Bug 1527203 Part 1 - Add interface to delay execution of debuggee content in workers during debugger registration, r=asuth.
authorBrian Hackett <bhackett1024@gmail.com>
Tue, 12 Feb 2019 13:04:18 -1000
changeset 519513 7ba97389b18333d03d4c07e101d1b156ec9b692f
parent 519512 afbcec667ac42d4a17303ad65bd499a29e43c403
child 519514 34d35ba26c07ee6be545909fed88f3a4d52bb6d3
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1527203
milestone67.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 1527203 Part 1 - Add interface to delay execution of debuggee content in workers during debugger registration, r=asuth.
dom/workers/WorkerDebugger.cpp
dom/workers/WorkerPrivate.cpp
dom/workers/WorkerPrivate.h
dom/workers/nsIWorkerDebugger.idl
--- a/dom/workers/WorkerDebugger.cpp
+++ b/dom/workers/WorkerDebugger.cpp
@@ -83,16 +83,27 @@ class CompileDebuggerScriptRunnable fina
       NS_WARNING("Failed to make global!");
       return false;
     }
 
     if (NS_WARN_IF(!aWorkerPrivate->EnsureClientSource())) {
       return false;
     }
 
+    if (NS_WARN_IF(!aWorkerPrivate->EnsureCSPEventListener())) {
+      return false;
+    }
+
+    // Initialize performance state which might be used on the main thread, as
+    // in CompileScriptRunnable. This runnable might execute first.
+    aWorkerPrivate->EnsurePerformanceStorage();
+    if (mozilla::StaticPrefs::dom_performance_enable_scheduler_timing()) {
+      aWorkerPrivate->EnsurePerformanceCounter();
+    }
+
     JS::Rooted<JSObject*> global(aCx, globalScope->GetWrapper());
 
     ErrorResult rv;
     JSAutoRealm ar(aCx, global);
     workerinternals::LoadMainScript(aWorkerPrivate, mScriptURL, DebuggerScript,
                                     rv);
     rv.WouldReportJSException();
     // Explicitly ignore NS_BINDING_ABORTED on rv.  Or more precisely, still
@@ -363,16 +374,22 @@ WorkerDebugger::RemoveListener(nsIWorker
   if (!mListeners.Contains(aListener)) {
     return NS_ERROR_INVALID_ARG;
   }
 
   mListeners.RemoveElement(aListener);
   return NS_OK;
 }
 
+NS_IMETHODIMP
+WorkerDebugger::SetDebuggerReady(bool aReady)
+{
+  return mWorkerPrivate->SetIsDebuggerReady(aReady);
+}
+
 void WorkerDebugger::Close() {
   MOZ_ASSERT(mWorkerPrivate);
   mWorkerPrivate = nullptr;
 
   nsTArray<nsCOMPtr<nsIWorkerDebuggerListener>> listeners(mListeners);
   for (size_t index = 0; index < listeners.Length(); ++index) {
     listeners[index]->OnClose();
   }
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -294,23 +294,24 @@ class ModifyBusyCountRunnable final : pu
       WorkerControlRunnable::PostRun(aCx, aWorkerPrivate, aRunResult);
       return;
     }
     // Don't do anything here as it's possible that aWorkerPrivate has been
     // deleted.
   }
 };
 
-class CompileScriptRunnable final : public WorkerRunnable {
+class CompileScriptRunnable final : public WorkerDebuggeeRunnable {
   nsString mScriptURL;
 
  public:
   explicit CompileScriptRunnable(WorkerPrivate* aWorkerPrivate,
                                  const nsAString& aScriptURL)
-      : WorkerRunnable(aWorkerPrivate), mScriptURL(aScriptURL) {}
+      : WorkerDebuggeeRunnable(aWorkerPrivate, WorkerThreadModifyBusyCount),
+        mScriptURL(aScriptURL) {}
 
  private:
   // We can't implement PreRun effectively, because at the point when that would
   // run we have not yet done our load so don't know things like our final
   // principal and whatnot.
 
   virtual bool WorkerRun(JSContext* aCx,
                          WorkerPrivate* aWorkerPrivate) override {
@@ -1327,61 +1328,70 @@ void WorkerPrivate::Traverse(nsCycleColl
     WorkerPrivate* tmp = this;
     NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mParentEventTargetRef);
   }
 }
 
 nsresult WorkerPrivate::Dispatch(already_AddRefed<WorkerRunnable> aRunnable,
                                  nsIEventTarget* aSyncLoopTarget) {
   // May be called on any thread!
+  MutexAutoLock lock(mMutex);
+  return DispatchLockHeld(std::move(aRunnable), aSyncLoopTarget, lock);
+}
+
+nsresult WorkerPrivate::DispatchLockHeld(already_AddRefed<WorkerRunnable> aRunnable,
+                                         nsIEventTarget* aSyncLoopTarget,
+                                         const MutexAutoLock& aProofOfLock) {
+  // May be called on any thread!
   RefPtr<WorkerRunnable> runnable(aRunnable);
 
-  {
-    MutexAutoLock lock(mMutex);
-
-    MOZ_ASSERT_IF(aSyncLoopTarget, mThread);
-
-    if (!mThread) {
-      if (ParentStatus() == Pending || mStatus == Pending) {
-        mPreStartRunnables.AppendElement(runnable);
-        return NS_OK;
-      }
-
-      NS_WARNING(
-          "Using a worker event target after the thread has already"
-          "been released!");
-      return NS_ERROR_UNEXPECTED;
+  MOZ_ASSERT_IF(aSyncLoopTarget, mThread);
+
+  if (mStatus == Dead || (!aSyncLoopTarget && ParentStatus() > Running)) {
+    NS_WARNING(
+        "A runnable was posted to a worker that is already shutting "
+        "down!");
+    return NS_ERROR_UNEXPECTED;
+  }
+
+  if (runnable->IsDebuggeeRunnable() && !mDebuggerReady) {
+    MOZ_RELEASE_ASSERT(!aSyncLoopTarget);
+    mDelayedDebuggeeRunnables.AppendElement(runnable);
+    return NS_OK;
+  }
+
+  if (!mThread) {
+    if (ParentStatus() == Pending || mStatus == Pending) {
+      mPreStartRunnables.AppendElement(runnable);
+      return NS_OK;
     }
 
-    if (mStatus == Dead || (!aSyncLoopTarget && ParentStatus() > Running)) {
-      NS_WARNING(
-          "A runnable was posted to a worker that is already shutting "
-          "down!");
-      return NS_ERROR_UNEXPECTED;
-    }
-
-    nsresult rv;
-    if (aSyncLoopTarget) {
-      rv = aSyncLoopTarget->Dispatch(runnable.forget(), NS_DISPATCH_NORMAL);
-    } else {
-      // WorkerDebuggeeRunnables don't need any special treatment here. True,
-      // they should not be delivered to a frozen worker. But frozen workers
-      // aren't drawing from the thread's main event queue anyway, only from
-      // mControlQueue.
-      rv = mThread->DispatchAnyThread(WorkerThreadFriendKey(),
-                                      runnable.forget());
-    }
-
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
-
-    mCondVar.Notify();
-  }
-
+    NS_WARNING(
+        "Using a worker event target after the thread has already"
+        "been released!");
+    return NS_ERROR_UNEXPECTED;
+  }
+
+  nsresult rv;
+  if (aSyncLoopTarget) {
+    rv = aSyncLoopTarget->Dispatch(runnable.forget(), NS_DISPATCH_NORMAL);
+  } else {
+    // WorkerDebuggeeRunnables don't need any special treatment here. True,
+    // they should not be delivered to a frozen worker. But frozen workers
+    // aren't drawing from the thread's main event queue anyway, only from
+    // mControlQueue.
+    rv = mThread->DispatchAnyThread(WorkerThreadFriendKey(),
+                                    runnable.forget());
+  }
+
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  mCondVar.Notify();
   return NS_OK;
 }
 
 void WorkerPrivate::EnableDebugger() {
   AssertIsOnParentThread();
 
   if (NS_FAILED(RegisterWorkerDebugger(this))) {
     NS_WARNING("Failed to register worker debugger!");
@@ -2056,16 +2066,17 @@ WorkerPrivate::WorkerPrivate(WorkerPriva
       mWorkerScriptExecutedSuccessfully(false),
       mFetchHandlerWasAdded(false),
       mMainThreadObjectsForgotten(false),
       mIsChromeWorker(aIsChromeWorker),
       mParentFrozen(false),
       mIsSecureContext(
           IsNewWorkerSecureContext(mParent, mWorkerType, mLoadInfo)),
       mDebuggerRegistered(false),
+      mDebuggerReady(true),
       mIsInAutomation(false),
       mPerformanceCounter(nullptr) {
   MOZ_ASSERT_IF(!IsDedicatedWorker(), NS_IsMainThread());
 
   if (aParent) {
     aParent->AssertIsOnWorkerThread();
 
     // Note that this copies our parent's secure context state into mJSSettings.
@@ -2238,16 +2249,49 @@ already_AddRefed<WorkerPrivate> WorkerPr
     return nullptr;
   }
 
   worker->mSelfRef = worker;
 
   return worker.forget();
 }
 
+nsresult
+WorkerPrivate::SetIsDebuggerReady(bool aReady)
+{
+  AssertIsOnParentThread();
+  MutexAutoLock lock(mMutex);
+
+  if (mDebuggerReady == aReady) {
+    return NS_OK;
+  }
+
+  if (!aReady && mDebuggerRegistered) {
+    // The debugger can only be marked as not ready during registration.
+    return NS_ERROR_FAILURE;
+  }
+
+  mDebuggerReady = aReady;
+
+  if (aReady && mDebuggerRegistered) {
+    // Dispatch all the delayed runnables without releasing the lock, to ensure
+    // that the order in which debuggee runnables execute is the same as the
+    // order in which they were originally dispatched.
+    auto pending = std::move(mDelayedDebuggeeRunnables);
+    for (uint32_t i = 0; i < pending.Length(); i++) {
+      RefPtr<WorkerRunnable> runnable = pending[i].forget();
+      nsresult rv = DispatchLockHeld(runnable.forget(), nullptr, lock);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
+    MOZ_RELEASE_ASSERT(mDelayedDebuggeeRunnables.IsEmpty());
+  }
+
+  return NS_OK;
+}
+
 // static
 nsresult WorkerPrivate::GetLoadInfo(JSContext* aCx, nsPIDOMWindowInner* aWindow,
                                     WorkerPrivate* aParent,
                                     const nsAString& aScriptURL,
                                     bool aIsChromeWorker,
                                     LoadGroupBehavior aLoadGroupBehavior,
                                     WorkerType aWorkerType,
                                     WorkerLoadInfo* aLoadInfo) {
--- a/dom/workers/WorkerPrivate.h
+++ b/dom/workers/WorkerPrivate.h
@@ -165,16 +165,18 @@ class WorkerPrivate : public RelativeTim
 
     MutexAutoLock lock(mMutex);
 
     while (mDebuggerRegistered != aDebuggerRegistered) {
       mCondVar.Wait();
     }
   }
 
+  nsresult SetIsDebuggerReady(bool aReady);
+
   WorkerDebugger* Debugger() const {
     AssertIsOnMainThread();
 
     MOZ_ASSERT(mDebugger);
     return mDebugger;
   }
 
   void SetDebugger(WorkerDebugger* aDebugger) {
@@ -902,16 +904,22 @@ class WorkerPrivate : public RelativeTim
   void NotifyHolders(WorkerStatus aStatus);
 
   bool HasActiveHolders() {
     MOZ_ACCESS_THREAD_BOUND(mWorkerThreadAccessible, data);
     return !(data->mChildWorkers.IsEmpty() && data->mTimeouts.IsEmpty() &&
              data->mHolders.IsEmpty());
   }
 
+  // Internal logic to dispatch a runnable. This is separate from Dispatch()
+  // to allow runnables to be atomically dispatched in bulk.
+  nsresult DispatchLockHeld(already_AddRefed<WorkerRunnable> aRunnable,
+                            nsIEventTarget* aSyncLoopTarget,
+                            const MutexAutoLock& aProofOfLock);
+
   class EventTarget;
   friend class EventTarget;
   friend class mozilla::dom::WorkerHolder;
   friend class AutoSyncLoopHolder;
 
   struct TimeoutInfo;
 
   class MemoryReporter;
@@ -1069,16 +1077,23 @@ class WorkerPrivate : public RelativeTim
   //
   // It's a bit unfortunate that we have to have an out-of-band boolean for
   // this, but we need access to this state from the parent thread, and we can't
   // use our global object's secure state there.
   const bool mIsSecureContext;
 
   bool mDebuggerRegistered;
 
+  // During registration, this worker may be marked as not being ready to
+  // execute debuggee runnables or content.
+  //
+  // Protected by mMutex.
+  bool mDebuggerReady;
+  nsTArray<RefPtr<WorkerRunnable>> mDelayedDebuggeeRunnables;
+
   // mIsInAutomation is true when we're running in test automation.
   // We expose some extra testing functions in that case.
   bool mIsInAutomation;
 
   // This pointer will be null if dom.performance.enable_scheduler_timing is
   // false (default value)
   RefPtr<mozilla::PerformanceCounter> mPerformanceCounter;
 };
--- a/dom/workers/nsIWorkerDebugger.idl
+++ b/dom/workers/nsIWorkerDebugger.idl
@@ -42,9 +42,22 @@ interface nsIWorkerDebugger : nsISupport
   void initialize(in AString url);
 
   [binaryname(PostMessageMoz)]
   void postMessage(in AString message);
 
   void addListener(in nsIWorkerDebuggerListener listener);
 
   void removeListener(in nsIWorkerDebuggerListener listener);
+
+  // Indicate whether the debugger has finished initializing. By default the
+  // debugger will be considered initialized when the onRegister hooks in all
+  // nsIWorkerDebuggerManagerListener have been called.
+  //
+  // setDebuggerReady(false) can be called during an onRegister hook to mark
+  // the debugger as not being ready yet. This will prevent all content from
+  // running in the worker, including the worker's main script and any messages
+  // posted to it. Other runnables will still execute in the worker as normal.
+  //
+  // When the debugger is ready, setDebuggerReady(true) should then be called
+  // to allow the worker to begin executing content.
+  void setDebuggerReady(in boolean ready);
 };