Bug 1529680 - [release 127] [Breakpoints] Only wait for the main thread to set/clear breakpoints. (#7996). r=dwalsh
authorBrian Hackett <bhackett1024@gmail.com>
Mon, 25 Feb 2019 10:00:16 -0500
changeset 518965 049f4a9c8486e9b9bd8cb54513688bd86abb2084
parent 518964 71adfc4d2e58ba2fc20dbece687005e7f6dad441
child 518966 183f1a860db46789f2bda30951c61e49c01cb9f6
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)
reviewersdwalsh
bugs1529680
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 1529680 - [release 127] [Breakpoints] Only wait for the main thread to set/clear breakpoints. (#7996). r=dwalsh
devtools/client/debugger/new/src/client/firefox/commands.js
--- a/devtools/client/debugger/new/src/client/firefox/commands.js
+++ b/devtools/client/debugger/new/src/client/firefox/commands.js
@@ -161,37 +161,42 @@ 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* getAllThreadClients() {
-  yield threadClient;
-  for (const { thread } of (Object.values(workerClients): any)) {
-    yield thread;
-  }
-}
-
 async function setBreakpoint(
   location: BreakpointLocation,
   options: BreakpointOptions
 ) {
   breakpoints[locationKey(location)] = { location, options };
-  for (const thread of getAllThreadClients()) {
-    await thread.setBreakpoint(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);
   }
 }
 
 async function removeBreakpoint(location: BreakpointLocation) {
   delete breakpoints[locationKey(location)];
-  for (const thread of getAllThreadClients()) {
-    await thread.removeBreakpoint(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);
   }
 }
 
 async function evaluateInFrame(script: Script, options: EvaluateParam) {
   return evaluate(script, options);
 }
 
 async function evaluateExpressions(scripts: Script[], options: EvaluateParam) {