Bug 1286487 - WorkerProxyToMainThreadRunnable must keep alive workers using WorkerHolder. r=bkelly, a=abillings
authorAndrea Marchesini <amarchesini@mozilla.com>
Sat, 23 Jul 2016 08:31:31 +0200
changeset 347715 4ea03e074e7f2f493b9b9a8ca979ee847094857b
parent 347714 0fda1a94617401c79765aacf86020e610aaf29d7
child 347716 d927ac40c9a6803c3338e5388d8222cada59a76f
push id6389
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:38:22 +0000
treeherdermozilla-beta@01d67bfe6c81 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly, abillings
bugs1286487
milestone50.0a2
Bug 1286487 - WorkerProxyToMainThreadRunnable must keep alive workers using WorkerHolder. r=bkelly, a=abillings
dom/workers/WorkerRunnable.cpp
dom/workers/WorkerRunnable.h
--- a/dom/workers/WorkerRunnable.cpp
+++ b/dom/workers/WorkerRunnable.cpp
@@ -672,23 +672,23 @@ WorkerProxyToMainThreadRunnable::WorkerP
 WorkerProxyToMainThreadRunnable::~WorkerProxyToMainThreadRunnable()
 {}
 
 bool
 WorkerProxyToMainThreadRunnable::Dispatch()
 {
   mWorkerPrivate->AssertIsOnWorkerThread();
 
-  if (NS_WARN_IF(!mWorkerPrivate->ModifyBusyCountFromWorker(true))) {
+  if (NS_WARN_IF(!HoldWorker())) {
     RunBackOnWorkerThread();
     return false;
   }
 
   if (NS_WARN_IF(NS_FAILED(NS_DispatchToMainThread(this)))) {
-    mWorkerPrivate->ModifyBusyCountFromWorker(false);
+    ReleaseWorker();
     RunBackOnWorkerThread();
     return false;
   }
 
   return true;
 }
 
 NS_IMETHODIMP
@@ -728,21 +728,55 @@ WorkerProxyToMainThreadRunnable::PostDis
     WorkerRun(JSContext* aCx, workers::WorkerPrivate* aWorkerPrivate) override
     {
       MOZ_ASSERT(aWorkerPrivate);
       aWorkerPrivate->AssertIsOnWorkerThread();
 
       mRunnable->RunBackOnWorkerThread();
 
       // Let's release the worker thread.
-      aWorkerPrivate->ModifyBusyCountFromWorker(false);
+      mRunnable->ReleaseWorker();
       return true;
     }
 
   private:
     ~ReleaseRunnable()
     {}
   };
 
   RefPtr<WorkerControlRunnable> runnable =
     new ReleaseRunnable(mWorkerPrivate, this);
   NS_WARN_IF(!runnable->Dispatch());
 }
+
+bool
+WorkerProxyToMainThreadRunnable::HoldWorker()
+{
+  mWorkerPrivate->AssertIsOnWorkerThread();
+  MOZ_ASSERT(!mWorkerHolder);
+
+  class SimpleWorkerHolder final : public WorkerHolder
+  {
+  public:
+    bool Notify(Status aStatus) override
+    {
+      // We don't care about the notification. We just want to keep the
+      // mWorkerPrivate alive.
+      return true;
+    }
+  };
+
+  UniquePtr<WorkerHolder> workerHolder(new SimpleWorkerHolder());
+  if (NS_WARN_IF(!workerHolder->HoldWorker(mWorkerPrivate))) {
+    return false;
+  }
+
+  mWorkerHolder = Move(workerHolder);
+  return true;
+}
+
+void
+WorkerProxyToMainThreadRunnable::ReleaseWorker()
+{
+  mWorkerPrivate->AssertIsOnWorkerThread();
+  MOZ_ASSERT(mWorkerHolder);
+  mWorkerHolder = nullptr;
+}
--- a/dom/workers/WorkerRunnable.h
+++ b/dom/workers/WorkerRunnable.h
@@ -428,18 +428,22 @@ protected:
 public:
   bool Dispatch();
 
 private:
   NS_IMETHOD Run() override;
 
   void PostDispatchOnMainThread();
 
+  bool HoldWorker();
+  void ReleaseWorker();
+
 protected:
   WorkerPrivate* mWorkerPrivate;
+  UniquePtr<WorkerHolder> mWorkerHolder;
 };
 
 // Class for checking API exposure.  This totally violates the "MUST" in the
 // comments on WorkerMainThreadRunnable::Dispatch, because API exposure checks
 // can't throw.  Maybe we should change it so they _could_ throw.  But for now
 // we are bad people and should be ashamed of ourselves.  Let's hope none of
 // them happen while a worker is shutting down.
 //