Backed out 2 changesets (bug 1798493) for causing wpt failures on partitioned-web-locks.tentative.https.html CLOSED TREE

Backed out changeset c8c8ab2aa130 (bug 1798493)
Backed out changeset f65876226f15 (bug 1798493)

caps/ContentPrincipalInfoHashKey.h

@@ -0,0 +1,56 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* 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/. */
+
+#ifndef CAPS_PRINCIPALHASHKEY_H_
+#define CAPS_PRINCIPALHASHKEY_H_
+
+#include "mozilla/ipc/PBackgroundSharedTypes.h"
+#include "PLDHashTable.h"
+#include "nsHashKeys.h"
+#include "nsUnicharUtils.h"
+
+namespace mozilla {
+
+class ContentPrincipalInfoHashKey : public PLDHashEntryHdr {
+ public:
+  using KeyType = const ipc::ContentPrincipalInfo&;
+  using KeyTypePointer = const ipc::ContentPrincipalInfo*;
+
+  explicit ContentPrincipalInfoHashKey(KeyTypePointer aKey)
+      : mPrincipalInfo(*aKey) {
+    MOZ_COUNT_CTOR(ContentPrincipalInfoHashKey);
+  }
+  ContentPrincipalInfoHashKey(ContentPrincipalInfoHashKey&& aOther) noexcept
+      : mPrincipalInfo(aOther.mPrincipalInfo) {
+    MOZ_COUNT_CTOR(ContentPrincipalInfoHashKey);
+  }
+
+  MOZ_COUNTED_DTOR(ContentPrincipalInfoHashKey)
+
+  KeyType GetKey() const { return mPrincipalInfo; }
+
+  bool KeyEquals(KeyTypePointer aKey) const {
+    // Mocks BasePrincipal::FastEquals()
+    return mPrincipalInfo.originNoSuffix() == aKey->originNoSuffix() &&
+           mPrincipalInfo.attrs() == aKey->attrs();
+  }
+
+  static KeyTypePointer KeyToPointer(KeyType aKey) { return &aKey; }
+  static PLDHashNumber HashKey(KeyTypePointer aKey) {
+    nsAutoCString suffix;
+    aKey->attrs().CreateSuffix(suffix);
+    return HashGeneric(HashString(aKey->originNoSuffix()), HashString(suffix));
+  }
+
+  enum { ALLOW_MEMMOVE = true };
+
+ protected:
+  const ipc::ContentPrincipalInfo mPrincipalInfo;
+};
+
+}  // namespace mozilla
+
+#endif  // CAPS_PRINCIPALHASHKEY_H_

caps/PrincipalHashKey.h

@@ -25,7 +25,7 @@ class PrincipalHashKey : public PLDHashEntryHdr {
     MOZ_ASSERT(aKey);
     MOZ_COUNT_CTOR(PrincipalHashKey);
   }
-  PrincipalHashKey(PrincipalHashKey&& aKey) noexcept
+  PrincipalHashKey(PrincipalHashKey&& aKey)
       : mPrincipal(std::move(aKey.mPrincipal)) {
     MOZ_COUNT_CTOR(PrincipalHashKey);
   }
@@ -43,7 +43,7 @@ class PrincipalHashKey : public PLDHashEntryHdr {
     return aKey;
   }
   static PLDHashNumber HashKey(const nsIPrincipal* aKey) {
-    const auto* bp = BasePrincipal::Cast(aKey);
+    auto* bp = BasePrincipal::Cast(aKey);
     return HashGeneric(bp->GetOriginNoSuffixHash(), bp->GetOriginSuffixHash());
   }
 

caps/moz.build

@@ -31,6 +31,7 @@ EXPORTS += [
 EXPORTS.mozilla = [
     "BasePrincipal.h",
     "ContentPrincipal.h",
+    "ContentPrincipalInfoHashKey.h",
     "ExpandedPrincipal.h",
     "NullPrincipal.h",
     "OriginAttributes.h",

dom/clients/manager/ClientInfo.cpp

@@ -34,10 +34,9 @@ ClientInfo& ClientInfo::operator=(const ClientInfo& aRight) {
   return *this;
 }
 
-ClientInfo::ClientInfo(ClientInfo&& aRight) noexcept
-    : mData(std::move(aRight.mData)) {}
+ClientInfo::ClientInfo(ClientInfo&& aRight) : mData(std::move(aRight.mData)) {}
 
-ClientInfo& ClientInfo::operator=(ClientInfo&& aRight) noexcept {
+ClientInfo& ClientInfo::operator=(ClientInfo&& aRight) {
   mData.reset();
   mData = std::move(aRight.mData);
   return *this;
@@ -90,14 +89,14 @@ const IPCClientInfo& ClientInfo::ToIPC() const { return *mData; }
 bool ClientInfo::IsPrivateBrowsing() const {
   switch (PrincipalInfo().type()) {
     case PrincipalInfo::TContentPrincipalInfo: {
-      const auto& p = PrincipalInfo().get_ContentPrincipalInfo();
+      auto& p = PrincipalInfo().get_ContentPrincipalInfo();
       return p.attrs().mPrivateBrowsingId != 0;
     }
     case PrincipalInfo::TSystemPrincipalInfo: {
       return false;
     }
     case PrincipalInfo::TNullPrincipalInfo: {
-      const auto& p = PrincipalInfo().get_NullPrincipalInfo();
+      auto& p = PrincipalInfo().get_NullPrincipalInfo();
       return p.attrs().mPrivateBrowsingId != 0;
     }
     default: {
@@ -108,6 +107,7 @@ bool ClientInfo::IsPrivateBrowsing() const {
 }
 
 Result<nsCOMPtr<nsIPrincipal>, nsresult> ClientInfo::GetPrincipal() const {
+  MOZ_ASSERT(NS_IsMainThread());
   return PrincipalInfoToPrincipal(PrincipalInfo());
 }
 

dom/clients/manager/ClientInfo.h

@@ -45,9 +45,9 @@ class ClientInfo final {
 
   ClientInfo& operator=(const ClientInfo& aRight);
 
-  ClientInfo(ClientInfo&& aRight) noexcept;
+  ClientInfo(ClientInfo&& aRight);
 
-  ClientInfo& operator=(ClientInfo&& aRight) noexcept;
+  ClientInfo& operator=(ClientInfo&& aRight);
 
   explicit ClientInfo(const IPCClientInfo& aData);
 

dom/locks/LockManager.cpp

@@ -39,16 +39,18 @@ LockManager::LockManager(nsIGlobalObject* aGlobal) : mOwner(aGlobal) {
     return;
   }
 
-  nsCOMPtr<nsIPrincipal> principal =
-      clientInfo->GetPrincipal().unwrapOr(nullptr);
-  if (!principal || !principal->GetIsContentPrincipal()) {
+  const mozilla::ipc::PrincipalInfo& principalInfo =
+      clientInfo->PrincipalInfo();
+
+  if (principalInfo.type() !=
+      mozilla::ipc::PrincipalInfo::TContentPrincipalInfo) {
     return;
   }
 
   mozilla::ipc::PBackgroundChild* backgroundActor =
       mozilla::ipc::BackgroundChild::GetOrCreateForCurrentThread();
   mActor = new locks::LockManagerChild(aGlobal);
-  backgroundActor->SendPLockManagerConstructor(mActor, WrapNotNull(principal),
+  backgroundActor->SendPLockManagerConstructor(mActor, principalInfo,
                                                clientInfo->Id());
 
   if (!NS_IsMainThread()) {
@@ -131,13 +133,7 @@ already_AddRefed<Promise> LockManager::Request(const nsAString& aName,
     return nullptr;
   }
 
-  const StorageAccess access = mOwner->GetStorageAccess();
-  bool allowed =
-      access > StorageAccess::eDeny ||
-      (StaticPrefs::
-           privacy_partition_always_partition_third_party_non_cookie_storage() &&
-       ShouldPartitionStorage(access));
-  if (!allowed) {
+  if (mOwner->GetStorageAccess() <= StorageAccess::eDeny) {
     // 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

dom/locks/LockManagerParent.cpp

@@ -7,6 +7,7 @@
 #include "LockManagerParent.h"
 #include "LockRequestParent.h"
 
+#include "mozilla/ContentPrincipalInfoHashKey.h"
 #include "mozilla/PrincipalHashKey.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/StaticPtr.h"
@@ -17,24 +18,27 @@
 
 namespace mozilla::dom::locks {
 
-static StaticAutoPtr<nsTHashMap<PrincipalHashKey, WeakPtr<ManagedLocks>>>
+static StaticAutoPtr<
+    nsTHashMap<ContentPrincipalInfoHashKey, WeakPtr<ManagedLocks>>>
     sManagedLocksMap;
 
 using IPCResult = mozilla::ipc::IPCResult;
 
-LockManagerParent::LockManagerParent(NotNull<nsIPrincipal*> aPrincipal,
-                                     const nsID& aClientId)
-    : mClientId(NSID_TrimBracketsUTF16(aClientId)), mPrincipal(aPrincipal) {
+LockManagerParent::LockManagerParent(
+    const mozilla::ipc::ContentPrincipalInfo& aPrincipalInfo,
+    const nsID& aClientId)
+    : mClientId(NSID_TrimBracketsUTF16(aClientId)),
+      mPrincipalInfo(aPrincipalInfo) {
   if (!sManagedLocksMap) {
     sManagedLocksMap =
-        new nsTHashMap<PrincipalHashKey, WeakPtr<ManagedLocks>>();
+        new nsTHashMap<ContentPrincipalInfoHashKey, WeakPtr<ManagedLocks>>();
   } else {
-    mManagedLocks = sManagedLocksMap->Get(aPrincipal);
+    mManagedLocks = sManagedLocksMap->Get(aPrincipalInfo);
   }
 
   if (!mManagedLocks) {
     mManagedLocks = new ManagedLocks();
-    sManagedLocksMap->LookupOrInsert(aPrincipal, mManagedLocks);
+    sManagedLocksMap->LookupOrInsert(aPrincipalInfo, mManagedLocks);
   }
 }
 
@@ -75,8 +79,8 @@ void LockManagerParent::ActorDestroy(ActorDestroyReason aWhy) {
   mManagedLocks = nullptr;
   // We just decreased the refcount and potentially deleted it, so check whether
   // the weak pointer still points to anything and remove the entry if not.
-  if (!sManagedLocksMap->Get(mPrincipal)) {
-    sManagedLocksMap->Remove(mPrincipal);
+  if (!sManagedLocksMap->Get(mPrincipalInfo)) {
+    sManagedLocksMap->Remove(mPrincipalInfo);
   }
 }
 

dom/locks/LockManagerParent.h

@@ -32,7 +32,8 @@ class LockManagerParent final : public PLockManagerParent {
  public:
   NS_INLINE_DECL_REFCOUNTING(LockManagerParent)
 
-  LockManagerParent(NotNull<nsIPrincipal*> aPrincipal, const nsID& aClientId);
+  LockManagerParent(const mozilla::ipc::ContentPrincipalInfo& aPrincipalInfo,
+                    const nsID& aClientId);
 
   void ProcessRequestQueue(nsTArray<RefPtr<LockRequestParent>>& aQueue);
   bool IsGrantableRequest(const IPCLockRequest& aRequest);
@@ -53,7 +54,7 @@ class LockManagerParent final : public PLockManagerParent {
 
   RefPtr<ManagedLocks> mManagedLocks;
   nsString mClientId;
-  NotNull<nsCOMPtr<nsIPrincipal>> mPrincipal;
+  mozilla::ipc::ContentPrincipalInfo mPrincipalInfo;
 };
 
 }  // namespace mozilla::dom::locks

ipc/glue/BackgroundParentImpl.cpp

@@ -1474,9 +1474,9 @@ bool BackgroundParentImpl::DeallocPMediaTransportParent(
 }
 
 already_AddRefed<dom::locks::PLockManagerParent>
-BackgroundParentImpl::AllocPLockManagerParent(NotNull<nsIPrincipal*> aPrincipal,
-                                              const nsID& aClientId) {
-  return MakeAndAddRef<mozilla::dom::locks::LockManagerParent>(aPrincipal,
+BackgroundParentImpl::AllocPLockManagerParent(
+    const ContentPrincipalInfo& aPrincipalInfo, const nsID& aClientId) {
+  return MakeAndAddRef<mozilla::dom::locks::LockManagerParent>(aPrincipalInfo,
                                                                aClientId);
 }
 

ipc/glue/BackgroundParentImpl.h

@@ -408,7 +408,7 @@ class BackgroundParentImpl : public PBackgroundParent {
       PWebSocketConnectionParent* actor, const uint32_t& aListenerId) override;
 
   already_AddRefed<PLockManagerParent> AllocPLockManagerParent(
-      NotNull<nsIPrincipal*> aPrincipal, const nsID& aClientId) final;
+      const ContentPrincipalInfo& aPrincipalInfo, const nsID& aClientId) final;
 
   already_AddRefed<PFetchParent> AllocPFetchParent() override;
 };

ipc/glue/PBackground.ipdl

@@ -313,7 +313,7 @@ parent:
 
   async PWebSocketConnection(uint32_t aListenerId);
 
-  async PLockManager(nsIPrincipal aPrincipalInfo, nsID aClientId);
+  async PLockManager(ContentPrincipalInfo aPrincipalInfo, nsID aClientId);
 
   async PIPCClientCerts();
 

testing/web-platform/meta/web-locks/partitioned-web-locks.tentative.https.html.ini

@@ -1,6 +1,11 @@
-prefs: [privacy.partition.always_partition_third_party_non_cookie_storage:true]
-
 [partitioned-web-locks.tentative.https.html]
+  expected:
+    if os == "android": ERROR
+    TIMEOUT
+  [WebLocks of an iframe under a 3rd-party site are partitioned]
+    expected:
+      if os == "android": FAIL
+      TIMEOUT
 
   [WebLocks of a nested iframe with a cross-site ancestor are partitioned]
-    expected: FAIL
+    expected: NOTRUN

testing/web-platform/tests/web-locks/partitioned-web-locks.tentative.https.html

@@ -29,10 +29,10 @@ let next_lock_id = 1;
 // back to the pop-up.
 // Step 6 (pop-up): intercept the result message from the iframe and
 // send it to the top-frame.
-// Step 7 (top-frame): add cleanup hook.
-// Step 8 (top-frame): ensure that the same-site iframe's web-lock
+// Step 7 (top-frame): ensure that the same-site iframe's web-lock
 // request succeeds since it and the top-level site are successfully
 // partitioned and each can hold an exclusive lock.
+// Step 8 (top-frame): clean up.
 
 async function third_party_test(t) {
   let target_url = HTTPS_ORIGIN + '/web-locks/resources/iframe.html';
@@ -59,19 +59,19 @@ async function third_party_test(t) {
   const result = await new Promise(resolve => window.onmessage = resolve);
 
   // Step 7.
+  // When 3rd party storage partitioning is enabled, the iframe should be able
+  // to aquire a lock with the same name as one exclusively held by the opener
+  // of its top window, even when that opener has the same origin.
+  assert_equals(result.data.failed, undefined,
+      'The 1p iframe failed to acquire the lock');
+
+  // Step 8.
   t.add_cleanup(() => {
     w.close()
     for(let i = 1; i < next_lock_id; i++){
       held.get(i)();
     }
   });
-
-  // Step 8.
-  // When 3rd party storage partitioning is enabled, the iframe should be able
-  // to acquire a lock with the same name as one exclusively held by the opener
-  // of its top window, even when that opener has the same origin.
-  assert_equals(result.data.failed, undefined,
-      'The 1p iframe failed to acquire the lock');
 }
 
 promise_test(t => {
@@ -101,10 +101,10 @@ promise_test(t => {
 // child iframe and send it to the pop-up.
 // Nested Step 9 (pop-up): intercept the result message from the parent
 // iframe and send it to the top frame.
-// Nested Step 10 (top frame): add cleanup hook
-// Nested Step 11 (top frame): ensure that the same-site iframe's web-lock
+// Nested Step 10 (top frame): ensure that the same-site iframe's web-lock
 // request succeeds since it and the top-level are successfully
 // partitioned and each can hold an exclusive lock.
+// Nested Step 11 (top frame): clean up.
 
 // Map of lock_id => function that releases a lock.
 const held_2 = new Map();
@@ -145,19 +145,19 @@ async function nested_iframe_test(t) {
   const result = await new Promise(resolve => window.onmessage = resolve);
 
   // Nested Step 10.
+  // With third-party storage partitioning enabled, the same-site iframe
+  // should be able to acquire the lock as it has a cross-site ancestor
+  // and is partitioned separately from the top-level site.
+  assert_equals(result.data.failed, undefined,
+      'The 1p iframe failed to acquire the lock');
+
+  // Nested Step 11.
   t.add_cleanup(() => {
     w.close()
     for(let i = 1; i < next_lock_id_2; i++){
       held_2.get(i)();
     }
   });
-
-  // Nested Step 11.
-  // With third-party storage partitioning enabled, the same-site iframe
-  // should be able to acquire the lock as it has a cross-site ancestor
-  // and is partitioned separately from the top-level site.
-  assert_equals(result.data.failed, undefined,
-      'The 1p iframe failed to acquire the lock');
 }
 
 promise_test(t => {

toolkit/components/antitracking/test/browser/browser.ini

@@ -142,7 +142,6 @@ skip-if = debug # Bug 1700551
 [browser_partitionedLocalStorage.js]
 [browser_partitionedLocalStorage_events.js]
 support-files = localStorageEvents.html
-[browser_partitionedLockManager.js]
 [browser_workerPropagation.js]
 support-files = workerIframe.html
 [browser_cookieBetweenTabs.js]
@@ -206,7 +205,7 @@ support-files = browser_staticPartition_CORS_preflight.sjs
 [browser_staticPartition_HSTS.js]
 support-files = browser_staticPartition_HSTS.sjs
 [browser_staticPartition_saveAs.js]
-skip-if =
+skip-if = 
   os == "linux" && bits == 64 # Bug 1775746
   win10_2004 && debug # Bug 1775746
 support-files =

toolkit/components/antitracking/test/browser/browser_partitionedLockManager.js

@@ -1,24 +0,0 @@
-/* import-globals-from partitionedstorage_head.js */
-
-PartitionedStorageHelper.runTest(
-  "LockManager works in both first and third party contexts",
-  async (win3rdParty, win1stParty, allowed) => {
-    let locks = [];
-    ok(win1stParty.isSecureContext, "1st party is in a secure context");
-    ok(win3rdParty.isSecureContext, "3rd party is in a secure context");
-    await win1stParty.navigator.locks.request("foo", lock => {
-      locks.push(lock);
-      ok(true, "locks.request succeeded for 1st party");
-    });
-
-    await win3rdParty.navigator.locks.request("foo", lock => {
-      locks.push(lock);
-      ok(true, "locks.request succeeded for 3rd party");
-    });
-
-    is(locks.length, 2, "We should have granted 2 lock requests at this point");
-  },
-  /* cleanupFunction */ undefined,
-  /* extraPrefs */ undefined,
-  { runInSecureContext: true }
-);