From db807e804a0bcb23d3b137da4cba5ea8d8234632 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Wed, 15 Sep 2021 21:40:53 +0300 Subject: [PATCH] alloy: Fix FrameHandlerTest failures with BackForwardCache enabled (see issue #2421) To test: Run `ceftests --gtest_filter=FrameHandlerTest.OrderSub* --enable-features=BackForwardCache` --- libcef/browser/browser_contents_delegate.cc | 4 ++++ libcef/browser/browser_host_base.cc | 1 + libcef/browser/browser_info.cc | 12 ++++++++++-- libcef/browser/browser_info.h | 10 ++++++++++ 4 files changed, 25 insertions(+), 2 deletions(-) diff --git a/libcef/browser/browser_contents_delegate.cc b/libcef/browser/browser_contents_delegate.cc index 08b70a1b0..f8ba4575e 100644 --- a/libcef/browser/browser_contents_delegate.cc +++ b/libcef/browser/browser_contents_delegate.cc @@ -365,6 +365,10 @@ void CefBrowserContentsDelegate::DidFinishNavigation( (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. diff --git a/libcef/browser/browser_host_base.cc b/libcef/browser/browser_host_base.cc index 9bf123b4e..a2c3c6441 100644 --- a/libcef/browser/browser_host_base.cc +++ b/libcef/browser/browser_host_base.cc @@ -798,6 +798,7 @@ void CefBrowserHostBase::OnBeforeClose() { handler->OnBeforeClose(this); } } + browser_info_->SetClosing(); } void CefBrowserHostBase::OnBrowserDestroyed() { diff --git a/libcef/browser/browser_info.cc b/libcef/browser/browser_info.cc index 1142db954..dbade2dd8 100644 --- a/libcef/browser/browser_info.cc +++ b/libcef/browser/browser_info.cc @@ -41,7 +41,9 @@ CefBrowserInfo::~CefBrowserInfo() { CefRefPtr CefBrowserInfo::browser() const { base::AutoLock lock_scope(lock_); - return browser_; + if (!is_closing_) + return browser_; + return nullptr; } void CefBrowserInfo::SetBrowser(CefRefPtr browser) { @@ -71,6 +73,12 @@ void CefBrowserInfo::SetBrowser(CefRefPtr browser) { } } +void CefBrowserInfo::SetClosing() { + base::AutoLock lock_scope(lock_); + DCHECK(!is_closing_); + is_closing_ = true; +} + void CefBrowserInfo::MaybeCreateFrame(content::RenderFrameHost* host, bool is_guest_view) { CEF_REQUIRE_UIT(); @@ -228,7 +236,7 @@ void CefBrowserInfo::RemoveFrame(content::RenderFrameHost* host) { CefRefPtr CefBrowserInfo::GetMainFrame() { NotificationStateLock lock_scope(this); // Early exit if called post-destruction. - if (!browser_) + if (!browser_ || is_closing_) return nullptr; CHECK(main_frame_); diff --git a/libcef/browser/browser_info.h b/libcef/browser/browser_info.h index 18863f089..fe61b823f 100644 --- a/libcef/browser/browser_info.h +++ b/libcef/browser/browser_info.h @@ -52,6 +52,13 @@ class CefBrowserInfo : public base::RefCountedThreadSafe { // (to set) and DestroyBrowser (to clear). 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. + void SetClosing(); + // 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. @@ -239,6 +246,9 @@ class CefBrowserInfo : public base::RefCountedThreadSafe { // The current main frame. CefRefPtr main_frame_; + // True if the browser is currently closing. + bool is_closing_ = false; + // Only accessed on the UI thread. std::vector draggable_regions_;