Bug 1584007 - let ClientHandleParents wait on FutureClientSourceParents r=dom-workers-and-storage-reviewers,mattwoodrow,asuth

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

dom/clients/manager/ClientHandleParent.cpp

@@ -27,13 +27,13 @@ void ClientHandleParent::ActorDestroy(ActorDestroyReason aReason) {
     mSource->DetachHandle(this);
     mSource = nullptr;
   } else {
-    mService->StopWaitingForSource(this, mClientId);
-  }
+    if (!mSourcePromiseHolder.IsEmpty()) {
+      CopyableErrorResult rv;
+      rv.ThrowAbortError("Client aborted");
+      mSourcePromiseHolder.Reject(rv, __func__);
+    }
 
-  if (mSourcePromise) {
-    CopyableErrorResult rv;
-    rv.ThrowAbortError("Client aborted");
-    mSourcePromise->Reject(rv, __func__);
+    mSourcePromiseRequestHolder.DisconnectIfExists();
   }
 }
 
@@ -63,13 +63,20 @@ ClientHandleParent::~ClientHandleParent() { MOZ_DIAGNOSTIC_ASSERT(!mSource); }
 void ClientHandleParent::Init(const IPCClientInfo& aClientInfo) {
   mClientId = aClientInfo.id();
   mPrincipalInfo = aClientInfo.principalInfo();
-  mSource = mService->FindSource(aClientInfo.id(), aClientInfo.principalInfo());
-  if (!mSource) {
-    mService->WaitForSource(this, aClientInfo.id());
-    return;
-  }
 
-  mSource->AttachHandle(this);
+  // Callbacks are disconnected in ActorDestroy, so capturing `this` is safe.
+  mService->FindSource(aClientInfo.id(), aClientInfo.principalInfo())
+      ->Then(
+          GetCurrentSerialEventTarget(), __func__,
+          [this](ClientSourceParent* aSource) {
+            mSourcePromiseRequestHolder.Complete();
+            FoundSource(aSource);
+          },
+          [this](const CopyableErrorResult&) {
+            mSourcePromiseRequestHolder.Complete();
+            Unused << Send__delete__(this);
+          })
+      ->Track(mSourcePromiseRequestHolder);
 }
 
 ClientSourceParent* ClientHandleParent::GetSource() const { return mSource; }
@@ -79,20 +86,18 @@ RefPtr<SourcePromise> ClientHandleParent::EnsureSource() {
     return SourcePromise::CreateAndResolve(mSource, __func__);
   }
 
-  if (!mSourcePromise) {
-    mSourcePromise = new SourcePromise::Private(__func__);
-  }
-  return mSourcePromise;
+  return mSourcePromiseHolder.Ensure(__func__);
 }
 
 void ClientHandleParent::FoundSource(ClientSourceParent* aSource) {
+  MOZ_ASSERT(aSource);
   MOZ_ASSERT(aSource->Info().Id() == mClientId);
   if (!ClientMatchPrincipalInfo(aSource->Info().PrincipalInfo(),
                                 mPrincipalInfo)) {
-    if (mSourcePromise) {
+    if (mSourcePromiseHolder.IsEmpty()) {
       CopyableErrorResult rv;
       rv.ThrowAbortError("Client aborted");
-      mSourcePromise->Reject(rv, __func__);
+      mSourcePromiseHolder.Reject(rv, __func__);
     }
     Unused << Send__delete__(this);
     return;
@@ -100,9 +105,7 @@ void ClientHandleParent::FoundSource(ClientSourceParent* aSource) {
 
   mSource = aSource;
   mSource->AttachHandle(this);
-  if (mSourcePromise) {
-    mSourcePromise->Resolve(aSource, __func__);
-  }
+  mSourcePromiseHolder.ResolveIfExists(aSource, __func__);
 }
 
 }  // namespace mozilla::dom

dom/clients/manager/ClientHandleParent.h

@@ -20,16 +20,18 @@ typedef MozPromise<ClientSourceParent*, CopyableErrorResult,
 
 class ClientHandleParent final : public PClientHandleParent {
   RefPtr<ClientManagerService> mService;
+
+  // mSource and mSourcePromiseHolder are mutually exclusive.
   ClientSourceParent* mSource;
 
+  // Operations will wait on this promise while mSource is null.
+  MozPromiseHolder<SourcePromise> mSourcePromiseHolder;
+
+  MozPromiseRequestHolder<SourcePromise> mSourcePromiseRequestHolder;
+
   nsID mClientId;
   PrincipalInfo mPrincipalInfo;
 
-  // A promise for HandleOps that want to access our ClientSourceParent.
-  // Resolved once FoundSource is called and we have a ClientSourceParent
-  // available.
-  RefPtr<SourcePromise::Private> mSourcePromise;
-
   // PClientHandleParent interface
   mozilla::ipc::IPCResult RecvTeardown() override;
 

dom/clients/manager/ClientManagerService.cpp

@@ -294,40 +294,31 @@ bool ClientManagerService::RemoveSource(ClientSourceParent* aSource) {
   return true;
 }
 
-ClientSourceParent* ClientManagerService::FindSource(
+RefPtr<SourcePromise> ClientManagerService::FindSource(
     const nsID& aID, const PrincipalInfo& aPrincipalInfo) {
   AssertIsOnBackgroundThread();
 
   auto entry = mSourceTable.Lookup(aID);
   if (!entry) {
-    return nullptr;
+    CopyableErrorResult rv;
+    rv.ThrowInvalidStateError("Unknown client.");
+    return SourcePromise::CreateAndReject(rv, __func__);
   }
 
   if (entry.Data().is<FutureClientSourceParent>()) {
-    return nullptr;
+    return entry.Data().as<FutureClientSourceParent>().Promise();
   }
 
   ClientSourceParent* source = entry.Data().as<ClientSourceParent*>();
   if (source->IsFrozen() ||
       NS_WARN_IF(!ClientMatchPrincipalInfo(source->Info().PrincipalInfo(),
                                            aPrincipalInfo))) {
-    return nullptr;
+    CopyableErrorResult rv;
+    rv.ThrowInvalidStateError("Unknown client.");
+    return SourcePromise::CreateAndReject(rv, __func__);
   }
 
-  return source;
-}
-
-void ClientManagerService::WaitForSource(ClientHandleParent* aHandle,
-                                         const nsID& aID) {
-  auto& entry = mPendingHandles.LookupOrInsert(aID);
-  entry.AppendElement(aHandle);
-}
-
-void ClientManagerService::StopWaitingForSource(ClientHandleParent* aHandle,
-                                                const nsID& aID) {
-  if (auto entry = mPendingHandles.Lookup(aID)) {
-    entry->RemoveElement(aHandle);
-  }
+  return SourcePromise::CreateAndResolve(source, __func__);
 }
 
 void ClientManagerService::AddManager(ClientManagerParent* aManager) {

dom/clients/manager/ClientManagerService.h

@@ -86,10 +86,6 @@ class ClientManagerService final {
   // optimize for insertion, removal, and lookup by UUID.
   nsTHashMap<nsIDHashKey, SourceTableEntry> mSourceTable;
 
-  // The set of handles waiting for their corresponding ClientSourceParent
-  // to be created.
-  nsTHashMap<nsIDHashKey, nsTArray<ClientHandleParent*>> mPendingHandles;
-
   nsTArray<ClientManagerParent*> mManagerList;
 
   bool mShutdown;
@@ -119,15 +115,9 @@ class ClientManagerService final {
 
   bool RemoveSource(ClientSourceParent* aSource);
 
-  ClientSourceParent* FindSource(
+  RefPtr<SourcePromise> FindSource(
       const nsID& aID, const mozilla::ipc::PrincipalInfo& aPrincipalInfo);
 
-  // Called when a ClientHandle is created before the corresponding
-  // ClientSource. Will call FoundSource on the ClientHandleParent when it
-  // becomes available.
-  void WaitForSource(ClientHandleParent* aHandle, const nsID& aID);
-  void StopWaitingForSource(ClientHandleParent* aHandle, const nsID& aID);
-
   void AddManager(ClientManagerParent* aManager);
 
   void RemoveManager(ClientManagerParent* aManager);