Commit b91e3f2d authored by Raphael Kubo da Costa's avatar Raphael Kubo da Costa Committed by Commit Bot

wake lock: Do not dispatch events if the context has been destroyed.

Both WakeLock and WakeLockSentinel inherit from ContextLifecycleObserver,
but by design a WakeLock is always created before a WakeLockSentinel, and
thus its ContextDestroyed() implementation is called before
WakeLockSentinel's.

This can result in the following call chain:
1. WakeLock::ContextDestroyed()
2. WakeLockManager::ClearWakeLocks()
3. WakeLockSentinel::DoRelease()

if a WakeLockSentinel has an event handler, DoRelease() will try to dispatch
an event after the context has been destroyed, as
WakeLockSentinel::ContextDestroyed(), which calls RemoveAllEventListeners(),
has not been called yet.

We now check if we have a usable context before dispatching events.

WakeLockSentinelTest.ContextDestruction() has been augmented and now tests
the entire flow (i.e. starting with a WakeLock object) rather than just
context destruction when a WakeLockSentinel is present.

Bug: 1023477
Change-Id: I3211d60d624456b950a884771c3fba79a4f8fa8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1917359
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: default avatarReilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715406}
parent d6e53cfd
...@@ -73,6 +73,7 @@ class MODULES_EXPORT WakeLock final : public ScriptWrappable, ...@@ -73,6 +73,7 @@ class MODULES_EXPORT WakeLock final : public ScriptWrappable,
// record per responsible document [...] internal slots. // record per responsible document [...] internal slots.
Member<WakeLockManager> managers_[kWakeLockTypeCount]; Member<WakeLockManager> managers_[kWakeLockTypeCount];
FRIEND_TEST_ALL_PREFIXES(WakeLockSentinelTest, ContextDestruction);
FRIEND_TEST_ALL_PREFIXES(WakeLockTest, RequestWakeLockGranted); FRIEND_TEST_ALL_PREFIXES(WakeLockTest, RequestWakeLockGranted);
FRIEND_TEST_ALL_PREFIXES(WakeLockTest, RequestWakeLockDenied); FRIEND_TEST_ALL_PREFIXES(WakeLockTest, RequestWakeLockDenied);
FRIEND_TEST_ALL_PREFIXES(WakeLockTest, LossOfDocumentActivity); FRIEND_TEST_ALL_PREFIXES(WakeLockTest, LossOfDocumentActivity);
......
...@@ -85,6 +85,11 @@ void WakeLockSentinel::DoRelease() { ...@@ -85,6 +85,11 @@ void WakeLockSentinel::DoRelease() {
manager_->UnregisterSentinel(this); manager_->UnregisterSentinel(this);
manager_.Clear(); manager_.Clear();
// This function may be called on ExecutionContext destruction. Events should
// not be dispatched in this case.
if (!GetExecutionContext() || GetExecutionContext()->IsContextDestroyed())
return;
// 6. Queue a task to fire an event named "release" at lock. // 6. Queue a task to fire an event named "release" at lock.
DispatchEvent(*Event::Create(event_type_names::kRelease)); DispatchEvent(*Event::Create(event_type_names::kRelease));
} }
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "third_party/blink/renderer/core/dom/document.h" #include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/dom/events/native_event_listener.h" #include "third_party/blink/renderer/core/dom/events/native_event_listener.h"
#include "third_party/blink/renderer/modules/event_target_modules_names.h" #include "third_party/blink/renderer/modules/event_target_modules_names.h"
#include "third_party/blink/renderer/modules/wake_lock/wake_lock.h"
#include "third_party/blink/renderer/modules/wake_lock/wake_lock_manager.h" #include "third_party/blink/renderer/modules/wake_lock/wake_lock_manager.h"
#include "third_party/blink/renderer/modules/wake_lock/wake_lock_test_utils.h" #include "third_party/blink/renderer/modules/wake_lock/wake_lock_test_utils.h"
#include "third_party/blink/renderer/platform/heap/handle.h" #include "third_party/blink/renderer/platform/heap/handle.h"
...@@ -85,10 +86,24 @@ TEST(WakeLockSentinelTest, ContextDestruction) { ...@@ -85,10 +86,24 @@ TEST(WakeLockSentinelTest, ContextDestruction) {
MockWakeLockService wake_lock_service; MockWakeLockService wake_lock_service;
WakeLockTestingContext context(&wake_lock_service); WakeLockTestingContext context(&wake_lock_service);
auto* sentinel = MakeGarbageCollected<WakeLockSentinel>( context.GetPermissionService().SetPermissionResponse(
context.GetScriptState(), WakeLockType::kScreen, WakeLockType::kScreen, mojom::blink::PermissionStatus::GRANTED);
MakeGarbageCollected<WakeLockManager>(context.GetDocument(),
WakeLockType::kScreen)); auto* screen_resolver =
MakeGarbageCollected<ScriptPromiseResolver>(context.GetScriptState());
ScriptPromise screen_promise = screen_resolver->Promise();
auto* wake_lock = MakeGarbageCollected<WakeLock>(*context.GetDocument());
wake_lock->DoRequest(WakeLockType::kScreen, screen_resolver);
WakeLockManager* manager =
wake_lock->managers_[static_cast<size_t>(WakeLockType::kScreen)];
ASSERT_TRUE(manager);
context.WaitForPromiseFulfillment(screen_promise);
auto* sentinel = ScriptPromiseUtils::GetPromiseResolutionAsWakeLockSentinel(
screen_promise);
ASSERT_TRUE(sentinel);
auto* event_listener = auto* event_listener =
MakeGarbageCollected<SyncEventListener>(WTF::Bind([]() { MakeGarbageCollected<SyncEventListener>(WTF::Bind([]() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment