Bug 1584007 - check ClientSource existence r=asuth

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

dom/clients/manager/ClientHandleOpParent.cpp

@@ -28,8 +28,16 @@ void ClientHandleOpParent::Init(ClientOpConstructorArgs&& aArgs) {
   handle->EnsureSource()
       ->Then(
           GetCurrentSerialEventTarget(), __func__,
-          [this, args = std::move(aArgs)](ClientSourceParent* source) mutable {
+          [this, handle, args = std::move(aArgs)](bool) mutable {
             mSourcePromiseRequestHolder.Complete();
+
+            auto source = handle->GetSource();
+            if (!source) {
+              CopyableErrorResult rv;
+              rv.ThrowAbortError("Client has been destroyed");
+              Unused << PClientHandleOpParent::Send__delete__(this, rv);
+              return;
+            }
             RefPtr<ClientOpPromise> p;
 
             // ClientPostMessageArgs can contain PBlob actors.  This means we

dom/clients/manager/ClientHandleParent.cpp

@@ -68,9 +68,13 @@ void ClientHandleParent::Init(const IPCClientInfo& aClientInfo) {
   mService->FindSource(aClientInfo.id(), aClientInfo.principalInfo())
       ->Then(
           GetCurrentSerialEventTarget(), __func__,
-          [this](ClientSourceParent* aSource) {
+          [this](bool) {
             mSourcePromiseRequestHolder.Complete();
-            FoundSource(aSource);
+            ClientSourceParent* source =
+                mService->FindExistingSource(mClientId, mPrincipalInfo);
+            if (source) {
+              FoundSource(source);
+            }
           },
           [this](const CopyableErrorResult&) {
             mSourcePromiseRequestHolder.Complete();
@@ -105,7 +109,7 @@ void ClientHandleParent::FoundSource(ClientSourceParent* aSource) {
 
   mSource = aSource;
   mSource->AttachHandle(this);
-  mSourcePromiseHolder.ResolveIfExists(aSource, __func__);
+  mSourcePromiseHolder.ResolveIfExists(true, __func__);
 }
 
 }  // namespace mozilla::dom

dom/clients/manager/ClientHandleParent.h

@@ -14,8 +14,7 @@ namespace dom {
 class ClientManagerService;
 class ClientSourceParent;
 
-typedef MozPromise<ClientSourceParent*, CopyableErrorResult,
-                   /* IsExclusive = */ false>
+typedef MozPromise<bool, CopyableErrorResult, /* IsExclusive = */ false>
     SourcePromise;
 
 class ClientHandleParent final : public PClientHandleParent {
@@ -54,6 +53,7 @@ class ClientHandleParent final : public PClientHandleParent {
 
   void FoundSource(ClientSourceParent* aSource);
 
+  // Should be called only once EnsureSource() has resolved. May return nullptr.
   ClientSourceParent* GetSource() const;
 
   RefPtr<SourcePromise> EnsureSource();

dom/clients/manager/ClientManagerService.cpp

@@ -266,7 +266,7 @@ bool ClientManagerService::AddSource(ClientSourceParent* aSource) {
       return false;
     }
 
-    placeHolder.ResolvePromiseIfExists(aSource);
+    placeHolder.ResolvePromiseIfExists();
     *entry = AsVariant(aSource);
     return true;
   }
@@ -363,7 +363,7 @@ RefPtr<SourcePromise> ClientManagerService::FindSource(
     return SourcePromise::CreateAndReject(rv, __func__);
   }
 
-  return SourcePromise::CreateAndResolve(source, __func__);
+  return SourcePromise::CreateAndResolve(true, __func__);
 }
 
 void ClientManagerService::AddManager(ClientManagerParent* aManager) {

dom/clients/manager/ClientManagerService.h

@@ -59,9 +59,8 @@ class ClientManagerService final {
       return mPromiseHolder.Ensure(__func__);
     }
 
-    void ResolvePromiseIfExists(ClientSourceParent* aSource) {
-      MOZ_ASSERT(aSource);
-      mPromiseHolder.ResolveIfExists(aSource, __func__);
+    void ResolvePromiseIfExists() {
+      mPromiseHolder.ResolveIfExists(true, __func__);
     }
 
     void RejectPromiseIfExists(const CopyableErrorResult& aRv) {
@@ -101,11 +100,6 @@ class ClientManagerService final {
   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();
 
@@ -123,9 +117,18 @@ class ClientManagerService final {
   // no longer exists.
   void ForgetFutureSource(const IPCClientInfo& aClientInfo);
 
+  // Returns a promise that resolves if/when the ClientSourceParent exists and
+  // rejects if/when it's known that the ClientSourceParent will never exist or
+  // if it's frozen. Note that the ClientSourceParent may not exist anymore
+  // by the time promise callbacks run.
   RefPtr<SourcePromise> FindSource(
       const nsID& aID, const mozilla::ipc::PrincipalInfo& aPrincipalInfo);
 
+  // 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;
+
   void AddManager(ClientManagerParent* aManager);
 
   void RemoveManager(ClientManagerParent* aManager);