wake lock: Tighten checks in WakeLockSentinel::HasPendingActivity()

We were previously only checking if a WakeLockSentinel had any registered
event listeners to decide whether it should be kept alive in
HasPendingActivity().

This is not enough, as if the sentinel has been released, either by
release() being called or because the WakeLock class ended up calling
WakeLockManager::ClearWakeLocks(), it should also be eligible for garbage
collection even if it still has one or more event listeners (as they will
never be called again anyway).

Bug: 1015788
Change-Id: I2fa328915e4e1d39344ef3ed5e56c985ef7ad451
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1868875
Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Auto-Submit: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707748}

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

@@ -51,7 +51,10 @@ void WakeLockSentinel::Trace(blink::Visitor* visitor) {
 }
 
 bool WakeLockSentinel::HasPendingActivity() const {
-  return HasEventListeners();
+  // This WakeLockSentinel needs to remain alive as long as:
+  // 1. DoRelease() has not not been called yet AND
+  // 2. It has at least one event listener.
+  return manager_ && HasEventListeners();
 }
 
 void WakeLockSentinel::ContextDestroyed(ExecutionContext*) {

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

@@ -103,4 +103,40 @@ TEST(WakeLockSentinelTest, ContextDestruction) {
   EXPECT_FALSE(sentinel->HasPendingActivity());
 }
 
+TEST(WakeLockSentinelTest, HasPendingActivityConditions) {
+  MockWakeLockService wake_lock_service;
+  WakeLockTestingContext context(&wake_lock_service);
+
+  auto* manager = MakeGarbageCollected<WakeLockManager>(context.GetDocument(),
+                                                        WakeLockType::kScreen);
+  auto* resolver =
+      MakeGarbageCollected<ScriptPromiseResolver>(context.GetScriptState());
+  ScriptPromise promise = resolver->Promise();
+  manager->AcquireWakeLock(resolver);
+  context.WaitForPromiseFulfillment(promise);
+  auto* sentinel =
+      ScriptPromiseUtils::GetPromiseResolutionAsWakeLockSentinel(promise);
+  ASSERT_TRUE(sentinel);
+
+  // A new WakeLockSentinel was created and it can be GC'ed.
+  EXPECT_FALSE(sentinel->HasPendingActivity());
+
+  base::RunLoop run_loop;
+  auto* event_listener =
+      MakeGarbageCollected<SyncEventListener>(run_loop.QuitClosure());
+  sentinel->addEventListener(event_type_names::kRelease, event_listener);
+
+  // The sentinel cannot be GC'ed, it has an event listener and it has not been
+  // released.
+  EXPECT_TRUE(sentinel->HasPendingActivity());
+
+  // An event such as a page visibility change will eventually call this method.
+  manager->ClearWakeLocks();
+  run_loop.Run();
+
+  // The sentinel can be GC'ed even though it still has an event listener, as
+  // it has already been released.
+  EXPECT_FALSE(sentinel->HasPendingActivity());
+}
+
 }  // namespace blink