diff --git a/include/capi/cef_life_span_handler_capi.h b/include/capi/cef_life_span_handler_capi.h index 52d5520b2..caae621a1 100644 --- a/include/capi/cef_life_span_handler_capi.h +++ b/include/capi/cef_life_span_handler_capi.h @@ -33,7 +33,7 @@ // by hand. See the translator.README.txt file in the tools directory for // more information. // -// $hash=9756fec933d13ecb320b0ec8c3bd8c9fcddfef47$ +// $hash=85d9f30e93e1c3759213074cc5876f848cf4b012$ // #ifndef CEF_INCLUDE_CAPI_CEF_LIFE_SPAN_HANDLER_CAPI_H_ @@ -202,9 +202,13 @@ typedef struct _cef_life_span_handler_t { /// // Called just before a browser is destroyed. Release all references to the // browser object and do not attempt to execute any functions on the browser - // object after this callback returns. This callback will be the last - // notification that references |browser|. See do_close() documentation for - // additional usage information. + // object (other than GetIdentifier or IsSame) after this callback returns. + // This callback will be the last notification that references |browser| on + // the UI thread. Any in-progress network requests associated with |browser| + // will be aborted when the browser is destroyed, and + // cef_resource_request_handler_t callbacks related to those requests may + // still arrive on the IO thread after this function is called. See do_close() + // documentation for additional usage information. /// void(CEF_CALLBACK* on_before_close)(struct _cef_life_span_handler_t* self, struct _cef_browser_t* browser); diff --git a/include/capi/cef_resource_request_handler_capi.h b/include/capi/cef_resource_request_handler_capi.h index c0ddb92ff..be4c522ae 100644 --- a/include/capi/cef_resource_request_handler_capi.h +++ b/include/capi/cef_resource_request_handler_capi.h @@ -33,7 +33,7 @@ // by hand. See the translator.README.txt file in the tools directory for // more information. // -// $hash=8d92ad3e0836dffa44a4b35a6b277e963af8144c$ +// $hash=af8ddd4d2d19e5b64d0a40778cb3c62fd5f1d8c9$ // #ifndef CEF_INCLUDE_CAPI_CEF_RESOURCE_REQUEST_HANDLER_CAPI_H_ @@ -172,7 +172,14 @@ typedef struct _cef_resource_request_handler_t { // and |response| represent the request and response respectively and cannot // be modified in this callback. |status| indicates the load completion // status. |received_content_length| is the number of response bytes actually - // read. + // read. This function will be called for all requests, including requests + // that are aborted due to CEF shutdown or destruction of the associated + // browser. In cases where the associated browser is destroyed this callback + // may arrive after the cef_life_span_handler_t::OnBeforeClose callback for + // that browser. The cef_frame_t::IsValid function can be used to test for + // this situation, and care should be taken not to call |browser| or |frame| + // functions that modify state (like LoadURL, SendProcessMessage, etc.) if the + // frame is invalid. /// void(CEF_CALLBACK* on_resource_load_complete)( struct _cef_resource_request_handler_t* self, diff --git a/include/cef_life_span_handler.h b/include/cef_life_span_handler.h index 391a3e425..c08cfcf6c 100644 --- a/include/cef_life_span_handler.h +++ b/include/cef_life_span_handler.h @@ -195,9 +195,13 @@ class CefLifeSpanHandler : public virtual CefBaseRefCounted { /// // Called just before a browser is destroyed. Release all references to the // browser object and do not attempt to execute any methods on the browser - // object after this callback returns. This callback will be the last - // notification that references |browser|. See DoClose() documentation for - // additional usage information. + // object (other than GetIdentifier or IsSame) after this callback returns. + // This callback will be the last notification that references |browser| on + // the UI thread. Any in-progress network requests associated with |browser| + // will be aborted when the browser is destroyed, and + // CefResourceRequestHandler callbacks related to those requests may still + // arrive on the IO thread after this method is called. See DoClose() + // documentation for additional usage information. /// /*--cef()--*/ virtual void OnBeforeClose(CefRefPtr browser) {} diff --git a/include/cef_request.h b/include/cef_request.h index 9c0185b7a..cbe2d0359 100644 --- a/include/cef_request.h +++ b/include/cef_request.h @@ -99,7 +99,7 @@ class CefRequest : public virtual CefBaseRefCounted { // fully qualified with an HTTP or HTTPS scheme component. Any username, // password or ref component will be removed. /// - /*--cef()--*/ + /*--cef(optional_param=referrer_url)--*/ virtual void SetReferrer(const CefString& referrer_url, ReferrerPolicy policy) = 0; @@ -154,7 +154,7 @@ class CefRequest : public virtual CefBaseRefCounted { // existing values will not be overwritten. The Referer value cannot be set // using this method. /// - /*--cef()--*/ + /*--cef(optional_param=value)--*/ virtual void SetHeaderByName(const CefString& name, const CefString& value, bool overwrite) = 0; @@ -193,7 +193,7 @@ class CefRequest : public virtual CefBaseRefCounted { // Set the URL to the first party for cookies used in combination with // CefURLRequest. /// - /*--cef()--*/ + /*--cef(optional_param=url)--*/ virtual void SetFirstPartyForCookies(const CefString& url) = 0; /// diff --git a/include/cef_resource_request_handler.h b/include/cef_resource_request_handler.h index 39e2f216e..b42b83af5 100644 --- a/include/cef_resource_request_handler.h +++ b/include/cef_resource_request_handler.h @@ -174,6 +174,13 @@ class CefResourceRequestHandler : public virtual CefBaseRefCounted { // |response| represent the request and response respectively and cannot be // modified in this callback. |status| indicates the load completion status. // |received_content_length| is the number of response bytes actually read. + // This method will be called for all requests, including requests that are + // aborted due to CEF shutdown or destruction of the associated browser. In + // cases where the associated browser is destroyed this callback may arrive + // after the CefLifeSpanHandler::OnBeforeClose callback for that browser. The + // CefFrame::IsValid method can be used to test for this situation, and care + // should be taken not to call |browser| or |frame| methods that modify state + // (like LoadURL, SendProcessMessage, etc.) if the frame is invalid. /// /*--cef(optional_param=browser,optional_param=frame)--*/ virtual void OnResourceLoadComplete(CefRefPtr browser, diff --git a/include/cef_response.h b/include/cef_response.h index 969522a4a..70dee994a 100644 --- a/include/cef_response.h +++ b/include/cef_response.h @@ -96,7 +96,7 @@ class CefResponse : public virtual CefBaseRefCounted { /// // Set the response status text. /// - /*--cef()--*/ + /*--cef(optional_param=statusText)--*/ virtual void SetStatusText(const CefString& statusText) = 0; /// @@ -108,7 +108,7 @@ class CefResponse : public virtual CefBaseRefCounted { /// // Set the response mime type. /// - /*--cef()--*/ + /*--cef(optional_param=mimeType)--*/ virtual void SetMimeType(const CefString& mimeType) = 0; /// @@ -120,7 +120,7 @@ class CefResponse : public virtual CefBaseRefCounted { /// // Set the response charset. /// - /*--cef()--*/ + /*--cef(optional_param=charset)--*/ virtual void SetCharset(const CefString& charset) = 0; /// @@ -150,7 +150,7 @@ class CefResponse : public virtual CefBaseRefCounted { /// // Set the resolved URL after redirects or changed as a result of HSTS. /// - /*--cef()--*/ + /*--cef(optional_param=url)--*/ virtual void SetURL(const CefString& url) = 0; }; diff --git a/libcef/browser/browser_host_impl.h b/libcef/browser/browser_host_impl.h index 72284f1ef..807770d7d 100644 --- a/libcef/browser/browser_host_impl.h +++ b/libcef/browser/browser_host_impl.h @@ -496,6 +496,7 @@ class CefBrowserHostImpl : public CefBrowserHost, void AddObserver(Observer* observer); void RemoveObserver(Observer* observer); bool HasObserver(Observer* observer) const; + bool StartAudioMirroring(); bool StopAudioMirroring(); diff --git a/libcef/browser/browser_info.cc b/libcef/browser/browser_info.cc index 790cc2f27..76c05f0e1 100644 --- a/libcef/browser/browser_info.cc +++ b/libcef/browser/browser_info.cc @@ -353,6 +353,17 @@ void CefBrowserInfo::RemoveAllFrames() { frame_id_map_.clear(); frame_tree_node_id_map_.clear(); + // Explicitly Detach main frames. + for (auto& info : frame_info_set_) { + if (info->frame_ && info->is_main_frame_) + info->frame_->Detach(); + } + + if (main_frame_) { + main_frame_->Detach(); + main_frame_ = nullptr; + } + // And finally delete the frame info. frame_info_set_.clear(); } diff --git a/libcef/browser/context.cc b/libcef/browser/context.cc index dfb21c515..dd22ac299 100644 --- a/libcef/browser/context.cc +++ b/libcef/browser/context.cc @@ -565,6 +565,21 @@ bool CefContext::ValidateCachePath(const base::FilePath& cache_path) { return true; } +void CefContext::AddObserver(Observer* observer) { + CEF_REQUIRE_UIT(); + observers_.AddObserver(observer); +} + +void CefContext::RemoveObserver(Observer* observer) { + CEF_REQUIRE_UIT(); + observers_.RemoveObserver(observer); +} + +bool CefContext::HasObserver(Observer* observer) const { + CEF_REQUIRE_UIT(); + return observers_.HasObserver(observer); +} + void CefContext::OnContextInitialized() { CEF_REQUIRE_UIT(); @@ -591,6 +606,9 @@ void CefContext::FinishShutdownOnUIThread( browser_info_manager_->DestroyAllBrowsers(); + for (auto& observer : observers_) + observer.OnContextDestroyed(); + if (trace_subscriber_.get()) trace_subscriber_.reset(NULL); diff --git a/libcef/browser/context.h b/libcef/browser/context.h index 21a656896..060e2c73b 100644 --- a/libcef/browser/context.h +++ b/libcef/browser/context.h @@ -12,6 +12,7 @@ #include "include/cef_app.h" +#include "base/observer_list.h" #include "base/threading/platform_thread.h" #include "third_party/skia/include/core/SkColor.h" @@ -36,6 +37,17 @@ class CefContext { public: typedef std::list> BrowserList; + // Interface to implement for observers that wish to be informed of changes + // to the context. All methods will be called on the UI thread. + class Observer { + public: + // Called before the context is destroyed. + virtual void OnContextDestroyed() = 0; + + protected: + virtual ~Observer() {} + }; + CefContext(); ~CefContext(); @@ -80,6 +92,13 @@ class CefContext { // Verify that |cache_path| is valid and create it if necessary. bool ValidateCachePath(const base::FilePath& cache_path); + // Manage observer objects. The observer must either outlive this object or + // remove itself before destruction. These methods can only be called on the + // UI thread. + void AddObserver(Observer* observer); + void RemoveObserver(Observer* observer); + bool HasObserver(Observer* observer) const; + private: void OnContextInitialized(); @@ -104,6 +123,9 @@ class CefContext { std::unique_ptr sm_main_params_; std::unique_ptr trace_subscriber_; std::unique_ptr browser_info_manager_; + + // Observers that want to be notified of changes to this object. + base::ObserverList::Unchecked observers_; }; // Helper macro that returns true if the global context is in a valid state. diff --git a/libcef/browser/net_service/browser_urlrequest_impl.cc b/libcef/browser/net_service/browser_urlrequest_impl.cc index d7e3fde7e..465ff42aa 100644 --- a/libcef/browser/net_service/browser_urlrequest_impl.cc +++ b/libcef/browser/net_service/browser_urlrequest_impl.cc @@ -113,15 +113,21 @@ class CefBrowserURLRequest::Context request_context_impl->GetBrowserContext(); DCHECK(browser_context); - content::RenderFrameHost* rfh = nullptr; + scoped_refptr loader_factory_getter; if (frame) { - // The request will be associated with this frame/browser. - rfh = static_cast(frame.get())->GetRenderFrameHost(); + // The request will be associated with this frame/browser if it's valid, + // otherwise the request will be canceled. + content::RenderFrameHost* rfh = + static_cast(frame.get())->GetRenderFrameHost(); + if (rfh) { + loader_factory_getter = + net_service::URLLoaderFactoryGetter::Create(rfh, browser_context); + } + } else { + loader_factory_getter = + net_service::URLLoaderFactoryGetter::Create(nullptr, browser_context); } - auto loader_factory_getter = - net_service::URLLoaderFactoryGetter::Create(rfh, browser_context); - task_runner->PostTask( FROM_HERE, base::BindOnce( @@ -138,6 +144,12 @@ class CefBrowserURLRequest::Context if (!url_request_) return; + if (!loader_factory_getter) { + // Cancel the request immediately. + Cancel(); + return; + } + DCHECK_EQ(status_, UR_IO_PENDING); loader_factory_getter_ = loader_factory_getter; diff --git a/libcef/browser/net_service/resource_handler_wrapper.cc b/libcef/browser/net_service/resource_handler_wrapper.cc index d2375430b..2f27bd112 100644 --- a/libcef/browser/net_service/resource_handler_wrapper.cc +++ b/libcef/browser/net_service/resource_handler_wrapper.cc @@ -24,7 +24,9 @@ class SkipCallbackWrapper : public CefResourceSkipCallback { ~SkipCallbackWrapper() override { if (!callback_.is_null()) { - std::move(callback_).Run(net::ERR_FAILED); + // Make sure it executes on the correct thread. + work_thread_task_runner_->PostTask( + FROM_HERE, base::BindOnce(std::move(callback_), net::ERR_FAILED)); } } @@ -32,7 +34,7 @@ class SkipCallbackWrapper : public CefResourceSkipCallback { if (!work_thread_task_runner_->RunsTasksInCurrentSequence()) { work_thread_task_runner_->PostTask( FROM_HERE, - base::Bind(&SkipCallbackWrapper::Continue, this, bytes_skipped)); + base::BindOnce(&SkipCallbackWrapper::Continue, this, bytes_skipped)); return; } if (!callback_.is_null()) { @@ -59,7 +61,9 @@ class ReadCallbackWrapper : public CefResourceReadCallback { ~ReadCallbackWrapper() override { if (!callback_.is_null()) { - std::move(callback_).Run(net::ERR_FAILED); + // Make sure it executes on the correct thread. + work_thread_task_runner_->PostTask( + FROM_HERE, base::BindOnce(std::move(callback_), net::ERR_FAILED)); } } @@ -67,7 +71,7 @@ class ReadCallbackWrapper : public CefResourceReadCallback { if (!work_thread_task_runner_->RunsTasksInCurrentSequence()) { work_thread_task_runner_->PostTask( FROM_HERE, - base::Bind(&ReadCallbackWrapper::Continue, this, bytes_read)); + base::BindOnce(&ReadCallbackWrapper::Continue, this, bytes_read)); return; } if (!callback_.is_null()) { @@ -86,47 +90,89 @@ class ReadCallbackWrapper : public CefResourceReadCallback { DISALLOW_COPY_AND_ASSIGN(ReadCallbackWrapper); }; +// Helper for accessing a CefResourceHandler without creating reference loops. +class HandlerProvider : public base::RefCountedThreadSafe { + public: + explicit HandlerProvider(CefRefPtr handler) + : handler_(handler) { + DCHECK(handler_); + } + virtual ~HandlerProvider() { + // Detach should have been called. + DCHECK(!handler_); + } + + CefRefPtr handler() const { + base::AutoLock lock_scope(lock_); + return handler_; + } + + void Detach() { + base::AutoLock lock_scope(lock_); + if (!handler_) + return; + + // Execute on the expected thread. + CEF_POST_TASK(CEF_IOT, + base::BindOnce(&CefResourceHandler::Cancel, handler_)); + + handler_ = nullptr; + } + + private: + mutable base::Lock lock_; + CefRefPtr handler_; +}; + class ReadResponseCallbackWrapper : public CefCallback { public: ~ReadResponseCallbackWrapper() override { if (callback_) { + // This will post to the correct thread if necessary. callback_->Continue(net::ERR_FAILED); } } void Continue() override { CEF_POST_TASK(CEF_IOT, - base::Bind(&ReadResponseCallbackWrapper::DoRead, this)); + base::BindOnce(&ReadResponseCallbackWrapper::DoRead, this)); } void Cancel() override { CEF_POST_TASK(CEF_IOT, - base::Bind(&ReadResponseCallbackWrapper::DoCancel, this)); + base::BindOnce(&ReadResponseCallbackWrapper::DoCancel, this)); } - static void ReadResponse(CefRefPtr handler, + static void ReadResponse(scoped_refptr handler_provider, net::IOBuffer* dest, int length, CefRefPtr callback) { CEF_POST_TASK( - CEF_IOT, base::Bind(ReadResponseCallbackWrapper::ReadResponseOnIOThread, - handler, base::Unretained(dest), length, callback)); + CEF_IOT, + base::BindOnce(ReadResponseCallbackWrapper::ReadResponseOnIOThread, + handler_provider, base::Unretained(dest), length, + callback)); } private: - ReadResponseCallbackWrapper(CefRefPtr handler, + ReadResponseCallbackWrapper(scoped_refptr handler_provider, net::IOBuffer* dest, int length, CefRefPtr callback) - : handler_(handler), dest_(dest), length_(length), callback_(callback) {} + : handler_provider_(handler_provider), + dest_(dest), + length_(length), + callback_(callback) {} - static void ReadResponseOnIOThread(CefRefPtr handler, - net::IOBuffer* dest, - int length, - CefRefPtr callback) { + static void ReadResponseOnIOThread( + scoped_refptr handler_provider, + net::IOBuffer* dest, + int length, + CefRefPtr callback) { CEF_REQUIRE_IOT(); CefRefPtr callbackWrapper = - new ReadResponseCallbackWrapper(handler, dest, length, callback); + new ReadResponseCallbackWrapper(handler_provider, dest, length, + callback); callbackWrapper->DoRead(); } @@ -135,9 +181,15 @@ class ReadResponseCallbackWrapper : public CefCallback { if (!callback_) return; + auto handler = handler_provider_->handler(); + if (!handler) { + DoCancel(); + return; + } + int bytes_read = 0; bool result = - handler_->ReadResponse(dest_->data(), length_, bytes_read, this); + handler->ReadResponse(dest_->data(), length_, bytes_read, this); if (result) { if (bytes_read > 0) { // Continue immediately. @@ -160,7 +212,7 @@ class ReadResponseCallbackWrapper : public CefCallback { } } - CefRefPtr handler_; + scoped_refptr handler_provider_; net::IOBuffer* const dest_; int length_; CefRefPtr callback_; @@ -171,15 +223,23 @@ class ReadResponseCallbackWrapper : public CefCallback { class InputStreamWrapper : public InputStream { public: - explicit InputStreamWrapper(CefRefPtr handler) - : handler_(handler) {} - ~InputStreamWrapper() override {} + explicit InputStreamWrapper(scoped_refptr handler_provider) + : handler_provider_(handler_provider) {} + + ~InputStreamWrapper() override { Cancel(); } // InputStream methods: bool Skip(int64_t n, int64_t* bytes_skipped, SkipCallback callback) override { + auto handler = handler_provider_->handler(); + if (!handler) { + // Cancel immediately. + *bytes_skipped = net::ERR_FAILED; + return false; + } + CefRefPtr callbackWrapper = new SkipCallbackWrapper(std::move(callback)); - bool result = handler_->Skip(n, *bytes_skipped, callbackWrapper.get()); + bool result = handler->Skip(n, *bytes_skipped, callbackWrapper.get()); if (result) { if (*bytes_skipped > 0) { // Continue immediately. @@ -196,10 +256,17 @@ class InputStreamWrapper : public InputStream { int length, int* bytes_read, ReadCallback callback) override { + auto handler = handler_provider_->handler(); + if (!handler) { + // Cancel immediately. + *bytes_read = net::ERR_FAILED; + return false; + } + CefRefPtr callbackWrapper = new ReadCallbackWrapper(std::move(callback)); - bool result = handler_->Read(dest->data(), length, *bytes_read, - callbackWrapper.get()); + bool result = + handler->Read(dest->data(), length, *bytes_read, callbackWrapper.get()); if (result) { if (*bytes_read > 0) { // Continue immediately. @@ -210,7 +277,7 @@ class InputStreamWrapper : public InputStream { if (*bytes_read == -1) { // Call ReadResponse on the IO thread. - ReadResponseCallbackWrapper::ReadResponse(handler_, dest, length, + ReadResponseCallbackWrapper::ReadResponse(handler_provider_, dest, length, callbackWrapper); *bytes_read = 0; return true; @@ -220,8 +287,13 @@ class InputStreamWrapper : public InputStream { return false; } + void Cancel() { + // Triggers a call to Cancel on the handler. + handler_provider_->Detach(); + } + private: - CefRefPtr handler_; + scoped_refptr handler_provider_; DISALLOW_COPY_AND_ASSIGN(InputStreamWrapper); }; @@ -236,14 +308,18 @@ class OpenCallbackWrapper : public CefCallback { ~OpenCallbackWrapper() override { if (!callback_.is_null()) { - Execute(std::move(callback_), std::move(stream_), false); + // Make sure it executes on the correct thread. + work_thread_task_runner_->PostTask( + FROM_HERE, + base::BindOnce(&OpenCallbackWrapper::Execute, std::move(callback_), + std::move(stream_), false)); } } void Continue() override { if (!work_thread_task_runner_->RunsTasksInCurrentSequence()) { work_thread_task_runner_->PostTask( - FROM_HERE, base::Bind(&OpenCallbackWrapper::Continue, this)); + FROM_HERE, base::BindOnce(&OpenCallbackWrapper::Continue, this)); return; } if (!callback_.is_null()) { @@ -254,7 +330,7 @@ class OpenCallbackWrapper : public CefCallback { void Cancel() override { if (!work_thread_task_runner_->RunsTasksInCurrentSequence()) { work_thread_task_runner_->PostTask( - FROM_HERE, base::Bind(&OpenCallbackWrapper::Cancel, this)); + FROM_HERE, base::BindOnce(&OpenCallbackWrapper::Cancel, this)); return; } if (!callback_.is_null()) { @@ -279,10 +355,16 @@ class OpenCallbackWrapper : public CefCallback { }; void CallProcessRequestOnIOThread( - CefRefPtr handler, + scoped_refptr handler_provider, CefRefPtr request, CefRefPtr callbackWrapper) { CEF_REQUIRE_IOT(); + auto handler = handler_provider->handler(); + if (!handler) { + callbackWrapper->Cancel(); + return; + } + if (!handler->ProcessRequest(request.get(), callbackWrapper.get())) { callbackWrapper->Cancel(); } @@ -292,10 +374,13 @@ class ResourceResponseWrapper : public ResourceResponse { public: ResourceResponseWrapper(const RequestId request_id, CefRefPtr handler) - : request_id_(request_id), handler_(handler) { - DCHECK(handler_); + : request_id_(request_id), + handler_provider_(new HandlerProvider(handler)) {} + + ~ResourceResponseWrapper() override { + // Triggers a call to Cancel on the handler. + handler_provider_->Detach(); } - ~ResourceResponseWrapper() override {} // ResourceResponse methods: bool OpenInputStream(const RequestId& request_id, @@ -303,16 +388,23 @@ class ResourceResponseWrapper : public ResourceResponse { OpenCallback callback) override { DCHECK_EQ(request_id, request_id_); + auto handler = handler_provider_->handler(); + if (!handler) { + // Cancel immediately. + return false; + } + // May be recreated on redirect. request_ = new CefRequestImpl(); request_->Set(&request, request_id.hash()); request_->SetReadOnly(true); CefRefPtr callbackWrapper = new OpenCallbackWrapper( - std::move(callback), std::make_unique(handler_)); + std::move(callback), + std::make_unique(handler_provider_)); bool handle_request = false; bool result = - handler_->Open(request_.get(), handle_request, callbackWrapper.get()); + handler->Open(request_.get(), handle_request, callbackWrapper.get()); if (result) { if (handle_request) { // Continue immediately. @@ -328,8 +420,9 @@ class ResourceResponseWrapper : public ResourceResponse { } // Call ProcessRequest on the IO thread. - CEF_POST_TASK(CEF_IOT, base::Bind(CallProcessRequestOnIOThread, handler_, - request_, callbackWrapper)); + CEF_POST_TASK( + CEF_IOT, base::BindOnce(CallProcessRequestOnIOThread, handler_provider_, + request_, callbackWrapper)); return true; } @@ -343,10 +436,17 @@ class ResourceResponseWrapper : public ResourceResponse { DCHECK_EQ(request_id, request_id_); CEF_REQUIRE_IOT(); + auto handler = handler_provider_->handler(); + if (!handler) { + // Cancel immediately. + *status_code = net::ERR_FAILED; + return; + } + CefRefPtr response = CefResponse::Create(); int64_t response_length = -1; CefString redirect_url; - handler_->GetResponseHeaders(response, response_length, redirect_url); + handler->GetResponseHeaders(response, response_length, redirect_url); const auto error_code = response->GetError(); if (error_code != ERR_NONE) { @@ -390,7 +490,7 @@ class ResourceResponseWrapper : public ResourceResponse { const RequestId request_id_; CefRefPtr request_; - CefRefPtr handler_; + scoped_refptr handler_provider_; DISALLOW_COPY_AND_ASSIGN(ResourceResponseWrapper); }; diff --git a/libcef/browser/net_service/resource_request_handler_wrapper.cc b/libcef/browser/net_service/resource_request_handler_wrapper.cc index d71a65cbd..455bb0b86 100644 --- a/libcef/browser/net_service/resource_request_handler_wrapper.cc +++ b/libcef/browser/net_service/resource_request_handler_wrapper.cc @@ -7,6 +7,7 @@ #include "libcef/browser/browser_host_impl.h" #include "libcef/browser/browser_platform_delegate.h" #include "libcef/browser/content_browser_client.h" +#include "libcef/browser/context.h" #include "libcef/browser/net_service/cookie_helper.h" #include "libcef/browser/net_service/proxy_url_loader_factory.h" #include "libcef/browser/net_service/resource_handler_wrapper.h" @@ -54,7 +55,7 @@ class RequestCallbackWrapper : public CefRequestCallback { if (!work_thread_task_runner_->RunsTasksInCurrentSequence()) { work_thread_task_runner_->PostTask( FROM_HERE, - base::Bind(&RequestCallbackWrapper::Continue, this, allow)); + base::BindOnce(&RequestCallbackWrapper::Continue, this, allow)); return; } if (!callback_.is_null()) { @@ -139,6 +140,12 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { callback_(std::move(callback)), cancel_callback_(std::move(cancel_callback)) {} + ~PendingRequest() { + if (cancel_callback_) { + std::move(cancel_callback_).Run(net::ERR_ABORTED); + } + } + void Run(InterceptedRequestHandlerWrapper* self) { self->OnBeforeRequest(id_, request_, request_was_redirected_, std::move(callback_), std::move(cancel_callback_)); @@ -151,9 +158,12 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { CancelRequestCallback cancel_callback_; }; - class BrowserObserver : public CefBrowserHostImpl::Observer { + // Observer to receive notification of CEF context or associated browser + // destruction. Only one of the *Destroyed() methods will be called. + class DestructionObserver : public CefBrowserHostImpl::Observer, + public CefContext::Observer { public: - BrowserObserver() = default; + DestructionObserver() = default; void SetWrapper(base::WeakPtr wrapper) { CEF_REQUIRE_IOT(); @@ -163,35 +173,57 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { void OnBrowserDestroyed(CefBrowserHostImpl* browser) override { CEF_REQUIRE_UIT(); browser->RemoveObserver(this); - CEF_POST_TASK( - CEF_IOT, - base::BindOnce(&InterceptedRequestHandlerWrapper::OnBrowserDestroyed, - wrapper_)); + NotifyOnDestroyed(); + } + + void OnContextDestroyed() override { + CEF_REQUIRE_UIT(); + CefContext::Get()->RemoveObserver(this); + NotifyOnDestroyed(); } private: + void NotifyOnDestroyed() { + // It's not safe to test the WeakPtr on the UI thread, so we'll just post + // a task which will be ignored if the WeakPtr is invalid. + CEF_POST_TASK(CEF_IOT, base::BindOnce( + &InterceptedRequestHandlerWrapper::OnDestroyed, + wrapper_)); + } + base::WeakPtr wrapper_; - DISALLOW_COPY_AND_ASSIGN(BrowserObserver); + DISALLOW_COPY_AND_ASSIGN(DestructionObserver); }; InterceptedRequestHandlerWrapper() : weak_ptr_factory_(this) {} ~InterceptedRequestHandlerWrapper() override { - if (browser_observer_) { + // There should be no pending or in-progress requests during destruction. + DCHECK(pending_requests_.empty()); + DCHECK(request_map_.empty()); + + if (destruction_observer_) { + destruction_observer_->SetWrapper(nullptr); CEF_POST_TASK( CEF_UIT, base::BindOnce(&InterceptedRequestHandlerWrapper:: - RemoveBrowserObserverOnUIThread, - browser_info_, std::move(browser_observer_))); + RemoveDestructionObserverOnUIThread, + browser_ ? browser_->browser_info() : nullptr, + std::move(destruction_observer_))); } } - static void RemoveBrowserObserverOnUIThread( + static void RemoveDestructionObserverOnUIThread( scoped_refptr browser_info, - std::unique_ptr observer) { - // The browser may already have been destroyed when this method is executed. - auto browser = browser_info->browser(); - if (browser) - browser->RemoveObserver(observer.get()); + std::unique_ptr observer) { + // Verify that the browser or context is still valid before attempting to + // remove the observer. + if (browser_info) { + auto browser = browser_info->browser(); + if (browser) + browser->RemoveObserver(observer.get()); + } else if (CONTEXT_STATE_VALID()) { + CefContext::Get()->RemoveObserver(observer.get()); + } } void Initialize(content::BrowserContext* browser_context, @@ -211,14 +243,20 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { static_cast(browser_context->GetResourceContext()); DCHECK(resource_context_); - // Don't hold a RefPtr to the CefBrowserHostImpl. - if (browser) { - browser_info_ = browser->browser_info(); + // We register to be notified of CEF context or browser destruction so that + // we can stop accepting new requests and cancel pending/in-progress + // requests in a timely manner (e.g. before we start asserting about leaked + // objects during CEF shutdown). + destruction_observer_.reset(new DestructionObserver()); - browser_observer_.reset(new BrowserObserver()); - browser->AddObserver(browser_observer_.get()); + if (browser) { + // These references will be released in OnDestroyed(). + browser_ = browser; + frame_ = frame; + browser->AddObserver(destruction_observer_.get()); + } else { + CefContext::Get()->AddObserver(destruction_observer_.get()); } - frame_ = frame; render_process_id_ = render_process_id; render_frame_id_ = render_frame_id; @@ -245,10 +283,21 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { CEF_REQUIRE_IOT(); initialized_ = true; - if (browser_observer_) { - browser_observer_->SetWrapper(weak_ptr_factory_.GetWeakPtr()); + // Check that the CEF context or associated browser was not destroyed + // between the calls to Initialize and SetInitialized, in which case + // we won't get an OnDestroyed callback from DestructionObserver. + if (browser_) { + if (!browser_->browser_info()->browser()) { + OnDestroyed(); + return; + } + } else if (!CONTEXT_STATE_VALID()) { + OnDestroyed(); + return; } + destruction_observer_->SetWrapper(weak_ptr_factory_.GetWeakPtr()); + // Continue any pending requests. if (!pending_requests_.empty()) { for (const auto& request : pending_requests_) @@ -265,6 +314,12 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { CancelRequestCallback cancel_callback) override { CEF_REQUIRE_IOT(); + if (shutting_down_) { + // Abort immediately. + std::move(cancel_callback).Run(net::ERR_ABORTED); + return; + } + if (!initialized_) { // Queue requests until we're initialized. pending_requests_.push_back(std::make_unique( @@ -305,8 +360,8 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { std::move(cancel_callback)); if (handler) { - state->cookie_filter_ = handler->GetCookieAccessFilter( - GetBrowser(), frame_, requestPtr.get()); + state->cookie_filter_ = + handler->GetCookieAccessFilter(browser_, frame_, requestPtr.get()); } auto exec_callback = @@ -366,7 +421,7 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { CefCookie cef_cookie; if (net_service::MakeCefCookie(cookie, cef_cookie)) { *allow = state->cookie_filter_->CanSendCookie( - GetBrowser(), frame_, state->pending_request_.get(), cef_cookie); + browser_, frame_, state->pending_request_.get(), cef_cookie); } } @@ -413,7 +468,11 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { CEF_REQUIRE_IOT(); RequestState* state = GetState(id); - DCHECK(state); + if (!state) { + // The request may have been canceled during destruction. + return; + } + // Must have a handler and/or scheme factory. DCHECK(state->handler_ || state->scheme_factory_); DCHECK(state->pending_request_); @@ -431,8 +490,7 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { base::Passed(std::move(callback)))); cef_return_value_t retval = state->handler_->OnBeforeResourceLoad( - GetBrowser(), frame_, state->pending_request_.get(), - callbackPtr.get()); + browser_, frame_, state->pending_request_.get(), callbackPtr.get()); if (retval != RV_CONTINUE_ASYNC) { // Continue or cancel the request immediately. callbackPtr->Continue(retval == RV_CONTINUE); @@ -493,18 +551,17 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { } CefRefPtr resource_handler; - CefRefPtr browser = GetBrowser(); if (state->handler_) { // Does the client want to handle the request? resource_handler = state->handler_->GetResourceHandler( - browser, frame_, state->pending_request_.get()); + browser_, frame_, state->pending_request_.get()); } if (!resource_handler && state->scheme_factory_) { // Does the scheme factory want to handle the request? - resource_handler = - state->scheme_factory_->Create(browser, frame_, request->url.scheme(), - state->pending_request_.get()); + resource_handler = state->scheme_factory_->Create( + browser_, frame_, request->url.scheme(), + state->pending_request_.get()); } std::unique_ptr resource_response; @@ -526,7 +583,11 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { CEF_REQUIRE_IOT(); RequestState* state = GetState(id); - DCHECK(state); + if (!state) { + // The request may have been canceled during destruction. + return; + } + if (!state->handler_) return; @@ -549,7 +610,11 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { CEF_REQUIRE_IOT(); RequestState* state = GetState(id); - DCHECK(state); + if (!state) { + // The request may have been canceled during destruction. + return; + } + if (!state->handler_) { // Cookies may come from a scheme handler. MaybeSaveCookies( @@ -581,7 +646,7 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { CefString newUrl = redirect_info.new_url.spec(); CefString oldUrl = newUrl; bool url_changed = false; - state->handler_->OnResourceRedirect(GetBrowser(), frame_, + state->handler_->OnResourceRedirect(browser_, frame_, state->pending_request_.get(), state->pending_response_.get(), newUrl); if (newUrl != oldUrl) { @@ -619,7 +684,7 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { auto response_mode = ResponseMode::CONTINUE; GURL new_url; - if (state->handler_->OnResourceResponse(GetBrowser(), frame_, + if (state->handler_->OnResourceResponse(browser_, frame_, state->pending_request_.get(), state->pending_response_.get())) { // The request may have been modified. @@ -703,7 +768,7 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { CefCookie cef_cookie; if (net_service::MakeCefCookie(cookie, cef_cookie)) { *allow = state->cookie_filter_->CanSaveCookie( - GetBrowser(), frame_, state->pending_request_.get(), + browser_, frame_, state->pending_request_.get(), state->pending_response_.get(), cef_cookie); } } @@ -723,11 +788,14 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { CEF_REQUIRE_IOT(); RequestState* state = GetState(id); - DCHECK(state); + if (!state) { + // The request may have been canceled during destruction. + return body; + } if (state->handler_) { auto filter = state->handler_->GetResourceResponseFilter( - GetBrowser(), frame_, state->pending_request_.get(), + browser_, frame_, state->pending_request_.get(), state->pending_response_.get()); if (filter) { return CreateResponseFilterHandler( @@ -763,7 +831,7 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { RequestState* state = GetState(id); if (!state) { - // The request may have been canceled. + // The request may have been canceled during destruction. return; } @@ -779,28 +847,13 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { if (state->handler_ && !ignore_result) { DCHECK(state->pending_request_); - if (!state->pending_response_) { - // If the request failed there may not be a response object yet. - state->pending_response_ = new CefResponseImpl(); - } else { - state->pending_response_->SetReadOnly(false); - } - state->pending_response_->SetError( - static_cast(status.error_code)); - state->pending_response_->SetReadOnly(true); - - CefRefPtr browser = GetBrowser(); - - state->handler_->OnResourceLoadComplete( - browser, frame_, state->pending_request_.get(), - state->pending_response_.get(), - status.error_code == 0 ? UR_SUCCESS : UR_FAILED, - status.encoded_body_length); + CallHandlerOnComplete(state, status); if (status.error_code != 0 && is_external_) { bool allow_os_execution = false; - state->handler_->OnProtocolExecution( - browser, frame_, state->pending_request_.get(), allow_os_execution); + state->handler_->OnProtocolExecution(browser_, frame_, + state->pending_request_.get(), + allow_os_execution); if (allow_os_execution) { CefBrowserPlatformDelegate::HandleExternalProtocol(request.url); } @@ -811,6 +864,34 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { } private: + void CallHandlerOnComplete(RequestState* state, + const network::URLLoaderCompletionStatus& status) { + if (!state->handler_ || !state->pending_request_) + return; + + // The request object may be currently flagged as writable in cases where we + // abort a request that is waiting on a pending callack. + if (!state->pending_request_->IsReadOnly()) { + state->pending_request_->SetReadOnly(true); + } + + if (!state->pending_response_) { + // If the request failed there may not be a response object yet. + state->pending_response_ = new CefResponseImpl(); + } else { + state->pending_response_->SetReadOnly(false); + } + state->pending_response_->SetError( + static_cast(status.error_code)); + state->pending_response_->SetReadOnly(true); + + state->handler_->OnResourceLoadComplete( + browser_, frame_, state->pending_request_.get(), + state->pending_response_.get(), + status.error_code == 0 ? UR_SUCCESS : UR_FAILED, + status.encoded_body_length); + } + // Returns the handler, if any, that should be used for this request. CefRefPtr GetHandler( const RequestId& id, @@ -821,10 +902,9 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { const int64 request_id = id.hash(); - CefRefPtr browser = GetBrowser(); - if (browser) { + if (browser_) { // Maybe the browser's client wants to handle it? - CefRefPtr client = browser->GetHost()->GetClient(); + CefRefPtr client = browser_->GetHost()->GetClient(); if (client) { CefRefPtr request_handler = client->GetRequestHandler(); @@ -832,7 +912,7 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { requestPtr = MakeRequest(request, request_id, true); handler = request_handler->GetResourceRequestHandler( - browser, frame_, requestPtr.get(), is_navigation_, is_download_, + browser_, frame_, requestPtr.get(), is_navigation_, is_download_, request_initiator_, *intercept_only); } } @@ -849,7 +929,7 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { requestPtr = MakeRequest(request, request_id, true); handler = context_handler->GetResourceRequestHandler( - browser, frame_, requestPtr.get(), is_navigation_, is_download_, + browser_, frame_, requestPtr.get(), is_navigation_, is_download_, request_initiator_, *intercept_only); } } @@ -880,29 +960,52 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { request_map_.erase(it); } - CefRefPtr GetBrowser() const { - if (browser_info_) - return browser_info_->browser().get(); - return nullptr; - } - - // Called when the associated browser, if any, is destroyed. - void OnBrowserDestroyed() { + // Stop accepting new requests and cancel pending/in-flight requests when the + // CEF context or associated browser is destroyed. + void OnDestroyed() { CEF_REQUIRE_IOT(); DCHECK(initialized_); - DCHECK(browser_observer_); - browser_observer_.reset(); + DCHECK(destruction_observer_); + destruction_observer_.reset(); - // Cancel any pending requests. - while (!request_map_.empty()) { - auto cancel_callback = - std::move(request_map_.begin()->second->cancel_callback_); - if (cancel_callback) { - // Results in a call to RemoveState(). - std::move(cancel_callback).Run(net::ERR_ABORTED); - } else { - RemoveState(request_map_.begin()->first); + // Stop accepting new requests. + shutting_down_ = true; + + // Stop the delivery of pending callbacks. + weak_ptr_factory_.InvalidateWeakPtrs(); + + // Take ownership of any pending requests. + PendingRequests pending_requests; + pending_requests.swap(pending_requests_); + + // Take ownership of any in-progress requests. + RequestMap request_map; + request_map.swap(request_map_); + + // Notify handlers for in-progress requests. + for (const auto& pair : request_map) { + CallHandlerOnComplete( + pair.second.get(), + network::URLLoaderCompletionStatus(net::ERR_ABORTED)); + } + + if (browser_) { + // Clear objects that reference the browser. + browser_ = nullptr; + frame_ = nullptr; + } + + // Execute cancel callbacks and delete pending and in-progress requests. + // This may result in the request being torn down sooner, or it may be + // ignored if the request is already in the process of being torn down. When + // the last callback is executed it may result in |this| being deleted. + pending_requests.clear(); + + for (auto& pair : request_map) { + auto state = std::move(pair.second); + if (state->cancel_callback_) { + std::move(state->cancel_callback_).Run(net::ERR_ABORTED); } } } @@ -921,11 +1024,12 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { } bool initialized_ = false; + bool shutting_down_ = false; // Only accessed on the UI thread. content::BrowserContext* browser_context_ = nullptr; - scoped_refptr browser_info_; + CefRefPtr browser_; CefRefPtr frame_; CefResourceContext* resource_context_ = nullptr; int render_process_id_ = 0; @@ -946,8 +1050,8 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler { using PendingRequests = std::vector>; PendingRequests pending_requests_; - // Used to recieve notification of browser destruction. - std::unique_ptr browser_observer_; + // Used to receive destruction notification. + std::unique_ptr destruction_observer_; base::WeakPtrFactory weak_ptr_factory_; @@ -961,8 +1065,12 @@ void InitOnUIThread( const network::ResourceRequest& request) { CEF_REQUIRE_UIT(); + // May return nullptr if the WebContents was destroyed while this callback was + // in-flight. content::WebContents* web_contents = web_contents_getter.Run(); - DCHECK(web_contents); + if (!web_contents) { + return; + } content::BrowserContext* browser_context = web_contents->GetBrowserContext(); DCHECK(browser_context); diff --git a/libcef/browser/net_service/stream_reader_url_loader.cc b/libcef/browser/net_service/stream_reader_url_loader.cc index 28f68194a..6d76bac1b 100644 --- a/libcef/browser/net_service/stream_reader_url_loader.cc +++ b/libcef/browser/net_service/stream_reader_url_loader.cc @@ -34,12 +34,16 @@ using OnInputStreamOpenedCallback = class OpenInputStreamWrapper : public base::RefCountedThreadSafe { public: - static void Open( + static base::OnceClosure Open( std::unique_ptr delegate, scoped_refptr work_thread_task_runner, const RequestId& request_id, const network::ResourceRequest& request, - OnInputStreamOpenedCallback callback) { + OnInputStreamOpenedCallback callback) WARN_UNUSED_RESULT { + scoped_refptr wrapper = new OpenInputStreamWrapper( + std::move(delegate), base::ThreadTaskRunnerHandle::Get(), + std::move(callback)); + work_thread_task_runner->PostTask( FROM_HERE, base::BindOnce(OpenInputStreamWrapper::OpenOnWorkThread, @@ -47,8 +51,9 @@ class OpenInputStreamWrapper // while the callback is executing on the background // thread. The delegate will be "returned" to the loader // once the InputStream open attempt is completed. - std::move(delegate), base::ThreadTaskRunnerHandle::Get(), - request_id, request, std::move(callback))); + wrapper, request_id, request)); + + return wrapper->GetCancelCallback(); } private: @@ -63,20 +68,26 @@ class OpenInputStreamWrapper callback_(std::move(callback)) {} virtual ~OpenInputStreamWrapper() {} - static void OpenOnWorkThread( - std::unique_ptr delegate, - scoped_refptr job_thread_task_runner, - const RequestId& request_id, - const network::ResourceRequest& request, - OnInputStreamOpenedCallback callback) { + static void OpenOnWorkThread(scoped_refptr wrapper, + const RequestId& request_id, + const network::ResourceRequest& request) { DCHECK(!CEF_CURRENTLY_ON_IOT()); DCHECK(!CEF_CURRENTLY_ON_UIT()); - scoped_refptr wrapper = new OpenInputStreamWrapper( - std::move(delegate), job_thread_task_runner, std::move(callback)); wrapper->Open(request_id, request); } + base::OnceClosure GetCancelCallback() { + return base::BindOnce(&OpenInputStreamWrapper::Cancel, + base::WrapRefCounted(this)); + } + + void Cancel() { + DCHECK(job_thread_task_runner_->RunsTasksInCurrentSequence()); + delegate_.reset(); + callback_.Reset(); + } + void Open(const RequestId& request_id, const network::ResourceRequest& request) { if (!delegate_->OpenInputStream( @@ -96,7 +107,7 @@ class OpenInputStreamWrapper return; } - DCHECK(!callback_.is_null()); + // May be null if we were canceled. if (callback_.is_null()) return; @@ -470,7 +481,12 @@ StreamReaderURLLoader::StreamReaderURLLoader( base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()}); } -StreamReaderURLLoader::~StreamReaderURLLoader() {} +StreamReaderURLLoader::~StreamReaderURLLoader() { + if (open_cancel_callback_) { + // Release the Delegate held by OpenInputStreamWrapper. + std::move(open_cancel_callback_).Run(); + } +} void StreamReaderURLLoader::Start() { DCHECK(thread_checker_.CalledOnValidThread()); @@ -503,7 +519,7 @@ void StreamReaderURLLoader::ContinueWithRequestHeaders( request_.headers = *headers; } - OpenInputStreamWrapper::Open( + open_cancel_callback_ = OpenInputStreamWrapper::Open( // This is intentional - the loader could be deleted while // the callback is executing on the background thread. The // delegate will be "returned" to the loader once the @@ -536,6 +552,7 @@ void StreamReaderURLLoader::OnInputStreamOpened( DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(returned_delegate); response_delegate_ = std::move(returned_delegate); + open_cancel_callback_.Reset(); if (!input_stream) { bool restarted = false; diff --git a/libcef/browser/net_service/stream_reader_url_loader.h b/libcef/browser/net_service/stream_reader_url_loader.h index 9f3e75685..301158e63 100644 --- a/libcef/browser/net_service/stream_reader_url_loader.h +++ b/libcef/browser/net_service/stream_reader_url_loader.h @@ -225,6 +225,8 @@ class StreamReaderURLLoader : public network::mojom::URLLoader { scoped_refptr stream_work_task_runner_; + base::OnceClosure open_cancel_callback_; + base::WeakPtrFactory weak_factory_; DISALLOW_COPY_AND_ASSIGN(StreamReaderURLLoader); diff --git a/libcef_dll/cpptoc/request_cpptoc.cc b/libcef_dll/cpptoc/request_cpptoc.cc index 7b035f7c9..6cb20c7a4 100644 --- a/libcef_dll/cpptoc/request_cpptoc.cc +++ b/libcef_dll/cpptoc/request_cpptoc.cc @@ -9,7 +9,7 @@ // implementations. See the translator.README.txt file in the tools directory // for more information. // -// $hash=fc10026d8f3e77868d19807ea625f9449678da60$ +// $hash=65f76c903a9c9f245ffe2b3275472d7d3d464f93$ // #include "libcef_dll/cpptoc/request_cpptoc.h" @@ -116,10 +116,7 @@ void CEF_CALLBACK request_set_referrer(struct _cef_request_t* self, DCHECK(self); if (!self) return; - // Verify param: referrer_url; type: string_byref_const - DCHECK(referrer_url); - if (!referrer_url) - return; + // Unverified params: referrer_url // Execute CefRequestCppToC::Get(self)->SetReferrer(CefString(referrer_url), policy); @@ -265,10 +262,7 @@ void CEF_CALLBACK request_set_header_by_name(struct _cef_request_t* self, DCHECK(name); if (!name) return; - // Verify param: value; type: string_byref_const - DCHECK(value); - if (!value) - return; + // Unverified params: value // Execute CefRequestCppToC::Get(self)->SetHeaderByName( @@ -357,10 +351,7 @@ request_set_first_party_for_cookies(struct _cef_request_t* self, DCHECK(self); if (!self) return; - // Verify param: url; type: string_byref_const - DCHECK(url); - if (!url) - return; + // Unverified params: url // Execute CefRequestCppToC::Get(self)->SetFirstPartyForCookies(CefString(url)); diff --git a/libcef_dll/cpptoc/response_cpptoc.cc b/libcef_dll/cpptoc/response_cpptoc.cc index 1a7808016..25dcff4b9 100644 --- a/libcef_dll/cpptoc/response_cpptoc.cc +++ b/libcef_dll/cpptoc/response_cpptoc.cc @@ -9,7 +9,7 @@ // implementations. See the translator.README.txt file in the tools directory // for more information. // -// $hash=4573a140180f11230e688f73e8c09503f9123c3d$ +// $hash=5c6b14610ef7bcadf5e4936a262c660896a32526$ // #include "libcef_dll/cpptoc/response_cpptoc.h" @@ -119,10 +119,7 @@ void CEF_CALLBACK response_set_status_text(struct _cef_response_t* self, DCHECK(self); if (!self) return; - // Verify param: statusText; type: string_byref_const - DCHECK(statusText); - if (!statusText) - return; + // Unverified params: statusText // Execute CefResponseCppToC::Get(self)->SetStatusText(CefString(statusText)); @@ -150,10 +147,7 @@ void CEF_CALLBACK response_set_mime_type(struct _cef_response_t* self, DCHECK(self); if (!self) return; - // Verify param: mimeType; type: string_byref_const - DCHECK(mimeType); - if (!mimeType) - return; + // Unverified params: mimeType // Execute CefResponseCppToC::Get(self)->SetMimeType(CefString(mimeType)); @@ -181,10 +175,7 @@ void CEF_CALLBACK response_set_charset(struct _cef_response_t* self, DCHECK(self); if (!self) return; - // Verify param: charset; type: string_byref_const - DCHECK(charset); - if (!charset) - return; + // Unverified params: charset // Execute CefResponseCppToC::Get(self)->SetCharset(CefString(charset)); @@ -275,10 +266,7 @@ void CEF_CALLBACK response_set_url(struct _cef_response_t* self, DCHECK(self); if (!self) return; - // Verify param: url; type: string_byref_const - DCHECK(url); - if (!url) - return; + // Unverified params: url // Execute CefResponseCppToC::Get(self)->SetURL(CefString(url)); diff --git a/libcef_dll/ctocpp/request_ctocpp.cc b/libcef_dll/ctocpp/request_ctocpp.cc index 7a6af1719..ad9284346 100644 --- a/libcef_dll/ctocpp/request_ctocpp.cc +++ b/libcef_dll/ctocpp/request_ctocpp.cc @@ -9,7 +9,7 @@ // implementations. See the translator.README.txt file in the tools directory // for more information. // -// $hash=8f57157cf8f05f46b5ae96335584b5e9df02b200$ +// $hash=30da50026aa5654b04de874040b194dcc87a7e30$ // #include "libcef_dll/ctocpp/request_ctocpp.h" @@ -118,10 +118,7 @@ void CefRequestCToCpp::SetReferrer(const CefString& referrer_url, // AUTO-GENERATED CONTENT - DELETE THIS COMMENT BEFORE MODIFYING - // Verify param: referrer_url; type: string_byref_const - DCHECK(!referrer_url.empty()); - if (referrer_url.empty()) - return; + // Unverified params: referrer_url // Execute _struct->set_referrer(_struct, referrer_url.GetStruct(), policy); @@ -274,10 +271,7 @@ void CefRequestCToCpp::SetHeaderByName(const CefString& name, DCHECK(!name.empty()); if (name.empty()) return; - // Verify param: value; type: string_byref_const - DCHECK(!value.empty()); - if (value.empty()) - return; + // Unverified params: value // Execute _struct->set_header_by_name(_struct, name.GetStruct(), value.GetStruct(), @@ -369,10 +363,7 @@ void CefRequestCToCpp::SetFirstPartyForCookies(const CefString& url) { // AUTO-GENERATED CONTENT - DELETE THIS COMMENT BEFORE MODIFYING - // Verify param: url; type: string_byref_const - DCHECK(!url.empty()); - if (url.empty()) - return; + // Unverified params: url // Execute _struct->set_first_party_for_cookies(_struct, url.GetStruct()); diff --git a/libcef_dll/ctocpp/response_ctocpp.cc b/libcef_dll/ctocpp/response_ctocpp.cc index ec170ac18..7015ec910 100644 --- a/libcef_dll/ctocpp/response_ctocpp.cc +++ b/libcef_dll/ctocpp/response_ctocpp.cc @@ -9,7 +9,7 @@ // implementations. See the translator.README.txt file in the tools directory // for more information. // -// $hash=781ea7e9b24023e8bdcc80ed93b27eb7ef80aa69$ +// $hash=0c197661e7e3e905b90bec127ff7b1ff23240fa2$ // #include "libcef_dll/ctocpp/response_ctocpp.h" @@ -118,10 +118,7 @@ void CefResponseCToCpp::SetStatusText(const CefString& statusText) { // AUTO-GENERATED CONTENT - DELETE THIS COMMENT BEFORE MODIFYING - // Verify param: statusText; type: string_byref_const - DCHECK(!statusText.empty()); - if (statusText.empty()) - return; + // Unverified params: statusText // Execute _struct->set_status_text(_struct, statusText.GetStruct()); @@ -151,10 +148,7 @@ void CefResponseCToCpp::SetMimeType(const CefString& mimeType) { // AUTO-GENERATED CONTENT - DELETE THIS COMMENT BEFORE MODIFYING - // Verify param: mimeType; type: string_byref_const - DCHECK(!mimeType.empty()); - if (mimeType.empty()) - return; + // Unverified params: mimeType // Execute _struct->set_mime_type(_struct, mimeType.GetStruct()); @@ -184,10 +178,7 @@ void CefResponseCToCpp::SetCharset(const CefString& charset) { // AUTO-GENERATED CONTENT - DELETE THIS COMMENT BEFORE MODIFYING - // Verify param: charset; type: string_byref_const - DCHECK(!charset.empty()); - if (charset.empty()) - return; + // Unverified params: charset // Execute _struct->set_charset(_struct, charset.GetStruct()); @@ -286,10 +277,7 @@ NO_SANITIZE("cfi-icall") void CefResponseCToCpp::SetURL(const CefString& url) { // AUTO-GENERATED CONTENT - DELETE THIS COMMENT BEFORE MODIFYING - // Verify param: url; type: string_byref_const - DCHECK(!url.empty()); - if (url.empty()) - return; + // Unverified params: url // Execute _struct->set_url(_struct, url.GetStruct()); diff --git a/tests/ceftests/resource_request_handler_unittest.cc b/tests/ceftests/resource_request_handler_unittest.cc index a1c40506e..35d3d70f7 100644 --- a/tests/ceftests/resource_request_handler_unittest.cc +++ b/tests/ceftests/resource_request_handler_unittest.cc @@ -20,18 +20,165 @@ namespace { +// Normal stream resource handler implementation that additionally verifies +// calls to Cancel. +class NormalResourceHandler : public CefStreamResourceHandler { + public: + NormalResourceHandler(int status_code, + const CefString& status_text, + const CefString& mime_type, + CefResponse::HeaderMap header_map, + CefRefPtr stream, + const base::Closure& destroy_callback) + : CefStreamResourceHandler(status_code, + status_text, + mime_type, + header_map, + stream), + destroy_callback_(destroy_callback) {} + + ~NormalResourceHandler() override { + if (IsNetworkServiceEnabled()) + EXPECT_EQ(1, cancel_ct_); + else + EXPECT_EQ(0, cancel_ct_); + destroy_callback_.Run(); + } + + void Cancel() override { + EXPECT_IO_THREAD(); + cancel_ct_++; + } + + private: + const base::Closure destroy_callback_; + int cancel_ct_ = 0; + + DISALLOW_COPY_AND_ASSIGN(NormalResourceHandler); +}; + +// Resource handler implementation that never completes. Used to test +// destruction handling behavior for in-progress requests. +class IncompleteResourceHandler : public CefResourceHandler { + public: + enum TestMode { + BLOCK_PROCESS_REQUEST, + BLOCK_READ_RESPONSE, + }; + + IncompleteResourceHandler(TestMode test_mode, + const std::string& mime_type, + const base::Closure& destroy_callback) + : test_mode_(test_mode), + mime_type_(mime_type), + destroy_callback_(destroy_callback) {} + + ~IncompleteResourceHandler() override { + EXPECT_EQ(1, process_request_ct_); + + if (IsNetworkServiceEnabled()) + EXPECT_EQ(1, cancel_ct_); + else + EXPECT_EQ(0, cancel_ct_); + + if (test_mode_ == BLOCK_READ_RESPONSE) { + EXPECT_EQ(1, get_response_headers_ct_); + EXPECT_EQ(1, read_response_ct_); + } else { + EXPECT_EQ(0, get_response_headers_ct_); + EXPECT_EQ(0, read_response_ct_); + } + + destroy_callback_.Run(); + } + + bool ProcessRequest(CefRefPtr request, + CefRefPtr callback) override { + EXPECT_IO_THREAD(); + + process_request_ct_++; + + if (test_mode_ == BLOCK_PROCESS_REQUEST) { + // Never release or execute this callback. + incomplete_callback_ = callback; + } else { + callback->Continue(); + } + return true; + } + + void GetResponseHeaders(CefRefPtr response, + int64& response_length, + CefString& redirectUrl) override { + EXPECT_IO_THREAD(); + EXPECT_EQ(test_mode_, BLOCK_READ_RESPONSE); + + get_response_headers_ct_++; + + response->SetStatus(200); + response->SetStatusText("OK"); + response->SetMimeType(mime_type_); + response_length = 100; + } + + bool ReadResponse(void* data_out, + int bytes_to_read, + int& bytes_read, + CefRefPtr callback) override { + EXPECT_IO_THREAD(); + EXPECT_EQ(test_mode_, BLOCK_READ_RESPONSE); + + read_response_ct_++; + + // Never release or execute this callback. + incomplete_callback_ = callback; + bytes_read = 0; + return true; + } + + void Cancel() override { + EXPECT_IO_THREAD(); + cancel_ct_++; + } + + private: + const TestMode test_mode_; + const std::string mime_type_; + const base::Closure destroy_callback_; + + int process_request_ct_ = 0; + int get_response_headers_ct_ = 0; + int read_response_ct_ = 0; + int cancel_ct_ = 0; + + CefRefPtr incomplete_callback_; + + IMPLEMENT_REFCOUNTING(IncompleteResourceHandler); + DISALLOW_COPY_AND_ASSIGN(IncompleteResourceHandler); +}; + class BasicResponseTest : public TestHandler { public: enum TestMode { // Normal load, nothing fancy. LOAD, + // Don't continue from OnBeforeResourceLoad, then close the browser to + // verify destruction handling of in-progress requests. + INCOMPLETE_BEFORE_RESOURCE_LOAD, + // Modify the request (add headers) in OnBeforeResourceLoad. MODIFY_BEFORE_RESOURCE_LOAD, // Redirect the request (change the URL) in OnBeforeResourceLoad. REDIRECT_BEFORE_RESOURCE_LOAD, + // Return a CefResourceHandler from GetResourceHandler that never completes, + // then close the browser to verify destruction handling of in-progress + // requests. + INCOMPLETE_REQUEST_HANDLER_PROCESS_REQUEST, + INCOMPLETE_REQUEST_HANDLER_READ_RESPONSE, + // Redirect the request using a CefResourceHandler returned from // GetResourceHandler. REDIRECT_REQUEST_HANDLER, @@ -157,6 +304,14 @@ class BasicResponseTest : public TestHandler { on_before_resource_load_ct_++; + if (mode_ == INCOMPLETE_BEFORE_RESOURCE_LOAD) { + incomplete_callback_ = callback; + + // Close the browser asynchronously to complete the test. + CloseBrowserAsync(); + return RV_CONTINUE_ASYNC; + } + if (mode_ == MODIFY_BEFORE_RESOURCE_LOAD) { // Expect this data in the request for future callbacks. SetCustomHeader(request); @@ -182,6 +337,12 @@ class BasicResponseTest : public TestHandler { get_resource_handler_ct_++; + if (IsIncompleteRequestHandler()) { + // Close the browser asynchronously to complete the test. + CloseBrowserAsync(); + return GetIncompleteResource(); + } + const std::string& url = request->GetURL(); if (url == GetURL(RESULT_HTML) && mode_ == RESTART_RESOURCE_RESPONSE) { if (get_resource_handler_ct_ == 1) { @@ -314,7 +475,7 @@ class BasicResponseTest : public TestHandler { VerifyState(kOnResourceLoadComplete, request, response); - if (unhandled_) { + if (unhandled_ || IsIncomplete()) { EXPECT_EQ(UR_FAILED, status); EXPECT_EQ(0, received_content_length); } else { @@ -324,6 +485,10 @@ class BasicResponseTest : public TestHandler { } on_resource_load_complete_ct_++; + + if (IsIncomplete()) { + MaybeDestroyTest(false); + } } void OnProtocolExecution(CefRefPtr browser, @@ -474,19 +639,51 @@ class BasicResponseTest : public TestHandler { else EXPECT_EQ(1, on_resource_response_ct_); } + } else if (IsIncomplete()) { + EXPECT_EQ(1, on_before_browse_ct_); + if (IsNetworkServiceEnabled()) { + EXPECT_EQ(1, get_resource_request_handler_ct_); + EXPECT_EQ(1, get_cookie_access_filter_ct_); + } + EXPECT_EQ(1, on_before_resource_load_ct_); + + if (IsIncompleteRequestHandler()) + EXPECT_EQ(1, get_resource_handler_ct_); + else + EXPECT_EQ(0, get_resource_handler_ct_); + + EXPECT_EQ(0, on_resource_redirect_ct_); + + if (mode_ == INCOMPLETE_REQUEST_HANDLER_READ_RESPONSE) { + EXPECT_EQ(1, get_resource_response_filter_ct_); + EXPECT_EQ(1, on_resource_response_ct_); + } else { + EXPECT_EQ(0, get_resource_response_filter_ct_); + EXPECT_EQ(0, on_resource_response_ct_); + } } else { NOTREACHED(); } + EXPECT_EQ(resource_handler_created_ct_, resource_handler_destroyed_ct_); EXPECT_EQ(1, on_resource_load_complete_ct_); - EXPECT_EQ(1, on_load_end_ct_); - if (custom_scheme_ && unhandled_) + if (IsIncomplete()) + EXPECT_EQ(0, on_load_end_ct_); + else + EXPECT_EQ(1, on_load_end_ct_); + + if (custom_scheme_ && unhandled_ && !IsIncomplete()) EXPECT_EQ(1, on_protocol_execution_ct_); else EXPECT_EQ(0, on_protocol_execution_ct_); TestHandler::DestroyTest(); + + if (!SignalCompletionWhenAllBrowsersClose()) { + // Complete asynchronously so the call stack has a chance to unwind. + CefPostTask(TID_UI, base::Bind(&BasicResponseTest::TestComplete, this)); + } } private: @@ -518,7 +715,7 @@ class BasicResponseTest : public TestHandler { } const char* GetStartupURL() const { - if (IsLoad()) { + if (IsLoad() || IsIncomplete()) { return GetURL(RESULT_HTML); } else if (mode_ == REDIRECT_RESOURCE_REDIRECT) { return GetURL(REDIRECT2_HTML); @@ -537,17 +734,23 @@ class BasicResponseTest : public TestHandler { return "Redirect"; } - CefRefPtr GetOKResource() const { + base::Closure GetResourceDestroyCallback() { + resource_handler_created_ct_++; + return base::Bind(&BasicResponseTest::MaybeDestroyTest, this, true); + } + + CefRefPtr GetOKResource() { const std::string& body = GetResponseBody(); CefRefPtr stream = CefStreamReader::CreateForData( const_cast(body.c_str()), body.size()); - return new CefStreamResourceHandler(200, "OK", "text/html", - CefResponse::HeaderMap(), stream); + return new NormalResourceHandler(200, "OK", "text/html", + CefResponse::HeaderMap(), stream, + GetResourceDestroyCallback()); } CefRefPtr GetRedirectResource( - const std::string& redirect_url) const { + const std::string& redirect_url) { const std::string& body = GetRedirectBody(); CefRefPtr stream = CefStreamReader::CreateForData( const_cast(body.c_str()), body.size()); @@ -555,8 +758,17 @@ class BasicResponseTest : public TestHandler { CefResponse::HeaderMap headerMap; headerMap.insert(std::make_pair("Location", redirect_url)); - return new CefStreamResourceHandler(307, "Temporary Redirect", "text/html", - headerMap, stream); + return new NormalResourceHandler(307, "Temporary Redirect", "text/html", + headerMap, stream, + GetResourceDestroyCallback()); + } + + CefRefPtr GetIncompleteResource() { + return new IncompleteResourceHandler( + mode_ == INCOMPLETE_REQUEST_HANDLER_PROCESS_REQUEST + ? IncompleteResourceHandler::BLOCK_PROCESS_REQUEST + : IncompleteResourceHandler::BLOCK_READ_RESPONSE, + "text/html", GetResourceDestroyCallback()); } bool IsLoad() const { @@ -564,6 +776,16 @@ class BasicResponseTest : public TestHandler { mode_ == RESTART_RESOURCE_RESPONSE; } + bool IsIncompleteRequestHandler() const { + return mode_ == INCOMPLETE_REQUEST_HANDLER_PROCESS_REQUEST || + mode_ == INCOMPLETE_REQUEST_HANDLER_READ_RESPONSE; + } + + bool IsIncomplete() const { + return mode_ == INCOMPLETE_BEFORE_RESOURCE_LOAD || + IsIncompleteRequestHandler(); + } + bool IsRedirect() const { return mode_ == REDIRECT_BEFORE_RESOURCE_LOAD || mode_ == REDIRECT_REQUEST_HANDLER || @@ -630,7 +852,7 @@ class BasicResponseTest : public TestHandler { EXPECT_EQ(request_id_, request->GetIdentifier()) << callback; } - if (IsLoad()) { + if (IsLoad() || IsIncomplete()) { EXPECT_STREQ("GET", request->GetMethod().ToString().c_str()) << callback; EXPECT_STREQ(GetURL(RESULT_HTML), request->GetURL().ToString().c_str()) << callback; @@ -686,8 +908,18 @@ class BasicResponseTest : public TestHandler { mode_ == RESTART_RESOURCE_RESPONSE) && get_resource_handler_ct_ == 1; - if (unhandled_ && !override_unhandled) { - EXPECT_EQ(ERR_UNKNOWN_URL_SCHEME, response->GetError()) << callback; + // True for tests where the request will be incomplete and never receive a + // response. + const bool incomplete_unhandled = + (mode_ == INCOMPLETE_BEFORE_RESOURCE_LOAD || + mode_ == INCOMPLETE_REQUEST_HANDLER_PROCESS_REQUEST); + + if ((unhandled_ && !override_unhandled) || incomplete_unhandled) { + if (incomplete_unhandled) { + EXPECT_EQ(ERR_ABORTED, response->GetError()) << callback; + } else { + EXPECT_EQ(ERR_UNKNOWN_URL_SCHEME, response->GetError()) << callback; + } EXPECT_EQ(0, response->GetStatus()) << callback; EXPECT_STREQ("", response->GetStatusText().ToString().c_str()) << callback; @@ -695,7 +927,13 @@ class BasicResponseTest : public TestHandler { EXPECT_STREQ("", response->GetMimeType().ToString().c_str()) << callback; EXPECT_STREQ("", response->GetCharset().ToString().c_str()) << callback; } else { - EXPECT_EQ(ERR_NONE, response->GetError()) << callback; + if (mode_ == INCOMPLETE_REQUEST_HANDLER_READ_RESPONSE && + callback == kOnResourceLoadComplete) { + // We got a response, but we also got aborted. + EXPECT_EQ(ERR_ABORTED, response->GetError()) << callback; + } else { + EXPECT_EQ(ERR_NONE, response->GetError()) << callback; + } EXPECT_EQ(200, response->GetStatus()) << callback; EXPECT_STREQ("OK", response->GetStatusText().ToString().c_str()) << callback; @@ -719,6 +957,45 @@ class BasicResponseTest : public TestHandler { EXPECT_STREQ("", response->GetCharset().ToString().c_str()) << callback; } + void CloseBrowserAsync() { + EXPECT_TRUE(IsIncomplete()); + SetSignalCompletionWhenAllBrowsersClose(false); + CefPostDelayedTask( + TID_UI, base::Bind(&TestHandler::CloseBrowser, GetBrowser(), false), + 100); + } + + void MaybeDestroyTest(bool from_handler) { + if (!CefCurrentlyOn(TID_UI)) { + CefPostTask(TID_UI, base::Bind(&BasicResponseTest::MaybeDestroyTest, this, + from_handler)); + return; + } + + if (from_handler) { + resource_handler_destroyed_ct_++; + } + + bool destroy_test = false; + if (IsIncomplete()) { + // Destroy the test if we got OnResourceLoadComplete and either the + // resource handler will never complete or it was destroyed. + destroy_test = + on_resource_load_complete_ct_ > 0 && + (!IsIncompleteRequestHandler() || + resource_handler_destroyed_ct_ == resource_handler_created_ct_); + } else { + // Destroy the test if we got OnLoadEnd and the expected number of + // resource handlers were destroyed. + destroy_test = on_load_end_ct_ > 0 && resource_handler_destroyed_ct_ == + resource_handler_created_ct_; + } + + if (destroy_test) { + DestroyTest(); + } + } + const TestMode mode_; const bool custom_scheme_; const bool unhandled_; @@ -726,6 +1003,8 @@ class BasicResponseTest : public TestHandler { int browser_id_ = 0; uint64 request_id_ = 0U; + int resource_handler_created_ct_ = 0; + int on_before_browse_ct_ = 0; int on_load_end_ct_ = 0; @@ -738,6 +1017,10 @@ class BasicResponseTest : public TestHandler { int get_resource_response_filter_ct_ = 0; int on_resource_load_complete_ct_ = 0; int on_protocol_execution_ct_ = 0; + int resource_handler_destroyed_ct_ = 0; + + // Used with INCOMPLETE_BEFORE_RESOURCE_LOAD. + CefRefPtr incomplete_callback_; DISALLOW_COPY_AND_ASSIGN(BasicResponseTest); IMPLEMENT_REFCOUNTING(BasicResponseTest); @@ -746,10 +1029,21 @@ class BasicResponseTest : public TestHandler { bool IsTestSupported(BasicResponseTest::TestMode test_mode, bool custom_scheme, bool unhandled) { - if (!IsNetworkServiceEnabled() && (custom_scheme || unhandled)) { - // The old network implementation does not support the same functionality - // for custom schemes and unhandled requests. - return false; + if (!IsNetworkServiceEnabled()) { + if (custom_scheme || unhandled) { + // The old network implementation does not support the same functionality + // for custom schemes and unhandled requests. + return false; + } + if (test_mode == BasicResponseTest::INCOMPLETE_BEFORE_RESOURCE_LOAD || + test_mode == + BasicResponseTest::INCOMPLETE_REQUEST_HANDLER_PROCESS_REQUEST || + test_mode == + BasicResponseTest::INCOMPLETE_REQUEST_HANDLER_READ_RESPONSE) { + // The old network implementation does not support the same behavior + // for canceling incomplete requests. + return false; + } } return true; } @@ -782,11 +1076,23 @@ bool IsTestSupported(BasicResponseTest::TestMode test_mode, BASIC_TEST(name##RestartResourceResponse, RESTART_RESOURCE_RESPONSE, custom, \ unhandled) +// Tests only supported in handled mode. +#define BASIC_TEST_HANDLED_MODES(name, custom) \ + BASIC_TEST(name##IncompleteBeforeResourceLoad, \ + INCOMPLETE_BEFORE_RESOURCE_LOAD, custom, false) \ + BASIC_TEST(name##IncompleteRequestHandlerProcessRequest, \ + INCOMPLETE_REQUEST_HANDLER_PROCESS_REQUEST, custom, false) \ + BASIC_TEST(name##IncompleteRequestHandlerReadResponse, \ + INCOMPLETE_REQUEST_HANDLER_READ_RESPONSE, custom, false) + BASIC_TEST_ALL_MODES(StandardHandled, false, false) BASIC_TEST_ALL_MODES(StandardUnhandled, false, true) BASIC_TEST_ALL_MODES(CustomHandled, true, false) BASIC_TEST_ALL_MODES(CustomUnhandled, true, true) +BASIC_TEST_HANDLED_MODES(StandardHandled, false) +BASIC_TEST_HANDLED_MODES(CustomHandled, true) + namespace { const char kSubresourceProcessMsg[] = "SubresourceMsg"; @@ -797,12 +1103,22 @@ class SubresourceResponseTest : public RoutingTestHandler { // Normal load, nothing fancy. LOAD, + // Don't continue from OnBeforeResourceLoad, then close the browser to + // verify destruction handling of in-progress requests. + INCOMPLETE_BEFORE_RESOURCE_LOAD, + // Modify the request (add headers) in OnBeforeResourceLoad. MODIFY_BEFORE_RESOURCE_LOAD, // Redirect the request (change the URL) in OnBeforeResourceLoad. REDIRECT_BEFORE_RESOURCE_LOAD, + // Return a CefResourceHandler from GetResourceHandler that never completes, + // then close the browser to verify destruction handling of in-progress + // requests. + INCOMPLETE_REQUEST_HANDLER_PROCESS_REQUEST, + INCOMPLETE_REQUEST_HANDLER_READ_RESPONSE, + // Redirect the request using a CefResourceHandler returned from // GetResourceHandler. REDIRECT_REQUEST_HANDLER, @@ -979,6 +1295,14 @@ class SubresourceResponseTest : public RoutingTestHandler { on_before_resource_load_ct_++; + if (mode_ == INCOMPLETE_BEFORE_RESOURCE_LOAD) { + incomplete_callback_ = callback; + + // Close the browser asynchronously to complete the test. + CloseBrowserAsync(); + return RV_CONTINUE_ASYNC; + } + if (mode_ == MODIFY_BEFORE_RESOURCE_LOAD) { // Expect this data in the request for future callbacks. SetCustomHeader(request); @@ -1014,6 +1338,12 @@ class SubresourceResponseTest : public RoutingTestHandler { get_resource_handler_ct_++; + if (IsIncompleteRequestHandler()) { + // Close the browser asynchronously to complete the test. + CloseBrowserAsync(); + return GetIncompleteResource(); + } + const std::string& url = request->GetURL(); if (url == GetURL(RESULT_JS) && mode_ == RESTART_RESOURCE_RESPONSE) { if (get_resource_handler_ct_ == 1) { @@ -1188,7 +1518,7 @@ class SubresourceResponseTest : public RoutingTestHandler { VerifyState(kOnResourceLoadComplete, request, response); - if (unhandled_) { + if (unhandled_ || IsIncomplete()) { EXPECT_EQ(UR_FAILED, status); EXPECT_EQ(0, received_content_length); } else { @@ -1198,6 +1528,10 @@ class SubresourceResponseTest : public RoutingTestHandler { } on_resource_load_complete_ct_++; + + if (IsIncomplete()) { + MaybeDestroyTest(false); + } } void OnProtocolExecution(CefRefPtr browser, @@ -1235,7 +1569,7 @@ class SubresourceResponseTest : public RoutingTestHandler { on_load_end_ct_++; TestHandler::OnLoadEnd(browser, frame, httpStatusCode); - MaybeDestroyTest(); + MaybeDestroyTest(false); } bool OnQuery(CefRefPtr browser, @@ -1254,18 +1588,11 @@ class SubresourceResponseTest : public RoutingTestHandler { callback->Success(""); on_query_ct_++; - MaybeDestroyTest(); + MaybeDestroyTest(false); return true; } - void MaybeDestroyTest() { - if (on_load_end_ct_ > (subframe_ ? 1 : 0) && - (on_query_ct_ > 0 || unhandled_)) { - DestroyTest(); - } - } - void DestroyTest() override { if (!IsNetworkServiceEnabled()) { // Called once for each other callback. @@ -1380,29 +1707,61 @@ class SubresourceResponseTest : public RoutingTestHandler { else EXPECT_EQ(1, on_resource_response_ct_); } + } else if (IsIncomplete()) { + if (IsNetworkServiceEnabled()) { + EXPECT_EQ(1, get_resource_request_handler_ct_); + EXPECT_EQ(1, get_cookie_access_filter_ct_); + } + EXPECT_EQ(1, on_before_resource_load_ct_); + + if (IsIncompleteRequestHandler()) + EXPECT_EQ(1, get_resource_handler_ct_); + else + EXPECT_EQ(0, get_resource_handler_ct_); + + EXPECT_EQ(0, on_resource_redirect_ct_); + + if (mode_ == INCOMPLETE_REQUEST_HANDLER_READ_RESPONSE) { + EXPECT_EQ(1, get_resource_response_filter_ct_); + EXPECT_EQ(1, on_resource_response_ct_); + } else { + EXPECT_EQ(0, get_resource_response_filter_ct_); + EXPECT_EQ(0, on_resource_response_ct_); + } } else { NOTREACHED(); } + EXPECT_EQ(resource_handler_created_ct_, resource_handler_destroyed_ct_); EXPECT_EQ(1, on_resource_load_complete_ct_); // Only called for the main and/or sub frame load. - if (subframe_) - EXPECT_EQ(2, on_load_end_ct_); - else - EXPECT_EQ(1, on_load_end_ct_); + if (IsIncomplete()) { + EXPECT_EQ(0, on_load_end_ct_); + } else { + if (subframe_) + EXPECT_EQ(2, on_load_end_ct_); + else + EXPECT_EQ(1, on_load_end_ct_); + } - if (unhandled_) + if (unhandled_ || IsIncomplete()) EXPECT_EQ(0, on_query_ct_); else EXPECT_EQ(1, on_query_ct_); - if (custom_scheme_ && unhandled_) + if (custom_scheme_ && unhandled_ && !IsIncomplete()) EXPECT_EQ(1, on_protocol_execution_ct_); else EXPECT_EQ(0, on_protocol_execution_ct_); TestHandler::DestroyTest(); + + if (!SignalCompletionWhenAllBrowsersClose()) { + // Complete asynchronously so the call stack has a chance to unwind. + CefPostTask(TID_UI, + base::Bind(&SubresourceResponseTest::TestComplete, this)); + } } private: @@ -1461,7 +1820,7 @@ class SubresourceResponseTest : public RoutingTestHandler { } const char* GetStartupURL() const { - if (IsLoad()) { + if (IsLoad() || IsIncomplete()) { return GetURL(RESULT_JS); } else if (mode_ == REDIRECT_RESOURCE_REDIRECT) { return GetURL(REDIRECT2_JS); @@ -1511,35 +1870,43 @@ class SubresourceResponseTest : public RoutingTestHandler { return "Redirect"; } - CefRefPtr GetMainResource() const { + base::Closure GetResourceDestroyCallback() { + resource_handler_created_ct_++; + return base::Bind(&SubresourceResponseTest::MaybeDestroyTest, this, true); + } + + CefRefPtr GetMainResource() { const std::string& body = GetMainResponseBody(); CefRefPtr stream = CefStreamReader::CreateForData( const_cast(body.c_str()), body.size()); - return new CefStreamResourceHandler(200, "OK", "text/html", - CefResponse::HeaderMap(), stream); + return new NormalResourceHandler(200, "OK", "text/html", + CefResponse::HeaderMap(), stream, + GetResourceDestroyCallback()); } - CefRefPtr GetSubResource() const { + CefRefPtr GetSubResource() { const std::string& body = GetSubResponseBody(); CefRefPtr stream = CefStreamReader::CreateForData( const_cast(body.c_str()), body.size()); - return new CefStreamResourceHandler(200, "OK", "text/html", - CefResponse::HeaderMap(), stream); + return new NormalResourceHandler(200, "OK", "text/html", + CefResponse::HeaderMap(), stream, + GetResourceDestroyCallback()); } - CefRefPtr GetOKResource() const { + CefRefPtr GetOKResource() { const std::string& body = GetResponseBody(); CefRefPtr stream = CefStreamReader::CreateForData( const_cast(body.c_str()), body.size()); - return new CefStreamResourceHandler(200, "OK", "text/javascript", - CefResponse::HeaderMap(), stream); + return new NormalResourceHandler(200, "OK", "text/javascript", + CefResponse::HeaderMap(), stream, + GetResourceDestroyCallback()); } CefRefPtr GetRedirectResource( - const std::string& redirect_url) const { + const std::string& redirect_url) { const std::string& body = GetRedirectBody(); CefRefPtr stream = CefStreamReader::CreateForData( const_cast(body.c_str()), body.size()); @@ -1547,8 +1914,17 @@ class SubresourceResponseTest : public RoutingTestHandler { CefResponse::HeaderMap headerMap; headerMap.insert(std::make_pair("Location", redirect_url)); - return new CefStreamResourceHandler(307, "Temporary Redirect", - "text/javascript", headerMap, stream); + return new NormalResourceHandler(307, "Temporary Redirect", + "text/javascript", headerMap, stream, + GetResourceDestroyCallback()); + } + + CefRefPtr GetIncompleteResource() { + return new IncompleteResourceHandler( + mode_ == INCOMPLETE_REQUEST_HANDLER_PROCESS_REQUEST + ? IncompleteResourceHandler::BLOCK_PROCESS_REQUEST + : IncompleteResourceHandler::BLOCK_READ_RESPONSE, + "text/javascript", GetResourceDestroyCallback()); } bool IsLoad() const { @@ -1556,6 +1932,16 @@ class SubresourceResponseTest : public RoutingTestHandler { mode_ == RESTART_RESOURCE_RESPONSE; } + bool IsIncompleteRequestHandler() const { + return mode_ == INCOMPLETE_REQUEST_HANDLER_PROCESS_REQUEST || + mode_ == INCOMPLETE_REQUEST_HANDLER_READ_RESPONSE; + } + + bool IsIncomplete() const { + return mode_ == INCOMPLETE_BEFORE_RESOURCE_LOAD || + IsIncompleteRequestHandler(); + } + bool IsRedirect() const { return mode_ == REDIRECT_BEFORE_RESOURCE_LOAD || mode_ == REDIRECT_REQUEST_HANDLER || @@ -1633,7 +2019,7 @@ class SubresourceResponseTest : public RoutingTestHandler { EXPECT_EQ(request_id_, request->GetIdentifier()) << callback; } - if (IsLoad()) { + if (IsLoad() || IsIncomplete()) { EXPECT_STREQ("GET", request->GetMethod().ToString().c_str()) << callback; EXPECT_STREQ(GetURL(RESULT_JS), request->GetURL().ToString().c_str()) << callback; @@ -1697,8 +2083,18 @@ class SubresourceResponseTest : public RoutingTestHandler { mode_ == RESTART_RESOURCE_RESPONSE) && get_resource_handler_ct_ == 1; - if (unhandled_ && !override_unhandled) { - EXPECT_EQ(ERR_UNKNOWN_URL_SCHEME, response->GetError()) << callback; + // True for tests where the request will be incomplete and never receive a + // response. + const bool incomplete_unhandled = + (mode_ == INCOMPLETE_BEFORE_RESOURCE_LOAD || + mode_ == INCOMPLETE_REQUEST_HANDLER_PROCESS_REQUEST); + + if ((unhandled_ && !override_unhandled) || incomplete_unhandled) { + if (incomplete_unhandled) { + EXPECT_EQ(ERR_ABORTED, response->GetError()) << callback; + } else { + EXPECT_EQ(ERR_UNKNOWN_URL_SCHEME, response->GetError()) << callback; + } EXPECT_EQ(0, response->GetStatus()) << callback; EXPECT_STREQ("", response->GetStatusText().ToString().c_str()) << callback; @@ -1706,7 +2102,13 @@ class SubresourceResponseTest : public RoutingTestHandler { EXPECT_STREQ("", response->GetMimeType().ToString().c_str()) << callback; EXPECT_STREQ("", response->GetCharset().ToString().c_str()) << callback; } else { - EXPECT_EQ(ERR_NONE, response->GetError()) << callback; + if (mode_ == INCOMPLETE_REQUEST_HANDLER_READ_RESPONSE && + callback == kOnResourceLoadComplete) { + // We got a response, but we also got aborted. + EXPECT_EQ(ERR_ABORTED, response->GetError()) << callback; + } else { + EXPECT_EQ(ERR_NONE, response->GetError()) << callback; + } EXPECT_EQ(200, response->GetStatus()) << callback; EXPECT_STREQ("OK", response->GetStatusText().ToString().c_str()) << callback; @@ -1731,6 +2133,47 @@ class SubresourceResponseTest : public RoutingTestHandler { EXPECT_STREQ("", response->GetCharset().ToString().c_str()) << callback; } + void CloseBrowserAsync() { + EXPECT_TRUE(IsIncomplete()); + SetSignalCompletionWhenAllBrowsersClose(false); + CefPostDelayedTask( + TID_UI, base::Bind(&TestHandler::CloseBrowser, GetBrowser(), false), + 100); + } + + void MaybeDestroyTest(bool from_handler) { + if (!CefCurrentlyOn(TID_UI)) { + CefPostTask(TID_UI, base::Bind(&SubresourceResponseTest::MaybeDestroyTest, + this, from_handler)); + return; + } + + if (from_handler) { + resource_handler_destroyed_ct_++; + } + + bool destroy_test = false; + if (IsIncomplete()) { + // Destroy the test if we got OnResourceLoadComplete and either the + // resource handler will never complete or it was destroyed. + destroy_test = + on_resource_load_complete_ct_ > 0 && + (!IsIncompleteRequestHandler() || + resource_handler_destroyed_ct_ == resource_handler_created_ct_); + } else { + // Destroy the test if we got the expected number of OnLoadEnd and + // OnQuery, and the expected number of resource handlers were destroyed. + destroy_test = + on_load_end_ct_ > (subframe_ ? 1 : 0) && + (on_query_ct_ > 0 || unhandled_) && + resource_handler_destroyed_ct_ == resource_handler_created_ct_; + } + + if (destroy_test) { + DestroyTest(); + } + } + const TestMode mode_; const bool custom_scheme_; const bool unhandled_; @@ -1740,6 +2183,8 @@ class SubresourceResponseTest : public RoutingTestHandler { int64 frame_id_ = 0; uint64 request_id_ = 0U; + int resource_handler_created_ct_ = 0; + int on_before_browse_ct_ = 0; int on_load_end_ct_ = 0; int on_query_ct_ = 0; @@ -1753,6 +2198,10 @@ class SubresourceResponseTest : public RoutingTestHandler { int get_resource_response_filter_ct_ = 0; int on_resource_load_complete_ct_ = 0; int on_protocol_execution_ct_ = 0; + int resource_handler_destroyed_ct_ = 0; + + // Used with INCOMPLETE_BEFORE_RESOURCE_LOAD. + CefRefPtr incomplete_callback_; DISALLOW_COPY_AND_ASSIGN(SubresourceResponseTest); IMPLEMENT_REFCOUNTING(SubresourceResponseTest); @@ -1762,10 +2211,21 @@ bool IsTestSupported(SubresourceResponseTest::TestMode test_mode, bool custom_scheme, bool unhandled, bool subframe) { - if (!IsNetworkServiceEnabled() && (custom_scheme || unhandled)) { - // The old network implementation does not support the same functionality - // for custom schemes and unhandled requests. - return false; + if (!IsNetworkServiceEnabled()) { + if (custom_scheme || unhandled) { + // The old network implementation does not support the same functionality + // for custom schemes and unhandled requests. + return false; + } + if (test_mode == SubresourceResponseTest::INCOMPLETE_BEFORE_RESOURCE_LOAD || + test_mode == SubresourceResponseTest:: + INCOMPLETE_REQUEST_HANDLER_PROCESS_REQUEST || + test_mode == + SubresourceResponseTest::INCOMPLETE_REQUEST_HANDLER_READ_RESPONSE) { + // The old network implementation does not support the same behavior + // for canceling incomplete requests. + return false; + } } return true; } @@ -1799,6 +2259,17 @@ bool IsTestSupported(SubresourceResponseTest::TestMode test_mode, SUBRESOURCE_TEST(name##RestartResourceResponse, RESTART_RESOURCE_RESPONSE, \ custom, unhandled, subframe) +// Tests only supported in handled mode. +#define SUBRESOURCE_TEST_HANDLED_MODES(name, custom, subframe) \ + SUBRESOURCE_TEST(name##IncompleteBeforeResourceLoad, \ + INCOMPLETE_BEFORE_RESOURCE_LOAD, custom, false, subframe) \ + SUBRESOURCE_TEST(name##IncompleteRequestHandlerProcessRequest, \ + INCOMPLETE_REQUEST_HANDLER_PROCESS_REQUEST, custom, false, \ + subframe) \ + SUBRESOURCE_TEST(name##IncompleteRequestHandlerReadResponse, \ + INCOMPLETE_REQUEST_HANDLER_READ_RESPONSE, custom, false, \ + subframe) + SUBRESOURCE_TEST_ALL_MODES(StandardHandledMainFrame, false, false, false) SUBRESOURCE_TEST_ALL_MODES(StandardUnhandledMainFrame, false, true, false) SUBRESOURCE_TEST_ALL_MODES(CustomHandledMainFrame, true, false, false) @@ -1809,6 +2280,12 @@ SUBRESOURCE_TEST_ALL_MODES(StandardUnhandledSubFrame, false, true, true) SUBRESOURCE_TEST_ALL_MODES(CustomHandledSubFrame, true, false, true) SUBRESOURCE_TEST_ALL_MODES(CustomUnhandledSubFrame, true, true, true) +SUBRESOURCE_TEST_HANDLED_MODES(StandardHandledMainFrame, false, false) +SUBRESOURCE_TEST_HANDLED_MODES(CustomHandledMainFrame, true, false) + +SUBRESOURCE_TEST_HANDLED_MODES(StandardHandledSubFrame, false, true) +SUBRESOURCE_TEST_HANDLED_MODES(CustomHandledSubFrame, true, true) + namespace { const char kResourceTestHtml[] = "http://test.com/resource.html"; diff --git a/tests/ceftests/urlrequest_unittest.cc b/tests/ceftests/urlrequest_unittest.cc index 75d999e30..1015bb263 100644 --- a/tests/ceftests/urlrequest_unittest.cc +++ b/tests/ceftests/urlrequest_unittest.cc @@ -34,8 +34,9 @@ using client::ClientAppRenderer; namespace { // Unique values for URLRequest tests. -const char* kRequestTestUrl = "http://tests/URLRequestTest.Test"; -const char* kRequestTestMsg = "URLRequestTest.Test"; +const char kRequestTestUrl[] = "http://tests/URLRequestTest.Test"; +const char kRequestTestMsg[] = "URLRequestTest.Test"; +const char kIncompleteRequestTestMsg[] = "URLRequestTest.IncompleteRequestTest"; // TEST DATA @@ -53,6 +54,9 @@ const char kRequestSaveCookieName[] = "urcookie_save"; const char kCacheControlHeader[] = "cache-control"; +// Used with incomplete tests for data that should not be sent. +const char kIncompleteDoNotSendData[] = "DO NOT SEND"; + enum RequestTestMode { REQTEST_GET = 0, REQTEST_GET_NODATA, @@ -75,6 +79,8 @@ enum RequestTestMode { REQTEST_CACHE_ONLY_SUCCESS_HEADER, REQTEST_CACHE_DISABLE_FLAG, REQTEST_CACHE_DISABLE_HEADER, + REQTEST_INCOMPLETE_PROCESS_REQUEST, + REQTEST_INCOMPLETE_READ_RESPONSE, }; enum ContextTestMode { @@ -85,18 +91,7 @@ enum ContextTestMode { // Defines test expectations for a request. struct RequestRunSettings { - RequestRunSettings() - : expect_upload_progress(false), - expect_download_progress(true), - expect_download_data(true), - expected_status(UR_SUCCESS), - expected_error_code(ERR_NONE), - expect_send_cookie(false), - expect_save_cookie(false), - expect_follow_redirect(true), - expect_response_was_cached(false), - expected_send_count(-1), - expected_receive_count(-1) {} + RequestRunSettings() {} // Set expectations for request failure. void SetRequestFailureExpected(cef_errorcode_t error_code) { @@ -118,44 +113,52 @@ struct RequestRunSettings { // Optional response data that will be returned by the backend. std::string response_data; + // Create an incomplete request to test shutdown behavior. + enum IncompleteType { + INCOMPLETE_NONE, + INCOMPLETE_PROCESS_REQUEST, + INCOMPLETE_READ_RESPONSE, + }; + IncompleteType incomplete_type = INCOMPLETE_NONE; + // If true upload progress notification will be expected. - bool expect_upload_progress; + bool expect_upload_progress = false; // If true download progress notification will be expected. - bool expect_download_progress; + bool expect_download_progress = true; // If true download data will be expected. - bool expect_download_data; + bool expect_download_data = true; // Expected status value. - CefURLRequest::Status expected_status; + CefURLRequest::Status expected_status = UR_SUCCESS; // Expected error code value. - CefURLRequest::ErrorCode expected_error_code; + CefURLRequest::ErrorCode expected_error_code = ERR_NONE; // If true the request cookie should be sent to the server. - bool expect_send_cookie; + bool expect_send_cookie = false; // If true the response cookie should be saved. - bool expect_save_cookie; + bool expect_save_cookie = false; // If specified the test will begin with this redirect request and response. CefRefPtr redirect_request; CefRefPtr redirect_response; // If true the redirect is expected to be followed. - bool expect_follow_redirect; + bool expect_follow_redirect = true; // If true the response is expected to be served from cache. - bool expect_response_was_cached; + bool expect_response_was_cached = false; // The expected number of requests to send, or -1 if unspecified. // Used only with the server backend. - int expected_send_count; + int expected_send_count = -1; // The expected number of requests to receive, or -1 if unspecified. // Used only with the server backend. - int expected_receive_count; + int expected_receive_count = -1; typedef base::Callback @@ -472,12 +475,20 @@ void GetNormalResponse(const RequestRunSettings* settings, // Serves request responses. class RequestSchemeHandler : public CefResourceHandler { public: - explicit RequestSchemeHandler(RequestRunSettings* settings) - : settings_(settings), offset_(0) {} + RequestSchemeHandler(RequestRunSettings* settings, + const base::Closure& destroy_callback) + : settings_(settings), destroy_callback_(destroy_callback) {} + + ~RequestSchemeHandler() override { + if (IsNetworkServiceEnabled()) + EXPECT_EQ(1, cancel_ct_); + + destroy_callback_.Run(); + } bool ProcessRequest(CefRefPtr request, CefRefPtr callback) override { - EXPECT_TRUE(CefCurrentlyOn(TID_IO)); + EXPECT_IO_THREAD(); VerifyNormalRequest(settings_, request, false); // HEAD requests are identical to GET requests except no response data is @@ -493,7 +504,7 @@ class RequestSchemeHandler : public CefResourceHandler { void GetResponseHeaders(CefRefPtr response, int64& response_length, CefString& redirectUrl) override { - EXPECT_TRUE(CefCurrentlyOn(TID_IO)); + EXPECT_IO_THREAD(); GetNormalResponse(settings_, response); response_length = response_data_.length(); } @@ -502,7 +513,7 @@ class RequestSchemeHandler : public CefResourceHandler { int bytes_to_read, int& bytes_read, CefRefPtr callback) override { - EXPECT_TRUE(CefCurrentlyOn(TID_IO)); + EXPECT_IO_THREAD(); bool has_data = false; bytes_read = 0; @@ -522,14 +533,20 @@ class RequestSchemeHandler : public CefResourceHandler { return has_data; } - void Cancel() override { EXPECT_TRUE(CefCurrentlyOn(TID_IO)); } + void Cancel() override { + EXPECT_IO_THREAD(); + cancel_ct_++; + } private: // |settings_| is not owned by this object. RequestRunSettings* settings_; + base::Closure destroy_callback_; std::string response_data_; - size_t offset_; + size_t offset_ = 0; + + int cancel_ct_ = 0; IMPLEMENT_REFCOUNTING(RequestSchemeHandler); }; @@ -537,13 +554,23 @@ class RequestSchemeHandler : public CefResourceHandler { // Serves redirect request responses. class RequestRedirectSchemeHandler : public CefResourceHandler { public: - explicit RequestRedirectSchemeHandler(CefRefPtr request, - CefRefPtr response) - : request_(request), response_(response) {} + RequestRedirectSchemeHandler(CefRefPtr request, + CefRefPtr response, + const base::Closure& destroy_callback) + : request_(request), + response_(response), + destroy_callback_(destroy_callback) {} + + ~RequestRedirectSchemeHandler() override { + if (IsNetworkServiceEnabled()) + EXPECT_EQ(1, cancel_ct_); + + destroy_callback_.Run(); + } bool ProcessRequest(CefRefPtr request, CefRefPtr callback) override { - EXPECT_TRUE(CefCurrentlyOn(TID_IO)); + EXPECT_IO_THREAD(); // Verify that the request was sent correctly. TestRequestEqual(request_, request, true); @@ -556,7 +583,7 @@ class RequestRedirectSchemeHandler : public CefResourceHandler { void GetResponseHeaders(CefRefPtr response, int64& response_length, CefString& redirectUrl) override { - EXPECT_TRUE(CefCurrentlyOn(TID_IO)); + EXPECT_IO_THREAD(); response->SetStatus(response_->GetStatus()); response->SetStatusText(response_->GetStatusText()); @@ -573,20 +600,128 @@ class RequestRedirectSchemeHandler : public CefResourceHandler { int bytes_to_read, int& bytes_read, CefRefPtr callback) override { - EXPECT_TRUE(CefCurrentlyOn(TID_IO)); + EXPECT_IO_THREAD(); NOTREACHED(); return false; } - void Cancel() override { EXPECT_TRUE(CefCurrentlyOn(TID_IO)); } + void Cancel() override { + EXPECT_IO_THREAD(); + cancel_ct_++; + } private: CefRefPtr request_; CefRefPtr response_; + base::Closure destroy_callback_; + + int cancel_ct_ = 0; IMPLEMENT_REFCOUNTING(RequestRedirectSchemeHandler); }; +// Resource handler implementation that never completes. Used to test +// destruction handling behavior for in-progress requests. +class IncompleteSchemeHandler : public CefResourceHandler { + public: + IncompleteSchemeHandler(RequestRunSettings* settings, + const base::Closure& destroy_callback) + : settings_(settings), destroy_callback_(destroy_callback) { + EXPECT_NE(settings_->incomplete_type, RequestRunSettings::INCOMPLETE_NONE); + } + + ~IncompleteSchemeHandler() override { + EXPECT_EQ(1, process_request_ct_); + + if (IsNetworkServiceEnabled()) + EXPECT_EQ(1, cancel_ct_); + else + EXPECT_EQ(0, cancel_ct_); + + if (settings_->incomplete_type == + RequestRunSettings::INCOMPLETE_READ_RESPONSE) { + EXPECT_EQ(1, get_response_headers_ct_); + EXPECT_EQ(1, read_response_ct_); + } else { + EXPECT_EQ(0, get_response_headers_ct_); + EXPECT_EQ(0, read_response_ct_); + } + + destroy_callback_.Run(); + } + + bool ProcessRequest(CefRefPtr request, + CefRefPtr callback) override { + EXPECT_IO_THREAD(); + + process_request_ct_++; + + if (settings_->incomplete_type == + RequestRunSettings::INCOMPLETE_PROCESS_REQUEST) { + // Never release or execute this callback. + incomplete_callback_ = callback; + } else { + callback->Continue(); + } + return true; + } + + void GetResponseHeaders(CefRefPtr response, + int64& response_length, + CefString& redirectUrl) override { + EXPECT_IO_THREAD(); + EXPECT_EQ(settings_->incomplete_type, + RequestRunSettings::INCOMPLETE_READ_RESPONSE); + + get_response_headers_ct_++; + + response->SetStatus(settings_->response->GetStatus()); + response->SetStatusText(settings_->response->GetStatusText()); + response->SetMimeType(settings_->response->GetMimeType()); + + CefResponse::HeaderMap headerMap; + settings_->response->GetHeaderMap(headerMap); + settings_->response->SetHeaderMap(headerMap); + + response_length = static_cast(settings_->response_data.size()); + } + + bool ReadResponse(void* data_out, + int bytes_to_read, + int& bytes_read, + CefRefPtr callback) override { + EXPECT_IO_THREAD(); + EXPECT_EQ(settings_->incomplete_type, + RequestRunSettings::INCOMPLETE_READ_RESPONSE); + + read_response_ct_++; + + // Never release or execute this callback. + incomplete_callback_ = callback; + bytes_read = 0; + return true; + } + + void Cancel() override { + EXPECT_IO_THREAD(); + cancel_ct_++; + } + + private: + RequestRunSettings* const settings_; + const base::Closure destroy_callback_; + + int process_request_ct_ = 0; + int get_response_headers_ct_ = 0; + int read_response_ct_ = 0; + int cancel_ct_ = 0; + + CefRefPtr incomplete_callback_; + + IMPLEMENT_REFCOUNTING(IncompleteSchemeHandler); + DISALLOW_COPY_AND_ASSIGN(IncompleteSchemeHandler); +}; + class RequestSchemeHandlerFactory : public CefSchemeHandlerFactory { public: RequestSchemeHandlerFactory() {} @@ -595,13 +730,22 @@ class RequestSchemeHandlerFactory : public CefSchemeHandlerFactory { CefRefPtr frame, const CefString& scheme_name, CefRefPtr request) override { - EXPECT_TRUE(CefCurrentlyOn(TID_IO)); + EXPECT_IO_THREAD(); + + handler_create_ct_++; + const base::Closure destroy_callback = + base::Bind(&RequestSchemeHandlerFactory::OnHandlerDestroyed, this); + RequestDataMap::Entry entry = data_map_.Find(request->GetURL()); if (entry.type == RequestDataMap::Entry::TYPE_NORMAL) { - return new RequestSchemeHandler(entry.settings); + if (entry.settings->incomplete_type == + RequestRunSettings::INCOMPLETE_NONE) { + return new RequestSchemeHandler(entry.settings, destroy_callback); + } + return new IncompleteSchemeHandler(entry.settings, destroy_callback); } else if (entry.type == RequestDataMap::Entry::TYPE_REDIRECT) { - return new RequestRedirectSchemeHandler(entry.redirect_request, - entry.redirect_response); + return new RequestRedirectSchemeHandler( + entry.redirect_request, entry.redirect_response, destroy_callback); } // Unknown test. @@ -629,6 +773,19 @@ class RequestSchemeHandlerFactory : public CefSchemeHandlerFactory { data_map_.AddSchemeHandler(settings); } + void OnHandlerDestroyed() { + if (!CefCurrentlyOn(TID_IO)) { + CefPostTask( + TID_IO, + base::Bind(&RequestSchemeHandlerFactory::OnHandlerDestroyed, this)); + return; + } + + handler_destroy_ct_++; + + MaybeShutdown(); + } + void Shutdown(const base::Closure& complete_callback) { if (!CefCurrentlyOn(TID_IO)) { CefPostTask(TID_IO, base::Bind(&RequestSchemeHandlerFactory::Shutdown, @@ -636,13 +793,29 @@ class RequestSchemeHandlerFactory : public CefSchemeHandlerFactory { return; } + EXPECT_TRUE(shutdown_callback_.is_null()); + shutdown_callback_ = complete_callback; + data_map_.SetOwnerTaskRunner(nullptr); - complete_callback.Run(); + + MaybeShutdown(); } private: + void MaybeShutdown() { + if (!shutdown_callback_.is_null() && + handler_create_ct_ == handler_destroy_ct_) { + shutdown_callback_.Run(); + shutdown_callback_.Reset(); + } + } + RequestDataMap data_map_; + int handler_create_ct_ = 0; + int handler_destroy_ct_ = 0; + base::Closure shutdown_callback_; + IMPLEMENT_REFCOUNTING(RequestSchemeHandlerFactory); }; @@ -878,7 +1051,12 @@ class RequestServerHandler : public CefServerHandler { int connection_id, CefRefPtr response, const std::string& response_data) { - int response_code = response->GetStatus(); + const int response_code = response->GetStatus(); + if (response_code <= 0) { + // Intentionally not responding for incomplete request tests. + return; + } + const CefString& content_type = response->GetMimeType(); int64 content_length = static_cast(response_data.size()); @@ -888,6 +1066,11 @@ class RequestServerHandler : public CefServerHandler { server->SendHttpResponse(connection_id, response_code, content_type, content_length, extra_headers); + if (response_data == kIncompleteDoNotSendData) { + // Intentionally not sending data for incomplete request tests. + return; + } + if (content_length != 0) { server->SendRawData(connection_id, response_data.data(), response_data.size()); @@ -949,16 +1132,7 @@ class RequestClient : public CefURLRequestClient { RequestCompleteCallback; explicit RequestClient(const RequestCompleteCallback& complete_callback) - : complete_callback_(complete_callback), - request_complete_ct_(0), - upload_progress_ct_(0), - download_progress_ct_(0), - download_data_ct_(0), - upload_total_(0), - download_total_(0), - status_(UR_UNKNOWN), - error_code_(ERR_NONE), - response_was_cached_(false) { + : complete_callback_(complete_callback) { EXPECT_FALSE(complete_callback_.is_null()); } @@ -1015,22 +1189,22 @@ class RequestClient : public CefURLRequestClient { } private: - RequestCompleteCallback complete_callback_; + const RequestCompleteCallback complete_callback_; public: - int request_complete_ct_; - int upload_progress_ct_; - int download_progress_ct_; - int download_data_ct_; + int request_complete_ct_ = 0; + int upload_progress_ct_ = 0; + int download_progress_ct_ = 0; + int download_data_ct_ = 0; - int64 upload_total_; - int64 download_total_; + int64 upload_total_ = 0; + int64 download_total_ = 0; std::string download_data_; CefRefPtr request_; - CefURLRequest::Status status_; - CefURLRequest::ErrorCode error_code_; + CefURLRequest::Status status_ = UR_UNKNOWN; + CefURLRequest::ErrorCode error_code_ = ERR_NONE; CefRefPtr response_; - bool response_was_cached_; + bool response_was_cached_ = false; private: IMPLEMENT_REFCOUNTING(RequestClient); @@ -1046,11 +1220,13 @@ class RequestTestRunner : public base::RefCountedThreadSafe { RequestTestRunner(bool is_browser_process, bool is_server_backend, bool use_frame_method, - bool run_in_browser_process) + bool run_in_browser_process, + const base::Closure& incomplete_request_callback) : is_browser_process_(is_browser_process), is_server_backend_(is_server_backend), use_frame_method_(use_frame_method), - run_in_browser_process_(run_in_browser_process) { + run_in_browser_process_(run_in_browser_process), + incomplete_request_callback_(incomplete_request_callback) { owner_task_runner_ = CefTaskRunner::GetForCurrentThread(); EXPECT_TRUE(owner_task_runner_.get()); EXPECT_TRUE(owner_task_runner_->BelongsToCurrentThread()); @@ -1096,11 +1272,16 @@ class RequestTestRunner : public base::RefCountedThreadSafe { MultipleRunTest); REGISTER_TEST(REQTEST_CACHE_DISABLE_HEADER, SetupCacheDisableHeaderTest, MultipleRunTest); + REGISTER_TEST(REQTEST_INCOMPLETE_PROCESS_REQUEST, + SetupIncompleteProcessRequestTest, SingleRunTest); + REGISTER_TEST(REQTEST_INCOMPLETE_READ_RESPONSE, + SetupIncompleteReadResponseTest, SingleRunTest); } void Destroy() { owner_task_runner_ = nullptr; request_context_ = nullptr; + incomplete_request_callback_.Reset(); } // Called in the browser process to set the request context that will be used @@ -1764,6 +1945,44 @@ class RequestTestRunner : public base::RefCountedThreadSafe { complete_callback.Run(); } + void SetupIncompleteProcessRequestTest( + const base::Closure& complete_callback) { + // Start with the normal get test. + SetupGetTestShared(); + + settings_.incomplete_type = RequestRunSettings::INCOMPLETE_PROCESS_REQUEST; + + // There will be no response and the request will be aborted. + settings_.response = CefResponse::Create(); + settings_.response_data.clear(); + settings_.expected_error_code = ERR_ABORTED; + settings_.expected_status = UR_FAILED; + settings_.expect_download_progress = false; + settings_.expect_download_data = false; + + complete_callback.Run(); + } + + void SetupIncompleteReadResponseTest(const base::Closure& complete_callback) { + // Start with the normal get test. + SetupGetTestShared(); + + settings_.incomplete_type = RequestRunSettings::INCOMPLETE_READ_RESPONSE; + + // There will be a response but the request will be aborted without + // receiving any data. + settings_.response_data = kIncompleteDoNotSendData; + settings_.expected_error_code = ERR_ABORTED; + settings_.expected_status = UR_FAILED; + // TODO(network): Download progress notifications are sent for incomplete + // (with no data sent) requests in the browser process but not the renderer + // process. Consider standardizing this behavior. + settings_.expect_download_progress = is_browser_process_; + settings_.expect_download_data = false; + + complete_callback.Run(); + } + // Send a request. |complete_callback| will be executed on request completion. void SendRequest( const RequestClient::RequestCompleteCallback& complete_callback) { @@ -1781,6 +2000,11 @@ class RequestTestRunner : public base::RefCountedThreadSafe { } else { CefURLRequest::Create(request, client.get(), request_context_); } + + if (settings_.incomplete_type != RequestRunSettings::INCOMPLETE_NONE) { + incomplete_request_callback_.Run(); + incomplete_request_callback_.Reset(); + } } // Verify a response. @@ -2018,6 +2242,9 @@ class RequestTestRunner : public base::RefCountedThreadSafe { const bool use_frame_method_; const bool run_in_browser_process_; + // Used with incomplete request tests. + base::Closure incomplete_request_callback_; + // Primary thread runner for the object that owns us. In the browser process // this will be the UI thread and in the renderer process this will be the // RENDERER thread. @@ -2074,8 +2301,9 @@ class RequestRendererTest : public ClientAppRenderer::Delegate { frame_ = frame; test_mode_ = static_cast(args->GetInt(0)); - test_runner_ = new RequestTestRunner(false, args->GetBool(1), - use_frame_method, false); + test_runner_ = new RequestTestRunner( + false, args->GetBool(1), use_frame_method, false, + base::Bind(&RequestRendererTest::OnIncompleteRequest, this)); // Setup the test. This will create the objects that we test against but // not register any backend (because we're in the render process). @@ -2107,9 +2335,34 @@ class RequestRendererTest : public ClientAppRenderer::Delegate { base::Bind(&RequestRendererTest::OnShutdownComplete, this)); } + void OnIncompleteRequest() { + EXPECT_TRUE(CefCurrentlyOn(TID_RENDERER)); + + // This method will only be called for incomplete requests. + EXPECT_NE(test_runner_->settings_.incomplete_type, + RequestRunSettings::INCOMPLETE_NONE); + + // Check if the test has failed. + bool result = !TestFailed(); + + // The browser will be closed to abort in-progress requests. + CefRefPtr return_msg = + CefProcessMessage::Create(kIncompleteRequestTestMsg); + EXPECT_TRUE(return_msg->GetArgumentList()->SetBool(0, result)); + browser_->GetMainFrame()->SendProcessMessage(PID_BROWSER, return_msg); + } + void OnShutdownComplete() { EXPECT_TRUE(CefCurrentlyOn(TID_RENDERER)); + if (test_runner_->settings_.incomplete_type != + RequestRunSettings::INCOMPLETE_NONE) { + // For incomplete tests there's a race between process destruction due to + // the browser closing, and the test possibly completing due to request + // cancellation. We therefore ignore test completion in this case. + return; + } + // Check if the test has failed. bool result = !TestFailed(); @@ -2179,8 +2432,9 @@ class RequestTestHandler : public TestHandler { void PreSetupContinue() { EXPECT_TRUE(CefCurrentlyOn(TID_UI)); - test_runner_ = new RequestTestRunner(true, test_server_backend_, - test_frame_method_, test_in_browser_); + test_runner_ = new RequestTestRunner( + true, test_server_backend_, test_frame_method_, test_in_browser_, + base::Bind(&RequestTestHandler::OnIncompleteRequest, this)); // Get or create the request context. if (context_mode_ == CONTEXT_GLOBAL) { @@ -2332,17 +2586,57 @@ class RequestTestHandler : public TestHandler { EXPECT_TRUE(message->IsReadOnly()); EXPECT_FALSE(test_in_browser_); + EXPECT_FALSE(got_message_); got_message_.yes(); if (message->GetArgumentList()->GetBool(0)) got_success_.yes(); - // Renderer process test is complete. - OnRunComplete(); + const std::string& message_name = message->GetName(); + if (message_name == kRequestTestMsg) { + // Renderer process test is complete. + OnRunComplete(); + } else if (message_name == kIncompleteRequestTestMsg) { + // Incomplete renderer tests will not complete normally. Instead, trigger + // browser close and then signal completion from OnBeforeClose. + OnIncompleteRequest(); + } return true; } + void OnBeforeClose(CefRefPtr browser) override { + if (!test_in_browser_ && test_runner_->settings_.incomplete_type != + RequestRunSettings::INCOMPLETE_NONE) { + // Incomplete tests running in the renderer process will never recieve the + // test complete process message, so call the method here. + OnRunComplete(); + } + + TestHandler::OnBeforeClose(browser); + } + + // Incomplete tests will not complete normally. Instead, we trigger a browser + // close to abort in-progress requests. + void OnIncompleteRequest() { + if (!CefCurrentlyOn(TID_UI)) { + CefPostTask(TID_UI, + base::Bind(&RequestTestHandler::OnIncompleteRequest, this)); + return; + } + + EXPECT_TRUE(test_frame_method_); + EXPECT_NE(RequestRunSettings::INCOMPLETE_NONE, + test_runner_->settings_.incomplete_type); + + // TestComplete will eventually be called from DestroyTest instead of being + // triggered by browser destruction. + SetSignalCompletionWhenAllBrowsersClose(false); + CefPostDelayedTask( + TID_UI, base::Bind(&TestHandler::CloseBrowser, GetBrowser(), false), + 100); + } + // Test run is complete. It ran in either the browser or render process. void OnRunComplete() { GetTestCookie(test_runner_->GetRequestContext(), test_server_backend_, @@ -2379,15 +2673,21 @@ class RequestTestHandler : public TestHandler { TestHandler::DestroyTest(); - // Need to call TestComplete() explicitly if testing in the browser and - // using the global context. Otherwise, TestComplete() will be called when - // the browser is destroyed (for render test + global context) or when the - // temporary context is destroyed. - const bool call_test_complete = (test_in_browser_ && !test_frame_method_ && - context_mode_ == CONTEXT_GLOBAL); + // For non-global contexts OnTestComplete() will be called when the + // RequestContextHandler is destroyed. + bool call_test_complete = false; + if (context_mode_ == CONTEXT_GLOBAL) { + if (test_in_browser_ && !test_frame_method_) { + // These tests don't create a browser that would signal implicitly. + call_test_complete = true; + } else if (!SignalCompletionWhenAllBrowsersClose()) { + // These tests close the browser to terminate in-progress requests + // before test completion. + call_test_complete = true; + } + } - // Release our reference to the context. Do not access any object members - // after this call because |this| might be deleted. + // Release references to the context and handler. test_runner_->Destroy(); if (call_test_complete) @@ -2401,6 +2701,9 @@ class RequestTestHandler : public TestHandler { return; } + EXPECT_FALSE(got_on_test_complete_); + got_on_test_complete_.yes(); + if (!context_tmpdir_.IsEmpty()) { // Wait a bit for cache file handles to close after browser or request // context destruction. @@ -2479,6 +2782,8 @@ class RequestTestHandler : public TestHandler { TrackCallback got_message_; TrackCallback got_success_; + TrackCallback got_on_test_complete_; + IMPLEMENT_REFCOUNTING(RequestTestHandler); }; @@ -2487,13 +2792,22 @@ bool IsTestSupported(RequestTestMode test_mode, bool test_in_browser, bool test_server_backend, bool test_frame_method) { - if (IsNetworkServiceEnabled() && !test_in_browser && !test_server_backend && - !test_frame_method) { - // When NetworkService is enabled requests from the render process can only - // reach non-server backends when using the CefFrame::CreateURLRequest - // method. - return false; + if (IsNetworkServiceEnabled()) { + if (!test_in_browser && !test_server_backend && !test_frame_method) { + // When NetworkService is enabled requests from the render process can + // only reach non-server backends when using the + // CefFrame::CreateURLRequest method. + return false; + } + } else { + if (test_mode == REQTEST_INCOMPLETE_PROCESS_REQUEST || + test_mode == REQTEST_INCOMPLETE_READ_RESPONSE) { + // The old network implementation does not support the same behavior + // for canceling incomplete requests. + return false; + } } + return true; } @@ -2599,6 +2913,33 @@ void RegisterURLRequestCustomSchemes( REQ_TEST_SET(WithoutFrame, false) REQ_TEST_SET(WithFrame, true) +// Define tests that can only run with a frame. +#define REQ_TEST_FRAME_SET_EX(suffix, context_mode, test_server_backend) \ + REQ_TEST(BrowserIncompleteProcessRequest##suffix, \ + REQTEST_INCOMPLETE_PROCESS_REQUEST, context_mode, true, \ + test_server_backend, true) \ + REQ_TEST(BrowserIncompleteReadResponse##suffix, \ + REQTEST_INCOMPLETE_READ_RESPONSE, context_mode, true, \ + test_server_backend, true) \ + REQ_TEST(RendererIncompleteProcessRequest##suffix, \ + REQTEST_INCOMPLETE_PROCESS_REQUEST, context_mode, false, \ + test_server_backend, true) \ + REQ_TEST(RendererIncompleteReadResponse##suffix, \ + REQTEST_INCOMPLETE_READ_RESPONSE, context_mode, false, \ + test_server_backend, true) + +#define REQ_TEST_FRAME_SET() \ + REQ_TEST_FRAME_SET_EX(ContextGlobalCustomWithFrame, CONTEXT_GLOBAL, false) \ + REQ_TEST_FRAME_SET_EX(ContextInMemoryCustomWithFrame, CONTEXT_INMEMORY, \ + false) \ + REQ_TEST_FRAME_SET_EX(ContextOnDiskCustomWithFrame, CONTEXT_ONDISK, false) \ + REQ_TEST_FRAME_SET_EX(ContextGlobalServerWithFrame, CONTEXT_GLOBAL, true) \ + REQ_TEST_FRAME_SET_EX(ContextInMemoryServerWithFrame, CONTEXT_INMEMORY, \ + true) \ + REQ_TEST_FRAME_SET_EX(ContextOnDiskServerWithFrame, CONTEXT_ONDISK, true) + +REQ_TEST_FRAME_SET() + // Cache tests can only be run with the server backend. #define REQ_TEST_CACHE_SET_EX(suffix, context_mode, test_frame_method) \ REQ_TEST(BrowserGETCacheWithControl##suffix, REQTEST_CACHE_WITH_CONTROL, \