Bug 1530056 - fix browser_dbg-windowless-workers.js intermittent. r=bhackett
☠☠ backed out by 5f699370c775 ☠ ☠
authorJason Laster <jlaster@mozilla.com>
Sat, 02 Mar 2019 03:14:59 +0000
changeset 520016 06b992be418341f5eb3051405826a812dc4a5b27
parent 520015 a6fd8376d3ec14c2fb5eacb4720d97813b0e28e9
child 520017 52fc794e22ee068c2444682cc3329aec53001a53
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)
reviewersbhackett
bugs1530056
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 1530056 - fix browser_dbg-windowless-workers.js intermittent. r=bhackett Differential Revision: https://phabricator.services.mozilla.com/D21800
devtools/client/debugger/new/src/client/firefox/commands.js
devtools/client/debugger/new/test/mochitest/browser_dbg-windowless-workers-early-breakpoint.js
devtools/client/debugger/new/test/mochitest/browser_dbg-windowless-workers.js
--- a/devtools/client/debugger/new/src/client/firefox/commands.js
+++ b/devtools/client/debugger/new/src/client/firefox/commands.js
@@ -34,16 +34,18 @@ import type {
 let workerClients: Object;
 let threadClient: ThreadClient;
 let tabTarget: TabTarget;
 let debuggerClient: DebuggerClient;
 let sourceActors: { [ActorId]: SourceId };
 let breakpoints: { [string]: Object };
 let supportsWasm: boolean;
 
+let shouldWaitForWorkers = false;
+
 type Dependencies = {
   threadClient: ThreadClient,
   tabTarget: TabTarget,
   debuggerClient: DebuggerClient,
   supportsWasm: boolean
 };
 
 function setupCommands(dependencies: Dependencies) {
@@ -84,16 +86,20 @@ function lookupThreadClient(thread: stri
 
 function lookupConsoleClient(thread: string) {
   if (thread == threadClient.actor) {
     return tabTarget.activeConsole;
   }
   return workerClients[thread].console;
 }
 
+function listWorkerThreadClients() {
+  return Object.values(workerClients).map(({ thread }) => thread);
+}
+
 function resume(thread: string): Promise<*> {
   return new Promise(resolve => {
     lookupThreadClient(thread).resume(resolve);
   });
 }
 
 function stepIn(thread: string): Promise<*> {
   return new Promise(resolve => {
@@ -161,42 +167,58 @@ function removeXHRBreakpoint(path: strin
 
 // Get the string key to use for a breakpoint location.
 // See also duplicate code in breakpoint-actor-map.js :(
 function locationKey(location) {
   const { sourceUrl, sourceId, line, column } = location;
   return `${(sourceUrl: any)}:${(sourceId: any)}:${line}:${(column: any)}`;
 }
 
+function waitForWorkers(shouldWait) {
+  shouldWaitForWorkers = shouldWait;
+}
+
 async function setBreakpoint(
   location: BreakpointLocation,
   options: BreakpointOptions
 ) {
   breakpoints[locationKey(location)] = { location, options };
   await threadClient.setBreakpoint(location, options);
 
   // Set breakpoints in other threads as well, but do not wait for the requests
   // to complete, so that we don't get hung up if one of the threads stops
   // responding. We don't strictly need to wait for the main thread to finish
   // setting its breakpoint, but this leads to more consistent behavior if the
   // user sets a breakpoint and immediately starts interacting with the page.
   // If the main thread stops responding then we're toast regardless.
-  for (const { thread } of (Object.values(workerClients): any)) {
-    thread.setBreakpoint(location, options);
+  if (shouldWaitForWorkers) {
+    for (const thread of listWorkerThreadClients()) {
+      await thread.setBreakpoint(location, options);
+    }
+  } else {
+    for (const thread of listWorkerThreadClients()) {
+      thread.setBreakpoint(location, options);
+    }
   }
 }
 
 async function removeBreakpoint(location: BreakpointLocation) {
   delete breakpoints[locationKey(location)];
   await threadClient.removeBreakpoint(location);
 
   // Remove breakpoints without waiting for the thread to respond, for the same
   // reason as in setBreakpoint.
-  for (const { thread } of (Object.values(workerClients): any)) {
-    thread.removeBreakpoint(location);
+  if (shouldWaitForWorkers) {
+    for (const thread of listWorkerThreadClients()) {
+      await thread.removeBreakpoint(location);
+    }
+  } else {
+    for (const thread of listWorkerThreadClients()) {
+      thread.removeBreakpoint(location);
+    }
   }
 }
 
 async function evaluateInFrame(script: Script, options: EvaluateParam) {
   return evaluate(script, options);
 }
 
 async function evaluateExpressions(scripts: Script[], options: EvaluateParam) {
@@ -444,12 +466,13 @@ const clientCommands = {
   getFrameScopes,
   pauseOnExceptions,
   fetchSources,
   registerSourceActor,
   fetchWorkers,
   getMainThread,
   sendPacket,
   setSkipPausing,
-  setEventListenerBreakpoints
+  setEventListenerBreakpoints,
+  waitForWorkers
 };
 
 export { setupCommands, clientCommands };
--- a/devtools/client/debugger/new/test/mochitest/browser_dbg-windowless-workers-early-breakpoint.js
+++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-windowless-workers-early-breakpoint.js
@@ -4,16 +4,21 @@
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 // Test that breakpoints at worker startup are hit when using windowless workers.
 add_task(async function() {
   const dbg = await initDebugger("doc-windowless-workers-early-breakpoint.html", "simple-worker.js");
 
   const workerSource = findSource(dbg, "simple-worker.js");
 
+  // NOTE: by default we do not wait on worker
+  // commands to complete because the thread could be
+  // shutting down.
+  dbg.client.waitForWorkers(true);
+
   await addBreakpoint(dbg, workerSource, 1);
   invokeInTab("startWorker");
   await waitForPaused(dbg, "simple-worker.js");
 
   // We should be paused at the first line of simple-worker.js
   assertPausedAtSourceAndLine(dbg, workerSource.id, 1);
   await removeBreakpoint(dbg, workerSource.id, 1);
   await resume(dbg);
--- a/devtools/client/debugger/new/test/mochitest/browser_dbg-windowless-workers.js
+++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-windowless-workers.js
@@ -27,16 +27,21 @@ function getValue(dbg, index) {
 // Test basic windowless worker functionality: the main thread and worker can be
 // separately controlled from the same debugger.
 add_task(async function() {
   await pushPref("devtools.debugger.features.windowless-workers", true);
 
   const dbg = await initDebugger("doc-windowless-workers.html");
   const mainThread = dbg.toolbox.threadClient.actor;
 
+  // NOTE: by default we do not wait on worker
+  // commands to complete because the thread could be
+  // shutting down.
+  dbg.client.waitForWorkers(true);
+
   const workers = await getWorkers(dbg);
   ok(workers.length == 2, "Got two workers");
   const worker1Thread = workers[0].actor;
   const worker2Thread = workers[1].actor;
 
   const mainThreadSource = findSource(dbg, "doc-windowless-workers.html");
   const workerSource = findSource(dbg, "simple-worker.js");