From d9efaee9b9aa268579f1be9180bb30c97500745b Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Tue, 18 May 2021 20:45:05 -0400 Subject: [PATCH] Fix routing of frame messages after cross-origin navigation (fixes issue #2849) When navigating cross-origin a speculative RenderFrameHost (RFH) and CefFrameHostImpl is created in the browser process for the new frame object created in a new renderer process. The FrameAttached message then arrives for the speculative RFH, and as a consequence interfaces are bound between the new CefFrameHostImpl and the speculative RFH. If the pending navigation commits then the existing RFH will be replaced with the previously speculative RFH. Since interfaces are already bound we must keep the new CefFrameHostImpl. This means that frame objects (including for the main frame) will now always change after cross-origin navigation, and the old frame object will be invalidated. --- libcef/browser/browser_frame.cc | 10 +++++++--- libcef/browser/browser_frame.h | 3 ++- libcef/browser/browser_info.cc | 28 ++++++++++++++-------------- libcef/browser/browser_info.h | 9 ++++++--- libcef/browser/frame_host_impl.cc | 2 ++ libcef/browser/frame_host_impl.h | 4 +++- 6 files changed, 34 insertions(+), 22 deletions(-) diff --git a/libcef/browser/browser_frame.cc b/libcef/browser/browser_frame.cc index 0fe8ee9ed..1d4ff880e 100644 --- a/libcef/browser/browser_frame.cc +++ b/libcef/browser/browser_frame.cc @@ -41,7 +41,9 @@ void CefBrowserFrame::SendMessage(const std::string& name, } void CefBrowserFrame::FrameAttached() { - if (auto host = GetFrameHost()) { + // Always send to the newly created RFH, which may be speculative when + // navigating cross-origin. + if (auto host = GetFrameHost(/*prefer_speculative=*/true)) { host->FrameAttached(); } } @@ -60,11 +62,13 @@ void CefBrowserFrame::UpdateDraggableRegions( } } -CefRefPtr CefBrowserFrame::GetFrameHost() const { +CefRefPtr CefBrowserFrame::GetFrameHost( + bool prefer_speculative) const { CEF_REQUIRE_UIT(); auto rfh = render_frame_host(); if (auto browser = CefBrowserHostBase::GetBrowserForHost(rfh)) { - return browser->browser_info()->GetFrameForHost(rfh); + return browser->browser_info()->GetFrameForHost(rfh, nullptr, + prefer_speculative); } NOTREACHED(); return nullptr; diff --git a/libcef/browser/browser_frame.h b/libcef/browser/browser_frame.h index 0b7cd00bb..d164b2b0b 100644 --- a/libcef/browser/browser_frame.h +++ b/libcef/browser/browser_frame.h @@ -42,7 +42,8 @@ class CefBrowserFrame // FrameServiceBase methods: bool ShouldCloseOnFinishNavigation() const override { return false; } - CefRefPtr GetFrameHost() const; + CefRefPtr GetFrameHost( + bool prefer_speculative = false) const; DISALLOW_COPY_AND_ASSIGN(CefBrowserFrame); }; diff --git a/libcef/browser/browser_info.cc b/libcef/browser/browser_info.cc index 03074ece3..fd1038a4f 100644 --- a/libcef/browser/browser_info.cc +++ b/libcef/browser/browser_info.cc @@ -86,14 +86,10 @@ void CefBrowserInfo::MaybeCreateFrame(content::RenderFrameHost* host, if (!info->is_guest_view_ && info->is_speculative_ && !is_speculative) { // Upgrade the frame info from speculative to non-speculative. if (info->is_main_frame_) { - if (main_frame_) { - // Update the existing main frame object. - main_frame_->SetRenderFrameHost(host); - info->frame_ = main_frame_; - } else { - // Set the main frame object. - main_frame_ = info->frame_; - } + // Set the main frame object. + if (main_frame_) + main_frame_->Detach(); + main_frame_ = info->frame_; } info->is_speculative_ = false; MaybeUpdateFrameTreeNodeIdMap(info); @@ -200,20 +196,23 @@ CefRefPtr CefBrowserInfo::CreateTempSubFrame( CefRefPtr CefBrowserInfo::GetFrameForHost( const content::RenderFrameHost* host, - bool* is_guest_view) const { + bool* is_guest_view, + bool prefer_speculative) const { if (is_guest_view) *is_guest_view = false; if (!host) return nullptr; - return GetFrameForId(CefFrameHostImpl::MakeFrameId(host), is_guest_view); + return GetFrameForId(CefFrameHostImpl::MakeFrameId(host), is_guest_view, + prefer_speculative); } CefRefPtr CefBrowserInfo::GetFrameForRoute( int32_t render_process_id, int32_t render_routing_id, - bool* is_guest_view) const { + bool* is_guest_view, + bool prefer_speculative) const { if (is_guest_view) *is_guest_view = false; @@ -222,12 +221,13 @@ CefRefPtr CefBrowserInfo::GetFrameForRoute( return GetFrameForId( CefFrameHostImpl::MakeFrameId(render_process_id, render_routing_id), - is_guest_view); + is_guest_view, prefer_speculative); } CefRefPtr CefBrowserInfo::GetFrameForId( int64_t frame_id, - bool* is_guest_view) const { + bool* is_guest_view, + bool prefer_speculative) const { if (is_guest_view) *is_guest_view = false; @@ -246,7 +246,7 @@ CefRefPtr CefBrowserInfo::GetFrameForId( return nullptr; } - if (info->is_speculative_) { + if (info->is_speculative_ && !prefer_speculative) { if (info->is_main_frame_ && main_frame_) { // Always prefer the non-speculative main frame. return main_frame_; diff --git a/libcef/browser/browser_info.h b/libcef/browser/browser_info.h index a4c825abe..24a35020a 100644 --- a/libcef/browser/browser_info.h +++ b/libcef/browser/browser_info.h @@ -88,7 +88,8 @@ class CefBrowserInfo : public base::RefCountedThreadSafe { // UI thread. CefRefPtr GetFrameForHost( const content::RenderFrameHost* host, - bool* is_guest_view = nullptr) const; + bool* is_guest_view = nullptr, + bool prefer_speculative = false) const; // Returns the frame object matching the specified IDs or nullptr if no match // is found. Nullptr will also be returned if a guest view match is found @@ -98,7 +99,8 @@ class CefBrowserInfo : public base::RefCountedThreadSafe { CefRefPtr GetFrameForRoute( int32_t render_process_id, int32_t render_routing_id, - bool* is_guest_view = nullptr) const; + bool* is_guest_view = nullptr, + bool prefer_speculative = false) const; // Returns the frame object matching the specified ID or nullptr if no match // is found. Nullptr will also be returned if a guest view match is found @@ -107,7 +109,8 @@ class CefBrowserInfo : public base::RefCountedThreadSafe { // thread. CefRefPtr GetFrameForId( int64_t frame_id, - bool* is_guest_view = nullptr) const; + bool* is_guest_view = nullptr, + bool prefer_speculative = false) const; // Returns the frame object matching the specified ID or nullptr if no match // is found. Nullptr will also be returned if a guest view match is found diff --git a/libcef/browser/frame_host_impl.cc b/libcef/browser/frame_host_impl.cc index 694a98915..7dc5c600e 100644 --- a/libcef/browser/frame_host_impl.cc +++ b/libcef/browser/frame_host_impl.cc @@ -89,6 +89,7 @@ void CefFrameHostImpl::SetRenderFrameHost(content::RenderFrameHost* host) { CHECK(browser_info_); render_frame_.reset(); + is_attached_ = false; render_frame_host_ = host; frame_id_ = MakeFrameId(host); @@ -534,6 +535,7 @@ void CefFrameHostImpl::SendMessage(const std::string& name, } void CefFrameHostImpl::FrameAttached() { + DCHECK(!is_attached_); if (!is_attached_) { is_attached_ = true; diff --git a/libcef/browser/frame_host_impl.h b/libcef/browser/frame_host_impl.h index 89635beb4..08cb1b3ac 100644 --- a/libcef/browser/frame_host_impl.h +++ b/libcef/browser/frame_host_impl.h @@ -41,7 +41,9 @@ class CefFrameHostImpl : public CefFrame, public cef::mojom::BrowserFrame { CefFrameHostImpl(scoped_refptr browser_info, content::RenderFrameHost* render_frame_host); - // Update an existing main frame object. + // Update an existing main frame object on creation or for same-origin + // navigations. A new CefFrameHostImpl will be created for cross-origin + // navigations. void SetRenderFrameHost(content::RenderFrameHost* host); ~CefFrameHostImpl() override;