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

@@ -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<CefFrameHostImpl> CefBrowserFrame::GetFrameHost() const {
+CefRefPtr<CefFrameHostImpl> 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;

libcef/browser/browser_frame.h

@@ -42,7 +42,8 @@ class CefBrowserFrame
   // FrameServiceBase methods:
   bool ShouldCloseOnFinishNavigation() const override { return false; }
 
-  CefRefPtr<CefFrameHostImpl> GetFrameHost() const;
+  CefRefPtr<CefFrameHostImpl> GetFrameHost(
+      bool prefer_speculative = false) const;
 
   DISALLOW_COPY_AND_ASSIGN(CefBrowserFrame);
 };

libcef/browser/browser_info.cc

@@ -86,15 +86,11 @@ 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.
+        if (main_frame_)
+          main_frame_->Detach();
         main_frame_ = info->frame_;
       }
-      }
       info->is_speculative_ = false;
       MaybeUpdateFrameTreeNodeIdMap(info);
     }
@@ -200,20 +196,23 @@ CefRefPtr<CefFrameHostImpl> CefBrowserInfo::CreateTempSubFrame(
 
 CefRefPtr<CefFrameHostImpl> 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<CefFrameHostImpl> 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<CefFrameHostImpl> CefBrowserInfo::GetFrameForRoute(
 
   return GetFrameForId(
       CefFrameHostImpl::MakeFrameId(render_process_id, render_routing_id),
-      is_guest_view);
+      is_guest_view, prefer_speculative);
 }
 
 CefRefPtr<CefFrameHostImpl> 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<CefFrameHostImpl> 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_;

libcef/browser/browser_info.h

@@ -88,7 +88,8 @@ class CefBrowserInfo : public base::RefCountedThreadSafe<CefBrowserInfo> {
   // UI thread.
   CefRefPtr<CefFrameHostImpl> 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<CefBrowserInfo> {
   CefRefPtr<CefFrameHostImpl> 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<CefBrowserInfo> {
   // thread.
   CefRefPtr<CefFrameHostImpl> 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

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;
 

libcef/browser/frame_host_impl.h

@@ -41,7 +41,9 @@ class CefFrameHostImpl : public CefFrame, public cef::mojom::BrowserFrame {
   CefFrameHostImpl(scoped_refptr<CefBrowserInfo> 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;