Bug 1527203 Part 2 - Prevent worker debuggee execution until the thread actor has attached, r=ochameau.
authorBrian Hackett <bhackett1024@gmail.com>
Tue, 12 Feb 2019 13:05:34 -1000
changeset 519514 34d35ba26c07ee6be545909fed88f3a4d52bb6d3
parent 519513 7ba97389b18333d03d4c07e101d1b156ec9b692f
child 519515 e5f0800182aef34737ce0793a31939ec12a47ae3
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)
reviewersochameau
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 2 - Prevent worker debuggee execution until the thread actor has attached, r=ochameau.
devtools/client/debugger/new/src/client/firefox.js
devtools/server/actors/targets/browsing-context.js
devtools/server/actors/thread.js
devtools/server/actors/worker/worker-target-actor-list.js
devtools/server/main.js
devtools/server/startup/worker.js
--- a/devtools/client/debugger/new/src/client/firefox.js
+++ b/devtools/client/debugger/new/src/client/firefox.js
@@ -39,16 +39,17 @@ export async function onConnect(connecti
     setupEvents({ threadClient, actions, supportsWasm });
   }
 
   tabTarget.on("will-navigate", actions.willNavigate);
   tabTarget.on("navigate", actions.navigated);
 
   await threadClient.reconfigure({
     observeAsmJS: true,
+    pauseWorkersUntilAttach: true,
     wasmBinarySource: supportsWasm,
     skipBreakpoints: prefs.skipPausing
   });
 
   // In Firefox, we need to initially request all of the sources. This
   // usually fires off individual `newSource` notifications as the
   // debugger finds them, but there may be existing sources already in
   // the debugger (if it's paused already, or if loading the page from
--- a/devtools/server/actors/targets/browsing-context.js
+++ b/devtools/server/actors/targets/browsing-context.js
@@ -622,29 +622,36 @@ const browsingContextTargetPrototype = {
     return {};
   },
 
   listFrames(request) {
     const windows = this._docShellsToWindows(this.docShells);
     return { frames: windows };
   },
 
-  listWorkers(request) {
-    if (!this.attached) {
-      return { error: "wrongState" };
-    }
-
+  ensureWorkerTargetActorList() {
     if (this._workerTargetActorList === null) {
       this._workerTargetActorList = new WorkerTargetActorList(this.conn, {
         type: Ci.nsIWorkerDebugger.TYPE_DEDICATED,
         window: this.window,
       });
     }
+    return this._workerTargetActorList;
+  },
 
-    return this._workerTargetActorList.getList().then((actors) => {
+  pauseWorkersUntilAttach(shouldPause) {
+    this.ensureWorkerTargetActorList().setPauseMatchingWorkers(shouldPause);
+  },
+
+  listWorkers(request) {
+    if (!this.attached) {
+      return { error: "wrongState" };
+    }
+
+    return this.ensureWorkerTargetActorList().getList().then((actors) => {
       const pool = new Pool(this.conn);
       for (const actor of actors) {
         pool.manage(actor);
       }
 
       // Do not destroy the pool before transfering ownership to the newly created
       // pool, so that we do not accidently destroy actors that are still in use.
       if (this._workerTargetActorPool) {
@@ -881,16 +888,17 @@ const browsingContextTargetPrototype = {
     if (this._targetScopedActorPool) {
       this._targetScopedActorPool.destroy();
       this._targetScopedActorPool = null;
     }
 
     // Make sure that no more workerListChanged notifications are sent.
     if (this._workerTargetActorList !== null) {
       this._workerTargetActorList.onListChanged = null;
+      this._workerTargetActorList.setPauseMatchingWorkers(false);
       this._workerTargetActorList = null;
     }
 
     if (this._workerTargetActorPool !== null) {
       this._workerTargetActorPool.destroy();
       this._workerTargetActorPool = null;
     }
 
--- a/devtools/server/actors/thread.js
+++ b/devtools/server/actors/thread.js
@@ -267,16 +267,24 @@ const ThreadActor = ActorClassWithSpec(t
     this._nestedEventLoops = new EventLoopStack({
       hooks: this._parent,
       connection: this.conn,
       thread: this,
     });
 
     this.dbg.addDebuggees();
     this.dbg.enabled = true;
+
+    // Notify the parent that we've finished attaching. If this is a worker
+    // thread which was paused until attaching, this will allow content to
+    // begin executing.
+    if (this._parent.onThreadAttached) {
+      this._parent.onThreadAttached();
+    }
+
     try {
       // Put ourselves in the paused state.
       const packet = this._paused();
       if (!packet) {
         return { error: "notAttached" };
       }
       packet.why = { type: "attached" };
 
@@ -444,16 +452,22 @@ const ThreadActor = ActorClassWithSpec(t
     if ("observeAsmJS" in options) {
       this.dbg.allowUnobservedAsmJS = !options.observeAsmJS;
     }
 
     if ("skipBreakpoints" in options) {
       this.skipBreakpoints = options.skipBreakpoints;
     }
 
+    if ("pauseWorkersUntilAttach" in options) {
+      if (this._parent.pauseWorkersUntilAttach) {
+        this._parent.pauseWorkersUntilAttach(options.pauseWorkersUntilAttach);
+      }
+    }
+
     Object.assign(this._options, options);
 
     // Update the global source store
     this.sources.setOptions(options);
 
     return {};
   },
 
--- a/devtools/server/actors/worker/worker-target-actor-list.js
+++ b/devtools/server/actors/worker/worker-target-actor-list.js
@@ -26,21 +26,50 @@ function matchWorkerDebugger(dbg, option
     if (window !== options.window) {
       return false;
     }
   }
 
   return true;
 }
 
+// When a new worker appears, in some cases (i.e. the debugger is running) we
+// want it to pause during registration until a later time (i.e. the debugger
+// finishes attaching to the worker). This is an optional WorkderDebuggerManager
+// listener that can be installed in addition to the WorkerTargetActorList
+// listener. It always listens to new workers and pauses any which are matched
+// by the WorkerTargetActorList.
+function PauseMatchingWorkers(options) {
+  this._options = options;
+  this.onRegister = this._onRegister.bind(this);
+  this.onUnregister = () => {};
+
+  wdm.addListener(this);
+}
+
+PauseMatchingWorkers.prototype = {
+  destroy() {
+    wdm.removeListener(this);
+  },
+
+  _onRegister(dbg) {
+    if (matchWorkerDebugger(dbg, this._options)) {
+      // Prevent the debuggee from executing in this worker until the debugger
+      // has finished attaching to it.
+      dbg.setDebuggerReady(false);
+    }
+  },
+};
+
 function WorkerTargetActorList(conn, options) {
   this._conn = conn;
   this._options = options;
   this._actors = new Map();
   this._onListChanged = null;
+  this._pauseMatchingWorkers = null;
   this._mustNotify = false;
   this.onRegister = this.onRegister.bind(this);
   this.onUnregister = this.onUnregister.bind(this);
 }
 
 WorkerTargetActorList.prototype = {
   getList() {
     // Create a set of debuggers.
@@ -118,11 +147,22 @@ WorkerTargetActorList.prototype = {
     }
   },
 
   onUnregister(dbg) {
     if (matchWorkerDebugger(dbg, this._options)) {
       this._notifyListChanged();
     }
   },
+
+  setPauseMatchingWorkers(shouldPause) {
+    if (shouldPause != !!this._pauseMatchingWorkers) {
+      if (shouldPause) {
+        this._pauseMatchingWorkers = new PauseMatchingWorkers(this._options);
+      } else {
+        this._pauseMatchingWorkers.destroy();
+        this._pauseMatchingWorkers = null;
+      }
+    }
+  },
 };
 
 exports.WorkerTargetActorList = WorkerTargetActorList;
--- a/devtools/server/main.js
+++ b/devtools/server/main.js
@@ -386,16 +386,21 @@ var DebuggerServer = {
         const listener = {
           onClose: () => {
             dbg.removeListener(listener);
           },
 
           onMessage: (message) => {
             message = JSON.parse(message);
             if (message.type !== "rpc") {
+              if (message.type == "attached") {
+                // The thread actor has finished attaching and can hit installed
+                // breakpoints. Allow content to begin executing in the worker.
+                dbg.setDebuggerReady(true);
+              }
               return;
             }
 
             Promise.resolve().then(() => {
               const method = {
                 "fetch": DevToolsUtils.fetch,
               }[message.method];
               if (!method) {
--- a/devtools/server/startup/worker.js
+++ b/devtools/server/startup/worker.js
@@ -84,16 +84,20 @@ this.addEventListener("message", functio
         get sources() {
           if (sources === null) {
             sources = new TabSources(threadActor);
           }
           return sources;
         },
 
         window: global,
+
+        onThreadAttached() {
+          postMessage(JSON.stringify({ type: "attached" }));
+        },
       };
 
       const threadActor = new ThreadActor(parent, global);
       pool.addActor(threadActor);
 
       // parentActor.threadActor is needed from the webconsole for grip previewing
       parent.threadActor = threadActor;