Bug 1619622 - Use the TargetList to fetch content process targets in the Debugger. r=jdescottes,jlast
authorAlexandre Poirot <poirot.alex@gmail.com>
Wed, 13 May 2020 14:44:45 +0000
changeset 529676 dbb5588616815aca455bfbcd5ee817196df7a68b
parent 529675 ad16a3b7b0b5058a066a803c5b5e4e7ee3ace5c7
child 529677 d6a9cab6867ecdcc6126b0a54e257514832db069
push id37414
push usernbeleuzu@mozilla.com
push dateThu, 14 May 2020 02:40:10 +0000
treeherdermozilla-central@045d696faa87 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdescottes, jlast
bugs1619622
milestone78.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 1619622 - Use the TargetList to fetch content process targets in the Debugger. r=jdescottes,jlast Differential Revision: https://phabricator.services.mozilla.com/D65125
devtools/client/debugger/src/client/firefox.js
devtools/client/debugger/src/client/firefox/events.js
devtools/client/debugger/src/client/firefox/targets.js
--- a/devtools/client/debugger/src/client/firefox.js
+++ b/devtools/client/debugger/src/client/firefox.js
@@ -93,19 +93,26 @@ async function onTargetAvailable({
   // debugger finds them, but there may be existing sources already in
   // the debugger (if it's paused already, or if loading the page from
   // bfcache) so explicity fire `newSource` events for all returned
   // sources.
   const sources = await clientCommands.fetchSources();
   await actions.newGeneratedSources(sources);
 
   await clientCommands.checkIfAlreadyPaused();
+
+  // TODO: optimize the thread updates to only update according to what changed
+  // i.e. just about this one target
+  await actions.updateThreads();
 }
 
 function onTargetDestroyed({ targetFront, isTopLevel }): void {
   if (isTopLevel) {
     targetFront.off("will-navigate", actions.willNavigate);
     targetFront.off("navigate", actions.navigated);
     removeEventsTopTarget(targetFront);
   }
+  // TODO: optimize the thread updates to only update according to what changed
+  // i.e. just about this one target
+  actions.updateThreads();
 }
 
 export { clientCommands, clientEvents };
--- a/devtools/client/debugger/src/client/firefox/events.js
+++ b/devtools/client/debugger/src/client/firefox/events.js
@@ -56,18 +56,16 @@ function attachAllTargets(currentTarget:
   return prefs.fission && currentTarget.isParentProcess;
 }
 
 function setupEvents(dependencies: Dependencies): void {
   const { devToolsClient } = dependencies;
   actions = dependencies.actions;
   sourceQueue.initialize(actions);
 
-  devToolsClient.mainRoot.on("processListChanged", threadListChanged);
-
   workersListener = new WorkersListener(devToolsClient.mainRoot);
 
   threadFrontListeners = new WeakMap();
 }
 
 function setupEventsTopTarget(targetFront: Target): void {
   targetFront.on("workerListChanged", threadListChanged);
   addThreadEventListeners(targetFront.threadFront);
--- a/devtools/client/debugger/src/client/firefox/targets.js
+++ b/devtools/client/debugger/src/client/firefox/targets.js
@@ -93,16 +93,17 @@ async function listWorkerTargets(args: A
     if (currentTarget.url && features.windowlessServiceWorkers) {
       allWorkers = await devToolsClient.mainRoot.listAllWorkerTargets();
       const {
         registrations,
       } = await devToolsClient.mainRoot.listServiceWorkerRegistrations();
       serviceWorkerRegistrations = registrations.filter(front =>
         sameOrigin(front.url, currentTarget.url)
       );
+      await pauseMatchingServiceWorkers({ devToolsClient, currentTarget });
     }
   }
 
   for (const front of serviceWorkerRegistrations) {
     const {
       activeWorker,
       waitingWorker,
       installingWorker,
@@ -129,48 +130,51 @@ async function listWorkerTargets(args: A
     if (!workers.includes(worker)) {
       workers.push(worker);
     }
   }
 
   return workers;
 }
 
-async function getAllProcessTargets(args): Promise<*> {
-  const { devToolsClient } = args;
+// Request to all content process to eagerly pause SW matching current page origin
+async function pauseMatchingServiceWorkers({ devToolsClient, currentTarget }) {
+  // Service workers associated with our target's origin need to pause until
+  // we attach, regardless of which process they are running in.
+  const origin = new URL(currentTarget.url).origin;
+  // Still call `RootFront.listProcesses` instead of using the TargetList
+  // as the TargetList doesn't iterate over processes in the content toolbox.
+  // We do not care about doing anything with the Content process targets here,
+  // the only goal is to call `pauseMatchingServiceWorkers` in all processes.
+  // So that the server eagerly freeze SW until we attach to them.
   const processes = await devToolsClient.mainRoot.listProcesses();
-  return Promise.all(
+  const targets = await Promise.all(
     processes
       .filter(descriptor => !descriptor.isParent)
       .map(descriptor => descriptor.getTarget())
   );
+  try {
+    await Promise.all(
+      targets.map(t => t.pauseMatchingServiceWorkers({ origin }))
+    );
+  } catch (e) {
+    // currentTarget.url might not be a full URL, and old servers without
+    // pauseMatchingServiceWorkers will throw.
+    // Old servers without pauseMatchingServiceWorkers will throw.
+    // @backward-compatibility: remove in Firefox 75
+  }
 }
 
 async function listProcessTargets(args: Args): Promise<*> {
   const { targetList } = args;
-  const currentTarget = targetList.targetFront;
-  if (!attachAllTargets(currentTarget)) {
-    if (currentTarget.url && features.windowlessServiceWorkers) {
-      // Service workers associated with our target's origin need to pause until
-      // we attach, regardless of which process they are running in.
-      try {
-        const origin = new URL(currentTarget.url).origin;
-        const targets = await getAllProcessTargets(args);
-        await Promise.all(
-          targets.map(t => t.pauseMatchingServiceWorkers({ origin }))
-        );
-      } catch (e) {
-        // currentTarget.url might not be a full URL, and old servers without
-        // pauseMatchingServiceWorkers will throw.
-        // @backward-compatibility: remove in Firefox 75
-      }
-    }
-    return [];
-  }
-
-  return getAllProcessTargets(args);
+  // First note that the TargetList will only fetch processes following the same
+  // rules as `attachAllTargets`. Only if we are attached to the `ParentProcessTarget`
+  // and if the browser toolbox fission pref is turned on.
+  // Also note that the `ParentProcessTarget` actor is considered to be a FRAME and not a PROCESS.
+  // But this is ok, as we expect to return only content processes here.
+  return targetList.getAllTargets(targetList.TYPES.PROCESS);
 }
 
 export async function updateTargets(args: Args): Promise<*> {
   const workers = await listWorkerTargets(args);
   const processes = await listProcessTargets(args);
   await attachTargets([...workers, ...processes], args);
 }