Bug 1584007 - let ClientManagerService store FutureClientSourceParent r=dom-workers-and-storage-reviewers,mattwoodrow,asuth
authorPerry Jiang <perry@mozilla.com>
Tue, 24 Mar 2020 15:16:50 +0000
changeset 520465 ac2765b4b4ec5d18ea22edefbdc3095893855f9c
parent 520464 24deb3d467831a8e74ee8addf3c53119ca019dd3
child 520466 00eda7b39a1316ec75ac112535a02ac80ab99307
push id37251
push usermalexandru@mozilla.com
push dateThu, 26 Mar 2020 09:33:08 +0000
treeherdermozilla-central@3e5a7430c8d7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdom-workers-and-storage-reviewers, mattwoodrow, asuth
bugs1584007
milestone76.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1584007 - let ClientManagerService store FutureClientSourceParent r=dom-workers-and-storage-reviewers,mattwoodrow,asuth The changes only make it possible for ClientManagerService to store FutureClientSourceParents, but it will not actually store them until following changesets. Differential Revision: https://phabricator.services.mozilla.com/D66145
dom/clients/manager/ClientManagerService.cpp
dom/clients/manager/ClientManagerService.h
--- a/dom/clients/manager/ClientManagerService.cpp
+++ b/dom/clients/manager/ClientManagerService.cpp
@@ -6,16 +6,17 @@
 
 #include "ClientManagerService.h"
 
 #include "ClientManagerParent.h"
 #include "ClientNavigateOpParent.h"
 #include "ClientOpenWindowUtils.h"
 #include "ClientPrincipalUtils.h"
 #include "ClientSourceParent.h"
+#include "mozilla/dom/ClientIPCTypes.h"
 #include "mozilla/dom/ContentParent.h"
 #include "mozilla/dom/ServiceWorkerManager.h"
 #include "mozilla/dom/ServiceWorkerUtils.h"
 #include "mozilla/ipc/BackgroundParent.h"
 #include "mozilla/ipc/PBackgroundSharedTypes.h"
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/MozPromise.h"
 #include "mozilla/SystemGroup.h"
@@ -135,17 +136,17 @@ ClientManagerService::ClientManagerServi
         svc->Shutdown();
       }
     });
   }
 }
 
 ClientManagerService::~ClientManagerService() {
   AssertIsOnBackgroundThread();
-  MOZ_DIAGNOSTIC_ASSERT(mSourceTable.Count() == 0);
+  MOZ_DIAGNOSTIC_ASSERT(mSourceTable.count() == 0);
   MOZ_DIAGNOSTIC_ASSERT(mManagerList.IsEmpty());
 
   MOZ_DIAGNOSTIC_ASSERT(sClientManagerServiceInstance == this);
   sClientManagerServiceInstance = nullptr;
 }
 
 void ClientManagerService::Shutdown() {
   AssertIsOnBackgroundThread();
@@ -161,16 +162,48 @@ void ClientManagerService::Shutdown() {
   // Begin destroying our various manager actors which will in turn destroy
   // all source, handle, and operation actors.
   AutoTArray<ClientManagerParent*, 16> list(mManagerList);
   for (auto actor : list) {
     Unused << PClientManagerParent::Send__delete__(actor);
   }
 }
 
+ClientSourceParent* ClientManagerService::MaybeUnwrapAsExistingSource(
+    const SourceTableEntry& aEntry) const {
+  AssertIsOnBackgroundThread();
+
+  if (aEntry.is<FutureClientSourceParent>()) {
+    return nullptr;
+  }
+
+  return aEntry.as<ClientSourceParent*>();
+}
+
+ClientSourceParent* ClientManagerService::FindExistingSource(
+    const nsID& aID, const PrincipalInfo& aPrincipalInfo) const {
+  AssertIsOnBackgroundThread();
+
+  auto entryPtr = mSourceTable.lookup(aID);
+
+  if (!entryPtr) {
+    return nullptr;
+  }
+
+  ClientSourceParent* source = MaybeUnwrapAsExistingSource(entryPtr->value());
+
+  if (!source || source->IsFrozen() ||
+      NS_WARN_IF(!ClientMatchPrincipalInfo(source->Info().PrincipalInfo(),
+                                           aPrincipalInfo))) {
+    return nullptr;
+  }
+
+  return source;
+}
+
 // static
 already_AddRefed<ClientManagerService>
 ClientManagerService::GetOrCreateInstance() {
   AssertIsOnBackgroundThread();
 
   if (!sClientManagerServiceInstance) {
     sClientManagerServiceInstance = new ClientManagerService();
   }
@@ -186,64 +219,106 @@ already_AddRefed<ClientManagerService> C
   if (!sClientManagerServiceInstance) {
     return nullptr;
   }
 
   RefPtr<ClientManagerService> ref(sClientManagerServiceInstance);
   return ref.forget();
 }
 
+namespace {
+
+bool IsNullPrincipalInfo(const PrincipalInfo& aPrincipalInfo) {
+  return aPrincipalInfo.type() == PrincipalInfo::TNullPrincipalInfo;
+}
+
+bool AreBothNullPrincipals(const PrincipalInfo& aPrincipalInfo1,
+                           const PrincipalInfo& aPrincipalInfo2) {
+  return IsNullPrincipalInfo(aPrincipalInfo1) &&
+         IsNullPrincipalInfo(aPrincipalInfo2);
+}
+
+}  // anonymous namespace
+
 bool ClientManagerService::AddSource(ClientSourceParent* aSource) {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aSource);
-  auto entry = mSourceTable.LookupForAdd(aSource->Info().Id());
-  // Do not permit overwriting an existing ClientSource with the same
-  // UUID.  This would allow a spoofed ClientParentSource actor to
-  // intercept postMessage() intended for the real actor.
-  if (NS_WARN_IF(!!entry)) {
-    return false;
-  }
-  entry.OrInsert([&] { return aSource; });
+
+  const nsID& id = aSource->Info().Id();
+  auto entryPtr = mSourceTable.lookupForAdd(id);
+
+  if (entryPtr) {
+    SourceTableEntry& entry = entryPtr->value();
+
+    // Do not permit overwriting an existing ClientSource with the same
+    // UUID.  This would allow a spoofed ClientParentSource actor to
+    // intercept postMessage() intended for the real actor.
+    if (NS_WARN_IF(entry.is<ClientSourceParent*>())) {
+      return false;
+    }
+
+    FutureClientSourceParent& placeHolder =
+        entry.as<FutureClientSourceParent>();
 
-  // Now that we've been created, notify any handles that were
-  // waiting on us.
-  auto* handles = mPendingHandles.GetValue(aSource->Info().Id());
-  if (handles) {
-    for (auto handle : *handles) {
-      handle->FoundSource(aSource);
+    const PrincipalInfo& placeHolderPrincipalInfo = placeHolder.PrincipalInfo();
+    const PrincipalInfo& sourcePrincipalInfo = aSource->Info().PrincipalInfo();
+
+    // The placeholder FutureClientSourceParent's PrincipalInfo must match the
+    // real ClientSourceParent's PrincipalInfo. The only exception is if both
+    // are null principals (two null principals are considered unequal).
+    if (!AreBothNullPrincipals(placeHolderPrincipalInfo, sourcePrincipalInfo) &&
+        NS_WARN_IF(!ClientMatchPrincipalInfo(placeHolderPrincipalInfo,
+                                             sourcePrincipalInfo))) {
+      return false;
     }
+
+    placeHolder.ResolvePromiseIfExists(aSource);
+
+    entry = AsVariant(aSource);
+    return true;
   }
-  mPendingHandles.Remove(aSource->Info().Id());
-  return true;
+
+  return mSourceTable.add(entryPtr, id, AsVariant(aSource));
 }
 
 bool ClientManagerService::RemoveSource(ClientSourceParent* aSource) {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aSource);
-  auto entry = mSourceTable.Lookup(aSource->Info().Id());
-  if (NS_WARN_IF(!entry)) {
+
+  auto entryPtr = mSourceTable.lookup(aSource->Info().Id());
+
+  if (NS_WARN_IF(!entryPtr)) {
     return false;
   }
-  entry.Remove();
+
+  mSourceTable.remove(entryPtr);
   return true;
 }
 
 ClientSourceParent* ClientManagerService::FindSource(
     const nsID& aID, const PrincipalInfo& aPrincipalInfo) {
   AssertIsOnBackgroundThread();
 
-  auto entry = mSourceTable.Lookup(aID);
-  if (!entry) {
+  auto entryPtr = mSourceTable.lookup(aID);
+
+  if (!entryPtr) {
     return nullptr;
   }
 
-  ClientSourceParent* source = entry.Data();
+  SourceTableEntry& entry = entryPtr->value();
+
+  if (entry.is<FutureClientSourceParent>()) {
+    return nullptr;
+  }
+
+  ClientSourceParent* source = entry.as<ClientSourceParent*>();
+
   if (source->IsFrozen() ||
-      !ClientMatchPrincipalInfo(source->Info().PrincipalInfo(),
-                                aPrincipalInfo)) {
+      NS_WARN_IF(!ClientMatchPrincipalInfo(source->Info().PrincipalInfo(),
+                                           aPrincipalInfo))) {
     return nullptr;
   }
 
   return source;
 }
 
 void ClientManagerService::WaitForSource(ClientHandleParent* aHandle,
                                          const nsID& aID) {
@@ -276,17 +351,17 @@ void ClientManagerService::RemoveManager
   MOZ_DIAGNOSTIC_ASSERT(aManager);
   DebugOnly<bool> removed = mManagerList.RemoveElement(aManager);
   MOZ_ASSERT(removed);
 }
 
 RefPtr<ClientOpPromise> ClientManagerService::Navigate(
     const ClientNavigateArgs& aArgs) {
   ClientSourceParent* source =
-      FindSource(aArgs.target().id(), aArgs.target().principalInfo());
+      FindExistingSource(aArgs.target().id(), aArgs.target().principalInfo());
   if (!source) {
     CopyableErrorResult rv;
     rv.ThrowInvalidStateError("Unknown client");
     return ClientOpPromise::CreateAndReject(rv, __func__);
   }
 
   const IPCServiceWorkerDescriptor& serviceWorker = aArgs.serviceWorker();
 
@@ -402,21 +477,21 @@ RefPtr<ClientOpPromise> ClientManagerSer
     const ClientMatchAllArgs& aArgs) {
   AssertIsOnBackgroundThread();
 
   ServiceWorkerDescriptor swd(aArgs.serviceWorker());
   const PrincipalInfo& principalInfo = swd.PrincipalInfo();
 
   RefPtr<PromiseListHolder> promiseList = new PromiseListHolder();
 
-  for (auto iter = mSourceTable.Iter(); !iter.Done(); iter.Next()) {
-    ClientSourceParent* source = iter.UserData();
-    MOZ_DIAGNOSTIC_ASSERT(source);
+  for (auto iter = mSourceTable.iter(); !iter.done(); iter.next()) {
+    ClientSourceParent* source =
+        MaybeUnwrapAsExistingSource(iter.get().value());
 
-    if (source->IsFrozen() || !source->ExecutionReady()) {
+    if (!source || source->IsFrozen() || !source->ExecutionReady()) {
       continue;
     }
 
     if (aArgs.type() != ClientType::All &&
         source->Info().Type() != aArgs.type()) {
       continue;
     }
 
@@ -497,21 +572,21 @@ RefPtr<ClientOpPromise> ClientManagerSer
     const ClientClaimArgs& aArgs) {
   AssertIsOnBackgroundThread();
 
   const IPCServiceWorkerDescriptor& serviceWorker = aArgs.serviceWorker();
   const PrincipalInfo& principalInfo = serviceWorker.principalInfo();
 
   RefPtr<PromiseListHolder> promiseList = new PromiseListHolder();
 
-  for (auto iter = mSourceTable.Iter(); !iter.Done(); iter.Next()) {
-    ClientSourceParent* source = iter.UserData();
-    MOZ_DIAGNOSTIC_ASSERT(source);
+  for (auto iter = mSourceTable.iter(); !iter.done(); iter.next()) {
+    ClientSourceParent* source =
+        MaybeUnwrapAsExistingSource(iter.get().value());
 
-    if (source->IsFrozen()) {
+    if (!source || source->IsFrozen()) {
       continue;
     }
 
     if (!ClientMatchPrincipalInfo(source->Info().PrincipalInfo(),
                                   principalInfo)) {
       continue;
     }
 
@@ -543,33 +618,33 @@ RefPtr<ClientOpPromise> ClientManagerSer
   // Maybe finish the promise now in case we didn't find any matching clients.
   promiseList->MaybeFinish();
 
   return promiseList->GetResultPromise();
 }
 
 RefPtr<ClientOpPromise> ClientManagerService::GetInfoAndState(
     const ClientGetInfoAndStateArgs& aArgs) {
-  ClientSourceParent* source = FindSource(aArgs.id(), aArgs.principalInfo());
+  ClientSourceParent* source =
+      FindExistingSource(aArgs.id(), aArgs.principalInfo());
 
   if (!source) {
     CopyableErrorResult rv;
     rv.ThrowInvalidStateError("Unknown client");
     return ClientOpPromise::CreateAndReject(rv, __func__);
   }
 
   if (!source->ExecutionReady()) {
     RefPtr<ClientManagerService> self = this;
 
-    // rejection ultimately converted to `undefined` in Clients::Get
     return source->ExecutionReadyPromise()->Then(
         GetCurrentThreadSerialEventTarget(), __func__,
-        [self, aArgs]() -> RefPtr<ClientOpPromise> {
+        [self = std::move(self), aArgs]() {
           ClientSourceParent* source =
-              self->FindSource(aArgs.id(), aArgs.principalInfo());
+              self->FindExistingSource(aArgs.id(), aArgs.principalInfo());
 
           if (!source) {
             CopyableErrorResult rv;
             rv.ThrowInvalidStateError("Unknown client");
             return ClientOpPromise::CreateAndReject(rv, __func__);
           }
 
           return source->StartOp(aArgs);
@@ -585,17 +660,18 @@ RefPtr<ClientOpPromise> ClientManagerSer
                      [aArgs]() { return ClientOpenWindow(aArgs); });
 }
 
 bool ClientManagerService::HasWindow(
     const Maybe<ContentParentId>& aContentParentId,
     const PrincipalInfo& aPrincipalInfo, const nsID& aClientId) {
   AssertIsOnBackgroundThread();
 
-  ClientSourceParent* source = FindSource(aClientId, aPrincipalInfo);
+  ClientSourceParent* source = FindExistingSource(aClientId, aPrincipalInfo);
+
   if (!source) {
     return false;
   }
 
   if (!source->ExecutionReady()) {
     return false;
   }
 
--- a/dom/clients/manager/ClientManagerService.h
+++ b/dom/clients/manager/ClientManagerService.h
@@ -77,33 +77,43 @@ class ClientManagerService final {
       return HashBytes(&aLookup, sizeof(Lookup));
     }
 
     static bool match(const Key& aKey, const Lookup& aLookup) {
       return aKey.Equals(aLookup);
     }
   };
 
-  // Store the ClientSourceParent objects in a hash table.  We want to
+  // Store the possible ClientSourceParent objects in a hash table.  We want to
   // optimize for insertion, removal, and lookup by UUID.
-  nsDataHashtable<nsIDHashKey, ClientSourceParent*> mSourceTable;
+  HashMap<nsID, SourceTableEntry, nsIDHasher> mSourceTable;
 
   // The set of handles waiting for their corresponding ClientSourceParent
   // to be created.
   nsDataHashtable<nsIDHashKey, nsTArray<ClientHandleParent*>> mPendingHandles;
 
   nsTArray<ClientManagerParent*> mManagerList;
 
   bool mShutdown;
 
   ClientManagerService();
   ~ClientManagerService();
 
   void Shutdown();
 
+  // Returns nullptr if aEntry isn't a ClientSourceParent (i.e. it's a
+  // FutureClientSourceParent).
+  ClientSourceParent* MaybeUnwrapAsExistingSource(
+      const SourceTableEntry& aEntry) const;
+
+  // Returns nullptr if the ClientSourceParent doesn't exist yet (i.e. it's a
+  // FutureClientSourceParent or has already been destroyed) or is frozen.
+  ClientSourceParent* FindExistingSource(
+      const nsID& aID, const mozilla::ipc::PrincipalInfo& aPrincipalInfo) const;
+
  public:
   static already_AddRefed<ClientManagerService> GetOrCreateInstance();
 
   // Returns nullptr if the service is not already created.
   static already_AddRefed<ClientManagerService> GetInstance();
 
   bool AddSource(ClientSourceParent* aSource);