BackForwardCache: Support MediaSessionImplOnServiceCreated for BFcache

Now all the experimental groups support MediaSessionImplOnServiceCreated for back-forward cache by default. This change removes the enum values and supports this feature. Bug: 1177661 Change-Id: Iea614650612e8a1b108740fde4e4b792b351bacd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3113356 Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org> Reviewed-by: Fergal Daly <fergal@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/main@{#914684}
FairyWorld · Aug 24, 2021 · afd7a0c · afd7a0c
1 parent 70ef1dc
commit afd7a0c
Show file tree

Hide file tree

Showing 11 changed files with 17 additions and 67 deletions.
diff --git a/content/browser/back_forward_cache_browsertest.cc b/content/browser/back_forward_cache_browsertest.cc
@@ -10824,16 +10824,14 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
   EXPECT_FALSE(rfh_b->GetIsolationInfoForSubresources().IsEmpty());
 }
 
-IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
-                       DoNotCacheIfMediaSessionExists) {
+IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, CacheWithMediaSession) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
   // 1) Navigate to a page using MediaSession.
   EXPECT_TRUE(NavigateToURL(
       shell(), embedded_test_server()->GetURL("a.com", "/title1.html")));
-  RenderFrameHost* rfh_a = shell()->web_contents()->GetMainFrame();
-  RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);
-  EXPECT_TRUE(ExecJs(rfh_a, R"(
+  RenderFrameHostImplWrapper rfh_a(current_frame_host());
+  EXPECT_TRUE(ExecJs(rfh_a.get(), R"(
     navigator.mediaSession.metadata = new MediaMetadata({
       artwork: [
         {src: "test_image.jpg", sizes: "1x1", type: "image/jpeg"},
@@ -10845,20 +10843,17 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
   // 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();
+  EXPECT_TRUE(rfh_a->IsInBackForwardCache());
 
   // 3) Go back.
   web_contents()->GetController().GoBack();
   EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
-
-  auto reason = BackForwardCacheDisable::DisabledReason(
-      BackForwardCacheDisable::DisabledReasonId::
-          kMediaSessionImplOnServiceCreated);
-  ExpectNotRestored({BackForwardCacheMetrics::NotRestoredReason::
-                         kDisableForRenderFrameHostCalled},
-                    {}, {}, {reason}, FROM_HERE);
+  EXPECT_EQ(rfh_a.get(), current_frame_host());
+  ExpectRestored(FROM_HERE);
+  // Check the media session state is reserved.
+  EXPECT_EQ("10x10", EvalJs(rfh_a.get(), R"(
+    navigator.mediaSession.metadata.artwork[1].sizes;
+  )"));
 }
 
 class BackForwardCacheBrowserTestWithSupportedFeatures
@@ -11255,17 +11250,7 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
   shell->Close();
 }
 
-class BackForwardCacheBrowserTestWithMediaSession
-    : public BackForwardCacheBrowserTest {
- protected:
-  void SetUpCommandLine(base::CommandLine* command_line) override {
-    EnableFeatureAndSetParams(features::kBackForwardCache, "supported_features",
-                              "MediaSessionImplOnServiceCreated");
-    BackForwardCacheBrowserTest::SetUpCommandLine(command_line);
-  }
-};
-
-IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTestWithMediaSession,
+IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
                        DoNotCacheIfMediaSessionPlaybackStateChanged) {
   ASSERT_TRUE(embedded_test_server()->Start());
 

diff --git a/content/browser/devtools/protocol/page_handler.cc b/content/browser/devtools/protocol/page_handler.cc
@@ -1520,9 +1520,6 @@ Page::BackForwardCacheNotRestoredReason BlocklistedFeatureToProtocol(
       return Page::BackForwardCacheNotRestoredReasonEnum::IsolatedWorldScript;
     case WebSchedulerTrackedFeature::kInjectedStyleSheet:
       return Page::BackForwardCacheNotRestoredReasonEnum::InjectedStyleSheet;
-    case WebSchedulerTrackedFeature::kMediaSessionImplOnServiceCreated:
-      return Page::BackForwardCacheNotRestoredReasonEnum::
-          MediaSessionImplOnServiceCreated;
   }
 }
 
@@ -1541,10 +1538,6 @@ DisableForRenderFrameHostReasonToProtocol(
           static_cast<BackForwardCacheDisable::DisabledReasonId>(reason.id)) {
         case BackForwardCacheDisable::DisabledReasonId::kUnknown:
           return Page::BackForwardCacheNotRestoredReasonEnum::Unknown;
-        case BackForwardCacheDisable::DisabledReasonId::
-            kMediaSessionImplOnServiceCreated:
-          return Page::BackForwardCacheNotRestoredReasonEnum::
-              ContentMediaSessionImplOnServiceCreated;
         case BackForwardCacheDisable::DisabledReasonId::kSecurityHandler:
           return Page::BackForwardCacheNotRestoredReasonEnum::
               ContentSecurityHandler;
@@ -1729,7 +1722,6 @@ Page::BackForwardCacheNotRestoredReasonType MapBlocklistedFeatureToType(
     case WebSchedulerTrackedFeature::kWebOTPService:
     case WebSchedulerTrackedFeature::kOutstandingNetworkRequestDirectSocket:
     case WebSchedulerTrackedFeature::kInjectedStyleSheet:
-    case WebSchedulerTrackedFeature::kMediaSessionImplOnServiceCreated:
     case WebSchedulerTrackedFeature::kWebTransport:
       return Page::BackForwardCacheNotRestoredReasonTypeEnum::PageSupportNeeded;
     case WebSchedulerTrackedFeature::kAppBanner:

diff --git a/content/browser/media/session/media_session_impl.cc b/content/browser/media/session/media_session_impl.cc
@@ -1355,13 +1355,6 @@ bool MediaSessionImpl::AddOneShotPlayer(MediaSessionPlayerObserver* observer,
 void MediaSessionImpl::OnServiceCreated(MediaSessionServiceImpl* service) {
   const auto rfh_id = service->GetRenderFrameHostId();
 
-  if (!BackForwardCacheImpl::IsMediaSessionImplOnServiceCreatedAllowed()) {
-    BackForwardCache::DisableForRenderFrameHost(
-        rfh_id, BackForwardCacheDisable::DisabledReason(
-                    BackForwardCacheDisable::DisabledReasonId::
-                        kMediaSessionImplOnServiceCreated));
-  }
-
   services_[rfh_id] = service;
   UpdateRoutedService();
 }

diff --git a/content/browser/renderer_host/back_forward_cache_disable.cc b/content/browser/renderer_host/back_forward_cache_disable.cc
@@ -12,9 +12,6 @@ std::string ReasonIdToString(
   switch (reason_id) {
     case BackForwardCacheDisable::DisabledReasonId::kUnknown:
       return "Unknown";
-    case BackForwardCacheDisable::DisabledReasonId::
-        kMediaSessionImplOnServiceCreated:
-      return "MediaSessionImpl::OnServiceCreated";
     case BackForwardCacheDisable::DisabledReasonId::kSecurityHandler:
       return "content::protocol::SecurityHandler";
     case BackForwardCacheDisable::DisabledReasonId::kWebAuthenticationAPI:

diff --git a/content/browser/renderer_host/back_forward_cache_disable.h b/content/browser/renderer_host/back_forward_cache_disable.h
@@ -20,7 +20,8 @@ class CONTENT_EXPORT BackForwardCacheDisable {
     // this enum is not logged directly as an enum (see
     // BackForwardCache::DisabledSource).
     kUnknown = 0,
-    kMediaSessionImplOnServiceCreated = 1,
+    // kMediaSessionImplOnServiceCreated = 1, Removed after implementing
+    // MediaSessionImplOnServiceCreated support in back/forward cache.
     kSecurityHandler = 2,
     kWebAuthenticationAPI = 3,
     kFileChooser = 4,

diff --git a/content/browser/renderer_host/back_forward_cache_impl.cc b/content/browser/renderer_host/back_forward_cache_impl.cc
@@ -238,8 +238,7 @@ BlockListedFeatures GetDisallowedFeatures(
       WebSchedulerTrackedFeature::kWebShare,
       WebSchedulerTrackedFeature::kWebSocket,
       WebSchedulerTrackedFeature::kWebTransport,
-      WebSchedulerTrackedFeature::kWebXR,
-      WebSchedulerTrackedFeature::kMediaSessionImplOnServiceCreated);
+      WebSchedulerTrackedFeature::kWebXR);
 
   WebSchedulerTrackedFeatures result = kAlwaysDisallowedFeatures;
 
@@ -1127,11 +1126,6 @@ bool BackForwardCacheImpl::IsQueryAllowed(const GURL& current_url) {
   return true;
 }
 
-bool BackForwardCacheImpl::IsMediaSessionImplOnServiceCreatedAllowed() {
-  return SupportedFeatures().Has(
-      WebSchedulerTrackedFeature::kMediaSessionImplOnServiceCreated);
-}
-
 void BackForwardCacheImpl::WillCommitNavigationToCachedEntry(
     Entry& bfcache_entry,
     base::OnceClosure done_callback) {

diff --git a/content/browser/renderer_host/back_forward_cache_impl.h b/content/browser/renderer_host/back_forward_cache_impl.h
@@ -150,10 +150,6 @@ class CONTENT_EXPORT BackForwardCacheImpl
     kOptInHeaderRequired,
   };
 
-  // Returns whether MediaSessionImpl::OnServiceCreated is allowed for the
-  // BackForwardCache.
-  static bool IsMediaSessionImplOnServiceCreatedAllowed();
-
   BackForwardCacheImpl();
   ~BackForwardCacheImpl() override;
 

diff --git a/third_party/blink/common/scheduler/web_scheduler_tracked_feature.cc b/third_party/blink/common/scheduler/web_scheduler_tracked_feature.cc
@@ -120,9 +120,6 @@ FeatureNames FeatureToNames(WebSchedulerTrackedFeature feature) {
       return {"IsolatedWorldScript", "Isolated world ran script"};
     case WebSchedulerTrackedFeature::kInjectedStyleSheet:
       return {"InjectedStyleSheet", "External systesheet injected"};
-    case WebSchedulerTrackedFeature::kMediaSessionImplOnServiceCreated:
-      return {"MediaSessionImplOnServiceCreated",
-              "MediaSessionImplOnServiceCreated"};
   }
   return {};
 }

diff --git a/third_party/blink/public/common/scheduler/web_scheduler_tracked_feature.h b/third_party/blink/public/common/scheduler/web_scheduler_tracked_feature.h
@@ -106,7 +106,8 @@ enum class WebSchedulerTrackedFeature : uint32_t {
   kOutstandingNetworkRequestDirectSocket = 53,
   kIsolatedWorldScript = 54,
   kInjectedStyleSheet = 55,
-  kMediaSessionImplOnServiceCreated = 56,
+  // kMediaSessionImplOnServiceCreated = 56, Removed after implementing
+  // MediaSessionImplOnServiceCreated support in back/forward cache.
   kWebTransport = 57,
 
   // NB: This enum is used in a bitmask, so kMaxValue must be less than 64.

diff --git a/third_party/blink/public/devtools_protocol/browser_protocol.pdl b/third_party/blink/public/devtools_protocol/browser_protocol.pdl
@@ -7903,9 +7903,7 @@ domain Page
       OutstandingNetworkRequestDirectSocket
       IsolatedWorldScript
       InjectedStyleSheet
-      MediaSessionImplOnServiceCreated
       # Disabled for render frame host reasons
-      ContentMediaSessionImplOnServiceCreated
       ContentSecurityHandler
       ContentWebAuthenticationAPI
       ContentFileChooser

diff --git a/...ocol/bfcache/report-back-forward-cache-status-disabled-for-render-frame-host-expected.txt b/...ocol/bfcache/report-back-forward-cache-status-disabled-for-render-frame-host-expected.txt
@@ -8,14 +8,10 @@ Test that back/forward navigations report the bfcache status
             type : Circumstantial
         }
         [1] : {
-            reason : ContentMediaSessionImplOnServiceCreated
-            type : Circumstantial
-        }
-        [2] : {
             reason : ContentMediaSession
             type : Circumstantial
         }
-        [3] : {
+        [2] : {
             reason : BrowsingInstanceNotSwapped
             type : Circumstantial
         }