Bug 1584007 - let ClientHandleParents wait on FutureClientSourceParen…

…ts r=dom-workers-and-storage-reviewers,mattwoodrow,asuth Differential Revision: https://phabricator.services.mozilla.com/D66154 --HG-- extra : moz-landing-system : lando
dothq · Mar 24, 2020 · 7dbfd81 · 7dbfd81
1 parent 21f57af
commit 7dbfd81
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 56 deletions.
diff --git a/dom/clients/manager/ClientHandleParent.cpp b/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(
+          GetCurrentThreadSerialEventTarget(), __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,30 +86,25 @@ 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->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;
   }
 
   mSource = aSource;
   mSource->AttachHandle(this);
-  if (mSourcePromise) {
-    mSourcePromise->Resolve(aSource, __func__);
-  }
+  mSourcePromiseHolder.ResolveIfExists(aSource, __func__);
 }
 
 }  // namespace dom

diff --git a/dom/clients/manager/ClientHandleParent.h b/dom/clients/manager/ClientHandleParent.h
@@ -6,6 +6,8 @@
 #ifndef _mozilla_dom_ClientHandleParent_h
 #define _mozilla_dom_ClientHandleParent_h
 
+#include "mozilla/MozPromise.h"
+#include "mozilla/RefPtr.h"
 #include "mozilla/dom/PClientHandleParent.h"
 #include "mozilla/ErrorResult.h"
 
@@ -21,16 +23,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;
 

diff --git a/dom/clients/manager/ClientManagerService.cpp b/dom/clients/manager/ClientManagerService.cpp
@@ -293,45 +293,35 @@ bool ClientManagerService::RemoveSource(ClientSourceParent* aSource) {
   return true;
 }
 
-ClientSourceParent* ClientManagerService::FindSource(
+RefPtr<SourcePromise> ClientManagerService::FindSource(
     const nsID& aID, const PrincipalInfo& aPrincipalInfo) {
   AssertIsOnBackgroundThread();
 
   auto entryPtr = mSourceTable.lookup(aID);
 
   if (!entryPtr) {
-    return nullptr;
+    CopyableErrorResult rv;
+    rv.ThrowInvalidStateError("Unknown client.");
+    return SourcePromise::CreateAndReject(rv, __func__);
   }
 
   SourceTableEntry& entry = entryPtr->value();
 
   if (entry.is<FutureClientSourceParent>()) {
-    return nullptr;
+    return entry.as<FutureClientSourceParent>().Promise();
   }
 
   ClientSourceParent* source = entry.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.GetOrInsert(aID);
-  entry.AppendElement(aHandle);
-}
-
-void ClientManagerService::StopWaitingForSource(ClientHandleParent* aHandle,
-                                                const nsID& aID) {
-  auto* entry = mPendingHandles.GetValue(aID);
-  if (entry) {
-    entry->RemoveElement(aHandle);
-  }
+  return SourcePromise::CreateAndResolve(source, __func__);
 }
 
 void ClientManagerService::AddManager(ClientManagerParent* aManager) {

diff --git a/dom/clients/manager/ClientManagerService.h b/dom/clients/manager/ClientManagerService.h
@@ -86,10 +86,6 @@ class ClientManagerService final {
   // optimize for insertion, removal, and lookup by UUID.
   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;
@@ -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);