Bug 1842289 - Part 2: Grab WorkerRef outside LockManager constructor r=dom-worker-reviewers,webidl,smaug

Differential Revision: https://phabricator.services.mozilla.com/D183048

dom/base/Navigator.cpp

@@ -2258,7 +2258,7 @@ webgpu::Instance* Navigator::Gpu() {
 
 dom::LockManager* Navigator::Locks() {
   if (!mLocks) {
-    mLocks = new dom::LockManager(GetWindow()->AsGlobal());
+    mLocks = dom::LockManager::Create(*GetWindow()->AsGlobal());
   }
   return mLocks;
 }

dom/locks/LockManager.cpp

@@ -36,32 +36,45 @@ JSObject* LockManager::WrapObject(JSContext* aCx,
 LockManager::LockManager(nsIGlobalObject* aGlobal) : mOwner(aGlobal) {
   Maybe<ClientInfo> clientInfo = aGlobal->GetClientInfo();
   if (!clientInfo) {
+    // Pass the nonworking object and let request()/query() throw.
     return;
   }
 
   nsCOMPtr<nsIPrincipal> principal =
       clientInfo->GetPrincipal().unwrapOr(nullptr);
   if (!principal || !principal->GetIsContentPrincipal()) {
+    // Same, the methods will throw instead of the constructor.
     return;
   }
 
   mozilla::ipc::PBackgroundChild* backgroundActor =
       mozilla::ipc::BackgroundChild::GetOrCreateForCurrentThread();
   mActor = new locks::LockManagerChild(aGlobal);
-  backgroundActor->SendPLockManagerConstructor(mActor, WrapNotNull(principal),
-                                               clientInfo->Id());
+
+  if (!backgroundActor->SendPLockManagerConstructor(
+          mActor, WrapNotNull(principal), clientInfo->Id())) {
+    // Failed to construct the actor. Pass the nonworking object and let the
+    // methods throw.
+    mActor = nullptr;
+    return;
+  }
+}
+
+already_AddRefed<LockManager> LockManager::Create(nsIGlobalObject& aGlobal) {
+  RefPtr<LockManager> manager = new LockManager(&aGlobal);
 
   if (!NS_IsMainThread()) {
-    mWorkerRef = WeakWorkerRef::Create(GetCurrentThreadWorkerPrivate(),
-                                       [self = RefPtr(this)]() {
-                                         // Others may grab a strong reference
-                                         // and block immediate destruction.
-                                         // Shutdown early as we don't have to
-                                         // wait for them.
-                                         self->Shutdown();
-                                         self->mWorkerRef = nullptr;
-                                       });
+    // Grabbing WorkerRef may fail and that will cause the methods throw later.
+    manager->mWorkerRef =
+        WeakWorkerRef::Create(GetCurrentThreadWorkerPrivate(), [manager]() {
+          // Others may grab a strong reference and block immediate destruction.
+          // Shutdown early as we don't have to wait for them.
+          manager->Shutdown();
+          manager->mWorkerRef = nullptr;
+        });
   }
+
+  return manager.forget();
 }
 
 static bool ValidateRequestArguments(const nsAString& name,
@@ -140,7 +153,7 @@ already_AddRefed<Promise> LockManager::Request(const nsAString& aName,
   if (!allowed) {
     // Step 4: If origin is an opaque origin, then return a promise rejected
     // with a "SecurityError" DOMException.
-    // But per https://wicg.github.io/web-locks/#lock-managers this really means
+    // But per https://w3c.github.io/web-locks/#lock-managers this really means
     // whether it has storage access.
     aRv.ThrowSecurityError("request() is not allowed in this context");
     return nullptr;
@@ -152,6 +165,11 @@ already_AddRefed<Promise> LockManager::Request(const nsAString& aName,
     return nullptr;
   }
 
+  if (!NS_IsMainThread() && !mWorkerRef) {
+    aRv.ThrowInvalidStateError("request() is not allowed at this point");
+    return nullptr;
+  }
+
   if (!ValidateRequestArguments(aName, aOptions, aRv)) {
     return nullptr;
   }
@@ -183,6 +201,11 @@ already_AddRefed<Promise> LockManager::Query(ErrorResult& aRv) {
     return nullptr;
   }
 
+  if (!NS_IsMainThread() && !mWorkerRef) {
+    aRv.ThrowInvalidStateError("query() is not allowed at this point");
+    return nullptr;
+  }
+
   RefPtr<Promise> promise = Promise::Create(mOwner, aRv);
   if (aRv.Failed()) {
     return nullptr;

dom/locks/LockManager.h

@@ -36,8 +36,12 @@ class LockManager final : public nsISupports, public nsWrapperCache {
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_WRAPPERCACHE_CLASS(LockManager)
 
+ private:
   explicit LockManager(nsIGlobalObject* aGlobal);
 
+ public:
+  static already_AddRefed<LockManager> Create(nsIGlobalObject& aGlobal);
+
   nsIGlobalObject* GetParentObject() const { return mOwner; }
 
   JSObject* WrapObject(JSContext* aCx,

dom/webidl/Lock.webidl

@@ -1,3 +1,12 @@
+/* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ * The origin of this IDL file is
+ * https://w3c.github.io/web-locks/
+ */
+
 [SecureContext, Exposed=(Window,Worker), Pref="dom.weblocks.enabled"]
 interface Lock {
   readonly attribute DOMString name;

dom/webidl/LockManager.webidl

@@ -1,3 +1,12 @@
+/* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ * The origin of this IDL file is
+ * https://w3c.github.io/web-locks/
+ */
+
 [SecureContext, Exposed=(Window,Worker), Pref="dom.weblocks.enabled"]
 interface LockManager {
   [NewObject]

dom/webidl/Navigator.webidl

@@ -338,7 +338,7 @@ partial interface Navigator {
   readonly attribute MediaSession mediaSession;
 };
 
-// https://wicg.github.io/web-locks/#navigator-mixins
+// https://w3c.github.io/web-locks/#navigator-mixins
 [SecureContext]
 interface mixin NavigatorLocks {
   [Pref="dom.weblocks.enabled"]

dom/webidl/WorkerNavigator.webidl

@@ -28,7 +28,7 @@ partial interface WorkerNavigator {
   readonly attribute MediaCapabilities mediaCapabilities;
 };
 
-// https://wicg.github.io/web-locks/#navigator-mixins
+// https://w3c.github.io/web-locks/#navigator-mixins
 WorkerNavigator includes NavigatorLocks;
 
 // https://gpuweb.github.io/gpuweb/#navigator-gpu

dom/workers/WorkerNavigator.cpp

@@ -284,7 +284,7 @@ dom::LockManager* WorkerNavigator::Locks() {
     nsIGlobalObject* global = workerPrivate->GlobalScope();
     MOZ_ASSERT(global);
 
-    mLocks = new dom::LockManager(global);
+    mLocks = dom::LockManager::Create(*global);
   }
   return mLocks;
 }