[bfcache] Don't cache pages with an active media session.

TBR=vakh@chromium.org

Change-Id: If78de3f54944ae0aed101c7ca692bdbe5d2d308c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1827280
Commit-Queue: Lowell Manners <lowell@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701531}

WATCHLISTS

@@ -245,7 +245,7 @@
       'filepath': 'content/browser/frame_host/back_forward_cache.*|'\
                   'content/browser/back_forward_cache_browsertest.cc|'\
                   'content/test/data/back_forward_cache/|'\
-                  'content/public/browser/back_forward_cache.h',
+                  'content/public/browser/back_forward_cache.*',
     },
     'binary_size': {
       'filepath': 'build/android/binary_size/|'\

components/safe_browsing/browser/threat_details.cc

@@ -311,17 +311,6 @@ void TrimElements(const std::set<int> target_ids,
   }
 }
 
-void DisableBackForwardCache(content::RenderFrameHost* rfh,
-                             base::StringPiece reason) {
-  content::WebContents::FromRenderFrameHost(rfh)
-      ->GetController()
-      .GetBackForwardCache()
-      .DisableForRenderFrameHost(
-          content::GlobalFrameRoutingId(rfh->GetProcess()->GetID(),
-                                        rfh->GetRoutingID()),
-          reason);
-}
-
 }  // namespace
 
 // The default ThreatDetailsFactory.  Global, made a singleton so we
@@ -652,7 +641,8 @@ void ThreatDetails::StartCollection() {
 }
 
 void ThreatDetails::RequestThreatDOMDetails(content::RenderFrameHost* frame) {
-  DisableBackForwardCache(frame, "safe_browsing::ThreatDetails");
+  content::BackForwardCache::DisableForRenderFrameHost(
+      frame, "safe_browsing::ThreatDetails");
   mojo::Remote<safe_browsing::mojom::ThreatReporter> threat_reporter;
   frame->GetRemoteInterfaces()->GetInterface(
       threat_reporter.BindNewPipeAndPassReceiver());

content/browser/media/session/media_session_browsertest.cc

@@ -12,14 +12,18 @@
 #include "base/synchronization/lock.h"
 #include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
+#include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
+#include "content/public/common/content_features.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
+#include "content/public/test/test_utils.h"
 #include "content/shell/browser/shell.h"
 #include "media/base/media_switches.h"
+#include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/http_request.h"
 #include "services/media_session/public/cpp/features.h"
 #include "services/media_session/public/cpp/test/audio_focus_test_util.h"
@@ -168,6 +172,10 @@ class MediaSessionBrowserTest : public ContentBrowserTest {
     return embedded_test_server()->GetURL(kMediaSessionTestImagePath);
   }
 
+ protected:
+  base::test::ScopedFeatureList scoped_feature_list_;
+  base::test::ScopedFeatureList disabled_feature_list_;
+
  private:
   class MediaStartStopObserver : public WebContentsObserver {
    public:
@@ -215,12 +223,26 @@ class MediaSessionBrowserTest : public ContentBrowserTest {
   base::Lock visited_urls_lock_;
   std::set<GURL> visited_urls_;
 
-  base::test::ScopedFeatureList scoped_feature_list_;
-  base::test::ScopedFeatureList disabled_feature_list_;
-
   DISALLOW_COPY_AND_ASSIGN(MediaSessionBrowserTest);
 };
 
+// A MediaSessionBrowserTest with BackForwardCache enabled.
+class MediaSessionBrowserTestWithBackForwardCache
+    : public MediaSessionBrowserTest {
+  void SetUpOnMainThread() override {
+    host_resolver()->AddRule("*", "127.0.0.1");
+    MediaSessionBrowserTest::SetUpOnMainThread();
+  }
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    scoped_feature_list_.InitWithFeaturesAndParameters(
+        {{features::kBackForwardCache,
+          {{"TimeToLiveInBackForwardCacheInSeconds", "3600"}}},
+         {media::kInternalMediaSession, {}}},
+        /*disabled_features=*/{});
+  }
+};
+
 }  // anonymous namespace
 
 IN_PROC_BROWSER_TEST_F(MediaSessionBrowserTest, MediaSessionNoOpWhenDisabled) {
@@ -428,4 +450,43 @@ IN_PROC_BROWSER_TEST_F(MediaSessionBrowserTest,
   EXPECT_FALSE(WasURLVisited(image.src));
 }
 
+IN_PROC_BROWSER_TEST_F(MediaSessionBrowserTestWithBackForwardCache,
+                       DoNotCacheIfMediaSessionExists) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  // 1) Navigate to a page containing media.
+  EXPECT_TRUE(NavigateToURL(shell(),
+                            GetTestUrl("media/session", "media-session.html")));
+
+  RenderFrameHost* rfh_a = shell()->web_contents()->GetMainFrame();
+  RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);
+
+  // 2) Navigate away.
+  EXPECT_TRUE(NavigateToURL(
+      shell(), embedded_test_server()->GetURL("b.com", "/title1.html")));
+
+  // The page should not have been cached in the back forward cache.
+  delete_observer_rfh_a.WaitUntilDeleted();
+}
+
+IN_PROC_BROWSER_TEST_F(MediaSessionBrowserTestWithBackForwardCache,
+                       CachesPageWithoutMedia) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  // 1) Navigate to a page not containing any media.
+  EXPECT_TRUE(NavigateToURL(
+      shell(), embedded_test_server()->GetURL("a.com", "/title1.html")));
+
+  RenderFrameHostImpl* rfh_a = static_cast<RenderFrameHostImpl*>(
+      shell()->web_contents()->GetMainFrame());
+  RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);
+
+  // 2) Navigate away.
+  EXPECT_TRUE(NavigateToURL(
+      shell(), embedded_test_server()->GetURL("b.com", "/title1.html")));
+
+  // The page should be cached in the back forward cache.
+  EXPECT_FALSE(delete_observer_rfh_a.deleted());
+  EXPECT_TRUE(rfh_a->is_in_back_forward_cache());
+}
 }  // namespace content

content/browser/media/session/media_session_impl.cc

@@ -20,6 +20,7 @@
 #include "content/browser/media/session/media_session_service_impl.h"
 #include "content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
+#include "content/public/browser/back_forward_cache.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/media_session.h"
 #include "content/public/browser/navigation_handle.h"
@@ -1123,6 +1124,9 @@ void MediaSessionImpl::OnServiceCreated(MediaSessionServiceImpl* service) {
   if (!rfh)
     return;
 
+  content::BackForwardCache::DisableForRenderFrameHost(
+      rfh, "MediaSessionImpl::OnServiceCreated");
+
   services_[rfh] = service;
   UpdateRoutedService();
 }

content/public/browser/BUILD.gn

@@ -53,6 +53,7 @@ jumbo_source_set("browser_sources") {
     "authenticator_request_client_delegate.h",
     "ax_event_notification_details.cc",
     "ax_event_notification_details.h",
+    "back_forward_cache.cc",
     "back_forward_cache.h",
     "background_fetch_delegate.cc",
     "background_fetch_delegate.h",

content/public/browser/back_forward_cache.cc

+// Copyright 2019 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.
+
+#include "content/public/browser/back_forward_cache.h"
+
+#include "base/strings/string_util.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/web_contents.h"
+
+namespace content {
+
+void BackForwardCache::DisableForRenderFrameHost(
+    RenderFrameHost* render_frame_host,
+    base::StringPiece reason) {
+  content::WebContents::FromRenderFrameHost(render_frame_host)
+      ->GetController()
+      .GetBackForwardCache()
+      .DisableForRenderFrameHost(content::GlobalFrameRoutingId(
+                                     render_frame_host->GetProcess()->GetID(),
+                                     render_frame_host->GetRoutingID()),
+                                 reason);
+}
+}  // namespace content

content/public/browser/back_forward_cache.h

@@ -11,6 +11,8 @@
 
 namespace content {
 
+class RenderFrameHost;
+
 // Public API for the BackForwardCache.
 //
 // After the user navigates away from a document, the old one might go into the
@@ -44,6 +46,10 @@ class CONTENT_EXPORT BackForwardCache {
   virtual void DisableForRenderFrameHost(GlobalFrameRoutingId id,
                                          base::StringPiece reason) = 0;
 
+  // Convenience static method for calling DisableForRenderFameHost.
+  static void DisableForRenderFrameHost(RenderFrameHost* render_frame_host,
+                                        base::StringPiece 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
   // this enum.