Add support for extension message and back/forward cache.

Previously we use to disallow back/forward cache on any page that
did extension messaging. Narrow that broad disablement to only if
there is an active message port open.

BUG=1222803

Change-Id: I51e456de5d774c997473924c7cd3e04e7df84a0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2980333
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#897103}

chrome/test/data/extensions/back_forward_cache/content_script/background.js

@@ -0,0 +1,12 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+chrome.runtime.onConnectExternal.addListener((p) => {
+  p.postMessage('connected');
+  p.onMessage.addListener((m) => {
+    if (m == 'disconnect') {
+      p.disconnect();
+    }
+  })
+});

chrome/test/data/extensions/back_forward_cache/content_script/change_page_title.js → chrome/test/data/extensions/back_forward_cache/content_script/content_script.js

@@ -3,3 +3,12 @@
 // found in the LICENSE file.
 
 document.title = "modified";
+
+chrome.runtime.onConnect.addListener((p) => {
+  p.postMessage('connected');
+  p.onMessage.addListener((m) => {
+    if (m == 'disconnect') {
+      p.disconnect();
+    }
+  });
+});

chrome/test/data/extensions/back_forward_cache/content_script/manifest.json

@@ -10,8 +10,12 @@
   "content_scripts": [
     {
       "matches": ["http://*/*", "https://*/*"],
-      "js": ["change_page_title.js"],
+      "js": ["content_script.js"],
       "run_at": "document_end"
     }
-  ]
+  ],
+  "background": {
+    "scripts": ["background.js"],
+    "persistent": false
+  }
 }

components/back_forward_cache/back_forward_cache_disable.cc

@@ -35,6 +35,8 @@ std::string ReasonIdToString(DisabledReasonId reason_id) {
       return "Extensions";
     case DisabledReasonId::kExtensionMessaging:
       return "ExtensionMessaging";
+    case DisabledReasonId::kExtensionMessagingForOpenPort:
+      return "ExtensionMessagingForOpenPort";
     case DisabledReasonId::kOomInterventionTabHelper:
       return "OomInterventionTabHelper";
   }

components/back_forward_cache/back_forward_cache_disable.h

@@ -31,6 +31,7 @@ enum class DisabledReasonId : content::BackForwardCache::DisabledReasonType {
   kModalDialog = 11,
   kExtensions = 12,
   kExtensionMessaging = 13,
+  kExtensionMessagingForOpenPort = 14,
   // New reasons should be accompanied by a comment as to why BackForwardCache
   // cannot be used in this case and a link to a bug to fix that if it is
   // fixable.

content/browser/renderer_host/back_forward_cache_impl.cc

@@ -906,6 +906,15 @@ void BackForwardCache::DisableForRenderFrameHost(
   DisableForRenderFrameHost(render_frame_host->GetGlobalId(), reason);
 }
 
+// static
+void BackForwardCache::ClearDisableReasonForRenderFrameHost(
+    GlobalRenderFrameHostId id,
+    BackForwardCache::DisabledReason reason) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (auto* rfh = RenderFrameHostImpl::FromID(id))
+    rfh->ClearDisableBackForwardCache(reason);
+}
+
 // static
 void BackForwardCache::DisableForRenderFrameHost(
     GlobalRenderFrameHostId id,

content/browser/renderer_host/render_frame_host_impl.cc

@@ -1729,6 +1729,11 @@ void RenderFrameHostImpl::DisableBackForwardCache(
   MaybeEvictFromBackForwardCache();
 }
 
+void RenderFrameHostImpl::ClearDisableBackForwardCache(
+    BackForwardCache::DisabledReason reason) {
+  back_forward_cache_disabled_reasons_.erase(reason);
+}
+
 void RenderFrameHostImpl::DisableProactiveBrowsingInstanceSwapForTesting() {
   // This should only be called on main frames.
   DCHECK(!GetParent());

content/browser/renderer_host/render_frame_host_impl.h

@@ -1298,6 +1298,7 @@ class CONTENT_EXPORT RenderFrameHostImpl
   void DisableBackForwardCache(BackForwardCache::DisabledSource source,
                                BackForwardCache::DisabledReasonType reason);
   void DisableBackForwardCache(BackForwardCache::DisabledReason reason);
+  void ClearDisableBackForwardCache(BackForwardCache::DisabledReason reason);
 
   bool is_evicted_from_back_forward_cache() {
     return is_evicted_from_back_forward_cache_;

content/public/browser/back_forward_cache.h

@@ -87,6 +87,9 @@ class CONTENT_EXPORT BackForwardCache {
   // its |id| can be used.
   static void DisableForRenderFrameHost(GlobalRenderFrameHostId id,
                                         DisabledReason reason);
+  // Clear a previously set `reason` for a `render_frame_host`.
+  static void ClearDisableReasonForRenderFrameHost(GlobalRenderFrameHostId id,
+                                                   DisabledReason reason);
 
   // List of reasons the BackForwardCache was disabled for a specific test. If a
   // test needs to be disabled for a reason not covered below, please add to

extensions/browser/api/messaging/extension_message_port.cc

@@ -8,11 +8,13 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/containers/contains.h"
 #include "base/scoped_observation.h"
 #include "base/strings/strcat.h"
+#include "components/back_forward_cache/back_forward_cache_disable.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/navigation_handle.h"
-#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_document_host_user_data.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
@@ -32,6 +34,60 @@ std::string PortIdToString(const extensions::PortId& port_id) {
                        base::NumberToString(port_id.GetChannelId().second)});
 }
 
+// This class is created per RenderDocument. It stores the number of
+// ExtensionMessagePort objects that have the associated `RenderFrameHost` in
+// the `ExtensionMessagePort::frames_` set.
+class MessagePortStatePerRenderDocument
+    : public content::RenderDocumentHostUserData<
+          MessagePortStatePerRenderDocument> {
+ public:
+  ~MessagePortStatePerRenderDocument() override {
+    if (ports_opened_ > 0) {
+      // Clear the disable reason as the new document may reuse the
+      // RenderFrameHost.
+      content::BackForwardCache::ClearDisableReasonForRenderFrameHost(
+          rfh_id_, back_forward_cache::DisabledReason(
+                       back_forward_cache::DisabledReasonId::
+                           kExtensionMessagingForOpenPort));
+    }
+  }
+
+  void PortOpened() {
+    ports_opened_++;
+    if (ports_opened_ != 1)
+      return;
+    content::BackForwardCache::DisableForRenderFrameHost(
+        rfh_id_, back_forward_cache::DisabledReason(
+                     back_forward_cache::DisabledReasonId::
+                         kExtensionMessagingForOpenPort));
+  }
+
+  void PortClosed() {
+    DCHECK_LT(0u, ports_opened_);
+    ports_opened_--;
+    if (ports_opened_ > 0)
+      return;
+
+    content::BackForwardCache::ClearDisableReasonForRenderFrameHost(
+        rfh_id_, back_forward_cache::DisabledReason(
+                     back_forward_cache::DisabledReasonId::
+                         kExtensionMessagingForOpenPort));
+  }
+
+ private:
+  explicit MessagePortStatePerRenderDocument(content::RenderFrameHost* rfh)
+      : rfh_id_(rfh->GetGlobalId()) {}
+  friend class content::RenderDocumentHostUserData<
+      MessagePortStatePerRenderDocument>;
+
+  size_t ports_opened_ = 0;
+  const content::GlobalRenderFrameHostId rfh_id_;
+
+  RENDER_DOCUMENT_HOST_USER_DATA_KEY_DECL();
+};
+
+RENDER_DOCUMENT_HOST_USER_DATA_KEY_IMPL(MessagePortStatePerRenderDocument)
+
 }  // namespace
 
 namespace extensions {
@@ -181,13 +237,29 @@ std::unique_ptr<ExtensionMessagePort> ExtensionMessagePort::CreateForEndpoint(
   return port;
 }
 
-ExtensionMessagePort::~ExtensionMessagePort() {}
+ExtensionMessagePort::~ExtensionMessagePort() {
+  ClearFrames();
+}
+
+void ExtensionMessagePort::ClearFrames() {
+  for (auto* rfh : frames_) {
+    auto* message_port_state =
+        MessagePortStatePerRenderDocument::GetForCurrentDocument(rfh);
+    if (message_port_state)
+      message_port_state->PortClosed();
+  }
+  frames_.clear();
+}
 
 void ExtensionMessagePort::RemoveCommonFrames(const MessagePort& port) {
   // Avoid overlap in the set of frames to make sure that it does not matter
   // when UnregisterFrame is called.
   for (auto it = frames_.begin(); it != frames_.end();) {
     if (port.HasFrame(*it)) {
+      auto* message_port_state =
+          MessagePortStatePerRenderDocument::GetForCurrentDocument(*it);
+      if (message_port_state)
+        message_port_state->PortClosed();
       frames_.erase(it++);
     } else {
       ++it;
@@ -326,7 +398,7 @@ void ExtensionMessagePort::ClosePort(int process_id,
     // The only non-frame-specific message is the response to an unhandled
     // onConnect event in the extension process.
     DCHECK(extension_process_);
-    frames_.clear();
+    ClearFrames();
     if (!HasReceivers())
       CloseChannel();
     return;
@@ -353,12 +425,23 @@ void ExtensionMessagePort::RegisterFrame(content::RenderFrameHost* rfh) {
   // ensure that we are notified of frame destruction. Without this check,
   // |frames_| can eventually contain a stale pointer because RenderFrameDeleted
   // is not triggered for |rfh|.
-  if (rfh->IsRenderFrameLive())
+  if (rfh->IsRenderFrameLive()) {
+    auto* message_port_state =
+        MessagePortStatePerRenderDocument::GetOrCreateForCurrentDocument(rfh);
+    message_port_state->PortOpened();
     frames_.insert(rfh);
+  }
 }
 
 void ExtensionMessagePort::UnregisterFrame(content::RenderFrameHost* rfh) {
-  if (frames_.erase(rfh) != 0 && !HasReceivers())
+  if (frames_.erase(rfh) == 0)
+    return;
+  auto* message_port_state =
+      MessagePortStatePerRenderDocument::GetForCurrentDocument(rfh);
+  if (message_port_state)
+    message_port_state->PortClosed();
+
+  if (!HasReceivers())
     CloseChannel();
 }
 

extensions/browser/api/messaging/extension_message_port.h

@@ -96,6 +96,9 @@ class ExtensionMessagePort : public MessagePort {
                        const ExtensionId& extension_id,
                        content::BrowserContext* browser_context);
 
+  // Clears the `frames_` set.
+  void ClearFrames();
+
   // Registers a frame as a receiver / sender.
   void RegisterFrame(content::RenderFrameHost* rfh);
 

extensions/browser/api/messaging/message_service.cc

@@ -19,7 +19,6 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/values.h"
 #include "build/build_config.h"
-#include "components/back_forward_cache/back_forward_cache_disable.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_frame_host.h"
@@ -111,14 +110,6 @@ const Extension* GetExtensionForNativeAppChannel(
                                                                 true);
 }
 
-void DisableBackForwardCacheForMessaging(content::RenderFrameHost* host) {
-  if (!host)
-    return;
-  content::BackForwardCache::DisableForRenderFrameHost(
-      host, back_forward_cache::DisabledReason(
-                back_forward_cache::DisabledReasonId::kExtensionMessaging));
-}
-
 }  // namespace
 
 struct MessageService::MessageChannel {
@@ -432,9 +423,6 @@ void MessageService::OpenChannelToNativeApp(
   content::RenderFrameHost* source_rfh =
       source.is_for_render_frame() ? source.GetRenderFrameHost() : nullptr;
 
-  // Disable back forward cache.
-  DisableBackForwardCacheForMessaging(source_rfh);
-
   std::string error = kReceivingEndDoesntExistError;
   const PortId receiver_port_id = source_port_id.GetOppositePortId();
   // NOTE: We're creating |receiver| with nullptr |source_rfh|, which seems to
@@ -500,9 +488,6 @@ void MessageService::OpenChannelToTab(const ChannelEndpoint& source,
     return;
   }
 
-  // Disable back forward cache.
-  DisableBackForwardCacheForMessaging(receiver_contents->GetMainFrame());
-
   const PortId receiver_port_id = source_port_id.GetOppositePortId();
   std::unique_ptr<MessagePort> receiver =
       messaging_delegate_->CreateReceiverForTab(weak_factory_.GetWeakPtr(),
@@ -898,10 +883,6 @@ void MessageService::OnOpenChannelAllowed(
     return;
   }
 
-  content::RenderFrameHost* source_rfh =
-      source.is_for_render_frame() ? source.GetRenderFrameHost() : nullptr;
-  DisableBackForwardCacheForMessaging(source_rfh);
-
   // The target might be a lazy background page or a Service Worker. In that
   // case, we have to check if it is loaded and ready, and if not, queue up the
   // task and load the page.

tools/metrics/histograms/enums.xml

@@ -6826,6 +6826,7 @@ others/histograms.xml -->
   <int value="196619" label="kEmbedder-kModalDialog"/>
   <int value="196620" label="kEmbedder-kExtensions"/>
   <int value="196621" label="kEmbedder-kExtensionMessaging"/>
+  <int value="196622" label="kEmbedder-kExtensionMessagingForOpenPort"/>
 </enum>
 
 <enum name="BackForwardCacheDisabledForRenderFrameHostReasonShort">