Bug 1457157 P1 Use DOMMozPromiseRequestHolder in the clients API binding objects. r=baku
authorBen Kelly <ben@wanderview.com>
Wed, 02 May 2018 06:29:26 -0700
changeset 472764 d667f861e2d11efa1fa2e6fd135bddc5b1900e03
parent 472763 3f9dd911845cf4979d94ed4239f5e0168582ad58
child 472765 c86679a9bf8360dd95790c63768a70913467c46a
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1457157
milestone61.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 1457157 P1 Use DOMMozPromiseRequestHolder in the clients API binding objects. r=baku
dom/clients/api/Client.cpp
dom/clients/api/ClientDOMUtil.h
dom/clients/api/Clients.cpp
--- a/dom/clients/api/Client.cpp
+++ b/dom/clients/api/Client.cpp
@@ -6,16 +6,17 @@
 
 #include "Client.h"
 
 #include "ClientDOMUtil.h"
 #include "mozilla/dom/ClientHandle.h"
 #include "mozilla/dom/ClientIPCTypes.h"
 #include "mozilla/dom/ClientManager.h"
 #include "mozilla/dom/ClientState.h"
+#include "mozilla/dom/DOMMozPromiseRequestHolder.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/WorkerPrivate.h"
 #include "mozilla/dom/WorkerScope.h"
 #include "nsIGlobalObject.h"
 
 namespace mozilla {
 namespace dom {
 
@@ -167,40 +168,33 @@ Client::Focus(ErrorResult& aRv)
     return outerPromise.forget();
   }
 
   if (!workerPrivate->GlobalScope()->WindowInteractionAllowed()) {
     outerPromise->MaybeReject(NS_ERROR_DOM_INVALID_ACCESS_ERR);
     return outerPromise.forget();
   }
 
-  // Hold the worker thread alive while we perform the async operation
-  // and also avoid invoking callbacks if the worker starts shutting
-  // down.
-  RefPtr<WorkerHolderToken> token =
-    WorkerHolderToken::Create(GetCurrentThreadWorkerPrivate(), Closing);
+  EnsureHandle();
 
-  EnsureHandle();
-  RefPtr<ClientStatePromise> innerPromise = mHandle->Focus();
-  RefPtr<Client> self = this;
+  IPCClientInfo ipcClientInfo(mData->info());
+  auto holder = MakeRefPtr<DOMMozPromiseRequestHolder<ClientStatePromise>>(mGlobal);
 
-  innerPromise->Then(mGlobal->EventTargetFor(TaskCategory::Other), __func__,
-    [self, token, outerPromise] (const ClientState& aResult) {
-      if (token->IsShuttingDown()) {
-        return;
-      }
-      RefPtr<Client> newClient =
-        new Client(self->mGlobal, ClientInfoAndState(self->mData->info(), aResult.ToIPC()));
+  mHandle->Focus()->Then(mGlobal->EventTargetFor(TaskCategory::Other), __func__,
+    [ipcClientInfo, holder, outerPromise] (const ClientState& aResult) {
+      holder->Complete();
+      NS_ENSURE_TRUE_VOID(holder->GetParentObject());
+      RefPtr<Client> newClient = new Client(holder->GetParentObject(),
+                                            ClientInfoAndState(ipcClientInfo,
+                                                               aResult.ToIPC()));
       outerPromise->MaybeResolve(newClient);
-    }, [self, token, outerPromise] (nsresult aResult) {
-      if (token->IsShuttingDown()) {
-        return;
-      }
+    }, [holder, outerPromise] (nsresult aResult) {
+      holder->Complete();
       outerPromise->MaybeReject(aResult);
-    });
+    })->Track(*holder);
 
   return outerPromise.forget();
 }
 
 already_AddRefed<Promise>
 Client::Navigate(const nsAString& aURL, ErrorResult& aRv)
 {
   MOZ_ASSERT(!NS_IsMainThread());
@@ -213,18 +207,17 @@ Client::Navigate(const nsAString& aURL, 
   if (aRv.Failed()) {
     return outerPromise.forget();
   }
 
   ClientNavigateArgs args(mData->info(), NS_ConvertUTF16toUTF8(aURL),
                           workerPrivate->GetLocationInfo().mHref);
   RefPtr<Client> self = this;
 
-  StartClientManagerOp(&ClientManager::Navigate, args,
-    mGlobal->EventTargetFor(TaskCategory::Other),
+  StartClientManagerOp(&ClientManager::Navigate, args, mGlobal,
     [self, outerPromise] (const ClientOpResult& aResult) {
       if (aResult.type() != ClientOpResult::TClientInfoAndState) {
         outerPromise->MaybeResolve(JS::NullHandleValue);
         return;
       }
       RefPtr<Client> newClient =
         new Client(self->mGlobal, aResult.get_ClientInfoAndState());
       outerPromise->MaybeResolve(newClient);
--- a/dom/clients/api/ClientDOMUtil.h
+++ b/dom/clients/api/ClientDOMUtil.h
@@ -3,49 +3,47 @@
 /* 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 _mozilla_dom_ClientDOMUtil_h
 #define _mozilla_dom_ClientDOMUtil_h
 
 #include "mozilla/dom/ClientIPCTypes.h"
 #include "mozilla/dom/ClientOpPromise.h"
+#include "mozilla/dom/DOMMozPromiseRequestHolder.h"
 #include "mozilla/dom/WorkerHolderToken.h"
 #include "mozilla/dom/WorkerPrivate.h"
 
 class nsIGlobalObject;
 
 namespace mozilla {
 namespace dom {
 
 // Utility method to properly execute a ClientManager operation.  It
 // will properly hold a worker thread alive and avoid executing callbacks
 // if the thread is shutting down.
 template<typename Func, typename Arg, typename Resolve, typename Reject>
 void
-StartClientManagerOp(Func aFunc, const Arg& aArg, nsISerialEventTarget* aTarget,
+StartClientManagerOp(Func aFunc, const Arg& aArg, nsIGlobalObject* aGlobal,
                      Resolve aResolve, Reject aReject)
 {
-  RefPtr<WorkerHolderToken> token;
-  if (!NS_IsMainThread()) {
-    token = WorkerHolderToken::Create(GetCurrentThreadWorkerPrivate(),
-                                      WorkerStatus::Closing);
-  }
+  MOZ_DIAGNOSTIC_ASSERT(aGlobal);
+
+  nsCOMPtr<nsISerialEventTarget> target =
+    aGlobal->EventTargetFor(TaskCategory::Other);
+
+  auto holder = MakeRefPtr<DOMMozPromiseRequestHolder<ClientOpPromise>>(aGlobal);
 
-  RefPtr<ClientOpPromise> promise = aFunc(aArg, aTarget);
-  promise->Then(aTarget, __func__,
-    [aResolve, token](const ClientOpResult& aResult) {
-      if (token && token->IsShuttingDown()) {
-        return;
-      }
+  aFunc(aArg, target)->Then(
+    target, __func__,
+    [aResolve, holder](const ClientOpResult& aResult) {
+      holder->Complete();
       aResolve(aResult);
-    }, [aReject, token](nsresult aResult) {
-      if (token && token->IsShuttingDown()) {
-        return;
-      }
+    }, [aReject, holder](nsresult aResult) {
+      holder->Complete();
       aReject(aResult);
-    });
+    })->Track(*holder);
 }
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // _mozilla_dom_ClientDOMUtil_h
--- a/dom/clients/api/Clients.cpp
+++ b/dom/clients/api/Clients.cpp
@@ -79,37 +79,41 @@ Clients::Get(const nsAString& aClientID,
   const PrincipalInfo& principalInfo = workerPrivate->GetPrincipalInfo();
   nsCOMPtr<nsISerialEventTarget> target =
     mGlobal->EventTargetFor(TaskCategory::Other);
 
   RefPtr<ClientOpPromise> innerPromise =
     ClientManager::GetInfoAndState(ClientGetInfoAndStateArgs(id, principalInfo),
                                    target);
 
-  nsCOMPtr<nsIGlobalObject> global = mGlobal;
   nsCString scope = workerPrivate->ServiceWorkerScope();
+  auto holder = MakeRefPtr<DOMMozPromiseRequestHolder<ClientOpPromise>>(mGlobal);
 
   innerPromise->Then(target, __func__,
-    [outerPromise, global, scope] (const ClientOpResult& aResult) {
-      RefPtr<Client> client = new Client(global, aResult.get_ClientInfoAndState());
+    [outerPromise, holder, scope] (const ClientOpResult& aResult) {
+      holder->Complete();
+      NS_ENSURE_TRUE_VOID(holder->GetParentObject());
+      RefPtr<Client> client = new Client(holder->GetParentObject(),
+                                         aResult.get_ClientInfoAndState());
       if (client->GetStorageAccess() == nsContentUtils::StorageAccess::eAllow) {
         outerPromise->MaybeResolve(Move(client));
         return;
       }
       nsCOMPtr<nsIRunnable> r =
-        NS_NewRunnableFunction("Clients::MatchAll() storage denied",
+        NS_NewRunnableFunction("Clients::Get() storage denied",
         [scope] {
           ServiceWorkerManager::LocalizeAndReportToAllClients(
             scope, "ServiceWorkerGetClientStorageError", nsTArray<nsString>());
         });
       SystemGroup::Dispatch(TaskCategory::Other, r.forget());
       outerPromise->MaybeResolveWithUndefined();
-    }, [outerPromise] (nsresult aResult) {
+    }, [outerPromise, holder] (nsresult aResult) {
+      holder->Complete();
       outerPromise->MaybeResolveWithUndefined();
-    });
+    })->Track(*holder);
 
   return outerPromise.forget();
 }
 
 namespace {
 
 class MatchAllComparator final
 {
@@ -161,18 +165,17 @@ Clients::MatchAll(const ClientQueryOptio
   }
 
   nsCOMPtr<nsIGlobalObject> global = mGlobal;
   nsCString scope = workerPrivate->ServiceWorkerScope();
 
   ClientMatchAllArgs args(workerPrivate->GetServiceWorkerDescriptor().ToIPC(),
                           aOptions.mType,
                           aOptions.mIncludeUncontrolled);
-  StartClientManagerOp(&ClientManager::MatchAll, args,
-    mGlobal->EventTargetFor(TaskCategory::Other),
+  StartClientManagerOp(&ClientManager::MatchAll, args, mGlobal,
     [outerPromise, global, scope] (const ClientOpResult& aResult) {
       nsTArray<RefPtr<Client>> clientList;
       bool storageDenied = false;
       for (const ClientInfoAndState& value : aResult.get_ClientList().values()) {
         RefPtr<Client> client = new Client(global, value);
         if (client->GetStorageAccess() != nsContentUtils::StorageAccess::eAllow) {
           storageDenied = true;
           continue;
@@ -224,18 +227,17 @@ Clients::OpenWindow(const nsAString& aUR
 
   const PrincipalInfo& principalInfo = workerPrivate->GetPrincipalInfo();
   nsCString baseURL = workerPrivate->GetLocationInfo().mHref;
   ClientOpenWindowArgs args(principalInfo, NS_ConvertUTF16toUTF8(aURL),
                             baseURL);
 
   nsCOMPtr<nsIGlobalObject> global = mGlobal;
 
-  StartClientManagerOp(&ClientManager::OpenWindow, args,
-    mGlobal->EventTargetFor(TaskCategory::Other),
+  StartClientManagerOp(&ClientManager::OpenWindow, args, mGlobal,
     [outerPromise, global] (const ClientOpResult& aResult) {
       if (aResult.type() != ClientOpResult::TClientInfoAndState) {
         outerPromise->MaybeResolve(JS::NullHandleValue);
         return;
       }
       RefPtr<Client> client =
         new Client(global, aResult.get_ClientInfoAndState());
       outerPromise->MaybeResolve(client);
@@ -266,18 +268,18 @@ Clients::Claim(ErrorResult& aRv)
     workerPrivate->GetServiceWorkerDescriptor();
 
   if (serviceWorker.State() != ServiceWorkerState::Activating &&
       serviceWorker.State() != ServiceWorkerState::Activated) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return outerPromise.forget();
   }
 
-  StartClientManagerOp(&ClientManager::Claim, ClientClaimArgs(serviceWorker.ToIPC()),
-    mGlobal->EventTargetFor(TaskCategory::Other),
+  StartClientManagerOp(
+    &ClientManager::Claim, ClientClaimArgs(serviceWorker.ToIPC()), mGlobal,
     [outerPromise] (const ClientOpResult& aResult) {
       outerPromise->MaybeResolveWithUndefined();
     }, [outerPromise] (nsresult aResult) {
       outerPromise->MaybeReject(aResult);
     });
 
   return outerPromise.forget();
 }