Bug 1250963 part 1. Change NotifyRunnable::Dispatch to not require a JSContext. r=khuey
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 25 Feb 2016 16:05:39 -0500
changeset 322012 a702f6bf94594534fa74dde6ef174a6d7639b6cd
parent 322011 5ee600d6dd400703f09d1d35f969885d874e5e21
child 322013 6d6d703d61e0b61717220b0a14e7332d7dfb1697
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey
bugs1250963
milestone47.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 1250963 part 1. Change NotifyRunnable::Dispatch to not require a JSContext. r=khuey The only reason NotifyRunnable::Dispatch needs a JSContext is so that it can call ModifyBusyCount in Pre/PostDispatch. The only reason that needs a JSContext is to call Cancel(), which only needs it to call Notify(), which only needs it to call NotifyPrivate, which only needs it to dispatch a NotifyRunnable.
dom/workers/RuntimeService.cpp
dom/workers/WorkerPrivate.cpp
dom/workers/WorkerPrivate.h
--- a/dom/workers/RuntimeService.cpp
+++ b/dom/workers/RuntimeService.cpp
@@ -2141,29 +2141,23 @@ void
 RuntimeService::CancelWorkersForWindow(nsPIDOMWindowInner* aWindow)
 {
   AssertIsOnMainThread();
 
   AutoTArray<WorkerPrivate*, MAX_WORKERS_PER_DOMAIN> workers;
   GetWorkersForWindow(aWindow, workers);
 
   if (!workers.IsEmpty()) {
-    AutoJSAPI jsapi;
-    if (NS_WARN_IF(!jsapi.InitWithLegacyErrorReporting(aWindow))) {
-      return;
-    }
-    JSContext* cx = jsapi.cx();
-
     for (uint32_t index = 0; index < workers.Length(); index++) {
       WorkerPrivate*& worker = workers[index];
 
       if (worker->IsSharedWorker()) {
         worker->CloseSharedWorkersForWindow(aWindow);
-      } else if (!worker->Cancel(cx)) {
-        JS_ReportPendingException(cx);
+      } else {
+        worker->Cancel();
       }
     }
   }
 }
 
 void
 RuntimeService::FreezeWorkersForWindow(nsPIDOMWindowInner* aWindow)
 {
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -842,29 +842,39 @@ public:
   NotifyRunnable(WorkerPrivate* aWorkerPrivate, Status aStatus)
   : WorkerControlRunnable(aWorkerPrivate, WorkerThreadUnchangedBusyCount),
     mStatus(aStatus)
   {
     MOZ_ASSERT(aStatus == Closing || aStatus == Terminating ||
                aStatus == Canceling || aStatus == Killing);
   }
 
+  // We can be dispatched without a JSContext, because all we do with the
+  // JSContext passed to Dispatch() normally for worker runnables is call
+  // ModifyBusyCount... but that doesn't actually use its JSContext argument.
+  bool Dispatch()
+  {
+    return WorkerControlRunnable::Dispatch(nullptr);
+  }
+
 private:
   virtual bool
   PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
   {
+    aWorkerPrivate->AssertIsOnParentThread();
     // Modify here, but not in PostRun! This busy count addition will be matched
     // by the CloseEventRunnable.
     return aWorkerPrivate->ModifyBusyCount(aCx, true);
   }
 
   virtual void
   PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
                bool aDispatchResult) override
   {
+    aWorkerPrivate->AssertIsOnParentThread();
     if (!aDispatchResult) {
       // We couldn't dispatch to the worker, which means it's already dead.
       // Undo the busy count modification.
       aWorkerPrivate->ModifyBusyCount(aCx, false);
     }
   }
 
   virtual bool
@@ -2514,17 +2524,17 @@ WorkerPrivateParent<Derived>::Start()
   }
 
   return false;
 }
 
 // aCx is null when called from the finalizer
 template <class Derived>
 bool
-WorkerPrivateParent<Derived>::NotifyPrivate(JSContext* aCx, Status aStatus)
+WorkerPrivateParent<Derived>::NotifyPrivate(Status aStatus)
 {
   AssertIsOnParentThread();
 
   bool pending;
   {
     MutexAutoLock lock(mMutex);
 
     if (mParentStatus >= aStatus) {
@@ -2566,17 +2576,17 @@ WorkerPrivateParent<Derived>::NotifyPriv
   NS_ASSERTION(aStatus != Terminating || mQueuedRunnables.IsEmpty(),
                "Shouldn't have anything queued!");
 
   // Anything queued will be discarded.
   mQueuedRunnables.Clear();
 
   RefPtr<NotifyRunnable> runnable =
     new NotifyRunnable(ParentAsWorkerPrivate(), aStatus);
-  return runnable->Dispatch(aCx);
+  return runnable->Dispatch();
 }
 
 template <class Derived>
 bool
 WorkerPrivateParent<Derived>::Freeze(JSContext* aCx,
                                      nsPIDOMWindowInner* aWindow)
 {
   AssertIsOnParentThread();
@@ -2773,17 +2783,18 @@ WorkerPrivateParent<Derived>::Close()
     }
   }
 
   return true;
 }
 
 template <class Derived>
 bool
-WorkerPrivateParent<Derived>::ModifyBusyCount(JSContext* aCx, bool aIncrease)
+WorkerPrivateParent<Derived>::ModifyBusyCount(JSContext* /* unused */,
+                                              bool aIncrease)
 {
   AssertIsOnParentThread();
 
   NS_ASSERTION(aIncrease || mBusyCount, "Mismatched busy count mods!");
 
   if (aIncrease) {
     mBusyCount++;
     return true;
@@ -2792,17 +2803,17 @@ WorkerPrivateParent<Derived>::ModifyBusy
   if (--mBusyCount == 0) {
 
     bool shouldCancel;
     {
       MutexAutoLock lock(mMutex);
       shouldCancel = mParentStatus == Terminating;
     }
 
-    if (shouldCancel && !Cancel(aCx)) {
+    if (shouldCancel && !Cancel()) {
       return false;
     }
   }
 
   return true;
 }
 
 template <class Derived>
@@ -3327,38 +3338,35 @@ WorkerPrivateParent<Derived>::CloseShare
   // if that was the last SharedWorker then it's time to cancel this worker.
 
   AutoSafeJSContext cx;
 
   if (!mSharedWorkers.IsEmpty()) {
     if (!Freeze(cx, nullptr)) {
       JS_ReportPendingException(cx);
     }
-  } else if (!Cancel(cx)) {
-    JS_ReportPendingException(cx);
+  } else {
+    Cancel();
   }
 }
 
 template <class Derived>
 void
 WorkerPrivateParent<Derived>::CloseAllSharedWorkers()
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(IsSharedWorker() || IsServiceWorker());
 
   for (uint32_t i = 0; i < mSharedWorkers.Length(); ++i) {
     mSharedWorkers[i]->Close();
   }
 
   mSharedWorkers.Clear();
 
-  AutoSafeJSContext cx;
-  if (!Cancel(cx)) {
-    JS_ReportPendingException(cx);
-  }
+  Cancel();
 }
 
 template <class Derived>
 void
 WorkerPrivateParent<Derived>::WorkerScriptLoaded()
 {
   AssertIsOnMainThread();
 
@@ -5180,17 +5188,17 @@ WorkerPrivate::NotifyFeatures(JSContext*
       NS_WARNING("Failed to notify feature!");
     }
   }
 
   AutoTArray<ParentType*, 10> children;
   children.AppendElements(mChildWorkers);
 
   for (uint32_t index = 0; index < children.Length(); index++) {
-    if (!children[index]->Notify(aCx, aStatus)) {
+    if (!children[index]->Notify(aStatus)) {
       NS_WARNING("Failed to notify child worker!");
     }
   }
 }
 
 void
 WorkerPrivate::CancelAllTimeouts(JSContext* aCx)
 {
--- a/dom/workers/WorkerPrivate.h
+++ b/dom/workers/WorkerPrivate.h
@@ -213,23 +213,23 @@ private:
   Derived*
   ParentAsWorkerPrivate() const
   {
     return static_cast<Derived*>(const_cast<WorkerPrivateParent*>(this));
   }
 
   // aCx is null when called from the finalizer
   bool
-  NotifyPrivate(JSContext* aCx, Status aStatus);
+  NotifyPrivate(Status aStatus);
 
   // aCx is null when called from the finalizer
   bool
   TerminatePrivate(JSContext* aCx)
   {
-    return NotifyPrivate(aCx, Terminating);
+    return NotifyPrivate(Terminating);
   }
 
   void
   PostMessageInternal(JSContext* aCx, JS::Handle<JS::Value> aMessage,
                       const Optional<Sequence<JS::Value>>& aTransferable,
                       UniquePtr<ServiceWorkerClientInfo>&& aClientInfo,
                       ErrorResult& aRv);
 
@@ -277,31 +277,31 @@ public:
   GetEventTarget();
 
   // May be called on any thread...
   bool
   Start();
 
   // Called on the parent thread.
   bool
-  Notify(JSContext* aCx, Status aStatus)
+  Notify(Status aStatus)
   {
-    return NotifyPrivate(aCx, aStatus);
+    return NotifyPrivate(aStatus);
   }
 
   bool
-  Cancel(JSContext* aCx)
+  Cancel()
   {
-    return Notify(aCx, Canceling);
+    return Notify(Canceling);
   }
 
   bool
   Kill(JSContext* aCx)
   {
-    return Notify(aCx, Killing);
+    return Notify(Killing);
   }
 
   // We can assume that an nsPIDOMWindow will be available for Freeze, Thaw
   // as these are only used for globals going in and out of the bfcache.
   bool
   Freeze(JSContext* aCx, nsPIDOMWindowInner* aWindow);
 
   bool
@@ -318,18 +318,20 @@ public:
   {
     AssertIsOnParentThread();
     return TerminatePrivate(aCx);
   }
 
   bool
   Close();
 
+  // The JSContext argument can be null, since it's not used for anything.  It's
+  // about to go away.
   bool
-  ModifyBusyCount(JSContext* aCx, bool aIncrease);
+  ModifyBusyCount(JSContext* /* unused */, bool aIncrease);
 
   void
   ForgetOverridenLoadGroup(nsCOMPtr<nsILoadGroup>& aLoadGroupOut);
 
   void
   ForgetMainThreadObjects(nsTArray<nsCOMPtr<nsISupports> >& aDoomed);
 
   void