Bug 1282026 - Add assertions in the DTOR of WorkerHolder - part 1 - WorkerHolder of Script loading, r=bkelly
authorAndrea Marchesini <amarchesini@mozilla.com>
Mon, 18 Jul 2016 09:12:40 +0200
changeset 305285 0619d5ea9d35e6f0800c044acfb8fb47ca98d8d8
parent 305284 6ce73fbf84723c3b9c9c89ef920db441de1e6bf0
child 305286 51d8d12d035cd86ed15badaa5d4a304844987713
push id79539
push useramarchesini@mozilla.com
push dateMon, 18 Jul 2016 07:15:46 +0000
treeherdermozilla-inbound@ff9608965b49 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1282026
milestone50.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 1282026 - Add assertions in the DTOR of WorkerHolder - part 1 - WorkerHolder of Script loading, r=bkelly
dom/workers/ScriptLoader.cpp
--- a/dom/workers/ScriptLoader.cpp
+++ b/dom/workers/ScriptLoader.cpp
@@ -536,20 +536,22 @@ private:
   ~LoaderListener() {}
 
   RefPtr<ScriptLoaderRunnable> mRunnable;
   uint32_t mIndex;
 };
 
 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;
 
   WorkerPrivate* mWorkerPrivate;
   nsCOMPtr<nsIEventTarget> mSyncLoopTarget;
   nsTArray<ScriptLoadInfo> mLoadInfos;
   RefPtr<CacheCreator> mCacheCreator;
@@ -713,18 +715,18 @@ private:
     cachePromise->AppendNativeHandler(promiseHandler);
 
     loadInfo.mCachePromise.swap(cachePromise);
     loadInfo.mCacheStatus = ScriptLoadInfo::WritingToCache;
 
     return NS_OK;
   }
 
-  virtual bool
-  Notify(Status aStatus) override
+  bool
+  Notify(Status aStatus)
   {
     mWorkerPrivate->AssertIsOnWorkerThread();
 
     if (aStatus >= Terminating && !mCanceled) {
       mCanceled = true;
 
       MOZ_ALWAYS_SUCCEEDS(
         NS_DispatchToMainThread(NewRunnableMethod(this,
@@ -1034,17 +1036,17 @@ private:
       principal = parentWorker->GetPrincipal();
     }
 
     // We don't mute the main worker script becase we've already done
     // same-origin checks on them so we should be able to see their errors.
     // 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
     // the 'requestSucceeded' attribute on the http channel.
     nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(request);
     nsAutoCString tCspHeaderValue, tCspROHeaderValue;
 
     if (httpChannel) {
@@ -1351,16 +1353,36 @@ private:
         MOZ_ASSERT(false, "This should never fail!");
       }
     }
   }
 };
 
 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,
                                  const uint8_t* aString)
 {
   return mRunnable->OnStreamComplete(aLoader, mIndex, aStatus, aStringLen, aString);
 }
 
@@ -2003,17 +2025,16 @@ ScriptExecutorRunnable::ShutdownScriptLo
         LogExceptionToConsole(aCx, aWorkerPrivate);
         mScriptLoader.mRv.Throw(NS_ERROR_DOM_NETWORK_ERR);
       }
     } else {
       mScriptLoader.mRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     }
   }
 
-  mScriptLoader.ReleaseWorker();
   aWorkerPrivate->StopSyncLoop(mSyncLoopTarget, aResult);
 }
 
 void
 ScriptExecutorRunnable::LogExceptionToConsole(JSContext* aCx,
                                               WorkerPrivate* aWorkerPrivate)
 {
   aWorkerPrivate->AssertIsOnWorkerThread();
@@ -2055,25 +2076,25 @@ LoadAllScripts(WorkerPrivate* aWorkerPri
 
   RefPtr<ScriptLoaderRunnable> loader =
     new ScriptLoaderRunnable(aWorkerPrivate, syncLoop.EventTarget(),
                              aLoadInfos, aIsMainScript, aWorkerScriptType,
                              aRv);
 
   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;
   }
 
   syncLoop.Run();
 }
 
 } /* anonymous namespace */