Bug 1738438 - Throw InvalidStateError as early as possible on LockMan…

…ager r=smaug The added WPT test is copied from web-platform-tests/wpt#31509. Differential Revision: https://phabricator.services.mozilla.com/D130504
iaxat · Nov 8, 2021 · 1894623 · 1894623
1 parent 11fd113
commit 1894623
Show file tree

Hide file tree

Showing 2 changed files with 85 additions and 14 deletions.
diff --git a/dom/locks/LockManager.cpp b/dom/locks/LockManager.cpp
@@ -113,6 +113,13 @@ already_AddRefed<Promise> LockManager::Request(const nsAString& aName,
                                                const LockOptions& aOptions,
                                                LockGrantedCallback& aCallback,
                                                ErrorResult& aRv) {
+  if (!mActor) {
+    aRv.ThrowInvalidStateError(
+        "The document of the lock manager is not fully active or web locks "
+        "aren't enabled for the document.");
+    return nullptr;
+  }
+
   if (mOwner->GetStorageAccess() <= StorageAccess::eDeny) {
     // Step 4: If origin is an opaque origin, then return a promise rejected
     // with a "SecurityError" DOMException.
@@ -125,14 +132,6 @@ already_AddRefed<Promise> LockManager::Request(const nsAString& aName,
     return nullptr;
   }
 
-  if (!mActor) {
-    // TODO: https://github.com/WICG/web-locks/issues/78
-    aRv.ThrowInvalidStateError(
-        "The document of the lock manager is not fully active or web locks "
-        "aren't enabled for the document.");
-    return nullptr;
-  }
-
   RefPtr<Promise> promise = Promise::Create(mOwner, aRv);
   if (aRv.Failed()) {
     return nullptr;
@@ -143,19 +142,18 @@ already_AddRefed<Promise> LockManager::Request(const nsAString& aName,
 };
 
 already_AddRefed<Promise> LockManager::Query(ErrorResult& aRv) {
-  if (mOwner->GetStorageAccess() <= StorageAccess::eDeny) {
-    aRv.ThrowSecurityError("query() is not allowed in this context");
-    return nullptr;
-  }
-
   if (!mActor) {
-    // TODO: https://github.com/WICG/web-locks/issues/78
     aRv.ThrowInvalidStateError(
         "The document of the lock manager is not fully active or web locks "
         "aren't enabled for the document.");
     return nullptr;
   }
 
+  if (mOwner->GetStorageAccess() <= StorageAccess::eDeny) {
+    aRv.ThrowSecurityError("query() is not allowed in this context");
+    return nullptr;
+  }
+
   RefPtr<Promise> promise = Promise::Create(mOwner, aRv);
   if (aRv.Failed()) {
     return nullptr;

diff --git a/testing/web-platform/tests/web-locks/non-fully-active.tentative.https.html b/testing/web-platform/tests/web-locks/non-fully-active.tentative.https.html
@@ -0,0 +1,73 @@
+<!DOCTYPE html>
+<meta charset=utf-8>
+<title>Web Locks API: Non-fully-active documents</title>
+<link rel=help href="https://wicg.github.io/web-locks/">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="resources/helpers.js"></script>
+
+<div></div>
+
+<script>
+  function createNonFullyActiveIframe(src) {
+    const iframe = document.createElement("iframe");
+    document.body.appendChild(iframe);
+    const { navigator, DOMException, postMessage } = iframe.contentWindow;
+    iframe.remove();
+    return { iframe, navigator, DOMException, postMessage };
+  }
+
+  promise_test(async t => {
+    const { navigator, DOMException } = createNonFullyActiveIframe();
+    const p = navigator.locks.request("foo", t.unreached_func());
+    await promise_rejects_dom(t, "InvalidStateError", DOMException, p, "Request should explicitly fail");
+  }, "request() on non-fully-active document must fail");
+
+  promise_test(async t => {
+    const { navigator, DOMException } = createNonFullyActiveIframe();
+    const p = navigator.locks.query();
+    await promise_rejects_dom(t, "InvalidStateError", DOMException, p, "Query should explicitly fail");
+  }, "query() on a non-fully-active document must fail");
+
+  promise_test(async t => {
+    const { navigator, DOMException, postMessage } = createNonFullyActiveIframe();
+
+    const p = navigator.locks.request("-", t.unreached_func());
+    await promise_rejects_dom(t, "InvalidStateError", DOMException, p, "Request should explicitly fail");
+  }, "request()'s fully-active check happens earlier than name validation");
+
+  promise_test(async t => {
+    const { iframe, navigator, DOMException } = createNonFullyActiveIframe();
+    document.body.append(iframe);
+    t.add_cleanup(() => iframe.remove());
+
+    // Appending should create a new browsing context with a new Navigator object
+    // https://html.spec.whatwg.org/multipage/iframe-embed-object.html#the-iframe-element:insert-an-element-into-a-document
+    // https://html.spec.whatwg.org/multipage/system-state.html#the-navigator-object:associated-navigator
+    assert_not_equals(navigator, iframe.contentWindow.navigator, "Navigator object changes");
+    assert_not_equals(navigator.locks, iframe.contentWindow.navigator.locks, "LockManager object changes");
+
+    const p = navigator.locks.request("foo", t.unreached_func());
+    await promise_rejects_dom(t, "InvalidStateError", DOMException, p, "Request on the previous LockManager still must fail");
+  }, "Reactivated iframe must not reuse the previous LockManager");
+
+  promise_test(async t => {
+    const iframe = document.createElement("iframe");
+    document.body.appendChild(iframe);
+    const worker = new iframe.contentWindow.Worker("resources/worker.js");
+
+    const name = uniqueName(t);
+    await postToWorkerAndWait(worker, { op: 'request', name });
+
+    let query = await navigator.locks.query();
+    assert_equals(query.held.length, 1, "One lock is present");
+
+    iframe.remove();
+
+    const lock = await navigator.locks.request(name, lock => lock);
+    assert_equals(lock.name, name, "The following lock should be processed");
+
+    query = await navigator.locks.query();
+    assert_equals(query.held.length, 0, "No lock is present");
+  }, "Workers owned by an unloaded iframe must release their locks");
+</script>