diff --git a/libcef/browser/alloy/alloy_browser_context.cc b/libcef/browser/alloy/alloy_browser_context.cc index cd4c3d016..c6523c8d4 100644 --- a/libcef/browser/alloy/alloy_browser_context.cc +++ b/libcef/browser/alloy/alloy_browser_context.cc @@ -194,6 +194,10 @@ void AlloyBrowserContext::Shutdown() { ChromePluginServiceFilter::GetInstance()->UnregisterProfile(this); + // Clear this reference before the associated KeyedServiceFactory is destroyed + // by PerformInterlockedTwoPhaseShutdown(). + extension_system_ = nullptr; + // Remove any BrowserContextKeyedServiceFactory associations. This must be // called before the ProxyService owned by AlloyBrowserContext is destroyed. // The SimpleDependencyManager should always be passed after the diff --git a/libcef/browser/alloy/alloy_browser_host_impl.cc b/libcef/browser/alloy/alloy_browser_host_impl.cc index 0d520e7b4..017ab7600 100644 --- a/libcef/browser/alloy/alloy_browser_host_impl.cc +++ b/libcef/browser/alloy/alloy_browser_host_impl.cc @@ -581,17 +581,6 @@ void AlloyBrowserHostImpl::DestroyBrowser() { destruction_state_ = DESTRUCTION_STATE_COMPLETED; - // Notify that this browser has been destroyed. These must be delivered in - // the expected order. - - // 1. Notify the platform delegate. With Views this will result in a call to - // CefBrowserViewDelegate::OnBrowserDestroyed(). - platform_delegate_->NotifyBrowserDestroyed(); - - // 2. Notify the browser's LifeSpanHandler. This must always be the last - // notification for this browser. - OnBeforeClose(); - // Destroy any platform constructs first. if (javascript_dialog_manager_.get()) { javascript_dialog_manager_->Destroy(); @@ -600,16 +589,8 @@ void AlloyBrowserHostImpl::DestroyBrowser() { menu_manager_->Destroy(); } - // Notify any observers that may have state associated with this browser. - OnBrowserDestroyed(); - - // If the WebContents still exists at this point, signal destruction before - // browser destruction. - if (web_contents()) { - WebContentsDestroyed(); - } - - // Disassociate the platform delegate from this browser. + // Disassociate the platform delegate from this browser. This will trigger + // WebContents destruction in most cases. platform_delegate_->BrowserDestroyed(this); // Delete objects created by the platform delegate that may be referenced by @@ -1408,10 +1389,22 @@ void AlloyBrowserHostImpl::AccessibilityLocationChangesReceived( } void AlloyBrowserHostImpl::WebContentsDestroyed() { + // In case we're notified before the CefBrowserContentsDelegate, + // reset it first for consistent state in DestroyWebContents. + if (GetWebContents()) { + contents_delegate_->WebContentsDestroyed(); + } + auto wc = web_contents(); content::WebContentsObserver::Observe(nullptr); - if (platform_delegate_) { - platform_delegate_->WebContentsDestroyed(wc); + DestroyWebContents(wc); + + if (destruction_state_ < DESTRUCTION_STATE_COMPLETED) { + // We were not called via DestroyBrowser. This can occur when (for example) + // a pending popup WebContents is destroyed during parent WebContents + // destruction. Try to close the associated browser now. + CEF_POST_TASK(CEF_UIT, base::BindOnce(&AlloyBrowserHostImpl::CloseBrowser, + this, /*force_close=*/true)); } } diff --git a/libcef/browser/alloy/alloy_content_browser_client.cc b/libcef/browser/alloy/alloy_content_browser_client.cc index d3ceedbd5..442538fd7 100644 --- a/libcef/browser/alloy/alloy_content_browser_client.cc +++ b/libcef/browser/alloy/alloy_content_browser_client.cc @@ -384,6 +384,10 @@ AlloyContentBrowserClient::AlloyContentBrowserClient() = default; AlloyContentBrowserClient::~AlloyContentBrowserClient() = default; +void AlloyContentBrowserClient::CleanupOnUIThread() { + browser_main_parts_ = nullptr; +} + std::unique_ptr AlloyContentBrowserClient::CreateBrowserMainParts( bool /* is_integration_test */) { diff --git a/libcef/browser/alloy/alloy_content_browser_client.h b/libcef/browser/alloy/alloy_content_browser_client.h index 7fcf80978..1d0209d90 100644 --- a/libcef/browser/alloy/alloy_content_browser_client.h +++ b/libcef/browser/alloy/alloy_content_browser_client.h @@ -34,6 +34,8 @@ class AlloyContentBrowserClient : public content::ContentBrowserClient { AlloyContentBrowserClient(); ~AlloyContentBrowserClient() override; + void CleanupOnUIThread(); + // ContentBrowserClient implementation. std::unique_ptr CreateBrowserMainParts( bool is_integration_test) override; diff --git a/libcef/browser/alloy/chrome_browser_process_alloy.cc b/libcef/browser/alloy/chrome_browser_process_alloy.cc index 00981ee39..bd2e66523 100644 --- a/libcef/browser/alloy/chrome_browser_process_alloy.cc +++ b/libcef/browser/alloy/chrome_browser_process_alloy.cc @@ -154,6 +154,7 @@ void ChromeBrowserProcessAlloy::CleanupOnUIThread() { } } + os_crypt_async_.reset(); local_state_.reset(); browser_policy_connector_.reset(); background_printing_manager_.reset(); diff --git a/libcef/browser/browser_contents_delegate.cc b/libcef/browser/browser_contents_delegate.cc index 39ab84865..6c3738e31 100644 --- a/libcef/browser/browser_contents_delegate.cc +++ b/libcef/browser/browser_contents_delegate.cc @@ -341,7 +341,7 @@ void CefBrowserContentsDelegate::RenderFrameCreated( void CefBrowserContentsDelegate::RenderFrameHostChanged( content::RenderFrameHost* old_host, content::RenderFrameHost* new_host) { - // Just in case RenderFrameCreated wasn't called for some reason. + // Update tracking for the RFH. RenderFrameCreated(new_host); } @@ -494,6 +494,11 @@ void CefBrowserContentsDelegate::DidFinishNavigation( return; } + if (browser_info_->IsClosing()) { + // Ignore notifications when the browser is closing. + return; + } + if (navigation_handle->IsInPrimaryMainFrame() && navigation_handle->HasCommitted()) { // A primary main frame navigation has occured. @@ -506,21 +511,15 @@ void CefBrowserContentsDelegate::DidFinishNavigation( const GURL& url = (error_code == net::OK ? navigation_handle->GetURL() : GURL()); - auto browser_info = browser_info_; - if (!browser_info->browser()) { - // Ignore notifications when the browser is closing. - return; - } - // May return NULL when starting a new navigation if the previous navigation // caused the renderer process to crash during load. CefRefPtr frame = - browser_info->GetFrameForGlobalId(global_id); + browser_info_->GetFrameForGlobalId(global_id); if (!frame) { if (is_main_frame) { - frame = browser_info->GetMainFrame(); + frame = browser_info_->GetMainFrame(); } else { - frame = browser_info->CreateTempSubFrame(frame_util::InvalidGlobalId()); + frame = browser_info_->CreateTempSubFrame(frame_util::InvalidGlobalId()); } } frame->RefreshAttributes(); diff --git a/libcef/browser/browser_context.cc b/libcef/browser/browser_context.cc index 10e86dec4..be02e7db0 100644 --- a/libcef/browser/browser_context.cc +++ b/libcef/browser/browser_context.cc @@ -132,7 +132,7 @@ class ImplManager { return all_.end(); } - using PathMap = std::map; + using PathMap = std::map>; PathMap map_; Vector all_; @@ -419,12 +419,12 @@ CefRefPtr CefBrowserContext::GetAnyRequestContext( if (prefer_no_handler) { for (const auto& request_context : request_context_set_) { if (!request_context->GetHandler()) { - return request_context; + return request_context.get(); } } } - return *request_context_set_.begin(); + return request_context_set_.begin()->get(); } CefBrowserContext::CookieableSchemes CefBrowserContext::GetCookieableSchemes() diff --git a/libcef/browser/browser_context.h b/libcef/browser/browser_context.h index 29ebcc301..71aafc912 100644 --- a/libcef/browser/browser_context.h +++ b/libcef/browser/browser_context.h @@ -235,7 +235,7 @@ class CefBrowserContext { std::unique_ptr media_router_manager_; // CefRequestContextImpl objects referencing this object. - std::set request_context_set_; + std::set> request_context_set_; // Map IDs to CefRequestContextHandler objects. CefRequestContextHandlerMap handler_map_; diff --git a/libcef/browser/browser_host_base.cc b/libcef/browser/browser_host_base.cc index a3a28145b..d4a486d22 100644 --- a/libcef/browser/browser_host_base.cc +++ b/libcef/browser/browser_host_base.cc @@ -262,17 +262,47 @@ void CefBrowserHostBase::InitializeBrowser() { WebContentsUserDataAdapter::Register(this); } +void CefBrowserHostBase::DestroyWebContents( + content::WebContents* web_contents) { + CEF_REQUIRE_UIT(); + + // GetWebContents() should return nullptr at this point. + DCHECK(!GetWebContents()); + + // Notify that this browser has been destroyed. These must be delivered in + // the expected order. + + // 1. Notify the platform delegate. With Views this will result in a call to + // CefBrowserViewDelegate::OnBrowserDestroyed(). + platform_delegate_->NotifyBrowserDestroyed(); + + // 2. Notify the browser's LifeSpanHandler. This must always be the last + // notification for this browser. + OnBeforeClose(); + + // Notify any observers that may have state associated with this browser. + OnBrowserDestroyed(); + + // Free objects that may have references to the WebContents. + devtools_protocol_manager_.reset(); + devtools_window_runner_.reset(); + context_menu_observer_ = nullptr; + + browser_info_->WebContentsDestroyed(); + platform_delegate_->WebContentsDestroyed(web_contents); +} + void CefBrowserHostBase::DestroyBrowser() { CEF_REQUIRE_UIT(); - devtools_protocol_manager_.reset(); - devtools_window_runner_.reset(); + // The WebContents should no longer be observed. + DCHECK(!contents_delegate_->web_contents()); + media_stream_registrar_.reset(); platform_delegate_.reset(); contents_delegate_->RemoveObserver(this); - contents_delegate_->ObserveWebContents(nullptr); if (unresponsive_process_callback_) { hang_monitor::Detach(unresponsive_process_callback_); @@ -280,7 +310,7 @@ void CefBrowserHostBase::DestroyBrowser() { } CefBrowserInfoManager::GetInstance()->RemoveBrowserInfo(browser_info_); - browser_info_->SetBrowser(nullptr); + browser_info_->BrowserDestroyed(); } CefRefPtr CefBrowserHostBase::GetBrowser() { @@ -889,7 +919,7 @@ void CefBrowserHostBase::SendMouseWheelEvent(const CefMouseEvent& event, } bool CefBrowserHostBase::IsValid() { - return browser_info_->browser() == this; + return browser_info_->IsValid(); } CefRefPtr CefBrowserHostBase::GetHost() { diff --git a/libcef/browser/browser_host_base.h b/libcef/browser/browser_host_base.h index b908fc3d0..7dfb5aae7 100644 --- a/libcef/browser/browser_host_base.h +++ b/libcef/browser/browser_host_base.h @@ -201,9 +201,32 @@ class CefBrowserHostBase : public CefBrowserHost, // the UI thread only. virtual bool WillBeDestroyed() const = 0; - // Called on the UI thread after the associated WebContents is destroyed. - // Also called from CefBrowserInfoManager::DestroyAllBrowsers if the browser - // was not properly shut down. + // Called on the UI thread to complete WebContents tear-down. In most cases + // this will be called via WebContentsObserver::WebContentsDestroyed. Any + // remaining objects that reference the WebContents (including RFH, etc) + // should be cleared in this callback. + virtual void DestroyWebContents(content::WebContents* web_contents); + + // Called on the UI thread to complete CefBrowserHost tear-down. + // + // With Chrome style the WebContents is owned by the Browser's TabStripModel + // and will usually be destroyed first: CloseBrowser -> (async) DoCloseBrowser + // -> [TabStripModel deletes the WebContents] -> OnWebContentsDestroyed -> + // DestroyWebContents -> (async) DestroyBrowser. + // + // With Alloy style the WebContents is owned by the + // CefBrowserPlatformDelegateAlloy and will usually be destroyed at the same + // time: CloseBrowser -> [OS/platform logic] -> (async) DestroyBrowser -> + // [CefBrowserPlatformDelegateAlloy deletes the WebContents] + // -> WebContentsDestroyed -> DestoyWebContents. + // + // There are a few exceptions to the above rules: + // 1. If the CefBrowserHost still exists at CefShutdown, in which case + // DestroyBrowser will be called first via + // CefBrowserInfoManager::DestroyAllBrowsers. + // 2. If a popup WebContents is still pending when the parent WebContents is + // destroyed, in which case WebContentsDestroyed will be called first via + // the parent WebContents destructor. virtual void DestroyBrowser(); // CefBrowserHost methods: diff --git a/libcef/browser/browser_info.cc b/libcef/browser/browser_info.cc index a4d1efe51..160d09f4e 100644 --- a/libcef/browser/browser_info.cc +++ b/libcef/browser/browser_info.cc @@ -55,43 +55,72 @@ CefBrowserInfo::~CefBrowserInfo() { CefRefPtr CefBrowserInfo::browser() const { base::AutoLock lock_scope(lock_); - if (!is_closing_) { - return browser_; - } - return nullptr; + return browser_; +} + +bool CefBrowserInfo::IsValid() const { + base::AutoLock lock_scope(lock_); + return browser_ && !is_closing_; +} + +bool CefBrowserInfo::IsClosing() const { + base::AutoLock lock_scope(lock_); + return is_closing_; } void CefBrowserInfo::SetBrowser(CefRefPtr browser) { - NotificationStateLock lock_scope(this); + base::AutoLock lock_scope(lock_); + DCHECK(browser); + DCHECK(!browser_); - if (browser) { - DCHECK(!browser_); - - // Cache the associated frame handler. - if (auto client = browser->GetClient()) { - frame_handler_ = client->GetFrameHandler(); - } - } else { - DCHECK(browser_); - } - - auto old_browser = browser_; browser_ = browser; - if (!browser_) { - RemoveAllFrames(old_browser); - - // Any future calls to MaybeExecuteFrameNotification will now fail. - // NotificationStateLock already took a reference for the delivery of any - // notifications that are currently queued due to RemoveAllFrames. - frame_handler_ = nullptr; + // Cache the associated frame handler. + if (auto client = browser->GetClient()) { + frame_handler_ = client->GetFrameHandler(); } } void CefBrowserInfo::SetClosing() { base::AutoLock lock_scope(lock_); - DCHECK(!is_closing_); - is_closing_ = true; + + // In most cases WebContentsDestroyed will be called first, except if the + // browser still exits at CefShitdown. + if (!is_closing_) { + is_closing_ = true; + } +} + +void CefBrowserInfo::WebContentsDestroyed() { + NotificationStateLock lock_scope(this); + + // Always called before BrowserDestroyed. + DCHECK(browser_); + + // We want GetMainFrame() to return nullptr at this point, but browser() + // should still be valid so as not to interfere with the net_service + // DestructionObserver. + if (!is_closing_) { + is_closing_ = true; + } + + RemoveAllFrames(browser_); + + // Any future calls to MaybeExecuteFrameNotification will now fail. + // NotificationStateLock already took a reference for the delivery of any + // notifications that are currently queued due to RemoveAllFrames. + frame_handler_ = nullptr; +} + +void CefBrowserInfo::BrowserDestroyed() { + base::AutoLock lock_scope(lock_); + + // Always called after SetClosing and WebContentsDestroyed. + DCHECK(is_closing_); + DCHECK(frame_info_set_.empty()); + + DCHECK(browser_); + browser_ = nullptr; } void CefBrowserInfo::MaybeCreateFrame(content::RenderFrameHost* host) { @@ -130,6 +159,9 @@ void CefBrowserInfo::MaybeCreateFrame(content::RenderFrameHost* host) { DCHECK_EQ(info->is_main_frame_, is_main_frame); #endif + // Update the associated RFH, which may have changed. + info->frame_->MaybeReAttach(this, host, /*require_detached=*/false); + if (info->is_speculative_ && !is_speculative) { // Upgrade the frame info from speculative to non-speculative. if (info->is_main_frame_) { @@ -142,7 +174,6 @@ void CefBrowserInfo::MaybeCreateFrame(content::RenderFrameHost* host) { } auto frame_info = new FrameInfo; - frame_info->host_ = host; frame_info->global_id_ = global_id; frame_info->is_main_frame_ = is_main_frame; frame_info->is_speculative_ = is_speculative; @@ -183,7 +214,7 @@ void CefBrowserInfo::FrameHostStateChanged( new_state == content::RenderFrameHost::LifecycleState::kActive) { if (auto frame = GetFrameForHost(host)) { // Update the associated RFH, which may have changed. - frame->MaybeReAttach(this, host); + frame->MaybeReAttach(this, host, /*require_detached=*/true); if (frame->IsMain()) { // Update the main frame object. @@ -519,8 +550,7 @@ void CefBrowserInfo::RemoveAllFrames( // Make sure any callbacks will see the correct state (e.g. like // CefBrowser::GetMainFrame returning nullptr and CefBrowser::IsValid // returning false). - DCHECK(!browser_); - DCHECK(old_browser); + DCHECK(is_closing_); // Clear the lookup maps. frame_id_map_.clear(); diff --git a/libcef/browser/browser_info.h b/libcef/browser/browser_info.h index 559aaf3ab..5b76574dd 100644 --- a/libcef/browser/browser_info.h +++ b/libcef/browser/browser_info.h @@ -49,21 +49,30 @@ class CefBrowserInfo : public base::RefCountedThreadSafe { bool print_preview_enabled() const { return print_preview_enabled_; } CefRefPtr extra_info() const { return extra_info_; } - // May return NULL if the browser has not yet been created or if the browser - // has been destroyed. + // May return nullptr if the browser has not yet been created (before + // SetBrowser) or if the browser has been destroyed (after BrowserDestroyed). CefRefPtr browser() const; - // Set or clear the browser. Called from CefBrowserHostBase InitializeBrowser - // (to set) and DestroyBrowser (to clear). + // Returns true if the browser has been created (after SetBrowser) and is not + // yet closing (before SetClosing or WebContentsDestroyed). + bool IsValid() const; + + // Returns true if the browser is closing (after SetClosing or + // WebContentsDestroyed). + bool IsClosing() const; + + // Called from CefBrowserHostBase constructor. void SetBrowser(CefRefPtr browser); - // Called after OnBeforeClose and before SetBrowser(nullptr). This will cause - // browser() and GetMainFrame() to return nullptr as expected by - // CefFrameHandler callbacks. Note that this differs from calling - // SetBrowser(nullptr) because the WebContents has not yet been destroyed and - // further frame-related callbacks are expected. + // Called from CefBrowserHostBase::OnBeforeClose. void SetClosing(); + // Called from CefBrowserHostBase::DestroyWebContents. + void WebContentsDestroyed(); + + // Called from CefBrowserHostBase::DestroyBrowser. + void BrowserDestroyed(); + // Ensure that a frame record exists for |host|. Called for the main frame // when the RenderView is created, or for a sub-frame when the associated // RenderFrame is created in the renderer process. @@ -170,7 +179,6 @@ class CefBrowserInfo : public base::RefCountedThreadSafe { return frame_ && is_main_frame_ && !is_speculative_ && !is_in_bfcache_; } - raw_ptr host_; content::GlobalRenderFrameHostId global_id_; bool is_main_frame_; bool is_speculative_; diff --git a/libcef/browser/chrome/chrome_browser_context.cc b/libcef/browser/chrome/chrome_browser_context.cc index 2cc97f62e..2fbbdc7d8 100644 --- a/libcef/browser/chrome/chrome_browser_context.cc +++ b/libcef/browser/chrome/chrome_browser_context.cc @@ -133,7 +133,8 @@ void ChromeBrowserContext::Shutdown() { // |g_browser_process| may be nullptr during shutdown. if (g_browser_process) { if (should_destroy_) { - GetPrimaryUserProfile()->DestroyOffTheRecordProfile(profile_); + GetPrimaryUserProfile()->DestroyOffTheRecordProfile( + profile_.ExtractAsDangling()); } else if (profile_) { OnProfileWillBeDestroyed(profile_); } diff --git a/libcef/browser/chrome/chrome_browser_host_impl.cc b/libcef/browser/chrome/chrome_browser_host_impl.cc index 426e28416..8e22411ab 100644 --- a/libcef/browser/chrome/chrome_browser_host_impl.cc +++ b/libcef/browser/chrome/chrome_browser_host_impl.cc @@ -146,16 +146,9 @@ void ChromeBrowserHostImpl::AddNewContents( void ChromeBrowserHostImpl::OnWebContentsDestroyed( content::WebContents* web_contents) { - // GetWebContents() should return nullptr at this point. - DCHECK(!GetWebContents()); - - // In most cases WebContents destruction will trigger browser destruction. - // The exception is if the browser still exists at CefShutdown, in which - // case DestroyBrowser() will be called first via - // CefBrowserInfoManager::DestroyAllBrowsers(). - if (platform_delegate_) { - platform_delegate_->WebContentsDestroyed(web_contents); + DestroyWebContents(web_contents); + if (!is_destroying_browser_) { // Destroy the browser asynchronously to allow the current call stack // to unwind (we may have been called via the TabStripModel owned by the // Browser). @@ -568,24 +561,14 @@ bool ChromeBrowserHostImpl::WillBeDestroyed() const { void ChromeBrowserHostImpl::DestroyBrowser() { CEF_REQUIRE_UIT(); - // Notify that this browser has been destroyed. These must be delivered in - // the expected order. + is_destroying_browser_ = true; - // 1. Notify the platform delegate. With Views this will result in a call to - // CefBrowserViewDelegate::OnBrowserDestroyed(). - platform_delegate_->NotifyBrowserDestroyed(); - - // 2. Notify the browser's LifeSpanHandler. This must always be the last - // notification for this browser. - OnBeforeClose(); - - // Notify any observers that may have state associated with this browser. - OnBrowserDestroyed(); - - // If the WebContents still exists at this point, signal destruction before - // browser destruction. - if (auto web_contents = GetWebContents()) { - platform_delegate_->WebContentsDestroyed(web_contents); + // If the WebContents still exists at this point, close the Browser and + // WebContents first. See comments on CefBrowserHostBase::DestroyBrowser. + if (GetWebContents()) { + // Triggers a call to OnWebContentsDestroyed. + DoCloseBrowser(/*force_close=*/true); + DCHECK(!GetWebContents()); } // Disassociate the platform delegate from this browser. @@ -604,6 +587,7 @@ void ChromeBrowserHostImpl::DoCloseBrowser(bool force_close) { // Like chrome::CloseTab() but specifying the WebContents. const int tab_index = GetCurrentTabIndex(); if (tab_index != TabStripModel::kNoTab) { + // This will trigger destruction of the Browser and WebContents. // TODO(chrome): Handle the case where this method returns false, // indicating that the contents were not closed immediately. browser_->tab_strip_model()->CloseWebContentsAt( diff --git a/libcef/browser/chrome/chrome_browser_host_impl.h b/libcef/browser/chrome/chrome_browser_host_impl.h index f8566142b..95817e7e9 100644 --- a/libcef/browser/chrome/chrome_browser_host_impl.h +++ b/libcef/browser/chrome/chrome_browser_host_impl.h @@ -177,6 +177,7 @@ class ChromeBrowserHostImpl : public CefBrowserHostBase { raw_ptr browser_ = nullptr; CefWindowHandle host_window_handle_ = kNullWindowHandle; + bool is_destroying_browser_ = false; base::WeakPtrFactory weak_ptr_factory_{this}; }; diff --git a/libcef/browser/chrome/chrome_content_browser_client_cef.cc b/libcef/browser/chrome/chrome_content_browser_client_cef.cc index 60ae18c9d..56df06933 100644 --- a/libcef/browser/chrome/chrome_content_browser_client_cef.cc +++ b/libcef/browser/chrome/chrome_content_browser_client_cef.cc @@ -84,6 +84,11 @@ void HandleExternalProtocolHelper( ChromeContentBrowserClientCef::ChromeContentBrowserClientCef() = default; ChromeContentBrowserClientCef::~ChromeContentBrowserClientCef() = default; +void ChromeContentBrowserClientCef::CleanupOnUIThread() { + browser_main_parts_ = nullptr; + ChromeContentBrowserClient::CleanupOnUIThread(); +} + std::unique_ptr ChromeContentBrowserClientCef::CreateBrowserMainParts( bool is_integration_test) { diff --git a/libcef/browser/chrome/chrome_content_browser_client_cef.h b/libcef/browser/chrome/chrome_content_browser_client_cef.h index 1622ede5f..ad04adf4b 100644 --- a/libcef/browser/chrome/chrome_content_browser_client_cef.h +++ b/libcef/browser/chrome/chrome_content_browser_client_cef.h @@ -26,6 +26,8 @@ class ChromeContentBrowserClientCef : public ChromeContentBrowserClient { ~ChromeContentBrowserClientCef() override; + void CleanupOnUIThread() override; + // ChromeContentBrowserClient overrides. std::unique_ptr CreateBrowserMainParts( bool is_integration_test) override; diff --git a/libcef/browser/chrome/views/chrome_browser_frame.cc b/libcef/browser/chrome/views/chrome_browser_frame.cc index d7513a199..346e90828 100644 --- a/libcef/browser/chrome/views/chrome_browser_frame.cc +++ b/libcef/browser/chrome/views/chrome_browser_frame.cc @@ -31,11 +31,10 @@ void ChromeBrowserFrame::Init(BrowserView* browser_view, DCHECK(browser_view); DCHECK(browser); - DCHECK(!browser_view_); - browser_view_ = browser_view; + DCHECK(!BrowserFrame::browser_view()); // Initialize BrowserFrame state. - InitBrowserView(browser_view); + SetBrowserView(browser_view); // Initialize BrowserView state. browser_view->InitBrowser(std::move(browser)); @@ -73,7 +72,7 @@ void ChromeBrowserFrame::AddAssociatedProfile(Profile* profile) { // Always call ThemeChanged() when the Chrome style BrowserView is added. bool call_theme_changed = - browser_view_ && browser_view_->GetProfile() == profile; + browser_view() && browser_view()->GetProfile() == profile; ProfileMap::iterator it = associated_profiles_.find(profile); if (it != associated_profiles_.end()) { @@ -127,8 +126,8 @@ void ChromeBrowserFrame::RemoveAssociatedProfile(Profile* profile) { Profile* ChromeBrowserFrame::GetThemeProfile() const { // Always prefer the Browser Profile, if any. - if (browser_view_) { - return browser_view_->GetProfile(); + if (browser_view()) { + return browser_view()->GetProfile(); } if (!associated_profiles_.empty()) { return associated_profiles_.begin()->first; @@ -137,9 +136,9 @@ Profile* ChromeBrowserFrame::GetThemeProfile() const { } bool ChromeBrowserFrame::ToggleFullscreenMode() { - if (browser_view_) { + if (browser_view()) { // Toggle fullscreen mode via the Chrome command for consistent behavior. - chrome::ToggleFullscreenMode(browser_view_->browser()); + chrome::ToggleFullscreenMode(browser_view()->browser()); return true; } return false; @@ -167,10 +166,10 @@ ChromeBrowserFrame::CreateNonClientFrameView() { } void ChromeBrowserFrame::Activate() { - if (browser_view_ && browser_view_->browser() && - browser_view_->browser()->is_type_devtools()) { + if (browser_view() && browser_view()->browser() && + browser_view()->browser()->is_type_devtools()) { if (auto browser_host = ChromeBrowserHostImpl::GetBrowserForBrowser( - browser_view_->browser())) { + browser_view()->browser())) { if (browser_host->platform_delegate()->HasExternalParent()) { // Handle activation of DevTools with external parent via the platform // delegate. On Windows the default platform implementation @@ -189,6 +188,7 @@ void ChromeBrowserFrame::Activate() { void ChromeBrowserFrame::OnNativeWidgetDestroyed() { window_view_ = nullptr; + SetBrowserView(nullptr); BrowserFrame::OnNativeWidgetDestroyed(); } @@ -207,7 +207,7 @@ void ChromeBrowserFrame::OnNativeThemeUpdated(ui::NativeTheme* observed_theme) { } ui::ColorProviderKey ChromeBrowserFrame::GetColorProviderKey() const { - if (browser_view_) { + if (browser_view()) { // Use the default Browser implementation. return BrowserFrame::GetColorProviderKey(); } @@ -220,7 +220,7 @@ ui::ColorProviderKey ChromeBrowserFrame::GetColorProviderKey() const { } void ChromeBrowserFrame::OnThemeChanged() { - if (browser_view_) { + if (browser_view()) { // Ignore these notifications if we have a Browser. return; } diff --git a/libcef/browser/chrome/views/chrome_browser_frame.h b/libcef/browser/chrome/views/chrome_browser_frame.h index 4d6ebfa80..d1f728295 100644 --- a/libcef/browser/chrome/views/chrome_browser_frame.h +++ b/libcef/browser/chrome/views/chrome_browser_frame.h @@ -141,8 +141,6 @@ class ChromeBrowserFrame : public BrowserFrame, // ThemeServiceObserver methods: void OnThemeChanged() override; - BrowserView* browser_view() const { return browser_view_; } - private: // CefColorProviderTracker::Observer methods: void OnColorProviderCacheResetMissed() override; @@ -150,13 +148,12 @@ class ChromeBrowserFrame : public BrowserFrame, void NotifyThemeColorsChanged(bool chrome_theme); raw_ptr window_view_; - raw_ptr browser_view_ = nullptr; bool initialized_ = false; bool native_theme_change_ = false; // Map of Profile* to count. - using ProfileMap = std::map; + using ProfileMap = std::map, size_t>; ProfileMap associated_profiles_; CefColorProviderTracker color_provider_tracker_{this}; diff --git a/libcef/browser/chrome/views/chrome_browser_view.cc b/libcef/browser/chrome/views/chrome_browser_view.cc index e4b61ea6c..b6a3b5889 100644 --- a/libcef/browser/chrome/views/chrome_browser_view.cc +++ b/libcef/browser/chrome/views/chrome_browser_view.cc @@ -100,3 +100,11 @@ ToolbarView* ChromeBrowserView::OverrideCreateToolbar() { return nullptr; } + +void ChromeBrowserView::WillDestroyToolbar() { + BrowserView::WillDestroyToolbar(); + if (cef_toolbar_) { + cef_toolbar_->Destroyed(); + cef_toolbar_ = nullptr; + } +} diff --git a/libcef/browser/chrome/views/chrome_browser_view.h b/libcef/browser/chrome/views/chrome_browser_view.h index 56d36f4bd..36899d2ba 100644 --- a/libcef/browser/chrome/views/chrome_browser_view.h +++ b/libcef/browser/chrome/views/chrome_browser_view.h @@ -48,6 +48,7 @@ class ChromeBrowserView // BrowserView methods: ToolbarView* OverrideCreateToolbar() override; + void WillDestroyToolbar() override; CefRefPtr cef_toolbar() const { return cef_toolbar_; } CefBrowserViewImpl* cef_browser_view() const { return cef_browser_view_; } diff --git a/libcef/browser/chrome/views/toolbar_view_impl.cc b/libcef/browser/chrome/views/toolbar_view_impl.cc index 48938b53b..a9869c422 100644 --- a/libcef/browser/chrome/views/toolbar_view_impl.cc +++ b/libcef/browser/chrome/views/toolbar_view_impl.cc @@ -30,6 +30,11 @@ CefToolbarViewImpl::CefToolbarViewImpl( browser_view_(browser_view), display_mode_(display_mode) {} +void CefToolbarViewImpl::Destroyed() { + browser_ = nullptr; + browser_view_ = nullptr; +} + CefToolbarViewView* CefToolbarViewImpl::CreateRootView() { return new CefToolbarViewView(delegate(), browser_, browser_view_, display_mode_); diff --git a/libcef/browser/chrome/views/toolbar_view_impl.h b/libcef/browser/chrome/views/toolbar_view_impl.h index 9b3f19acb..437b648f5 100644 --- a/libcef/browser/chrome/views/toolbar_view_impl.h +++ b/libcef/browser/chrome/views/toolbar_view_impl.h @@ -22,6 +22,8 @@ class CefToolbarViewImpl CefToolbarViewImpl(const CefToolbarViewImpl&) = delete; CefToolbarViewImpl& operator=(const CefToolbarViewImpl&) = delete; + void Destroyed(); + // Create a new CefToolbarViewImpl instance. |delegate| may be nullptr. static CefRefPtr Create( CefRefPtr delegate, @@ -47,8 +49,8 @@ class CefToolbarViewImpl CefToolbarViewView* CreateRootView() override; void InitializeRootView() override; - const raw_ptr browser_; - const raw_ptr browser_view_; + raw_ptr browser_; + raw_ptr browser_view_; std::optional const display_mode_; IMPLEMENT_REFCOUNTING_DELETE_ON_UIT(CefToolbarViewImpl); diff --git a/libcef/browser/download_manager_delegate_impl.cc b/libcef/browser/download_manager_delegate_impl.cc index e993febcb..cea133cb2 100644 --- a/libcef/browser/download_manager_delegate_impl.cc +++ b/libcef/browser/download_manager_delegate_impl.cc @@ -438,7 +438,7 @@ CefDownloadManagerDelegateImpl::GetOrAssociateBrowser( download::DownloadItem* item) { ItemBrowserMap::const_iterator it = item_browser_map_.find(item); if (it != item_browser_map_.end()) { - return it->second; + return it->second.get(); } CefRefPtr browser; @@ -469,7 +469,7 @@ CefRefPtr CefDownloadManagerDelegateImpl::GetBrowser( DownloadItem* item) { ItemBrowserMap::const_iterator it = item_browser_map_.find(item); if (it != item_browser_map_.end()) { - return it->second; + return it->second.get(); } // If the download is rejected (e.g. ALT+click on an invalid protocol link) diff --git a/libcef/browser/download_manager_delegate_impl.h b/libcef/browser/download_manager_delegate_impl.h index 8cafd725e..56390e057 100644 --- a/libcef/browser/download_manager_delegate_impl.h +++ b/libcef/browser/download_manager_delegate_impl.h @@ -65,7 +65,8 @@ class CefDownloadManagerDelegateImpl // Map of DownloadItem to originating CefBrowserHostBase. Maintaining this // map is necessary because DownloadItem::GetWebContents() may return NULL if // the browser navigates while the download is in progress. - using ItemBrowserMap = std::map; + using ItemBrowserMap = + std::map, raw_ptr>; ItemBrowserMap item_browser_map_; }; diff --git a/libcef/browser/extensions/extension_system.cc b/libcef/browser/extensions/extension_system.cc index 67010ead3..7a89a909f 100644 --- a/libcef/browser/extensions/extension_system.cc +++ b/libcef/browser/extensions/extension_system.cc @@ -255,9 +255,7 @@ void CefExtensionSystem::Init() { // any, are handled in the guest renderer process by ChromePDFPrintClient // and CefPrintRenderFrameHelperDelegate. // 20.When navigating away from the PDF file or closing the owner CefBrowser - // the guest WebContents will be destroyed. This triggers a call to - // CefMimeHandlerViewGuestDelegate::OnGuestDetached which removes the - // routing ID association with the owner CefBrowser. + // the guest WebContents will be destroyed. if (PdfExtensionEnabled()) { if (auto manifest = ParseManifest(pdf_extension_util::GetManifest())) { LoadExtension(std::move(*manifest), @@ -392,6 +390,7 @@ void CefExtensionSystem::Shutdown() { DCHECK(!cef_extension->loader_context()); } #endif + renderer_helper_ = nullptr; extension_map_.clear(); } diff --git a/libcef/browser/extensions/value_store/cef_value_store_factory.h b/libcef/browser/extensions/value_store/cef_value_store_factory.h index d713af26f..eeede5418 100644 --- a/libcef/browser/extensions/value_store/cef_value_store_factory.h +++ b/libcef/browser/extensions/value_store/cef_value_store_factory.h @@ -49,7 +49,8 @@ class CefValueStoreFactory : public ValueStoreFactory { std::unique_ptr CreateStore(); base::FilePath db_path_; - raw_ptr last_created_store_ = nullptr; + raw_ptr last_created_store_ = + nullptr; // A mapping from directories to their ValueStore. None of these value // stores are owned by this factory, so care must be taken when calling diff --git a/libcef/browser/file_dialog_manager.cc b/libcef/browser/file_dialog_manager.cc index fe2717d89..b94260af7 100644 --- a/libcef/browser/file_dialog_manager.cc +++ b/libcef/browser/file_dialog_manager.cc @@ -230,7 +230,7 @@ class CefSelectFileDialogListener : public ui::SelectFileDialog::Listener { void* params) override { DCHECK_EQ(params, params_); executing_ = true; - listener_->FileSelected(file, index, params); + listener_.ExtractAsDangling()->FileSelected(file, index, params); Destroy(); } @@ -238,14 +238,14 @@ class CefSelectFileDialogListener : public ui::SelectFileDialog::Listener { void* params) override { DCHECK_EQ(params, params_); executing_ = true; - listener_->MultiFilesSelected(files, params); + listener_.ExtractAsDangling()->MultiFilesSelected(files, params); Destroy(); } void FileSelectionCanceled(void* params) override { DCHECK_EQ(params, params_); executing_ = true; - listener_->FileSelectionCanceled(params); + listener_.ExtractAsDangling()->FileSelectionCanceled(params); Destroy(); } @@ -254,7 +254,7 @@ class CefSelectFileDialogListener : public ui::SelectFileDialog::Listener { delete this; } - const raw_ptr listener_; + raw_ptr listener_; const raw_ptr params_; base::OnceClosure callback_; diff --git a/libcef/browser/file_dialog_manager.h b/libcef/browser/file_dialog_manager.h index 2254b679d..78ee27fa0 100644 --- a/libcef/browser/file_dialog_manager.h +++ b/libcef/browser/file_dialog_manager.h @@ -91,7 +91,7 @@ class CefFileDialogManager { raw_ptr dialog_listener_ = nullptr; // List of all currently active listeners. - std::set active_listeners_; + std::set> active_listeners_; base::WeakPtrFactory weak_ptr_factory_{this}; }; diff --git a/libcef/browser/frame_host_impl.cc b/libcef/browser/frame_host_impl.cc index 7fc38dade..b2e6273a7 100644 --- a/libcef/browser/frame_host_impl.cc +++ b/libcef/browser/frame_host_impl.cc @@ -521,7 +521,7 @@ bool CefFrameHostImpl::Detach(DetachReason reason) { bool first_detach = false; // Should not be called for temporary frames. - DCHECK(!is_temporary()); + CHECK(!is_temporary()); { base::AutoLock lock_scope(state_lock_); @@ -548,17 +548,32 @@ bool CefFrameHostImpl::Detach(DetachReason reason) { void CefFrameHostImpl::MaybeReAttach( scoped_refptr browser_info, - content::RenderFrameHost* render_frame_host) { + content::RenderFrameHost* render_frame_host, + bool require_detached) { CEF_REQUIRE_UIT(); if (render_frame_.is_bound() && render_frame_host_ == render_frame_host) { // Nothing to do here. return; } - // We expect that Detach() was called previously. + // Should not be called for temporary frames. CHECK(!is_temporary()); - CHECK(!render_frame_.is_bound()); - CHECK(!render_frame_host_); + + if (require_detached) { + // We expect that Detach() was called previously. + CHECK(!render_frame_.is_bound()); + CHECK(!render_frame_host_); + } else if (render_frame_host_) { + // Intentionally not clearing |queued_renderer_actions_|, as we may be + // changing RFH during initial browser navigation. + VLOG(1) << GetDebugString() + << " detached (reason=RENDER_FRAME_CHANGED, is_connected=" + << render_frame_.is_bound() << ")"; + if (render_frame_.is_bound()) { + render_frame_->FrameDetached(); + } + render_frame_.reset(); + } // The RFH may change but the frame token should remain the same. CHECK(*frame_token_ == render_frame_host->GetGlobalFrameToken()); diff --git a/libcef/browser/frame_host_impl.h b/libcef/browser/frame_host_impl.h index 0fb3306a4..f1c18c6bc 100644 --- a/libcef/browser/frame_host_impl.h +++ b/libcef/browser/frame_host_impl.h @@ -144,7 +144,8 @@ class CefFrameHostImpl : public CefFrame, public cef::mojom::BrowserFrame { // cache. We may need to re-attach if the RFH has changed. See // https://crbug.com/1179502#c8 for additional background. void MaybeReAttach(scoped_refptr browser_info, - content::RenderFrameHost* render_frame_host); + content::RenderFrameHost* render_frame_host, + bool require_detached); // cef::mojom::BrowserFrame methods forwarded from CefBrowserFrame. void SendMessage(const std::string& name, diff --git a/libcef/browser/net_service/proxy_url_loader_factory.cc b/libcef/browser/net_service/proxy_url_loader_factory.cc index 24881c47a..dcdad0639 100644 --- a/libcef/browser/net_service/proxy_url_loader_factory.cc +++ b/libcef/browser/net_service/proxy_url_loader_factory.cc @@ -233,7 +233,7 @@ class InterceptedRequest : public network::mojom::URLLoader, mojo::PendingReceiver receiver); // Called from InterceptDelegate::OnInputStreamOpenFailed. - void InputStreamFailed(bool restart_needed); + bool InputStreamFailed(); // mojom::TrustedHeaderClient methods: void OnBeforeSendHeaders(const net::HttpRequestHeaders& headers, @@ -328,7 +328,6 @@ class InterceptedRequest : public network::mojom::URLLoader, const raw_ptr factory_; const int32_t id_; const uint32_t options_; - bool input_stream_previously_failed_ = false; bool request_was_redirected_ = false; int redirect_limit_ = net::URLRequest::kMaxRedirects; bool redirect_in_progress_ = false; @@ -374,7 +373,7 @@ class InterceptedRequest : public network::mojom::URLLoader, mojo::Receiver header_client_receiver_{ this}; - raw_ptr stream_loader_ = nullptr; + std::unique_ptr stream_loader_; base::WeakPtrFactory weak_factory_; }; @@ -391,9 +390,8 @@ class InterceptDelegate : public StreamReaderURLLoader::Delegate { return response_->OpenInputStream(request_id, request, std::move(callback)); } - void OnInputStreamOpenFailed(int32_t request_id, bool* restarted) override { - request_->InputStreamFailed(false /* restart_needed */); - *restarted = false; + bool OnInputStreamOpenFailed(int32_t request_id) override { + return request_->InputStreamFailed(); } void GetResponseHeaders(int32_t request_id, @@ -456,7 +454,11 @@ InterceptedRequest::~InterceptedRequest() { } void InterceptedRequest::Restart() { - stream_loader_ = nullptr; + // May exist if the previous stream resulted in a redirect. + if (stream_loader_) { + stream_loader_.reset(); + } + if (proxied_client_receiver_.is_bound()) { proxied_client_receiver_.reset(); target_loader_.reset(); @@ -555,24 +557,16 @@ void InterceptedRequest::OnLoaderCreated( header_client_receiver_.Bind(std::move(receiver)); } -void InterceptedRequest::InputStreamFailed(bool restart_needed) { - DCHECK(!input_stream_previously_failed_); - +bool InterceptedRequest::InputStreamFailed() { if (intercept_only_) { // This can happen for unsupported schemes, when no proper // response from the intercept handler is received, i.e. // the provided input stream in response failed to load. In // this case we send and error and stop loading. SendErrorAndCompleteImmediately(net::ERR_UNKNOWN_URL_SCHEME); - return; + return true; } - - if (!restart_needed) { - return; - } - - input_stream_previously_failed_ = true; - Restart(); + return false; } // TrustedHeaderClient methods. @@ -791,7 +785,7 @@ void InterceptedRequest::BeforeRequestReceived(const GURL& original_url, intercept_request_ = intercept_request; intercept_only_ = intercept_only; - if (input_stream_previously_failed_ || !intercept_request_) { + if (!intercept_request_) { // Equivalent to no interception. InterceptResponseReceived(original_url, nullptr); } else { @@ -894,7 +888,8 @@ void InterceptedRequest::ContinueAfterInterceptWithOverride( // avoid having Set-Cookie headers stripped by the IPC layer. current_request_uses_header_client_ = true; - stream_loader_ = new StreamReaderURLLoader( + DCHECK(!stream_loader_); + stream_loader_ = std::make_unique( id_, request_, proxied_client_receiver_.BindNewPipeAndPassRemote(), header_client_receiver_.BindNewPipeAndPassRemote(), traffic_annotation_, std::move(current_cached_metadata_), @@ -1092,6 +1087,9 @@ void InterceptedRequest::ContinueToResponseStarted(int error_code) { const bool is_redirect = redirect_url.is_valid() || (headers && headers->IsRedirect(&location)); if (stream_loader_ && is_redirect) { + // Don't continue reading from the stream loader. + stream_loader_->Cancel(); + // Redirecting from OnReceiveResponse generally isn't supported by the // NetworkService, so we can only support it when using a custom loader. // TODO(network): Remove this special case. @@ -1126,6 +1124,9 @@ void InterceptedRequest::ContinueToResponseStarted(int error_code) { if (!result.has_value() && !HasCrossOriginWhitelistEntry(*request_.request_initiator, url::Origin::Create(request_.url))) { + // Don't continue reading from the stream loader. + stream_loader_->Cancel(); + SendErrorStatusAndCompleteImmediately( network::URLLoaderCompletionStatus(result.error())); return; @@ -1138,6 +1139,11 @@ void InterceptedRequest::ContinueToResponseStarted(int error_code) { proxied_client_receiver_.Resume(); } + if (stream_loader_) { + // Continue reading from the stream loader. + stream_loader_->Continue(); + } + target_client_->OnReceiveResponse( std::move(current_response_), factory_->request_handler_->OnFilterResponseBody( diff --git a/libcef/browser/net_service/stream_reader_url_loader.cc b/libcef/browser/net_service/stream_reader_url_loader.cc index 8c2afb1cd..77650d8b5 100644 --- a/libcef/browser/net_service/stream_reader_url_loader.cc +++ b/libcef/browser/net_service/stream_reader_url_loader.cc @@ -523,6 +523,29 @@ void StreamReaderURLLoader::Start() { } } +void StreamReaderURLLoader::Continue() { + DCHECK(thread_checker_.CalledOnValidThread()); + + DCHECK(need_client_callback_ && !got_client_callback_); + got_client_callback_ = true; + + writable_handle_watcher_.Watch( + producer_handle_.get(), MOJO_HANDLE_SIGNAL_WRITABLE, + base::BindRepeating(&StreamReaderURLLoader::OnDataPipeWritable, + base::Unretained(this))); + + ReadMore(); +} + +void StreamReaderURLLoader::Cancel() { + DCHECK(thread_checker_.CalledOnValidThread()); + + DCHECK(need_client_callback_ && !got_client_callback_); + got_client_callback_ = true; + + CleanUp(); +} + void StreamReaderURLLoader::ContinueWithRequestHeaders( int32_t result, const std::optional& headers) { @@ -571,13 +594,8 @@ void StreamReaderURLLoader::OnInputStreamOpened( open_cancel_callback_.Reset(); if (!input_stream) { - bool restarted = false; - response_delegate_->OnInputStreamOpenFailed(request_id_, &restarted); - if (restarted) { - // The request has been restarted with a new loader. - // |this| will be deleted. - CleanUp(); - } else { + // May delete |this| iff returns true. + if (!response_delegate_->OnInputStreamOpenFailed(request_id_)) { HeadersComplete(net::HTTP_NOT_FOUND, -1); } return; @@ -700,13 +718,15 @@ void StreamReaderURLLoader::ContinueWithResponseHeaders( pending_response->encoded_body_length = nullptr; const GURL new_location = has_redirect_url ? *redirect_url : request_.url.Resolve(location); + + CleanUp(); + + // The client will restart the request with a new loader. + // May delete |this|. client_->OnReceiveRedirect( MakeRedirectInfo(request_, pending_headers.get(), new_location, pending_headers->response_code()), std::move(pending_response)); - // The client will restart the request with a new loader. - // |this| will be deleted. - CleanUp(); } else { mojo::ScopedDataPipeConsumerHandle consumer_handle; if (CreateDataPipe(nullptr /*options*/, producer_handle_, @@ -714,15 +734,13 @@ void StreamReaderURLLoader::ContinueWithResponseHeaders( RequestComplete(net::ERR_FAILED); return; } - writable_handle_watcher_.Watch( - producer_handle_.get(), MOJO_HANDLE_SIGNAL_WRITABLE, - base::BindRepeating(&StreamReaderURLLoader::OnDataPipeWritable, - base::Unretained(this))); + need_client_callback_ = true; client_->OnReceiveResponse(std::move(pending_response), std::move(consumer_handle), std::move(cached_metadata_)); - ReadMore(); + + // Wait for the client to call Continue() or Cancel(). } } @@ -808,20 +826,21 @@ void StreamReaderURLLoader::RequestComplete(int status_code) { // We don't support decoders, so use the same value. status.decoded_body_length = total_bytes_read_; - client_->OnComplete(status); CleanUp(); + + // May delete |this|. + client_->OnComplete(status); } void StreamReaderURLLoader::CleanUp() { DCHECK(thread_checker_.CalledOnValidThread()); + weak_factory_.InvalidateWeakPtrs(); + // Resets the watchers and pipes, so that we will never be called back. writable_handle_watcher_.Cancel(); pending_buffer_ = nullptr; producer_handle_.reset(); - - // Manages its own lifetime. - delete this; } bool StreamReaderURLLoader::ParseRange(const net::HttpRequestHeaders& headers) { diff --git a/libcef/browser/net_service/stream_reader_url_loader.h b/libcef/browser/net_service/stream_reader_url_loader.h index 5c1ac58fc..8c4096029 100644 --- a/libcef/browser/net_service/stream_reader_url_loader.h +++ b/libcef/browser/net_service/stream_reader_url_loader.h @@ -104,11 +104,9 @@ class StreamReaderURLLoader : public network::mojom::URLLoader { // on the IO thread unless otherwise indicated. class Delegate : public ResourceResponse { public: - // This method is called if the result of calling OpenInputStream was null. - // The |restarted| parameter is set to true if the request was restarted - // with a new loader. - virtual void OnInputStreamOpenFailed(int32_t request_id, - bool* restarted) = 0; + // Called if the result of calling OpenInputStream was nullptr. Returns + // true if the failure was handled. + virtual bool OnInputStreamOpenFailed(int32_t request_id) = 0; }; StreamReaderURLLoader( @@ -127,6 +125,10 @@ class StreamReaderURLLoader : public network::mojom::URLLoader { void Start(); + // Called by the client in response to OnReceiveResponse. + void Continue(); + void Cancel(); + // network::mojom::URLLoader methods: void FollowRedirect( const std::vector& removed_headers, @@ -186,6 +188,9 @@ class StreamReaderURLLoader : public network::mojom::URLLoader { base::OnceClosure open_cancel_callback_; + bool need_client_callback_ = false; + bool got_client_callback_ = false; + base::WeakPtrFactory weak_factory_; }; diff --git a/libcef/browser/osr/browser_platform_delegate_osr.cc b/libcef/browser/osr/browser_platform_delegate_osr.cc index 287ebdd67..34e6b0450 100644 --- a/libcef/browser/osr/browser_platform_delegate_osr.cc +++ b/libcef/browser/osr/browser_platform_delegate_osr.cc @@ -87,9 +87,9 @@ void CefBrowserPlatformDelegateOsr::BrowserCreated( void CefBrowserPlatformDelegateOsr::BrowserDestroyed( CefBrowserHostBase* browser) { - CefBrowserPlatformDelegateAlloy::BrowserDestroyed(browser); - view_osr_ = nullptr; + current_rvh_for_drag_ = nullptr; + CefBrowserPlatformDelegateAlloy::BrowserDestroyed(browser); } SkColor CefBrowserPlatformDelegateOsr::GetBackgroundColor() const { diff --git a/libcef/browser/osr/render_widget_host_view_osr.cc b/libcef/browser/osr/render_widget_host_view_osr.cc index 582e198bb..18a9aac51 100644 --- a/libcef/browser/osr/render_widget_host_view_osr.cc +++ b/libcef/browser/osr/render_widget_host_view_osr.cc @@ -307,8 +307,16 @@ void CefRenderWidgetHostViewOSR::ReleaseCompositor() { content::DelegatedFrameHost::HiddenCause::kOther); } delegated_frame_host_->DetachFromCompositor(); - delegated_frame_host_.reset(nullptr); + + content::RenderWidgetHostImpl* render_widget_host_impl = + content::RenderWidgetHostImpl::From(render_widget_host_); + if (render_widget_host_impl) { + render_widget_host_impl->SetCompositorForFlingScheduler(nullptr); + } + + host_display_client_ = nullptr; + compositor_.reset(nullptr); } diff --git a/libcef/browser/osr/render_widget_host_view_osr.h b/libcef/browser/osr/render_widget_host_view_osr.h index 5dd54eb7b..5c9acf614 100644 --- a/libcef/browser/osr/render_widget_host_view_osr.h +++ b/libcef/browser/osr/render_widget_host_view_osr.h @@ -425,7 +425,7 @@ class CefRenderWidgetHostViewOSR raw_ptr parent_host_view_; raw_ptr popup_host_view_ = nullptr; raw_ptr child_host_view_ = nullptr; - std::set guest_host_views_; + std::set> guest_host_views_; CefRefPtr browser_impl_; diff --git a/libcef/browser/simple_menu_model_impl.cc b/libcef/browser/simple_menu_model_impl.cc index 8c7d74016..41f8e00d5 100644 --- a/libcef/browser/simple_menu_model_impl.cc +++ b/libcef/browser/simple_menu_model_impl.cc @@ -62,16 +62,16 @@ CefSimpleMenuModelImpl::~CefSimpleMenuModelImpl() { void CefSimpleMenuModelImpl::Detach() { DCHECK(VerifyContext()); - if (!submenumap_.empty()) { + while (!submenumap_.empty()) { auto it = submenumap_.begin(); - for (; it != submenumap_.end(); ++it) { - it->second->Detach(); - } - submenumap_.clear(); + auto impl = it->second; + // Clear the raw_ptr reference before calling Detach(). + submenumap_.erase(it); + impl->Detach(); } if (is_owned_) { - delete model_; + model_.ClearAndDelete(); } model_ = nullptr; } diff --git a/libcef/browser/simple_menu_model_impl.h b/libcef/browser/simple_menu_model_impl.h index 9a33dbe2f..71e4ac200 100644 --- a/libcef/browser/simple_menu_model_impl.h +++ b/libcef/browser/simple_menu_model_impl.h @@ -163,7 +163,7 @@ class CefSimpleMenuModelImpl : public CefMenuModel { // Keep the submenus alive until they're removed, or we're destroyed. using SubMenuMap = - std::map>; + std::map, CefRefPtr>; SubMenuMap submenumap_; IMPLEMENT_REFCOUNTING(CefSimpleMenuModelImpl); diff --git a/libcef/browser/stream_impl.cc b/libcef/browser/stream_impl.cc index 3e0bd6a96..18eca67dc 100644 --- a/libcef/browser/stream_impl.cc +++ b/libcef/browser/stream_impl.cc @@ -6,8 +6,11 @@ #include +#include + #include "base/files/file_util.h" #include "base/logging.h" +#include "base/numerics/safe_conversions.h" #include "base/threading/thread_restrictions.h" // Static functions @@ -33,7 +36,7 @@ CefRefPtr CefStreamReader::CreateForData(void* data, DCHECK(size > 0); CefRefPtr reader; if (data && size > 0) { - reader = new CefBytesReader(data, size, true); + reader = new CefBytesReader(data, size); } return reader; } @@ -147,19 +150,15 @@ int CefFileWriter::Flush() { // CefBytesReader -CefBytesReader::CefBytesReader(void* data, int64_t datasize, bool copy) { - SetData(data, datasize, copy); -} - -CefBytesReader::~CefBytesReader() { - SetData(nullptr, 0, false); +CefBytesReader::CefBytesReader(void* data, int64_t datasize) { + SetData(data, datasize); } size_t CefBytesReader::Read(void* ptr, size_t size, size_t n) { base::AutoLock lock_scope(lock_); - size_t s = (datasize_ - offset_) / size; - size_t ret = (n < s ? n : s); - memcpy(ptr, (reinterpret_cast(data_.get())) + offset_, ret * size); + size_t s = (data_.size() - offset_) / size; + size_t ret = std::min(n, s); + memcpy(ptr, data_.data() + offset_, ret * size); offset_ += ret * size; return ret; } @@ -167,9 +166,12 @@ size_t CefBytesReader::Read(void* ptr, size_t size, size_t n) { int CefBytesReader::Seek(int64_t offset, int whence) { int rv = -1L; base::AutoLock lock_scope(lock_); + + const int64_t size = base::checked_cast(data_.size()); + switch (whence) { case SEEK_CUR: - if (offset_ + offset > datasize_ || offset_ + offset < 0) { + if (offset_ + offset > size || offset_ + offset < 0) { break; } offset_ += offset; @@ -177,15 +179,15 @@ int CefBytesReader::Seek(int64_t offset, int whence) { break; case SEEK_END: { int64_t offset_abs = std::abs(offset); - if (offset_abs > datasize_) { + if (offset_abs > size) { break; } - offset_ = datasize_ - offset_abs; + offset_ = size - offset_abs; rv = 0; break; } case SEEK_SET: - if (offset > datasize_ || offset < 0) { + if (offset > size || offset < 0) { break; } offset_ = offset; @@ -203,120 +205,20 @@ int64_t CefBytesReader::Tell() { int CefBytesReader::Eof() { base::AutoLock lock_scope(lock_); - return (offset_ >= datasize_); + return (offset_ >= base::checked_cast(data_.size())); } -void CefBytesReader::SetData(void* data, int64_t datasize, bool copy) { +void CefBytesReader::SetData(void* data, int64_t datasize) { base::AutoLock lock_scope(lock_); - if (copy_) { - free(data_); - } - copy_ = copy; offset_ = 0; - datasize_ = datasize; - if (copy) { - data_ = malloc(datasize); - DCHECK(data_ != nullptr); - if (data_) { - memcpy(data_, data, datasize); - } + if (data && datasize > 0) { + data_.reserve(datasize); + std::copy(static_cast(data), + static_cast(data) + datasize, + std::back_inserter(data_)); } else { - data_ = data; + data_.clear(); } } - -// CefBytesWriter - -CefBytesWriter::CefBytesWriter(size_t grow) : grow_(grow), datasize_(grow) { - DCHECK(grow > 0); - data_ = malloc(grow); - DCHECK(data_ != nullptr); -} - -CefBytesWriter::~CefBytesWriter() { - base::AutoLock lock_scope(lock_); - if (data_) { - free(data_); - } -} - -size_t CefBytesWriter::Write(const void* ptr, size_t size, size_t n) { - base::AutoLock lock_scope(lock_); - size_t rv; - if (offset_ + static_cast(size * n) >= datasize_ && - Grow(size * n) == 0) { - rv = 0; - } else { - memcpy(reinterpret_cast(data_.get()) + offset_, ptr, size * n); - offset_ += size * n; - rv = n; - } - - return rv; -} - -int CefBytesWriter::Seek(int64_t offset, int whence) { - int rv = -1L; - base::AutoLock lock_scope(lock_); - switch (whence) { - case SEEK_CUR: - if (offset_ + offset > datasize_ || offset_ + offset < 0) { - break; - } - offset_ += offset; - rv = 0; - break; - case SEEK_END: { - int64_t offset_abs = std::abs(offset); - if (offset_abs > datasize_) { - break; - } - offset_ = datasize_ - offset_abs; - rv = 0; - break; - } - case SEEK_SET: - if (offset > datasize_ || offset < 0) { - break; - } - offset_ = offset; - rv = 0; - break; - } - - return rv; -} - -int64_t CefBytesWriter::Tell() { - base::AutoLock lock_scope(lock_); - return offset_; -} - -int CefBytesWriter::Flush() { - return 0; -} - -std::string CefBytesWriter::GetDataString() { - base::AutoLock lock_scope(lock_); - std::string str(reinterpret_cast(data_.get()), offset_); - return str; -} - -size_t CefBytesWriter::Grow(size_t size) { - base::AutoLock lock_scope(lock_); - size_t rv; - size_t s = (size > grow_ ? size : grow_); - void* tmp = realloc(data_, datasize_ + s); - DCHECK(tmp != nullptr); - if (tmp) { - data_ = tmp; - datasize_ += s; - rv = datasize_; - } else { - rv = 0; - } - - return rv; -} diff --git a/libcef/browser/stream_impl.h b/libcef/browser/stream_impl.h index 96b6e09bf..d1bcae475 100644 --- a/libcef/browser/stream_impl.h +++ b/libcef/browser/stream_impl.h @@ -8,7 +8,7 @@ #include -#include +#include #include "base/memory/raw_ptr.h" #include "base/synchronization/lock.h" @@ -59,8 +59,8 @@ class CefFileWriter : public CefStreamWriter { // Implementation of CefStreamReader for byte buffers. class CefBytesReader : public CefStreamReader { public: - CefBytesReader(void* data, int64_t datasize, bool copy); - ~CefBytesReader() override; + // |data| is always copied. + CefBytesReader(void* data, int64_t datasize); size_t Read(void* ptr, size_t size, size_t n) override; int Seek(int64_t offset, int whence) override; @@ -68,15 +68,11 @@ class CefBytesReader : public CefStreamReader { int Eof() override; bool MayBlock() override { return false; } - void SetData(void* data, int64_t datasize, bool copy); - - void* GetData() { return data_; } - size_t GetDataSize() { return offset_; } + // |data| is always copied. + void SetData(void* data, int64_t datasize); protected: - raw_ptr data_ = nullptr; - int64_t datasize_ = 0; - bool copy_ = false; + std::vector data_; int64_t offset_ = 0; base::Lock lock_; @@ -84,35 +80,6 @@ class CefBytesReader : public CefStreamReader { IMPLEMENT_REFCOUNTING(CefBytesReader); }; -// Implementation of CefStreamWriter for byte buffers. -class CefBytesWriter : public CefStreamWriter { - public: - explicit CefBytesWriter(size_t grow); - ~CefBytesWriter() override; - - size_t Write(const void* ptr, size_t size, size_t n) override; - int Seek(int64_t offset, int whence) override; - int64_t Tell() override; - int Flush() override; - bool MayBlock() override { return false; } - - void* GetData() { return data_; } - int64_t GetDataSize() { return offset_; } - std::string GetDataString(); - - protected: - size_t Grow(size_t size); - - size_t grow_; - raw_ptr data_; - int64_t datasize_; - int64_t offset_ = 0; - - base::Lock lock_; - - IMPLEMENT_REFCOUNTING(CefBytesWriter); -}; - // Implementation of CefStreamReader for handlers. class CefHandlerReader : public CefStreamReader { public: diff --git a/libcef/browser/views/layout_impl.h b/libcef/browser/views/layout_impl.h index 18053d6ba..6e6e0003c 100644 --- a/libcef/browser/views/layout_impl.h +++ b/libcef/browser/views/layout_impl.h @@ -56,8 +56,8 @@ class CefLayoutImpl : public CefLayoutAdapter, public CefLayoutClass { owner_view_ = owner_view; layout_ref_ = CreateLayout(); DCHECK(layout_ref_); - owner_view->SetLayoutManager(base::WrapUnique(layout_ref_.get())); layout_util::Assign(this, owner_view); + owner_view->SetLayoutManager(base::WrapUnique(layout_ref_.get())); } // Create the views::LayoutManager object. diff --git a/libcef/browser/views/native_widget_mac.h b/libcef/browser/views/native_widget_mac.h index b244a038c..bb7e7e354 100644 --- a/libcef/browser/views/native_widget_mac.h +++ b/libcef/browser/views/native_widget_mac.h @@ -51,7 +51,7 @@ class CefNativeWidgetMac : public views::NativeWidgetMac { // Returns true if the CefWindow is fully initialized. bool IsCefWindowInitialized() const; - raw_ptr browser_view_ = nullptr; + raw_ptr browser_view_ = nullptr; }; #endif // CEF_LIBCEF_BROWSER_VIEWS_NATIVE_WIDGET_MAC_H_ diff --git a/libcef/browser/views/overlay_view_host.cc b/libcef/browser/views/overlay_view_host.cc index 6a11ac0f3..eb3793acc 100644 --- a/libcef/browser/views/overlay_view_host.cc +++ b/libcef/browser/views/overlay_view_host.cc @@ -55,13 +55,13 @@ class CefOverlayControllerImpl : public CefOverlayController { if (IsValid()) { // Results in a call to Destroyed(). host_->Close(); - host_ = nullptr; } } void Destroyed() { DCHECK(view_); view_ = nullptr; + host_ = nullptr; } void SetBounds(const CefRect& bounds) override { diff --git a/libcef/browser/views/view_view.h b/libcef/browser/views/view_view.h index 0936ab7be..ecd0d7826 100644 --- a/libcef/browser/views/view_view.h +++ b/libcef/browser/views/view_view.h @@ -38,6 +38,15 @@ CEF_VIEW_VIEW_T class CefViewView : public ViewsViewClass { explicit CefViewView(CefViewDelegateClass* cef_delegate, Args... args) : ParentClass(args...), cef_delegate_(cef_delegate) {} + ~CefViewView() override { + // Clear the reference to the delegate which may be released by the + // CefViewImpl when it's destroyed via UserData. + cef_delegate_ = nullptr; + + // Remove any UserData references to class members before they're destroyed. + ParentClass::ClearAllUserData(); + } + // Should be called from InitializeRootView() in the CefViewImpl-derived // class that created this object. This method will be called after // CefViewImpl registration has completed so it is safe to call complex @@ -88,7 +97,7 @@ CEF_VIEW_VIEW_T class CefViewView : public ViewsViewClass { const views::ViewHierarchyChangedDetails& details); // Not owned by this object. - const raw_ptr cef_delegate_; + raw_ptr cef_delegate_; }; CEF_VIEW_VIEW_T gfx::Size CEF_VIEW_VIEW_D::CalculatePreferredSize() const { diff --git a/libcef/browser/views/widget_impl.h b/libcef/browser/views/widget_impl.h index 3a633d1b3..58df5441c 100644 --- a/libcef/browser/views/widget_impl.h +++ b/libcef/browser/views/widget_impl.h @@ -89,7 +89,7 @@ class CefWidgetImpl : public views::Widget, bool initialized_ = false; // Map of Profile* to count. - using ProfileMap = std::map; + using ProfileMap = std::map, size_t>; ProfileMap associated_profiles_; CefColorProviderTracker color_provider_tracker_{this}; diff --git a/libcef/browser/views/window_view.cc b/libcef/browser/views/window_view.cc index 3c26439d2..db17c48c7 100644 --- a/libcef/browser/views/window_view.cc +++ b/libcef/browser/views/window_view.cc @@ -54,31 +54,29 @@ class ClientViewEx : public views::ClientView { public: ClientViewEx(views::Widget* widget, views::View* contents_view, - CefWindowView::Delegate* window_delegate) - : views::ClientView(widget, contents_view), - window_delegate_(window_delegate) { - DCHECK(window_delegate_); - } + base::WeakPtr view) + : views::ClientView(widget, contents_view), view_(std::move(view)) {} ClientViewEx(const ClientViewEx&) = delete; ClientViewEx& operator=(const ClientViewEx&) = delete; views::CloseRequestResult OnWindowCloseRequested() override { - return window_delegate_->CanWidgetClose() + return view_->window_delegate()->CanWidgetClose() ? views::CloseRequestResult::kCanClose : views::CloseRequestResult::kCannotClose; } private: - // Not owned by this object. - raw_ptr window_delegate_; + const base::WeakPtr view_; }; // Extend NativeFrameView with draggable region handling. class NativeFrameViewEx : public views::NativeFrameView { public: - NativeFrameViewEx(views::Widget* widget, CefWindowView* view) - : views::NativeFrameView(widget), widget_(widget), view_(view) {} + NativeFrameViewEx(views::Widget* widget, base::WeakPtr view) + : views::NativeFrameView(widget), + widget_(widget), + view_(std::move(view)) {} NativeFrameViewEx(const NativeFrameViewEx&) = delete; NativeFrameViewEx& operator=(const NativeFrameViewEx&) = delete; @@ -155,7 +153,7 @@ class NativeFrameViewEx : public views::NativeFrameView { private: // Not owned by this object. raw_ptr widget_; - raw_ptr view_; + const base::WeakPtr view_; }; // The area inside the frame border that can be clicked and dragged for resizing @@ -170,8 +168,8 @@ const int kResizeAreaCornerSize = 16; // with a resizable border. Based on AppWindowFrameView and CustomFrameView. class CaptionlessFrameView : public views::NonClientFrameView { public: - CaptionlessFrameView(views::Widget* widget, CefWindowView* view) - : widget_(widget), view_(view) {} + CaptionlessFrameView(views::Widget* widget, base::WeakPtr view) + : widget_(widget), view_(std::move(view)) {} CaptionlessFrameView(const CaptionlessFrameView&) = delete; CaptionlessFrameView& operator=(const CaptionlessFrameView&) = delete; @@ -288,7 +286,7 @@ class CaptionlessFrameView : public views::NonClientFrameView { // Not owned by this object. raw_ptr widget_; - raw_ptr view_; + const base::WeakPtr view_; // The bounds of the client view, in this view's coordinates. gfx::Rect client_view_bounds_; @@ -680,8 +678,8 @@ void CefWindowView::WindowClosing() { // Close any overlays now, before the Widget is destroyed. // Use a copy of the array because the original may be modified while // iterating. - std::vector overlay_hosts = overlay_hosts_; - for (auto* overlay_host : overlay_hosts) { + std::vector> overlay_hosts = overlay_hosts_; + for (auto& overlay_host : overlay_hosts) { overlay_host->Close(); } @@ -711,18 +709,21 @@ views::View* CefWindowView::GetContentsView() { } views::ClientView* CefWindowView::CreateClientView(views::Widget* widget) { - return new ClientViewEx(widget, GetContentsView(), window_delegate_); + return new ClientViewEx(widget, GetContentsView(), + weak_ptr_factory_.GetWeakPtr()); } std::unique_ptr CefWindowView::CreateNonClientFrameView(views::Widget* widget) { if (is_frameless_) { // Custom frame type that doesn't render a caption. - return std::make_unique(widget, this); + return std::make_unique( + widget, weak_ptr_factory_.GetWeakPtr()); } else if (widget->ShouldUseNativeFrame()) { // DesktopNativeWidgetAura::CreateNonClientFrameView() returns // NativeFrameView by default. Extend that type. - return std::make_unique(widget, this); + return std::make_unique(widget, + weak_ptr_factory_.GetWeakPtr()); } // Use Chromium provided CustomFrameView. In case if we would like to diff --git a/libcef/browser/views/window_view.h b/libcef/browser/views/window_view.h index fe1be369d..e3d0d7e1d 100644 --- a/libcef/browser/views/window_view.h +++ b/libcef/browser/views/window_view.h @@ -10,6 +10,7 @@ #include #include "base/memory/raw_ptr.h" +#include "base/memory/weak_ptr.h" #include "cef/include/views/cef_window.h" #include "cef/include/views/cef_window_delegate.h" #include "cef/libcef/browser/views/overlay_view_host.h" @@ -141,6 +142,8 @@ class CefWindowView bool IsAlloyStyle() const { return is_alloy_style_; } bool IsChromeStyle() const { return !is_alloy_style_; } + Delegate* window_delegate() const { return window_delegate_.get(); } + private: // Called after Widget teardown starts, before |this| is deleted. void DeleteDelegate(); @@ -173,7 +176,9 @@ class CefWindowView std::unique_ptr host_widget_destruction_observer_; // Hosts for overlay widgets. - std::vector overlay_hosts_; + std::vector> overlay_hosts_; + + base::WeakPtrFactory weak_ptr_factory_{this}; }; #endif // CEF_LIBCEF_BROWSER_VIEWS_WINDOW_VIEW_H_ diff --git a/libcef/common/alloy/alloy_main_runner_delegate.cc b/libcef/common/alloy/alloy_main_runner_delegate.cc index 60ef671d8..01b5a322f 100644 --- a/libcef/common/alloy/alloy_main_runner_delegate.cc +++ b/libcef/common/alloy/alloy_main_runner_delegate.cc @@ -5,6 +5,7 @@ #include "cef/libcef/common/alloy/alloy_main_runner_delegate.h" +#include "cef/libcef/browser/alloy/alloy_content_browser_client.h" #include "cef/libcef/browser/alloy/chrome_browser_process_alloy.h" #include "cef/libcef/common/alloy/alloy_main_delegate.h" #include "cef/libcef/renderer/alloy/alloy_content_renderer_client.h" @@ -43,6 +44,9 @@ void AlloyMainRunnerDelegate::AfterUIThreadInitialize() { } void AlloyMainRunnerDelegate::BeforeUIThreadShutdown() { + static_cast( + CefAppManager::Get()->GetContentClient()->browser()) + ->CleanupOnUIThread(); static_cast(g_browser_process) ->CleanupOnUIThread(); diff --git a/libcef/common/chrome/chrome_main_runner_delegate.cc b/libcef/common/chrome/chrome_main_runner_delegate.cc index cffd45b75..80c35d5d8 100644 --- a/libcef/common/chrome/chrome_main_runner_delegate.cc +++ b/libcef/common/chrome/chrome_main_runner_delegate.cc @@ -7,10 +7,10 @@ #include "base/command_line.h" #include "base/run_loop.h" +#include "cef/libcef/browser/chrome/chrome_content_browser_client_cef.h" #include "cef/libcef/common/app_manager.h" #include "cef/libcef/common/chrome/chrome_main_delegate_cef.h" #include "chrome/browser/browser_process_impl.h" -#include "chrome/browser/chrome_content_browser_client.h" #include "chrome/browser/chrome_process_singleton.h" #include "chrome/common/profiler/main_thread_stack_sampling_profiler.h" #include "components/keep_alive_registry/keep_alive_types.h" @@ -96,7 +96,7 @@ void ChromeMainRunnerDelegate::BeforeUIThreadInitialize() { } void ChromeMainRunnerDelegate::BeforeUIThreadShutdown() { - static_cast( + static_cast( CefAppManager::Get()->GetContentClient()->browser()) ->CleanupOnUIThread(); main_delegate_->CleanupOnUIThread(); diff --git a/libcef/common/value_base.cc b/libcef/common/value_base.cc index 304b3b7a4..cad6643f4 100644 --- a/libcef/common/value_base.cc +++ b/libcef/common/value_base.cc @@ -70,13 +70,32 @@ void CefValueController::Remove(void* value, bool notify_object) { // Remove all dependencies. dependency_map_.clear(); } else { - ReferenceMap::iterator it = reference_map_.find(value); - if (it != reference_map_.end()) { - // Remove the reference. - if (notify_object) { - it->second->OnControlRemoved(); + { + ReferenceMap::iterator it = reference_map_.find(value); + if (it != reference_map_.end()) { + // Remove the reference. + if (notify_object) { + it->second->OnControlRemoved(); + } + reference_map_.erase(it); + } + } + + if (!dependency_map_.empty()) { + // Remove any instance from dependency map sets. + DependencyMap::iterator it = dependency_map_.begin(); + while (it != dependency_map_.end()) { + DependencySet& set = it->second; + DependencySet::iterator it_set = set.find(value); + if (it_set != set.end()) { + set.erase(it_set); + } + if (set.empty()) { + it = dependency_map_.erase(it); + } else { + ++it; + } } - reference_map_.erase(it); } } } diff --git a/libcef/common/value_base.h b/libcef/common/value_base.h index ca2eb0e73..e3d616698 100644 --- a/libcef/common/value_base.h +++ b/libcef/common/value_base.h @@ -133,12 +133,12 @@ class CefValueController raw_ptr owner_object_ = nullptr; // Map of reference objects. - using ReferenceMap = std::map; + using ReferenceMap = std::map, raw_ptr>; ReferenceMap reference_map_; // Map of dependency objects. - using DependencySet = std::set; - using DependencyMap = std::map; + using DependencySet = std::set>; + using DependencyMap = std::map, DependencySet>; DependencyMap dependency_map_; }; @@ -323,7 +323,7 @@ class CefValueBase : public CefType, public CefValueController::Object { controller()->RemoveDependencies(value_); // Delete the value. - DeleteValue(value_.get()); + value_.ClearAndDelete(); } controller_ = nullptr; @@ -375,9 +375,6 @@ class CefValueBase : public CefType, public CefValueController::Object { value_ = nullptr; } - // Override to customize value deletion. - virtual void DeleteValue(ValueType* value) { delete value; } - // Returns a mutable reference to the value. inline ValueType* mutable_value() const { DCHECK(value_); diff --git a/libcef/common/values_impl.cc b/libcef/common/values_impl.cc index 24ec92690..bb6ab801b 100644 --- a/libcef/common/values_impl.cc +++ b/libcef/common/values_impl.cc @@ -1036,10 +1036,10 @@ bool CefDictionaryValueImpl::SetList(const CefString& key, } bool CefDictionaryValueImpl::RemoveInternal(const CefString& key) { - // The ExtractKey() call below which removes the Value from the dictionary + // The Extract() call below which removes the Value from the dictionary // will return a new Value object with the moved contents of the Value that - // exists in the implementation std::map. Consequently we use FindKey() to - // retrieve the actual Value pointer as it current exists first, for later + // exists in the implementation std::map. Consequently we use Find() to + // retrieve the actual Value pointer as it current exists first, for // comparison purposes. const base::Value* actual_value = const_value().GetDict().Find(std::string_view(key.ToString())); @@ -1047,21 +1047,19 @@ bool CefDictionaryValueImpl::RemoveInternal(const CefString& key) { return false; } - // |actual_value| is no longer valid after this call. - std::optional out_value = - mutable_value()->GetDict().Extract(std::string_view(key.ToString())); - if (!out_value.has_value()) { - return false; - } - // Remove the value. controller()->Remove(const_cast(actual_value), true); // Only list and dictionary types may have dependencies. - if (out_value->is_list() || out_value->is_dict()) { + if (actual_value->is_list() || actual_value->is_dict()) { controller()->RemoveDependencies(const_cast(actual_value)); } + // |actual_value| is no longer valid after this call. + std::optional out_value = + mutable_value()->GetDict().Extract(std::string_view(key.ToString())); + DCHECK(out_value.has_value()); + return true; } @@ -1492,22 +1490,22 @@ bool CefListValueImpl::RemoveInternal(size_t index) { // The std::move() call below which removes the Value from the list will // return a new Value object with the moved contents of the Value that exists // in the implementation std::vector. Consequently we use operator[] to - // retrieve the actual Value pointer as it current exists first, for later + // retrieve the actual Value pointer as it current exists first, for // comparison purposes. const base::Value& actual_value = list[index]; - // |actual_value| is no longer valid after this call. - auto out_value = std::move(list[index]); - list.erase(list.begin() + index); - // Remove the value. controller()->Remove(const_cast(&actual_value), true); // Only list and dictionary types may have dependencies. - if (out_value.is_list() || out_value.is_dict()) { + if (actual_value.is_list() || actual_value.is_dict()) { controller()->RemoveDependencies(const_cast(&actual_value)); } + // |actual_value| is no longer valid after this call. + auto out_value = std::move(list[index]); + list.erase(list.begin() + index); + return true; } diff --git a/patch/patch.cfg b/patch/patch.cfg index ccda02fae..981ba40e5 100644 --- a/patch/patch.cfg +++ b/patch/patch.cfg @@ -801,5 +801,10 @@ patches = [ # https://issues.chromium.org/issues/340815319 # https://chromium-review.googlesource.com/c/chromium/src/+/5542485 'name': 'base_allocator_instance_tracer_5542485' + }, + { + # Fix dangling raw_ptr errors. + # https://github.com/chromiumembedded/cef/issues/3239 + 'name': 'raw_ptr_3239' } ] diff --git a/patch/patches/chrome_runtime_views.patch b/patch/patches/chrome_runtime_views.patch index 32865efae..c3a903cff 100644 --- a/patch/patches/chrome_runtime_views.patch +++ b/patch/patches/chrome_runtime_views.patch @@ -233,10 +233,10 @@ index 0ccfe39eb5696..c9424316b6d14 100644 return gfx::Rect(); } diff --git chrome/browser/ui/views/frame/browser_frame.cc chrome/browser/ui/views/frame/browser_frame.cc -index 8dd3620ba2720..e3bac8207de24 100644 +index 8dd3620ba2720..bb69a77f9551e 100644 --- chrome/browser/ui/views/frame/browser_frame.cc +++ chrome/browser/ui/views/frame/browser_frame.cc -@@ -114,15 +114,23 @@ ui::ColorProviderKey::SchemeVariant GetSchemeVariant( +@@ -114,15 +114,25 @@ ui::ColorProviderKey::SchemeVariant GetSchemeVariant( //////////////////////////////////////////////////////////////////////////////// // BrowserFrame, public: @@ -253,16 +253,18 @@ index 8dd3620ba2720..e3bac8207de24 100644 // Don't focus anything on creation, selecting a tab will set the focus. set_focus_on_creation(false); + if (browser_view) -+ InitBrowserView(browser_view); ++ SetBrowserView(browser_view); +} + -+void BrowserFrame::InitBrowserView(BrowserView* browser_view) { ++void BrowserFrame::SetBrowserView(BrowserView* browser_view) { + browser_view_ = browser_view; -+ browser_view_->set_frame(this); ++ if (browser_view_) { ++ browser_view_->set_frame(this); ++ } } BrowserFrame::~BrowserFrame() {} -@@ -228,10 +236,20 @@ void BrowserFrame::LayoutWebAppWindowTitle( +@@ -228,10 +238,20 @@ void BrowserFrame::LayoutWebAppWindowTitle( } int BrowserFrame::GetTopInset() const { @@ -283,7 +285,7 @@ index 8dd3620ba2720..e3bac8207de24 100644 browser_frame_view_->UpdateThrobber(running); } -@@ -240,6 +258,8 @@ BrowserNonClientFrameView* BrowserFrame::GetFrameView() const { +@@ -240,6 +260,8 @@ BrowserNonClientFrameView* BrowserFrame::GetFrameView() const { } bool BrowserFrame::UseCustomFrame() const { @@ -292,7 +294,7 @@ index 8dd3620ba2720..e3bac8207de24 100644 return native_browser_frame_->UseCustomFrame(); } -@@ -253,20 +273,30 @@ bool BrowserFrame::ShouldDrawFrameHeader() const { +@@ -253,20 +275,30 @@ bool BrowserFrame::ShouldDrawFrameHeader() const { void BrowserFrame::GetWindowPlacement(gfx::Rect* bounds, ui::WindowShowState* show_state) const { @@ -323,7 +325,7 @@ index 8dd3620ba2720..e3bac8207de24 100644 browser_frame_view_->OnBrowserViewInitViewsComplete(); } -@@ -367,6 +397,8 @@ ui::ColorProviderKey::ThemeInitializerSupplier* BrowserFrame::GetCustomTheme() +@@ -367,6 +399,8 @@ ui::ColorProviderKey::ThemeInitializerSupplier* BrowserFrame::GetCustomTheme() } void BrowserFrame::OnNativeWidgetWorkspaceChanged() { @@ -332,7 +334,7 @@ index 8dd3620ba2720..e3bac8207de24 100644 chrome::SaveWindowWorkspace(browser_view_->browser(), GetWorkspace()); chrome::SaveWindowVisibleOnAllWorkspaces(browser_view_->browser(), IsVisibleOnAllWorkspaces()); -@@ -572,6 +604,13 @@ void BrowserFrame::SelectNativeTheme() { +@@ -572,6 +606,13 @@ void BrowserFrame::SelectNativeTheme() { return; } @@ -346,7 +348,7 @@ index 8dd3620ba2720..e3bac8207de24 100644 // Ignore the system theme for web apps with window-controls-overlay as the // display_override so the web contents can blend with the overlay by using // the developer-provided theme color for a better experience. Context: -@@ -637,5 +676,8 @@ bool BrowserFrame::RegenerateFrameOnThemeChange( +@@ -637,5 +678,8 @@ bool BrowserFrame::RegenerateFrameOnThemeChange( } bool BrowserFrame::IsIncognitoBrowser() const { @@ -356,20 +358,18 @@ index 8dd3620ba2720..e3bac8207de24 100644 return browser_view_->browser()->profile()->IsIncognitoProfile(); } diff --git chrome/browser/ui/views/frame/browser_frame.h chrome/browser/ui/views/frame/browser_frame.h -index 2e973c9e279b0..8662f9cf14b17 100644 +index 2e973c9e279b0..07d04a364d60c 100644 --- chrome/browser/ui/views/frame/browser_frame.h +++ chrome/browser/ui/views/frame/browser_frame.h -@@ -58,7 +58,9 @@ enum class TabDragKind { +@@ -58,6 +58,7 @@ enum class TabDragKind { // This is a virtual interface that allows system specific browser frames. class BrowserFrame : public views::Widget, public views::ContextMenuController { public: + BrowserFrame(); explicit BrowserFrame(BrowserView* browser_view); -+ void InitBrowserView(BrowserView* browser_view); BrowserFrame(const BrowserFrame&) = delete; - BrowserFrame& operator=(const BrowserFrame&) = delete; -@@ -137,7 +139,7 @@ class BrowserFrame : public views::Widget, public views::ContextMenuController { +@@ -137,7 +138,7 @@ class BrowserFrame : public views::Widget, public views::ContextMenuController { // ThemeService calls this when a user has changed their theme, indicating // that it's time to redraw everything. @@ -378,7 +378,16 @@ index 2e973c9e279b0..8662f9cf14b17 100644 // views::Widget: views::internal::RootView* CreateRootView() override; -@@ -175,17 +177,17 @@ class BrowserFrame : public views::Widget, public views::ContextMenuController { +@@ -170,22 +171,26 @@ class BrowserFrame : public views::Widget, public views::ContextMenuController { + void SetTabDragKind(TabDragKind tab_drag_kind); + TabDragKind tab_drag_kind() const { return tab_drag_kind_; } + ++ BrowserView* browser_view() const { return browser_view_.get(); } ++ + protected: ++ void SetBrowserView(BrowserView* browser_view); ++ + // views::Widget: void OnNativeThemeUpdated(ui::NativeTheme* observed_theme) override; ui::ColorProviderKey GetColorProviderKey() const override; @@ -402,7 +411,7 @@ index 2e973c9e279b0..8662f9cf14b17 100644 // regenerated. bool RegenerateFrameOnThemeChange(BrowserThemeChangeType theme_change_type); diff --git chrome/browser/ui/views/frame/browser_view.cc chrome/browser/ui/views/frame/browser_view.cc -index 110812d2874d7..89b28bdb32d90 100644 +index 110812d2874d7..6ac4f560d7c27 100644 --- chrome/browser/ui/views/frame/browser_view.cc +++ chrome/browser/ui/views/frame/browser_view.cc @@ -346,11 +346,10 @@ using content::NativeWebKeyboardEvent; @@ -486,9 +495,17 @@ index 110812d2874d7..89b28bdb32d90 100644 // Stop the animation timer explicitly here to avoid running it in a nested // message loop, which may run by Browser destructor. -@@ -1031,12 +1057,14 @@ BrowserView::~BrowserView() { - // child views and it is an observer for avatar toolbar button if any. - autofill_bubble_handler_.reset(); +@@ -1026,17 +1052,18 @@ BrowserView::~BrowserView() { + // Immersive mode may need to reparent views before they are removed/deleted. + immersive_mode_controller_.reset(); + +- // Reset autofill bubble handler to make sure it does not out-live toolbar, +- // since it is responsible for showing autofill related bubbles from toolbar's +- // child views and it is an observer for avatar toolbar button if any. +- autofill_bubble_handler_.reset(); ++ // If the Toolbar is not overloaded it will be destroyed via ++ // RemoveAllChildViews(). ++ WillDestroyToolbar(); + if (browser_) { auto* global_registry = @@ -501,7 +518,7 @@ index 110812d2874d7..89b28bdb32d90 100644 // The TabStrip attaches a listener to the model. Make sure we shut down the // TabStrip first so that it can cleanly remove the listener. -@@ -1060,7 +1088,9 @@ BrowserView::~BrowserView() { +@@ -1060,7 +1087,9 @@ BrowserView::~BrowserView() { // `SidePanelUI::RemoveSidePanelUIForBrowser()` deletes the // SidePanelCoordinator. @@ -511,7 +528,21 @@ index 110812d2874d7..89b28bdb32d90 100644 } // static -@@ -2032,9 +2062,14 @@ void BrowserView::OnExclusiveAccessUserInput() { +@@ -1621,6 +1650,13 @@ gfx::Point BrowserView::GetThemeOffsetFromBrowserView() const { + ThemeProperties::kFrameHeightAboveTabs - browser_view_origin.y()); + } + ++void BrowserView::WillDestroyToolbar() { ++ // Reset autofill bubble handler to make sure it does not out-live toolbar, ++ // since it is responsible for showing autofill related bubbles from toolbar's ++ // child views and it is an observer for avatar toolbar button if any. ++ autofill_bubble_handler_.reset(); ++} ++ + // static: + BrowserView::DevToolsDockedPlacement BrowserView::GetDevToolsDockedPlacement( + const gfx::Rect& contents_webview_bounds, +@@ -2032,9 +2068,14 @@ void BrowserView::OnExclusiveAccessUserInput() { bool BrowserView::ShouldHideUIForFullscreen() const { // Immersive mode needs UI for the slide-down top panel. @@ -527,7 +558,7 @@ index 110812d2874d7..89b28bdb32d90 100644 return frame_->GetFrameView()->ShouldHideTopUIForFullscreen(); } -@@ -3170,7 +3205,8 @@ DownloadShelf* BrowserView::GetDownloadShelf() { +@@ -3170,7 +3211,8 @@ DownloadShelf* BrowserView::GetDownloadShelf() { } DownloadBubbleUIController* BrowserView::GetDownloadBubbleUIController() { @@ -537,7 +568,7 @@ index 110812d2874d7..89b28bdb32d90 100644 if (auto* download_button = toolbar_button_provider_->GetDownloadButton()) return download_button->bubble_controller(); return nullptr; -@@ -3725,7 +3761,8 @@ void BrowserView::ReparentTopContainerForEndOfImmersive() { +@@ -3725,7 +3767,8 @@ void BrowserView::ReparentTopContainerForEndOfImmersive() { if (top_container()->parent() == this) return; @@ -547,7 +578,7 @@ index 110812d2874d7..89b28bdb32d90 100644 top_container()->DestroyLayer(); AddChildViewAt(top_container(), 0); EnsureFocusOrder(); -@@ -4207,11 +4244,38 @@ void BrowserView::GetAccessiblePanes(std::vector* panes) { +@@ -4207,11 +4250,38 @@ void BrowserView::GetAccessiblePanes(std::vector* panes) { bool BrowserView::ShouldDescendIntoChildForEventHandling( gfx::NativeView child, const gfx::Point& location) { @@ -588,7 +619,7 @@ index 110812d2874d7..89b28bdb32d90 100644 // Draggable regions are defined relative to the web contents. gfx::Point point_in_contents_web_view_coords(location); views::View::ConvertPointToTarget(GetWidget()->GetRootView(), -@@ -4220,7 +4284,7 @@ bool BrowserView::ShouldDescendIntoChildForEventHandling( +@@ -4220,7 +4290,7 @@ bool BrowserView::ShouldDescendIntoChildForEventHandling( // Draggable regions should be ignored for clicks into any browser view's // owned widgets, for example alerts, permission prompts or find bar. @@ -597,7 +628,7 @@ index 110812d2874d7..89b28bdb32d90 100644 point_in_contents_web_view_coords.x(), point_in_contents_web_view_coords.y()) || WidgetOwnedByAnchorContainsPoint(point_in_contents_web_view_coords); -@@ -4331,8 +4395,10 @@ void BrowserView::Layout(PassKey) { +@@ -4331,8 +4401,10 @@ void BrowserView::Layout(PassKey) { // TODO(jamescook): Why was this in the middle of layout code? toolbar_->location_bar()->omnibox_view()->SetFocusBehavior( @@ -610,7 +641,7 @@ index 110812d2874d7..89b28bdb32d90 100644 // Some of the situations when the BrowserView is laid out are: // - Enter/exit immersive fullscreen mode. -@@ -4398,6 +4464,11 @@ void BrowserView::AddedToWidget() { +@@ -4398,6 +4470,11 @@ void BrowserView::AddedToWidget() { SetThemeProfileForWindow(GetNativeWindow(), browser_->profile()); #endif @@ -622,7 +653,7 @@ index 110812d2874d7..89b28bdb32d90 100644 toolbar_->Init(); // TODO(pbos): Investigate whether the side panels should be creatable when -@@ -4445,13 +4516,9 @@ void BrowserView::AddedToWidget() { +@@ -4445,13 +4522,9 @@ void BrowserView::AddedToWidget() { EnsureFocusOrder(); @@ -638,7 +669,7 @@ index 110812d2874d7..89b28bdb32d90 100644 using_native_frame_ = frame_->ShouldUseNativeFrame(); MaybeInitializeWebUITabStrip(); -@@ -4882,7 +4949,8 @@ void BrowserView::ProcessFullscreen(bool fullscreen, +@@ -4882,7 +4955,8 @@ void BrowserView::ProcessFullscreen(bool fullscreen, // Undo our anti-jankiness hacks and force a re-layout. in_process_fullscreen_ = false; ToolbarSizeChanged(false); @@ -648,7 +679,7 @@ index 110812d2874d7..89b28bdb32d90 100644 } bool BrowserView::ShouldUseImmersiveFullscreenForUrl(const GURL& url) const { -@@ -5304,6 +5372,8 @@ Profile* BrowserView::GetProfile() { +@@ -5304,6 +5378,8 @@ Profile* BrowserView::GetProfile() { } void BrowserView::UpdateUIForTabFullscreen() { @@ -657,7 +688,7 @@ index 110812d2874d7..89b28bdb32d90 100644 frame()->GetFrameView()->UpdateFullscreenTopUI(); } -@@ -5326,6 +5396,8 @@ void BrowserView::HideDownloadShelf() { +@@ -5326,6 +5402,8 @@ void BrowserView::HideDownloadShelf() { } bool BrowserView::CanUserExitFullscreen() const { @@ -667,7 +698,7 @@ index 110812d2874d7..89b28bdb32d90 100644 } diff --git chrome/browser/ui/views/frame/browser_view.h chrome/browser/ui/views/frame/browser_view.h -index 46cdfe23b1234..14cbb302de0a2 100644 +index 46cdfe23b1234..4f3b2b7650b72 100644 --- chrome/browser/ui/views/frame/browser_view.h +++ chrome/browser/ui/views/frame/browser_view.h @@ -138,11 +138,16 @@ class BrowserView : public BrowserWindow, @@ -687,11 +718,21 @@ index 46cdfe23b1234..14cbb302de0a2 100644 void set_frame(BrowserFrame* frame) { frame_ = frame; paint_as_active_subscription_ = -@@ -873,6 +878,9 @@ class BrowserView : public BrowserWindow, +@@ -850,6 +855,10 @@ class BrowserView : public BrowserWindow, + // TopContainerBackground::PaintThemeCustomImage for details. + gfx::Point GetThemeOffsetFromBrowserView() const; + ++ // Called during Toolbar destruction to remove dependent objects that have ++ // dangling references. ++ virtual void WillDestroyToolbar(); ++ + protected: + // Enumerates where the devtools are docked relative to the browser's main + // web contents. +@@ -873,6 +882,8 @@ class BrowserView : public BrowserWindow, const gfx::Rect& contents_webview_bounds, const gfx::Rect& local_webview_container_bounds); -+ protected: + virtual ToolbarView* OverrideCreateToolbar() { return nullptr; } + private: @@ -980,7 +1021,7 @@ index 880d83324cfa6..a6a80cd0b3def 100644 } diff --git chrome/browser/ui/views/toolbar/toolbar_view.cc chrome/browser/ui/views/toolbar/toolbar_view.cc -index e97342ef97514..e373f6374fa4c 100644 +index e97342ef97514..03b140c9fc7c6 100644 --- chrome/browser/ui/views/toolbar/toolbar_view.cc +++ chrome/browser/ui/views/toolbar/toolbar_view.cc @@ -191,7 +191,7 @@ class TabstripLikeBackground : public views::Background { @@ -1008,7 +1049,12 @@ index e97342ef97514..e373f6374fa4c 100644 SetID(VIEW_ID_TOOLBAR); container_view_ = AddChildView(std::make_unique()); -@@ -251,6 +252,19 @@ ToolbarView::~ToolbarView() { +@@ -248,9 +249,24 @@ ToolbarView::~ToolbarView() { + + for (const auto& view_and_command : GetViewCommandMap()) + chrome::RemoveCommandObserver(browser_, view_and_command.second, this); ++ ++ browser_view_->WillDestroyToolbar(); } void ToolbarView::Init() { @@ -1028,7 +1074,7 @@ index e97342ef97514..e373f6374fa4c 100644 #if defined(USE_AURA) // Avoid generating too many occlusion tracking calculation events before this // function returns. The occlusion status will be computed only once once this -@@ -275,12 +289,12 @@ void ToolbarView::Init() { +@@ -275,12 +291,12 @@ void ToolbarView::Init() { auto location_bar = std::make_unique( browser_, browser_->profile(), browser_->command_controller(), this, @@ -1043,7 +1089,7 @@ index e97342ef97514..e373f6374fa4c 100644 download_button = std::make_unique(browser_view_); } -@@ -362,8 +376,10 @@ void ToolbarView::Init() { +@@ -362,8 +378,10 @@ void ToolbarView::Init() { } } std::unique_ptr cast; @@ -1055,7 +1101,7 @@ index e97342ef97514..e373f6374fa4c 100644 std::unique_ptr media_button; if (base::FeatureList::IsEnabled(media::kGlobalMediaControls)) { -@@ -373,7 +389,8 @@ void ToolbarView::Init() { +@@ -373,7 +391,8 @@ void ToolbarView::Init() { std::unique_ptr send_tab_to_self_button; @@ -1065,7 +1111,7 @@ index e97342ef97514..e373f6374fa4c 100644 send_tab_to_self_button = std::make_unique( browser_view_); -@@ -452,7 +469,7 @@ void ToolbarView::Init() { +@@ -452,7 +471,7 @@ void ToolbarView::Init() { send_tab_to_self_button_ = container_view_->AddChildView(std::move(send_tab_to_self_button)); @@ -1074,7 +1120,7 @@ index e97342ef97514..e373f6374fa4c 100644 if (companion::IsCompanionFeatureEnabled()) { side_panel_container_ = container_view_->AddChildView( std::make_unique(browser_view_)); -@@ -818,7 +835,7 @@ void ToolbarView::Layout(PassKey) { +@@ -818,7 +837,7 @@ void ToolbarView::Layout(PassKey) { if (display_mode_ == DisplayMode::NORMAL) { LayoutCommon(); diff --git a/patch/patches/raw_ptr_3239.patch b/patch/patches/raw_ptr_3239.patch new file mode 100644 index 000000000..69631056c --- /dev/null +++ b/patch/patches/raw_ptr_3239.patch @@ -0,0 +1,26 @@ +diff --git net/base/directory_lister.cc net/base/directory_lister.cc +index ae3a99f31e2ec..213cc33f2cc8e 100644 +--- net/base/directory_lister.cc ++++ net/base/directory_lister.cc +@@ -200,7 +200,7 @@ void DirectoryLister::OnListFile(const DirectoryListerData& data) { + } + + void DirectoryLister::OnListDone(int error) { +- delegate_->OnListDone(error); ++ delegate_.ExtractAsDangling()->OnListDone(error); + } + + } // namespace net +diff --git net/base/directory_lister.h net/base/directory_lister.h +index 991d15b79878e..b4a5534ea2c87 100644 +--- net/base/directory_lister.h ++++ net/base/directory_lister.h +@@ -133,7 +133,7 @@ class NET_EXPORT DirectoryLister { + void OnListDone(int error); + + scoped_refptr core_; +- const raw_ptr delegate_; ++ raw_ptr delegate_; + }; + + } // namespace net diff --git a/tests/cefsimple/simple_app.cc b/tests/cefsimple/simple_app.cc index ab24337b0..2f56f9daa 100644 --- a/tests/cefsimple/simple_app.cc +++ b/tests/cefsimple/simple_app.cc @@ -144,7 +144,7 @@ void SimpleApp::OnContextInitialized() { // that instead of the default URL. url = command_line->GetSwitchValue("url"); if (url.empty()) { - url = "http://www.google.com"; + url = "https://www.google.com"; } // Views is enabled by default with the Chrome bootstrap (add `--use-native` diff --git a/tests/ceftests/frame_handler_unittest.cc b/tests/ceftests/frame_handler_unittest.cc index 222d31c53..a99e5341d 100644 --- a/tests/ceftests/frame_handler_unittest.cc +++ b/tests/ceftests/frame_handler_unittest.cc @@ -423,8 +423,10 @@ struct FrameStatus { auto main_frame = browser->GetMainFrame(); if (expect_valid) { EXPECT_TRUE(main_frame) << func; - EXPECT_TRUE(main_frame->IsValid()) << func; - EXPECT_TRUE(main_frame->IsMain()) << func; + if (main_frame) { + EXPECT_TRUE(main_frame->IsValid()) << func; + EXPECT_TRUE(main_frame->IsMain()) << func; + } } else { // GetMainFrame() returns nullptr after OnBeforeClose. EXPECT_FALSE(main_frame) << func; @@ -561,7 +563,9 @@ class OrderMainTestHandler : public RoutingTestHandler, public CefFrameHandler { got_before_close_ = true; EXPECT_TRUE(current_main_frame_); - current_main_frame_->OnBeforeClose(browser); + if (current_main_frame_) { + current_main_frame_->OnBeforeClose(browser); + } RoutingTestHandler::OnBeforeClose(browser); } diff --git a/tests/ceftests/request_context_unittest.cc b/tests/ceftests/request_context_unittest.cc index 894cca537..0f85cc83e 100644 --- a/tests/ceftests/request_context_unittest.cc +++ b/tests/ceftests/request_context_unittest.cc @@ -3,6 +3,7 @@ // can be found in the LICENSE file. #include "include/base/cef_callback.h" +#include "include/base/cef_weak_ptr.h" #include "include/cef_request_context.h" #include "include/cef_request_context_handler.h" #include "include/wrapper/cef_closure_task.h" @@ -486,6 +487,11 @@ class PopupNavTestHandler : public TestHandler { AddResource(kPopupNavPopupUrl2, "Popup2", "text/html"); } + // By default, TestHandler signals test completion when all CefBrowsers + // have closed. For this test we instead want to wait for an explicit call + // to DestroyTest before signaling test completion. + SetSignalTestCompletionCount(1U); + CreateTestRequestContext( rc_mode_, rc_cache_path_, base::BindOnce(&PopupNavTestHandler::RunTestContinue, this)); @@ -609,9 +615,10 @@ class PopupNavTestHandler : public TestHandler { if (mode_ == DENY) { // Wait a bit to make sure the popup window isn't created. - CefPostDelayedTask( - TID_UI, base::BindOnce(&PopupNavTestHandler::DestroyTest, this), - 200); + CefPostDelayedTask(TID_UI, + base::BindOnce(&PopupNavTestHandler::DestroyTest, + weak_ptr_factory_.GetWeakPtr()), + 200); } } else if (url == kPopupNavPopupUrl) { EXPECT_FALSE(got_popup_load_end_); @@ -664,8 +671,9 @@ class PopupNavTestHandler : public TestHandler { } if (destroy_test) { - CefPostTask(TID_UI, - base::BindOnce(&PopupNavTestHandler::DestroyTest, this)); + // This may race with OnBeforeClose() for the remaining browser. + CefPostTask(TID_UI, base::BindOnce(&PopupNavTestHandler::DestroyTest, + weak_ptr_factory_.GetWeakPtr())); } } @@ -714,8 +722,14 @@ class PopupNavTestHandler : public TestHandler { EXPECT_TRUE(got_popup_load_end2_); } - // Will trigger destruction of all remaining browsers. + // Invalidate WeakPtrs now as |this| may be deleted on a different thread. + weak_ptr_factory_.InvalidateWeakPtrs(); + + // Will trigger destruction of any remaining browsers. TestHandler::DestroyTest(); + + // Allow the the test to complete once all browsers are gone. + SignalTestCompletion(); } const TestMode mode_; @@ -733,6 +747,8 @@ class PopupNavTestHandler : public TestHandler { TrackCallback got_popup_load_error2_; TrackCallback got_popup_load_end2_; + base::WeakPtrFactory weak_ptr_factory_{this}; + IMPLEMENT_REFCOUNTING(PopupNavTestHandler); }; diff --git a/tests/ceftests/scheme_handler_unittest.cc b/tests/ceftests/scheme_handler_unittest.cc index 1a59ec9ea..1f51539de 100644 --- a/tests/ceftests/scheme_handler_unittest.cc +++ b/tests/ceftests/scheme_handler_unittest.cc @@ -1437,7 +1437,7 @@ TEST(SchemeHandlerTest, CustomStandardXHRDifferentOriginSync) { EXPECT_TRUE(test_results.got_read); EXPECT_TRUE(test_results.got_output); EXPECT_TRUE(test_results.got_sub_request); - EXPECT_TRUE(test_results.got_sub_read); + EXPECT_FALSE(test_results.got_sub_read); EXPECT_FALSE(test_results.git_exit_success); ClearTestSchemes(&test_results); @@ -1470,7 +1470,7 @@ TEST(SchemeHandlerTest, CustomStandardXHRDifferentOriginAsync) { EXPECT_TRUE(test_results.got_read); EXPECT_TRUE(test_results.got_output); EXPECT_TRUE(test_results.got_sub_request); - EXPECT_TRUE(test_results.got_sub_read); + EXPECT_FALSE(test_results.got_sub_read); EXPECT_FALSE(test_results.git_exit_success); ClearTestSchemes(&test_results); @@ -1503,7 +1503,7 @@ TEST(SchemeHandlerTest, CustomStandardFetchDifferentOrigin) { EXPECT_TRUE(test_results.got_read); EXPECT_TRUE(test_results.got_output); EXPECT_TRUE(test_results.got_sub_request); - EXPECT_TRUE(test_results.got_sub_read); + EXPECT_FALSE(test_results.got_sub_read); EXPECT_FALSE(test_results.git_exit_success); ClearTestSchemes(&test_results); @@ -1676,7 +1676,7 @@ TEST(SchemeHandlerTest, HttpXHRDifferentOriginSync) { EXPECT_TRUE(test_results.got_read); EXPECT_TRUE(test_results.got_output); EXPECT_TRUE(test_results.got_sub_request); - EXPECT_TRUE(test_results.got_sub_read); + EXPECT_FALSE(test_results.got_sub_read); EXPECT_FALSE(test_results.git_exit_success); ClearTestSchemes(&test_results); @@ -1709,7 +1709,7 @@ TEST(SchemeHandlerTest, HttpXHRDifferentOriginAsync) { EXPECT_TRUE(test_results.got_read); EXPECT_TRUE(test_results.got_output); EXPECT_TRUE(test_results.got_sub_request); - EXPECT_TRUE(test_results.got_sub_read); + EXPECT_FALSE(test_results.got_sub_read); EXPECT_FALSE(test_results.git_exit_success); ClearTestSchemes(&test_results); @@ -1743,7 +1743,7 @@ TEST(SchemeHandlerTest, HttpFetchDifferentOriginAsync) { EXPECT_TRUE(test_results.got_read); EXPECT_TRUE(test_results.got_output); EXPECT_TRUE(test_results.got_sub_request); - EXPECT_TRUE(test_results.got_sub_read); + EXPECT_FALSE(test_results.got_sub_read); EXPECT_FALSE(test_results.git_exit_success); ClearTestSchemes(&test_results); @@ -2311,7 +2311,7 @@ TEST(SchemeHandlerTest, CustomStandardXHRDifferentOriginRedirectSync) { EXPECT_TRUE(test_results.got_output); EXPECT_TRUE(test_results.got_sub_redirect); EXPECT_TRUE(test_results.got_sub_request); - EXPECT_TRUE(test_results.got_sub_read); + EXPECT_FALSE(test_results.got_sub_read); EXPECT_FALSE(test_results.git_exit_success); ClearTestSchemes(&test_results); @@ -2346,7 +2346,7 @@ TEST(SchemeHandlerTest, CustomStandardXHRDifferentOriginRedirectAsync) { EXPECT_TRUE(test_results.got_output); EXPECT_TRUE(test_results.got_sub_redirect); EXPECT_TRUE(test_results.got_sub_request); - EXPECT_TRUE(test_results.got_sub_read); + EXPECT_FALSE(test_results.got_sub_read); EXPECT_FALSE(test_results.git_exit_success); ClearTestSchemes(&test_results); @@ -2382,7 +2382,7 @@ TEST(SchemeHandlerTest, CustomStandardFetchDifferentOriginRedirect) { EXPECT_TRUE(test_results.got_output); EXPECT_TRUE(test_results.got_sub_redirect); EXPECT_TRUE(test_results.got_sub_request); - EXPECT_TRUE(test_results.got_sub_read); + EXPECT_FALSE(test_results.got_sub_read); EXPECT_FALSE(test_results.git_exit_success); ClearTestSchemes(&test_results); diff --git a/tests/ceftests/values_unittest.cc b/tests/ceftests/values_unittest.cc index 565674e1b..6ac10ed74 100644 --- a/tests/ceftests/values_unittest.cc +++ b/tests/ceftests/values_unittest.cc @@ -266,6 +266,8 @@ void TestDictionary(CefRefPtr value, // Test the size. EXPECT_EQ(0U, value->GetSize()); + // Should be a no-op. + EXPECT_TRUE(value->Clear()); // Re-add some values. TestDictionaryNull(value); diff --git a/tests/ceftests/views/panel_unittest.cc b/tests/ceftests/views/panel_unittest.cc index 8f2bf66ad..82bf189be 100644 --- a/tests/ceftests/views/panel_unittest.cc +++ b/tests/ceftests/views/panel_unittest.cc @@ -757,6 +757,10 @@ void ChildThemeImpl() { // Restore the default background color for the global theme. window->SetThemeColor(CEF_ColorPrimaryBackground, default_color); EXPECT_EQ(default_color, window->GetThemeColor(CEF_ColorPrimaryBackground)); + + // Close the windows to avoid dangling references. + window->Close(); + new_window->Close(); } } // namespace