[bfcache] Enable bfcache when WakeLock is released

This CL makes a page bfcache-eligible when WakeLock is released.

Bug: 1039569
Change-Id: Ie951c5db8eaf0f08c0ff5dd27742c1ac7cf1b8db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2428504
Commit-Queue: Yuzu Saijo <yuzus@chromium.org>
Reviewed-by: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813072}

content/browser/back_forward_cache_browsertest.cc

@@ -2044,10 +2044,10 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, DoesNotCacheIfWebHID) {
 
 IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
                        DoesNotCacheIfAcquiredWakeLock) {
-  ASSERT_TRUE(embedded_test_server()->Start());
+  ASSERT_TRUE(CreateHttpsServer()->Start());
 
   // 1) Navigate to a page with WakeLock usage.
-  GURL url(embedded_test_server()->GetURL("/back_forward_cache/empty.html"));
+  GURL url(https_server()->GetURL("a.com", "/back_forward_cache/empty.html"));
   EXPECT_TRUE(NavigateToURL(shell(), url));
 
   RenderFrameHostImpl* rfh_a = current_frame_host();
@@ -2055,18 +2055,18 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
 
   // Acquire WakeLock.
   EXPECT_EQ("DONE", EvalJs(rfh_a, R"(
-  new Promise(async resolve => {
-    try {
-      await navigator.wakeLock.request('screen');
-      resolve('DONE');
-    } catch (error) {
-      resolve('error: request failed');
-    }
-  });
+    new Promise(async resolve => {
+      try {
+        await navigator.wakeLock.request('screen');
+        resolve('DONE');
+      } catch (error) {
+        resolve('error: request failed');
+      }
+    });
   )"));
 
   // 2) Navigate away.
-  shell()->LoadURL(embedded_test_server()->GetURL("b.com", "/title1.html"));
+  shell()->LoadURL(https_server()->GetURL("b.com", "/title1.html"));
 
   // The page uses WakeLock so it should be deleted.
   deleted.WaitUntilDeleted();
@@ -2081,55 +2081,78 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
       blink::scheduler::WebSchedulerTrackedFeature::kWakeLock, FROM_HERE);
 }
 
-// TODO(yuzus): By releasing wakelock, the page should become cacheable again.
-// Fix and re-enable the rest of this test.
-IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
-                       DISABLED_CacheIfReleasedWakeLock) {
-  ASSERT_TRUE(embedded_test_server()->Start());
+IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, CacheIfReleasedWakeLock) {
+  ASSERT_TRUE(CreateHttpsServer()->Start());
 
   // 1) Navigate to a page with WakeLock usage.
-  GURL url(embedded_test_server()->GetURL(
-      "/back_forward_cache/page_with_wakelock.html"));
+  GURL url(https_server()->GetURL("a.com", "/back_forward_cache/empty.html"));
   EXPECT_TRUE(NavigateToURL(shell(), url));
-
+  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
   RenderFrameHostImpl* rfh_a = current_frame_host();
-  RenderFrameDeletedObserver deleted(current_frame_host());
+  // Acquire and release WakeLock.
+  EXPECT_EQ("DONE", EvalJs(rfh_a, R"(
+    new Promise(async resolve => {
+      try {
+        const lock = await navigator.wakeLock.request('screen');
+        await lock.release();
+        resolve('DONE');
+      } catch (error) {
+        resolve('error: request failed');
+      }
+    });
+  )"));
 
-  // Acquire WakeLock.
-  EXPECT_EQ("DONE", EvalJs(rfh_a, "requestWakeLock()"));
   // 2) Navigate away.
-  shell()->LoadURL(embedded_test_server()->GetURL("b.com", "/title1.html"));
-
-  // The page uses WakeLock so it should be deleted.
-  deleted.WaitUntilDeleted();
+  shell()->LoadURL(https_server()->GetURL("b.com", "/title1.html"));
+  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
+  EXPECT_TRUE(rfh_a->IsInBackForwardCache());
 
   // 3) Go back to the page with WakeLock.
   web_contents()->GetController().GoBack();
   EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
-  ExpectNotRestored(
-      {BackForwardCacheMetrics::NotRestoredReason::kBlocklistedFeatures},
-      FROM_HERE);
-  ExpectBlocklistedFeature(
-      blink::scheduler::WebSchedulerTrackedFeature::kWakeLock, FROM_HERE);
+  EXPECT_EQ(current_frame_host(), rfh_a);
 
-  // Release WakeLock.
-  EXPECT_EQ("DONE", EvalJs(current_frame_host(), "releaseWakeLock()"));
+  // This time the page is restored from cache because WakeLock is released.
+  ExpectOutcome(BackForwardCacheMetrics::HistoryNavigationOutcome::kRestored,
+                FROM_HERE);
+}
 
-  // 4) Navigate away.
-  web_contents()->GetController().GoBack();
+IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
+                       DoesNotCacheIfAnyWakeLockHeld) {
+  ASSERT_TRUE(CreateHttpsServer()->Start());
+
+  // 1) Navigate to a page with WakeLock usage.
+  GURL url(https_server()->GetURL("/back_forward_cache/empty.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), url));
   EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
+  RenderFrameHostImpl* rfh_a = current_frame_host();
+  // Acquire and release WakeLock.
+  EXPECT_EQ("DONE", EvalJs(rfh_a, R"(
+    new Promise(async resolve => {
+      try {
+         const lock1 = await navigator.wakeLock.request('screen');
+         const lock2 = await navigator.wakeLock.request('screen');
+         await lock1.release();
+         resolve('DONE');
+      } catch (error) {
+         resolve('error: request failed');
+      }
+    });
+  )"));
 
-  EXPECT_TRUE(rfh_a->IsInBackForwardCache());
+  // 2) Navigate away.
+  shell()->LoadURL(https_server()->GetURL("b.com", "/title1.html"));
+  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
 
-  // 5) Go back to the page with WakeLock.
+  // 3) Go back to the page with WakeLock.
   web_contents()->GetController().GoBack();
   EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
-  EXPECT_EQ(current_frame_host(), rfh_a);
-  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
 
-  // This time the page is restored from cache because WakeLock is released.
-  ExpectOutcome(BackForwardCacheMetrics::HistoryNavigationOutcome::kRestored,
-                FROM_HERE);
+  ExpectNotRestored(
+      {BackForwardCacheMetrics::NotRestoredReason::kBlocklistedFeatures},
+      FROM_HERE);
+  ExpectBlocklistedFeature(
+      blink::scheduler::WebSchedulerTrackedFeature::kWakeLock, FROM_HERE);
 }
 
 IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,

content/test/data/back_forward_cache/page_with_wakelock.html

-<html>
-  <title>wakelock</title>
-</html>
-<script>
-let lock;
-
-async function requestWakeLock() {
-  try {
-    lock = await navigator.wakeLock.request('screen');
-    return 'DONE';
-  } catch (error) {
-    return `${error}`;
-  }
-}
-
-async function releaseWakeLock() {
-  await lock.release();
-  return 'DONE';
-}
-
-</script>

third_party/blink/renderer/modules/wake_lock/wake_lock.cc

@@ -18,7 +18,6 @@
 #include "third_party/blink/renderer/modules/wake_lock/wake_lock_type.h"
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
 #include "third_party/blink/renderer/platform/instrumentation/use_counter.h"
-#include "third_party/blink/renderer/platform/scheduler/public/frame_or_worker_scheduler.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
@@ -34,11 +33,7 @@ WakeLock::WakeLock(LocalDOMWindow& window)
       managers_{
           MakeGarbageCollected<WakeLockManager>(&window, WakeLockType::kScreen),
           MakeGarbageCollected<WakeLockManager>(&window,
-                                                WakeLockType::kSystem)} {
-  window.GetScheduler()->RegisterStickyFeature(
-      SchedulingPolicy::Feature::kWakeLock,
-      {SchedulingPolicy::RecordMetricsForBackForwardCache()});
-}
+                                                WakeLockType::kSystem)} {}
 
 WakeLock::WakeLock(DedicatedWorkerGlobalScope& worker_scope)
     : ExecutionContextLifecycleObserver(&worker_scope),

third_party/blink/renderer/modules/wake_lock/wake_lock_manager.cc

@@ -51,6 +51,11 @@ void WakeLockManager::AcquireWakeLock(ScriptPromiseResolver* resolver) {
     wake_lock_.set_disconnect_handler(WTF::Bind(
         &WakeLockManager::OnWakeLockConnectionError, WrapWeakPersistent(this)));
     wake_lock_->RequestWakeLock();
+
+    feature_handle_for_scheduler_ =
+        execution_context_->GetScheduler()->RegisterFeature(
+            SchedulingPolicy::Feature::kWakeLock,
+            {SchedulingPolicy::RecordMetricsForBackForwardCache()});
   }
   // https://w3c.github.io/screen-wake-lock/#the-request-method
   // 5.2. Let lock be a new WakeLockSentinel object with its type attribute set
@@ -86,6 +91,9 @@ void WakeLockManager::UnregisterSentinel(WakeLockSentinel* sentinel) {
     // 5.2. If success is true and type is "screen" run the following:
     // 5.2.1. Reset the platform-specific inactivity timer after which the
     //        screen is actually turned off.
+
+    // Make the page bfcache-eligible if there is no WakeLock held.
+    feature_handle_for_scheduler_.reset();
   }
 }
 

third_party/blink/renderer/modules/wake_lock/wake_lock_manager.h

@@ -12,6 +12,7 @@
 #include "third_party/blink/renderer/platform/heap/handle.h"
 #include "third_party/blink/renderer/platform/mojo/heap_mojo_remote.h"
 #include "third_party/blink/renderer/platform/mojo/heap_mojo_wrapper_mode.h"
+#include "third_party/blink/renderer/platform/scheduler/public/frame_or_worker_scheduler.h"
 
 namespace blink {
 
@@ -51,6 +52,11 @@ class MODULES_EXPORT WakeLockManager final
   // ExecutionContext from which we will connect to |wake_lock_service_|.
   Member<ExecutionContext> execution_context_;
 
+  // Do not put a page into BackForwardCache if a page has acquired WakeLock.
+  // The page becomes cache-able when all locks are released.
+  FrameOrWorkerScheduler::SchedulingAffectingFeatureHandle
+      feature_handle_for_scheduler_;
+
   FRIEND_TEST_ALL_PREFIXES(WakeLockManagerTest, AcquireWakeLock);
   FRIEND_TEST_ALL_PREFIXES(WakeLockManagerTest, ReleaseAllWakeLocks);
   FRIEND_TEST_ALL_PREFIXES(WakeLockManagerTest, ReleaseOneWakeLock);

third_party/blink/renderer/platform/scheduler/common/scheduling_policy.cc

@@ -29,6 +29,7 @@ bool SchedulingPolicy::IsFeatureSticky(SchedulingPolicy::Feature feature) {
     case Feature::kPortal:
     case Feature::kSpeechRecognizer:
     case Feature::kSpeechSynthesis:
+    case Feature::kWakeLock:
       return false;
     case Feature::kMainResourceHasCacheControlNoStore:
     case Feature::kMainResourceHasCacheControlNoCache:
@@ -50,7 +51,6 @@ bool SchedulingPolicy::IsFeatureSticky(SchedulingPolicy::Feature feature) {
     case Feature::kRequestedBackForwardCacheBlockedSensors:
     case Feature::kRequestedBackgroundWorkPermission:
     case Feature::kWebLocks:
-    case Feature::kWakeLock:
     case Feature::kRequestedStorageAccessGrant:
     case Feature::kWebNfc:
     case Feature::kWebFileSystem: