From fa2bc5a5d6e2da9eec949d4e884a3707f0119219 Mon Sep 17 00:00:00 2001
From: Andrea Marchesini <amarchesini@mozilla.com>
Date: Mon, 18 Jul 2016 09:12:40 +0200
Subject: [PATCH] Bug 1282026 - Add assertions in the DTOR of WorkerHolder -
 part 1 - WorkerHolder of Script loading, r=bkelly

---
 dom/workers/ScriptLoader.cpp | 39 +++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp
index 1f173f23d07b..aebc14b8c4f9 100644
--- a/dom/workers/ScriptLoader.cpp
+++ b/dom/workers/ScriptLoader.cpp
@@ -541,10 +541,12 @@ private:
 
 NS_IMPL_ISUPPORTS(LoaderListener, nsIStreamLoaderObserver, nsIRequestObserver)
 
-class ScriptLoaderRunnable final : public WorkerHolder
-                                 , public nsIRunnable
+class ScriptLoaderHolder;
+
+class ScriptLoaderRunnable final : public nsIRunnable
 {
   friend class ScriptExecutorRunnable;
+  friend class ScriptLoaderHolder;
   friend class CachePromiseHandler;
   friend class CacheScriptLoader;
   friend class LoaderListener;
@@ -718,8 +720,8 @@ private:
     return NS_OK;
   }
 
-  virtual bool
-  Notify(Status aStatus) override
+  bool
+  Notify(Status aStatus)
   {
     mWorkerPrivate->AssertIsOnWorkerThread();
 
@@ -1039,7 +1041,7 @@ private:
     // Note that for data: url, where we allow it through the same-origin check
     // but then give it a different origin.
     aLoadInfo.mMutedErrorFlag.emplace(IsMainWorkerScript()
-                                        ? false 
+                                        ? false
                                         : !principal->Subsumes(channelPrincipal));
 
     // Make sure we're not seeing the result of a 404 or something by checking
@@ -1356,6 +1358,26 @@ private:
 
 NS_IMPL_ISUPPORTS(ScriptLoaderRunnable, nsIRunnable)
 
+class MOZ_STACK_CLASS ScriptLoaderHolder final : public WorkerHolder
+{
+  // Raw pointer because this holder object follows the mRunnable life-time.
+  ScriptLoaderRunnable* mRunnable;
+
+public:
+  explicit ScriptLoaderHolder(ScriptLoaderRunnable* aRunnable)
+    : mRunnable(aRunnable)
+  {
+    MOZ_ASSERT(aRunnable);
+  }
+
+  virtual bool
+  Notify(Status aStatus) override
+  {
+    mRunnable->Notify(aStatus);
+    return true;
+  }
+};
+
 NS_IMETHODIMP
 LoaderListener::OnStreamComplete(nsIStreamLoader* aLoader, nsISupports* aContext,
                                  nsresult aStatus, uint32_t aStringLen,
@@ -2008,7 +2030,6 @@ ScriptExecutorRunnable::ShutdownScriptLoader(JSContext* aCx,
     }
   }
 
-  mScriptLoader.ReleaseWorker();
   aWorkerPrivate->StopSyncLoop(mSyncLoopTarget, aResult);
 }
 
@@ -2060,15 +2081,15 @@ LoadAllScripts(WorkerPrivate* aWorkerPrivate,
 
   NS_ASSERTION(aLoadInfos.IsEmpty(), "Should have swapped!");
 
-  if (NS_WARN_IF(!loader->HoldWorker(aWorkerPrivate))) {
+  ScriptLoaderHolder workerHolder(loader);
+
+  if (NS_WARN_IF(!workerHolder.HoldWorker(aWorkerPrivate))) {
     aRv.Throw(NS_ERROR_FAILURE);
     return;
   }
 
   if (NS_FAILED(NS_DispatchToMainThread(loader))) {
     NS_ERROR("Failed to dispatch!");
-
-    loader->ReleaseWorker();
     aRv.Throw(NS_ERROR_FAILURE);
     return;
   }