Don't reconnect after intentional browser frame detach (see issue #3260)

Send a FrameDetached message from CefFrameHostImpl::Detach before closing
the RenderFrame connection to avoid an immediate reconnect attempt by the
renderer.

When BFCache is disabled the intentionally detached frame will never be
reconnected. When BFCache is enabled the intentionally detached frame will
be reconnected via CefFrameImpl::OnWasShown if/when it exits the cache.

libcef/browser/frame_host_impl.cc

@@ -480,6 +480,10 @@ bool CefFrameHostImpl::Detach(DetachReason reason) {
     queued_renderer_actions_.pop();
   }
 
+  if (render_frame_.is_bound()) {
+    render_frame_->FrameDetached();
+  }
+
   render_frame_.reset();
   render_frame_host_ = nullptr;
 

libcef/common/mojom/cef.mojom

@@ -52,6 +52,9 @@ interface RenderFrame {
   // Browser process has received the FrameAttached() message.
   FrameAttachedAck();
 
+  // Browser process has intentionally detached.
+  FrameDetached();
+
   // Send a message to the render process.
   SendMessage(string name, mojo_base.mojom.Value arguments);
 

libcef/renderer/frame_impl.cc

@@ -531,11 +531,16 @@ void CefFrameImpl::ConnectBrowserFrame(ConnectReason reason) {
   auto& browser_frame = GetBrowserFrame(/*expect_acked=*/false);
   CHECK(browser_frame);
 
+  // True if this connection is a retry or if the frame just exited the
+  // BackForwardCache.
+  const bool reattached =
+      browser_connect_retry_ct_ > 0 || reason == ConnectReason::WAS_SHOWN;
+
   // If the channel is working we should get a call to FrameAttachedAck().
   // Otherwise, OnDisconnect() should be called to retry the
   // connection.
   browser_frame->FrameAttached(receiver_.BindNewPipeAndPassRemote(),
-                               browser_connect_retry_ct_ > 0);
+                               reattached);
   receiver_.set_disconnect_handler(
       base::BindOnce(&CefFrameImpl::OnDisconnect, this,
                      DisconnectReason::RENDER_FRAME_DISCONNECT));
@@ -580,6 +585,9 @@ void CefFrameImpl::OnDisconnect(DisconnectReason reason) {
       case DisconnectReason::DETACHED:
         reason_str = "DETACHED";
         break;
+      case DisconnectReason::BROWSER_FRAME_DETACHED:
+        reason_str = "BROWSER_FRAME_DETACHED";
+        break;
       case DisconnectReason::CONNECT_TIMEOUT:
         reason_str = "CONNECT_TIMEOUT";
         break;
@@ -619,8 +627,9 @@ void CefFrameImpl::OnDisconnect(DisconnectReason reason) {
   browser_connection_state_ = ConnectionState::DISCONNECTED;
   browser_connect_timer_.Stop();
 
-  // Only retry if the frame is still valid.
-  if (frame_) {
+  // Only retry if the frame is still valid and the browser process has not
+  // intentionally detached.
+  if (frame_ && reason != DisconnectReason::BROWSER_FRAME_DETACHED) {
     if (browser_connect_retry_ct_++ < kConnectionRetryMaxCt) {
       VLOG(1) << GetDebugString() << " connection retry scheduled";
 
@@ -708,6 +717,12 @@ void CefFrameImpl::FrameAttachedAck() {
   }
 }
 
+void CefFrameImpl::FrameDetached() {
+  // Sent from the browser process in response to CefFrameHostImpl::Detach().
+  CHECK_EQ(ConnectionState::CONNECTION_ACKED, browser_connection_state_);
+  OnDisconnect(DisconnectReason::BROWSER_FRAME_DETACHED);
+}
+
 void CefFrameImpl::SendMessage(const std::string& name, base::Value arguments) {
   if (auto app = CefAppManager::Get()->GetApplication()) {
     if (auto handler = app->GetRenderProcessHandler()) {

libcef/renderer/frame_impl.h

@@ -127,6 +127,7 @@ class CefFrameImpl
 
   enum class DisconnectReason {
     DETACHED,
+    BROWSER_FRAME_DETACHED,
     CONNECT_TIMEOUT,
     RENDER_FRAME_DISCONNECT,
     BROWSER_FRAME_DISCONNECT,
@@ -147,6 +148,7 @@ class CefFrameImpl
 
   // cef::mojom::RenderFrame methods:
   void FrameAttachedAck() override;
+  void FrameDetached() override;
   void SendMessage(const std::string& name, base::Value arguments) override;
   void SendSharedMemoryRegion(const std::string& name,
                               base::ReadOnlySharedMemoryRegion region) override;