Bug 1619622 - Update target/thread individually from the TargetList listeners. r=jdescottes,jlast

Differential Revision: https://phabricator.services.mozilla.com/D72685

devtools/client/debugger/src/actions/threads.js

@@ -5,6 +5,8 @@
 // @flow
 
 import { differenceBy } from "lodash";
+import type { Target } from "../client/firefox/types";
+import type { Thread, ThreadList, ActorId } from "../types";
 import type { Action, ThunkArgs } from "./types";
 import { removeSourceActors } from "./source-actors";
 import { newGeneratedSources } from "./sources";
@@ -16,10 +18,66 @@ import {
   getSourceActorsForThread,
 } from "../selectors";
 
-import type { ActorId } from "../types";
+function addThreads(
+  { dispatch, client, getState }: ThunkArgs,
+  addedThreads: ThreadList
+) {
+  const cx = getContext(getState());
+  dispatch(({ type: "INSERT_THREADS", cx, threads: addedThreads }: Action));
+
+  // Fetch the sources and install breakpoints on any new workers.
+  // NOTE: This runs in the background and fails quietly because it is
+  // pretty easy for sources to throw during the fetch if their thread
+  // shuts down, which would cause test failures.
+  for (const thread of addedThreads) {
+    client
+      .fetchThreadSources(thread.actor)
+      .then(sources => dispatch(newGeneratedSources(sources)))
+      .catch(e => console.error(e));
+  }
+}
+
+function removeThreads(
+  { dispatch, client, getState }: ThunkArgs,
+  removedThreads: ThreadList
+) {
+  const cx = getContext(getState());
+  const sourceActors = getSourceActorsForThread(
+    getState(),
+    removedThreads.map(t => t.actor)
+  );
+  dispatch(removeSourceActors(sourceActors));
+  dispatch(
+    ({
+      type: "REMOVE_THREADS",
+      cx,
+      threads: removedThreads.map(t => t.actor),
+    }: Action)
+  );
+}
+
+export function addTarget(targetFront: Target) {
+  return async function(args: ThunkArgs) {
+    const { client } = args;
+    const thread = await client.attachThread(targetFront);
+    return addThreads(args, [thread]);
+  };
+}
+export function removeTarget(targetFront: Target) {
+  return async function(args: ThunkArgs) {
+    const { getState } = args;
+    const currentThreads = getThreads(getState());
+    const { actorID } = targetFront.threadFront;
+    const thread: void | Thread = currentThreads.find(t => t.actor == actorID);
+    if (thread) {
+      return removeThreads(args, [thread]);
+    }
+  };
+}
 
 export function updateThreads() {
-  return async function({ dispatch, getState, client }: ThunkArgs) {
+  return async function(args: ThunkArgs) {
+    const { dispatch, getState, client } = args;
     const cx = getContext(getState());
     const threads = await client.fetchThreads();
 
@@ -28,32 +86,10 @@ export function updateThreads() {
     const addedThreads = differenceBy(threads, currentThreads, t => t.actor);
     const removedThreads = differenceBy(currentThreads, threads, t => t.actor);
     if (removedThreads.length > 0) {
-      const sourceActors = getSourceActorsForThread(
-        getState(),
-        removedThreads.map(t => t.actor)
-      );
-      dispatch(removeSourceActors(sourceActors));
-      dispatch(
-        ({
-          type: "REMOVE_THREADS",
-          cx,
-          threads: removedThreads.map(t => t.actor),
-        }: Action)
-      );
+      removeThreads(args, removedThreads);
     }
     if (addedThreads.length > 0) {
-      dispatch(({ type: "INSERT_THREADS", cx, threads: addedThreads }: Action));
-
-      // Fetch the sources and install breakpoints on any new workers.
-      // NOTE: This runs in the background and fails quietly because it is
-      // pretty easy for sources to throw during the fetch if their thread
-      // shuts down, which would cause test failures.
-      for (const thread of addedThreads) {
-        client
-          .fetchThreadSources(thread.actor)
-          .then(sources => dispatch(newGeneratedSources(sources)))
-          .catch(e => console.error(e));
-      }
+      await addThreads(args, addedThreads);
     }
 
     // Update the status of any service workers.

devtools/client/debugger/src/client/firefox.js

@@ -97,11 +97,9 @@ async function onTargetAvailable({
   const sources = await clientCommands.fetchSources();
   await actions.newGeneratedSources(sources);
 
-  await clientCommands.checkIfAlreadyPaused();
+  await actions.addTarget(targetFront);
 
-  // TODO: optimize the thread updates to only update according to what changed
-  // i.e. just about this one target
-  await actions.updateThreads();
+  await clientCommands.checkIfAlreadyPaused();
 }
 
 function onTargetDestroyed({ targetFront, isTopLevel }): void {
@@ -110,9 +108,7 @@ function onTargetDestroyed({ targetFront, isTopLevel }): void {
     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();
+  actions.removeTarget(targetFront);
 }
 
 export { clientCommands, clientEvents };

devtools/client/debugger/src/client/firefox/commands.js

@@ -5,7 +5,7 @@
 // @flow
 
 import { prepareSourcePayload, createThread, createFrame } from "./create";
-import { updateTargets } from "./targets";
+import { updateTargets, attachTarget } from "./targets";
 import { clientEvents } from "./events";
 
 import Reps from "devtools-reps";
@@ -470,6 +470,19 @@ async function fetchThreads() {
   );
 }
 
+async function attachThread(targetFront: Target) {
+  const options = {
+    breakpoints,
+    eventBreakpoints,
+    observeAsmJS: true,
+  };
+
+  await attachTarget(targetFront, targets, options);
+  const threadFront: ThreadFront = await targetFront.getFront("thread");
+
+  return createThread(threadFront.actorID, targetFront);
+}
+
 function getMainThread() {
   return currentThreadFront().actor;
 }
@@ -557,6 +570,7 @@ const clientCommands = {
   checkIfAlreadyPaused,
   registerSourceActor,
   fetchThreads,
+  attachThread,
   getMainThread,
   sendPacket,
   setSkipPausing,

devtools/client/debugger/src/client/firefox/events.js

@@ -68,7 +68,6 @@ function setupEvents(dependencies: Dependencies): void {
 
 function setupEventsTopTarget(targetFront: Target): void {
   targetFront.on("workerListChanged", threadListChanged);
-  addThreadEventListeners(targetFront.threadFront);
 
   if (features.windowlessServiceWorkers || attachAllTargets(targetFront)) {
     workersListener.addListener(threadListChanged);

devtools/client/debugger/src/client/firefox/targets.js

@@ -19,49 +19,94 @@ type Args = {
   targetList: TargetList,
 };
 
+/**
+ * Ensure attaching to the given Target.
+ * The two following actions are done only for workers. For other targets,
+ * the toolbox is already doing that.
+ *  - Attach the Target
+ *  - Attach the Thread = instantiate the Thread actor and register breakpoints.
+ *                        Note that, for now, the Toolbox don't have access to breakpoints
+ *                        and so, we have to register them somewhere else.
+ *                        This might be done via onConnect and syncBreakpoint.
+ *
+ *  - Register the Thread Actor ID into `targets` arguments, which collect
+ *  all Thread Fronts instances keyed by their actor ID.
+ *  - Listen to all meaningful RDP event sent by the Thread Actor.
+ *
+ * @param {Target} targetFront
+ *        The target to attach.
+ * @param {Object} targets
+ *        An object of Target Fronts keyed by their actor ID.
+ * @param {Object} options
+ *        Thread Actor options to provide to the actor when attaching to worker targets.
+ *        Typically includes breakpoints and breaking options.
+ */
+export async function attachTarget(
+  targetFront: Target,
+  targets: { [string]: Target },
+  options: Object
+) {
+  try {
+    // For all targets but workers, this will already be done by the Toolbox onTargetAvailable function.
+    await targetFront.attach();
+
+    const threadActorID = targetFront.targetForm.threadActor;
+    if (targets[threadActorID]) {
+      return;
+    }
+    targets[threadActorID] = targetFront;
+
+    // Content process targets have already been attached by the toolbox.
+    // And the thread front has been initialized from there.
+    // So we only need to retrieve it here.
+    let threadFront = targetFront.threadFront;
+
+    // But workers targets are still only managed by the debugger codebase
+    // and so we have to attach their thread actor
+    if (!threadFront) {
+      threadFront = await targetFront.attachThread({
+        ...defaultThreadOptions(),
+        ...options,
+      });
+      // NOTE: resume is not necessary for ProcessDescriptors and can be removed
+      // once we switch to WorkerDescriptors
+      threadFront.resume();
+    }
+
+    addThreadEventListeners(threadFront);
+  } catch (e) {
+    // If any of the workers have terminated since the list command initiated
+    // then we will get errors. Ignore these.
+  }
+}
+
+/**
+ * Process a new list of targets to attach to.
+ *
+ * @param {Array<Target>} targetLists
+ *        List of all targets we should be attached to.
+ *        Any targets which is not on this list will be "detached" and stop being used
+ *        Any new targets, which wasn't already passed to a previous call to this method
+ *        will be attached and start being watched by the Debugger.
+ * @param {Args} args
+ *        Environment object.
+ */
 async function attachTargets(targetLists, args): Promise<*> {
   const { targets } = args;
 
   targetLists = targetLists.filter(target => !!target);
 
+  // First remove from the known list, targets which are no longer in the new list
   for (const actor of Object.keys(targets)) {
     if (!targetLists.some(target => target.targetForm.threadActor == actor)) {
       delete targets[actor];
     }
   }
 
+  // Then attach on all of them. `attachTarget` will be a no-op if it was already
+  // attached.
   for (const targetFront of targetLists) {
-    try {
-      await targetFront.attach();
-
-      const threadActorID = targetFront.targetForm.threadActor;
-      if (targets[threadActorID]) {
-        continue;
-      }
-      targets[threadActorID] = targetFront;
-
-      // Content process targets have already been attached by the toolbox.
-      // And the thread front has been initialized from there.
-      // So we only need to retrieve it here.
-      let { threadFront } = targetFront;
-
-      // But workers targets are still only managed by the debugger codebase
-      // and so we have to attach their thread actor
-      if (!threadFront) {
-        threadFront = await targetFront.attachThread({
-          ...defaultThreadOptions(),
-          ...args.options,
-        });
-        // NOTE: resume is not necessary for ProcessDescriptors and can be removed
-        // once we switch to WorkerDescriptors
-        threadFront.resume();
-      }
-
-      addThreadEventListeners(threadFront);
-    } catch (e) {
-      // If any of the workers have terminated since the list command initiated
-      // then we will get errors. Ignore these.
-    }
+    await attachTarget(targetFront, targets, args.options);
   }
 }
 
@@ -174,7 +219,8 @@ async function listProcessTargets(args: Args): Promise<*> {
 }
 
 export async function updateTargets(args: Args): Promise<*> {
+  const currentTopLevelTarget = args.targetList.targetFront;
   const workers = await listWorkerTargets(args);
   const processes = await listProcessTargets(args);
-  await attachTargets([...workers, ...processes], args);
+  await attachTargets([currentTopLevelTarget, ...workers, ...processes], args);
 }

devtools/client/debugger/src/client/firefox/types.js

@@ -13,9 +13,7 @@
 import type {
   BreakpointLocation,
   BreakpointOptions,
-  FrameId,
   ActorId,
-  Script,
   PendingLocation,
   SourceId,
   Range,
@@ -189,7 +187,7 @@ export type Target = {
   off: (string, Function) => void,
   on: (string, Function) => void,
   emit: (string, any) => void,
-  getFront: string => Promise<ConsoleFront>,
+  getFront: string => Promise<*>,
   form: { consoleActor: any },
   root: any,
   navigateTo: ({ url: URL }) => Promise<*>,
@@ -214,21 +212,6 @@ export type Target = {
   debuggerServiceWorkerStatus: string,
 };
 
-type ConsoleFront = {
-  evaluateJSAsync: (
-    script: Script,
-    func: Function,
-    params?: { frameActor: ?FrameId }
-  ) => Promise<{ result: ExpressionResult }>,
-  autocomplete: (
-    input: string,
-    cursor: number,
-    func: Function,
-    frameId: ?string
-  ) => void,
-  emit: (string, any) => void,
-};
-
 /**
  * Clients for accessing the Firefox debug server and browser
  * @memberof firefox/clients