diff --git a/devtools/client/debugger/src/client/firefox/commands.js b/devtools/client/debugger/src/client/firefox/commands.js index 44fbacda8461..8343a059d6e1 100644 --- a/devtools/client/debugger/src/client/firefox/commands.js +++ b/devtools/client/debugger/src/client/firefox/commands.js @@ -49,8 +49,6 @@ let breakpoints: { [string]: Object }; let eventBreakpoints: ?EventListenerActiveList; let supportsWasm: boolean; -let shouldWaitForWorkers = false; - type Dependencies = { threadFront: ThreadFront, tabTarget: TabTarget, @@ -121,15 +119,23 @@ function listWorkerThreadFronts() { return (Object.values(workerClients): any).map(({ thread }) => thread); } -function forEachWorkerThread(iteratee) { - const promises = listWorkerThreadFronts().map(thread => iteratee(thread)); - - // Do not return promises for the caller to wait on unless a flag is set. - // Currently, worker threads are not guaranteed to respond to all requests, - // if we send a request while they are shutting down. See bug 1529163. - if (shouldWaitForWorkers) { - return Promise.all(promises); - } +function forEachThread(iteratee) { + // We have to be careful here to atomically initiate the operation on every + // thread, with no intervening await. Otherwise, other code could run and + // trigger additional thread operations. Requests on server threads will + // resolve in FIFO order, and this could result in client and server state + // going out of sync. + const mainThreadPromise = iteratee(threadFront); + const workerPromises = listWorkerThreadFronts().map(t => { + try { + iteratee(t); + } catch (e) { + // If a worker thread shuts down while sending the message then it will + // throw. Ignore these errors. + console.error(e); + } + }); + return Promise.all([mainThreadPromise, ...workerPromises]); } function resume(thread: string): Promise<*> { @@ -186,10 +192,6 @@ function locationKey(location: BreakpointLocation) { return `${(sourceUrl: any)}:${(sourceId: any)}:${line}:${(column: any)}`; } -function waitForWorkers(shouldWait: boolean) { - shouldWaitForWorkers = shouldWait; -} - function detachWorkers() { for (const thread of listWorkerThreadFronts()) { thread.detach(); @@ -217,7 +219,7 @@ function hasBreakpoint(location: BreakpointLocation) { return !!breakpoints[locationKey(location)]; } -async function setBreakpoint( +function setBreakpoint( location: BreakpointLocation, options: BreakpointOptions ) { @@ -225,27 +227,14 @@ async function setBreakpoint( options = maybeGenerateLogGroupId(options); breakpoints[locationKey(location)] = { location, options }; - // We have to be careful here to atomically initiate the setBreakpoint() call - // on every thread, with no intervening await. Otherwise, other code could run - // and change or remove the breakpoint before we finish calling setBreakpoint - // on all threads. Requests on server threads will resolve in FIFO order, and - // this could result in the breakpoint state here being out of sync with the - // breakpoints that are installed in the server. - const mainThreadPromise = threadFront.setBreakpoint(location, options); - - await forEachWorkerThread(thread => thread.setBreakpoint(location, options)); - await mainThreadPromise; + return forEachThread(thread => thread.setBreakpoint(location, options)); } -async function removeBreakpoint(location: PendingLocation) { +function removeBreakpoint(location: PendingLocation) { maybeClearLogpoint((location: any)); delete breakpoints[locationKey((location: any))]; - // Delay waiting on this promise, for the same reason as in setBreakpoint. - const mainThreadPromise = threadFront.removeBreakpoint(location); - - await forEachWorkerThread(thread => thread.removeBreakpoint(location)); - await mainThreadPromise; + return forEachThread(thread => thread.removeBreakpoint(location)); } async function evaluateInFrame(script: Script, options: EvaluateParam) { @@ -325,20 +314,15 @@ async function getFrameScopes(frame: Frame): Promise<*> { return sourceThreadFront.getEnvironment(frame.id); } -async function pauseOnExceptions( +function pauseOnExceptions( shouldPauseOnExceptions: boolean, shouldPauseOnCaughtExceptions: boolean ): Promise<*> { - await threadFront.pauseOnExceptions( - shouldPauseOnExceptions, - // Providing opposite value because server - // uses "shouldIgnoreCaughtExceptions" - !shouldPauseOnCaughtExceptions - ); - - await forEachWorkerThread(thread => + return forEachThread(thread => thread.pauseOnExceptions( shouldPauseOnExceptions, + // Providing opposite value because server + // uses "shouldIgnoreCaughtExceptions" !shouldPauseOnCaughtExceptions ) ); @@ -357,20 +341,18 @@ async function blackBox( } } -async function setSkipPausing(shouldSkip: boolean) { - await threadFront.skipBreakpoints(shouldSkip); - await forEachWorkerThread(thread => thread.skipBreakpoints(shouldSkip)); +function setSkipPausing(shouldSkip: boolean) { + return forEachThread(thread => thread.skipBreakpoints(shouldSkip)); } function interrupt(thread: string): Promise<*> { return lookupThreadFront(thread).interrupt(); } -async function setEventListenerBreakpoints(ids: string[]) { +function setEventListenerBreakpoints(ids: string[]) { eventBreakpoints = ids; - await threadFront.setActiveEventBreakpoints(ids); - await forEachWorkerThread(thread => thread.setActiveEventBreakpoints(ids)); + return forEachThread(thread => thread.setActiveEventBreakpoints(ids)); } // eslint-disable-next-line @@ -563,7 +545,6 @@ const clientCommands = { setSkipPausing, setEventListenerBreakpoints, getEventListenerBreakpointTypes, - waitForWorkers, detachWorkers, hasWasmSupport, lookupConsoleClient, diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-windowless-workers-early-breakpoint.js b/devtools/client/debugger/test/mochitest/browser_dbg-windowless-workers-early-breakpoint.js index 8cad0c164ce8..551a5aa8a1c7 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-windowless-workers-early-breakpoint.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-windowless-workers-early-breakpoint.js @@ -9,11 +9,6 @@ add_task(async function() { 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 selectSource(dbg, "simple-worker.js"); await waitForSelectedSource(dbg, "simple-worker.js"); await addBreakpoint(dbg, workerSource, 1); diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-windowless-workers.js b/devtools/client/debugger/test/mochitest/browser_dbg-windowless-workers.js index 21d934a7f454..334e3035e19d 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-windowless-workers.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-windowless-workers.js @@ -44,11 +44,6 @@ add_task(async function() { const dbg = await initDebugger("doc-windowless-workers.html"); const mainThread = dbg.toolbox.threadFront.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 thread1 = workers[0].actor; diff --git a/devtools/client/debugger/test/mochitest/helpers.js b/devtools/client/debugger/test/mochitest/helpers.js index c2e792feb2ec..fc1ead5c5d51 100644 --- a/devtools/client/debugger/test/mochitest/helpers.js +++ b/devtools/client/debugger/test/mochitest/helpers.js @@ -531,7 +531,6 @@ async function initDebugger(url, ...sources) { await clearDebuggerPreferences(); const toolbox = await openNewTabAndToolbox(EXAMPLE_URL + url, "jsdebugger"); const dbg = createDebuggerContext(toolbox); - dbg.client.waitForWorkers(false); await waitForSources(dbg, ...sources); return dbg;