From ec1fc5a17ba09d03a7fcfb261e11824798126b3c Mon Sep 17 00:00:00 2001 From: "ali.ibrahim" Date: Tue, 4 Oct 2016 17:12:17 +0200 Subject: [PATCH] Implementing error, close, finish, abort events management. --- atom/browser/api/atom_api_net.cc | 9 +- atom/browser/api/atom_api_net.h | 1 + atom/browser/api/atom_api_url_request.cc | 219 +++++++++++++++++++++-- atom/browser/api/atom_api_url_request.h | 116 +++++++++--- atom/browser/net/atom_url_request.cc | 92 ++++++---- atom/browser/net/atom_url_request.h | 12 +- lib/browser/api/net.js | 59 +++--- 7 files changed, 391 insertions(+), 117 deletions(-) diff --git a/atom/browser/api/atom_api_net.cc b/atom/browser/api/atom_api_net.cc index 2464393fcd02..5d23f04886de 100644 --- a/atom/browser/api/atom_api_net.cc +++ b/atom/browser/api/atom_api_net.cc @@ -29,7 +29,9 @@ void Net::BuildPrototype(v8::Isolate* isolate, v8::Local prototype) { prototype->SetClassName(mate::StringToV8(isolate, "Net")); mate::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate()) - .SetProperty("URLRequest", &Net::URLRequest); + .SetProperty("URLRequest", &Net::URLRequest) + .SetMethod("RequestGarbageCollectionForTesting", + &Net::RequestGarbageCollectionForTesting); } v8::Local Net::URLRequest(v8::Isolate* isolate) { @@ -37,6 +39,11 @@ v8::Local Net::URLRequest(v8::Isolate* isolate) { } +void Net::RequestGarbageCollectionForTesting() { + isolate()->RequestGarbageCollectionForTesting( + v8::Isolate::GarbageCollectionType::kFullGarbageCollection); +} + } // namespace api diff --git a/atom/browser/api/atom_api_net.h b/atom/browser/api/atom_api_net.h index 640a6445ebc4..04b882d2599b 100644 --- a/atom/browser/api/atom_api_net.h +++ b/atom/browser/api/atom_api_net.h @@ -24,6 +24,7 @@ class Net : public mate::EventEmitter { ~Net() override; private: + void RequestGarbageCollectionForTesting(); DISALLOW_COPY_AND_ASSIGN(Net); }; diff --git a/atom/browser/api/atom_api_url_request.cc b/atom/browser/api/atom_api_url_request.cc index 016edbe0c7ff..29736a5d0633 100644 --- a/atom/browser/api/atom_api_url_request.cc +++ b/atom/browser/api/atom_api_url_request.cc @@ -83,6 +83,84 @@ struct Converter> { namespace atom { namespace api { + +template +URLRequest::StateBase::StateBase(Flags initialState) + : state_(initialState) { +} + +template +void URLRequest::StateBase::SetFlag(Flags flag) { + state_ = static_cast(static_cast(state_) & + static_cast(flag)); +} + +template +bool URLRequest::StateBase::operator==(Flags flag) const { + return state_ == flag; +} + +template +bool URLRequest::StateBase::IsFlagSet(Flags flag) const { + return static_cast(state_) & static_cast(flag); +} + +URLRequest::RequestState::RequestState() + : StateBase(RequestStateFlags::kNotStarted) { +} + +bool URLRequest::RequestState::NotStarted() const { + return *this == RequestStateFlags::kNotStarted; +} + +bool URLRequest::RequestState::Started() const { + return IsFlagSet(RequestStateFlags::kStarted); +} + +bool URLRequest::RequestState::Finished() const { + return IsFlagSet(RequestStateFlags::kFinished); +} + +bool URLRequest::RequestState::Canceled() const { + return IsFlagSet(RequestStateFlags::kCanceled); +} + +bool URLRequest::RequestState::Failed() const { + return IsFlagSet(RequestStateFlags::kFailed); +} + +bool URLRequest::RequestState::Closed() const { + return IsFlagSet(RequestStateFlags::kClosed); +} + +URLRequest::ResponseState::ResponseState() + : StateBase(ResponseStateFlags::kNotStarted) { +} + +bool URLRequest::ResponseState::NotStarted() const { + return *this == ResponseStateFlags::kNotStarted; +} + +bool URLRequest::ResponseState::Started() const { + return IsFlagSet(ResponseStateFlags::kStarted); +} + +bool URLRequest::ResponseState::Ended() const { + return IsFlagSet(ResponseStateFlags::kEnded); +} + +bool URLRequest::ResponseState::Canceled() const { + return IsFlagSet(ResponseStateFlags::kCanceled); +} + +bool URLRequest::ResponseState::Failed() const { + return IsFlagSet(ResponseStateFlags::kFailed); +} + +bool URLRequest::ResponseState::Closed() const { + return IsFlagSet(ResponseStateFlags::kClosed); +} + URLRequest::URLRequest(v8::Isolate* isolate, v8::Local wrapper) : weak_ptr_factory_(this) { InitWith(isolate, wrapper); @@ -127,10 +205,12 @@ void URLRequest::BuildPrototype(v8::Isolate* isolate, // Request API .MakeDestroyable() .SetMethod("write", &URLRequest::Write) - .SetMethod("abort", &URLRequest::Abort) + .SetMethod("cancel", &URLRequest::Cancel) .SetMethod("setExtraHeader", &URLRequest::SetExtraHeader) .SetMethod("removeExtraHeader", &URLRequest::RemoveExtraHeader) .SetMethod("setChunkedUpload", &URLRequest::SetChunkedUpload) + .SetProperty("notStarted", &URLRequest::NotStarted) + .SetProperty("finished", &URLRequest::Finished) // Response APi .SetProperty("statusCode", &URLRequest::StatusCode) .SetProperty("statusMessage", &URLRequest::StatusMessage) @@ -139,19 +219,81 @@ void URLRequest::BuildPrototype(v8::Isolate* isolate, .SetProperty("httpVersionMinor", &URLRequest::ResponseHttpVersionMinor); } +bool URLRequest::NotStarted() const { + return request_state_.NotStarted(); +} + +bool URLRequest::Finished() const { + return request_state_.Finished(); +} + +bool URLRequest::Canceled() const { + return request_state_.Canceled(); +} + bool URLRequest::Write( scoped_refptr buffer, bool is_last) { - return atom_request_->Write(buffer, is_last); + if (request_state_.Canceled() || + request_state_.Failed() || + request_state_.Finished() || + request_state_.Closed()) { + return false; + } + + if (request_state_.NotStarted()) { + request_state_.SetFlag(RequestStateFlags::kStarted); + // Pin on first write. + pin(); + } + + if (is_last) { + request_state_.SetFlag(RequestStateFlags::kFinished); + EmitRequestEvent(true, "finish"); + } + + DCHECK(atom_request_); + if (atom_request_) { + return atom_request_->Write(buffer, is_last); + } + return false; } -void URLRequest::Abort() { - atom_request_->Abort(); +void URLRequest::Cancel() { + if (request_state_.Canceled()) { + // Cancel only once. + return; + } + + // Mark as canceled. + request_state_.SetFlag(RequestStateFlags::kCanceled); + + if (request_state_.Started()) { + // Really cancel if it was started. + atom_request_->Cancel(); + } + + if (!request_state_.Closed()) { + EmitRequestEvent(true, "abort"); + } + + + response_state_.SetFlag(ResponseStateFlags::kCanceled); + if (response_state_.Started() && !response_state_.Closed()) { + EmitResponseEvent(true, "aborted"); + } + Close(); } bool URLRequest::SetExtraHeader(const std::string& name, const std::string& value) { + // State must be equal to not started. + if (!request_state_.NotStarted()) { + // Cannot change headers after send. + return false; + } + if (!net::HttpUtil::IsValidHeaderName(name)) { return false; } @@ -165,45 +307,84 @@ bool URLRequest::SetExtraHeader(const std::string& name, } void URLRequest::RemoveExtraHeader(const std::string& name) { + // State must be equal to not started. + if (!request_state_.NotStarted()) { + // Cannot change headers after send. + return; + } atom_request_->RemoveExtraHeader(name); } void URLRequest::SetChunkedUpload(bool is_chunked_upload) { + // State must be equal to not started. + if (!request_state_.NotStarted()) { + // Cannot change headers after send. + return; + } atom_request_->SetChunkedUpload(is_chunked_upload); } void URLRequest::OnAuthenticationRequired( scoped_refptr auth_info) { EmitRequestEvent( + false, "login", auth_info.get(), base::Bind(&AtomURLRequest::PassLoginInformation, atom_request_)); } - void URLRequest::OnResponseStarted() { + if (request_state_.Canceled() || + request_state_.Failed() || + request_state_.Closed()) { + // Don't emit any event after request cancel. + return; + } + response_state_.SetFlag(ResponseStateFlags::kStarted); Emit("response"); } void URLRequest::OnResponseData( scoped_refptr buffer) { + if (request_state_.Canceled()) { + // Don't emit any event after request cancel. + return; + } if (!buffer || !buffer->data() || !buffer->size()) { return; } - EmitResponseEvent("data", buffer); + if (!response_state_.Closed()) { + EmitResponseEvent(false, "data", buffer); + } } void URLRequest::OnResponseCompleted() { - EmitResponseEvent("end"); - unpin(); - atom_request_ = nullptr; + response_state_.SetFlag(ResponseStateFlags::kEnded); + if (request_state_.Canceled()) { + // Don't emit any event after request cancel. + return; + } + if (!response_state_.Closed()) { + EmitResponseEvent(false, "end"); + } + Close(); } -void URLRequest::OnError(const std::string& error) { +void URLRequest::OnRequestError(const std::string& error) { + request_state_.SetFlag(RequestStateFlags::kFailed); auto error_object = v8::Exception::Error(mate::StringToV8(isolate(), error)); - EmitRequestEvent("error", error_object); + EmitRequestEvent(false, "error", error_object); + Close(); } +void URLRequest::OnResponseError(const std::string& error) { + response_state_.SetFlag(ResponseStateFlags::kFailed); + auto error_object = v8::Exception::Error(mate::StringToV8(isolate(), error)); + EmitResponseEvent(false, "error", error_object); + Close(); +} + + int URLRequest::StatusCode() const { if (auto response_headers = atom_request_->GetResponseHeaders()) { return response_headers->response_code(); @@ -238,6 +419,22 @@ uint32_t URLRequest::ResponseHttpVersionMinor() const { return 0; } +void URLRequest::Close() { + if (!response_state_.Closed()) { + response_state_.SetFlag(ResponseStateFlags::kClosed); + if (response_state_.Started()) { + // Emit a close event if we really have a response object. + EmitResponseEvent(true, "close"); + } + } + if (!request_state_.Closed()) { + request_state_.SetFlag(RequestStateFlags::kClosed); + EmitRequestEvent(true, "close"); + } + unpin(); + atom_request_ = nullptr; +} + void URLRequest::pin() { if (wrapper_.IsEmpty()) { wrapper_.Reset(isolate(), GetWrapper()); diff --git a/atom/browser/api/atom_api_url_request.h b/atom/browser/api/atom_api_url_request.h index 20eb66d3f0f2..e5baed4f0555 100644 --- a/atom/browser/api/atom_api_url_request.h +++ b/atom/browser/api/atom_api_url_request.h @@ -25,28 +25,28 @@ namespace api { // // The URLRequest class implements the V8 binding between the JavaScript API -// and Chromium native net library. It is responsible for handling HTTP/HTTPS +// and Chromium native net library. It is responsible for handling HTTP/HTTPS // requests. // -// The current class provides only the binding layer. Two other JavaScript -// classes (ClientRequest and IncomingMessage) in the net module provide the +// The current class provides only the binding layer. Two other JavaScript +// classes (ClientRequest and IncomingMessage) in the net module provide the // final API, including some state management and arguments validation. // -// URLRequest's methods fall into two main categories: command and event methods. -// They are always executed on the Browser's UI thread. +// URLRequest's methods fall into two main categories: command and event +// methods. They are always executed on the Browser's UI thread. // Command methods are called directly from JavaScript code via the API defined -// in BuildPrototype. A command method is generally implemented by forwarding -// the call to a corresponding method on AtomURLRequest which does the -// synchronization on the Browser IO thread. The latter then calls into Chromium -// net library. On the other hand, net library events originate on the IO +// in BuildPrototype. A command method is generally implemented by forwarding +// the call to a corresponding method on AtomURLRequest which does the +// synchronization on the Browser IO thread. The latter then calls into Chromium +// net library. On the other hand, net library events originate on the IO // thread in AtomURLRequest and are synchronized back on the UI thread, then -// forwarded to a corresponding event method in URLRequest and then to +// forwarded to a corresponding event method in URLRequest and then to // JavaScript via the EmitRequestEvent/EmitResponseEvent helpers. // -// URLRequest lifetime management: we followed the Wrapper/Wrappable pattern +// URLRequest lifetime management: we followed the Wrapper/Wrappable pattern // defined in native_mate. However, we augment that pattern with a pin/unpin // mechanism. The main reason is that we want the JS API to provide a similar -// lifetime guarantees as the XMLHttpRequest. +// lifetime guarantees as the XMLHttpRequest. // https://xhr.spec.whatwg.org/#garbage-collection // // The primary motivation is to not garbage collect a URLInstance as long as the @@ -61,32 +61,32 @@ namespace api { // }); // })(); // -// we still want data to be logged even if the response/request objects are no +// we still want data to be logged even if the response/request objects are n // more referenced in JavaScript. // // Binding by simply following the native_mate Wrapper/Wrappable pattern will // delete the URLRequest object when the corresponding JS object is collected. -// The v8 handle is a private member in WrappableBase and it is always weak, +// The v8 handle is a private member in WrappableBase and it is always weak, // there is no way to make it strong without changing native_mate. -// The solution we implement consists of maintaining some kind of state that +// The solution we implement consists of maintaining some kind of state that // prevents collection of JS wrappers as long as the request is emitting network -// events. At initialization, the object is unpinned. When the request starts, -// it is pinned. When no more events would be emitted, the object is unpinned -// and lifetime is again managed by the standard native mate Wrapper/Wrappable +// events. At initialization, the object is unpinned. When the request starts, +// it is pinned. When no more events would be emitted, the object is unpinned +// and lifetime is again managed by the standard native mate Wrapper/Wrappable // pattern. // -// pin/unpin: are implemented by constructing/reseting a V8 strong persistent +// pin/unpin: are implemented by constructing/reseting a V8 strong persistent // handle. // -// The URLRequest/AtmURLRequest interaction could have been implemented in a +// The URLRequest/AtmURLRequest interaction could have been implemented in a // single class. However, it implies that the resulting class lifetime will be -// managed by two conflicting mechanisms: JavaScript garbage collection and -// Chromium reference counting. Reasoning about lifetime issues become much +// managed by two conflicting mechanisms: JavaScript garbage collection and +// Chromium reference counting. Reasoning about lifetime issues become much // more complex. // -// We chose to split the implementation into two classes linked via a strong/weak -// pointers. A URLRequest instance is deleted if it is unpinned and the -// corresponding JS wrapper object is garbage collected. On the other hand, +// We chose to split the implementation into two classes linked via a +// strong/weak pointers. A URLRequest instance is deleted if it is unpinned and +// the corresponding JS wrapper object is garbage collected. On the other hand, // an AtmURLRequest instance lifetime is totally governed by reference counting. // class URLRequest : public mate::EventEmitter { @@ -103,7 +103,8 @@ class URLRequest : public mate::EventEmitter { void OnResponseStarted(); void OnResponseData(scoped_refptr data); void OnResponseCompleted(); - void OnError(const std::string& error); + void OnRequestError(const std::string& error); + void OnResponseError(const std::string& error); protected: explicit URLRequest(v8::Isolate* isolate, @@ -111,13 +112,70 @@ class URLRequest : public mate::EventEmitter { ~URLRequest() override; private: + template + class StateBase { + public: + void SetFlag(Flags flag); + protected: + explicit StateBase(Flags initialState); + bool operator==(Flags flag) const; + bool IsFlagSet(Flags flag) const; + private: + Flags state_; + }; + + enum class RequestStateFlags { + kNotStarted = 0x0, + kStarted = 0x1, + kFinished = 0x2, + kCanceled = 0x4, + kFailed = 0x8, + kClosed = 0x10 + }; + + class RequestState : public StateBase { + public: + RequestState(); + bool NotStarted() const; + bool Started() const; + bool Finished() const; + bool Canceled() const; + bool Failed() const; + bool Closed() const; + }; + + enum class ResponseStateFlags { + kNotStarted = 0x0, + kStarted = 0x1, + kEnded = 0x2, + kCanceled = 0x4, + kFailed = 0x8, + kClosed = 0x10 + }; + + class ResponseState : public StateBase { + public: + ResponseState(); + bool NotStarted() const; + bool Started() const; + bool Ended() const; + bool Canceled() const; + bool Failed() const; + bool Closed() const; + }; + + bool NotStarted() const; + bool Finished() const; + bool Canceled() const; + bool Failed() const; bool Write(scoped_refptr buffer, bool is_last); - void Abort(); + void Cancel(); bool SetExtraHeader(const std::string& name, const std::string& value); void RemoveExtraHeader(const std::string& name); void SetChunkedUpload(bool is_chunked_upload); + bool CanReadHeaders() const; int StatusCode() const; std::string StatusMessage() const; scoped_refptr RawResponseHeaders() const; @@ -134,16 +192,20 @@ class URLRequest : public mate::EventEmitter { template void EmitResponseEvent(ArgTypes... args); + void Close(); void pin(); void unpin(); scoped_refptr atom_request_; + RequestState request_state_; + ResponseState response_state_; // Used to implement pin/unpin. v8::Global wrapper_; base::WeakPtrFactory weak_ptr_factory_; + DISALLOW_COPY_AND_ASSIGN(URLRequest); }; diff --git a/atom/browser/net/atom_url_request.cc b/atom/browser/net/atom_url_request.cc index 6a20dc476861..f985915018a7 100644 --- a/atom/browser/net/atom_url_request.cc +++ b/atom/browser/net/atom_url_request.cc @@ -106,12 +106,12 @@ void AtomURLRequest::SetChunkedUpload(bool is_chunked_upload) { } -void AtomURLRequest::Abort() const { +void AtomURLRequest::Cancel() const { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); content::BrowserThread::PostTask( content::BrowserThread::IO, FROM_HERE, - base::Bind(&AtomURLRequest::DoAbort, this)); + base::Bind(&AtomURLRequest::DoCancel, this)); } void AtomURLRequest::SetExtraHeader(const std::string& name, @@ -202,7 +202,7 @@ void AtomURLRequest::DoWriteBuffer( } } -void AtomURLRequest::DoAbort() const { +void AtomURLRequest::DoCancel() const { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); request_->Cancel(); } @@ -235,38 +235,32 @@ void AtomURLRequest::OnResponseStarted(net::URLRequest* request) { const auto& status = request_->status(); if (status.is_success()) { - // Cache net::HttpResponseHeaders instance, a read-only objects - // so that headers and other http metainformation can be simultaneously - // read from UI thread while request data is simulataneously streaming - // on IO thread. - response_headers_ = request_->response_headers(); + // Success or pending trigger a Read. content::BrowserThread::PostTask( content::BrowserThread::UI, FROM_HERE, base::Bind(&AtomURLRequest::InformDelegateResponseStarted, this)); ReadResponse(); - } else { + } else if (status.status() == net::URLRequestStatus::Status::FAILED) { + // Report error on Start. + DoCancel(); auto error = net::ErrorToString(status.ToNetError()); content::BrowserThread::PostTask( content::BrowserThread::UI, FROM_HERE, - base::Bind(&AtomURLRequest::InformDelegateErrorOccured, + base::Bind(&AtomURLRequest::InformDelegateRequestErrorOccured, this, std::move(error))); } + // We don't report an error is the request is canceled. } void AtomURLRequest::ReadResponse() { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); - // Some servers may treat HEAD requests as GET requests. To free up the - // network connection as soon as possible, signal that the request has - // completed immediately, without trying to read any data back (all we care - // about is the response code and headers, which we already have). - int bytes_read = 0; - if (request_->status().is_success()) - if (!request_->Read(response_read_buffer_.get(), kBufferSize, &bytes_read)) - bytes_read = -1; - OnReadCompleted(request_.get(), bytes_read); + int bytes_read = -1; + if (request_->Read(response_read_buffer_.get(), kBufferSize, &bytes_read)) { + OnReadCompleted(request_.get(), bytes_read); + } } @@ -277,36 +271,53 @@ void AtomURLRequest::OnReadCompleted(net::URLRequest* request, DCHECK_EQ(request, request_.get()); const auto status = request_->status(); + + bool response_error = false; + bool data_ended = false; + bool data_transfer_error = false; do { - if (!status.is_success() || bytes_read <= 0) { - auto error = net::ErrorToString(status.ToNetError()); - content::BrowserThread::PostTask( - content::BrowserThread::UI, FROM_HERE, - base::Bind(&AtomURLRequest::InformDelegateErrorOccured, - this, - std::move(error))); + if (!status.is_success()) { + response_error = true; + break; + } + if (bytes_read == 0) { + data_ended = true; + break; + } + if (bytes_read < 0 || !CopyAndPostBuffer(bytes_read)) { + data_transfer_error = true; break; } - - const auto result = CopyAndPostBuffer(bytes_read); - if (!result) - // Failed to transfer data to UI thread. - return; } while (request_->Read(response_read_buffer_.get(), kBufferSize, &bytes_read)); - - if (!status.is_io_pending()) - + if (response_error) { + DoCancel(); + auto error = net::ErrorToString(status.ToNetError()); + content::BrowserThread::PostTask( + content::BrowserThread::UI, FROM_HERE, + base::Bind(&AtomURLRequest::InformDelegateResponseErrorOccured, + this, + std::move(error))); + } else if (data_ended) { content::BrowserThread::PostTask( content::BrowserThread::UI, FROM_HERE, base::Bind(&AtomURLRequest::InformDelegateResponseCompleted, this)); + } else if (data_transfer_error) { + // We abort the request on corrupted data transfer. + DoCancel(); + content::BrowserThread::PostTask( + content::BrowserThread::UI, FROM_HERE, + base::Bind(&AtomURLRequest::InformDelegateResponseErrorOccured, + this, + "Failed to transfer data from IO to UI thread.")); + } } bool AtomURLRequest::CopyAndPostBuffer(int bytes_read) { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); - // data is only a wrapper for the async response_read_buffer_. + // data is only a wrapper for the asynchronous response_read_buffer_. // Make a deep copy of payload and transfer ownership to the UI thread. auto buffer_copy = new net::IOBufferWithSize(bytes_read); memcpy(buffer_copy->data(), response_read_buffer_->data(), bytes_read); @@ -349,13 +360,20 @@ void AtomURLRequest::InformDelegateResponseCompleted() const { delegate_->OnResponseCompleted(); } -void AtomURLRequest::InformDelegateErrorOccured( +void AtomURLRequest::InformDelegateRequestErrorOccured( const std::string& error) const { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); if (delegate_) - delegate_->OnError(error); + delegate_->OnRequestError(error); } +void AtomURLRequest::InformDelegateResponseErrorOccured( + const std::string& error) const { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + + if (delegate_) + delegate_->OnResponseError(error); +} } // namespace atom diff --git a/atom/browser/net/atom_url_request.h b/atom/browser/net/atom_url_request.h index f065bc5249ac..f0708c0ac2ef 100644 --- a/atom/browser/net/atom_url_request.h +++ b/atom/browser/net/atom_url_request.h @@ -33,7 +33,7 @@ class AtomURLRequest : public base::RefCountedThreadSafe, bool Write(scoped_refptr buffer, bool is_last); void SetChunkedUpload(bool is_chunked_upload); - void Abort() const; + void Cancel() const; void SetExtraHeader(const std::string& name, const std::string& value) const; void RemoveExtraHeader(const std::string& name) const; void PassLoginInformation(const base::string16& username, @@ -51,12 +51,12 @@ class AtomURLRequest : public base::RefCountedThreadSafe, private: friend class base::RefCountedThreadSafe; - explicit AtomURLRequest(base::WeakPtr delegate); - ~AtomURLRequest()override; + explicit AtomURLRequest(base::WeakPtr delegate); + ~AtomURLRequest()override; void DoWriteBuffer(scoped_refptr buffer, bool is_last); - void DoAbort() const; + void DoCancel() const; void DoSetAuth(const base::string16& username, const base::string16& password) const; void DoCancelAuth() const; @@ -70,7 +70,8 @@ class AtomURLRequest : public base::RefCountedThreadSafe, void InformDelegateResponseData( scoped_refptr data) const; void InformDelegateResponseCompleted() const; - void InformDelegateErrorOccured(const std::string& error) const; + void InformDelegateRequestErrorOccured(const std::string& error) const; + void InformDelegateResponseErrorOccured(const std::string& error) const; base::WeakPtr delegate_; std::unique_ptr request_; @@ -81,7 +82,6 @@ class AtomURLRequest : public base::RefCountedThreadSafe, std::vector> upload_element_readers_; scoped_refptr response_read_buffer_; - scoped_refptr response_headers_; DISALLOW_COPY_AND_ASSIGN(AtomURLRequest); }; diff --git a/lib/browser/api/net.js b/lib/browser/api/net.js index 1471ae4f1dc7..5f2c7345d4a3 100644 --- a/lib/browser/api/net.js +++ b/lib/browser/api/net.js @@ -50,12 +50,24 @@ class IncomingMessage extends EventEmitter { } -URLRequest.prototype._emitRequestEvent = function () { - this._request.emit.apply(this._request, arguments) +URLRequest.prototype._emitRequestEvent = function (async, ...rest) { + if (async) { + process.nextTick(() => { + this._request.emit.apply(this._request, rest) + }) + } else { + this._request.emit.apply(this._request, rest) + } } -URLRequest.prototype._emitResponseEvent = function () { - this._response.emit.apply(this._response, arguments) +URLRequest.prototype._emitResponseEvent = function (async, ...rest) { + if (async) { + process.nextTick(() => { + this._request.emit.apply(this._request, rest) + }) + } else { + this._response.emit.apply(this._response, rest) + } } class ClientRequest extends EventEmitter { @@ -127,16 +139,7 @@ class ClientRequest extends EventEmitter { } } - // Flag to prevent request's headers modifications after - // headers flush. - this._started = false - - this._aborted = false - - // Flag to prevent writings after end. - this._finished = false - - // Set when the request uses chuned encoding. Can be switched + // Set when the request uses chunked encoding. Can be switched // to true only once and never set back to false. this._chunkedEncoding = false @@ -161,7 +164,7 @@ class ClientRequest extends EventEmitter { } set chunkedEncoding (value) { - if (this._started) { + if (!this._url_request.notStarted) { throw new Error('Can\'t set the transfer encoding, headers have been sent.') } this._chunkedEncoding = value @@ -174,7 +177,7 @@ class ClientRequest extends EventEmitter { if (value === undefined) { throw new Error('`value` required in setHeader("' + name + '", value).') } - if (this._started) { + if (!this._url_request.notStarted) { throw new Error('Can\'t set headers after they are sent.') } @@ -201,7 +204,7 @@ class ClientRequest extends EventEmitter { throw new Error('`name` is required for removeHeader(name).') } - if (this._started) { + if (!this._url_request.notStarted) { throw new Error('Can\'t remove headers after they are sent.') } @@ -229,9 +232,8 @@ class ClientRequest extends EventEmitter { // Since writing to the network is asynchronous, we conservatively // assume that request headers are written after delivering the first // buffer to the network IO thread. - if (!this._started) { + if (!this._url_request.notStarted) { this._url_request.setChunkedUpload(this.chunkedEncoding) - this._started = true } // The write callback is fired asynchronously to mimic Node.js. @@ -243,7 +245,7 @@ class ClientRequest extends EventEmitter { } write (data, encoding, callback) { - if (this._finished) { + if (this._url_request.finished) { let error = new Error('Write after end.') process.nextTick(writeAfterEndNT, this, error, callback) return true @@ -253,12 +255,10 @@ class ClientRequest extends EventEmitter { } end (data, encoding, callback) { - if (this._finished) { + if (this._url_request.finished) { return false } - this._finished = true - if (typeof data === 'function') { callback = data encoding = null @@ -274,18 +274,7 @@ class ClientRequest extends EventEmitter { } abort () { - if (!this._started) { - // Does nothing if stream - return - } - - if (!this._aborted) { - this._url_request.abort() - this._aborted = true - process.nextTick(() => { - this.emit('abort') - }) - } + this._url_request.cancel() } }