Bug 1602734 - Pass one option object instead of arguments to watchTargets callbacks. r=jdescottes

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

--HG--
extra : moz-landing-system : lando

devtools/client/framework/toolbox.js

@@ -628,7 +628,7 @@ Toolbox.prototype = {
    * This method will be called for the top-level target, as well as any potential
    * additional targets we may care about.
    */
-  async _onTargetAvailable(type, targetFront, isTopLevel) {
+  async _onTargetAvailable({ type, targetFront, isTopLevel }) {
     if (isTopLevel) {
       // Attach to a new top-level target.
       // For now, register these event listeners only on the top level target
@@ -648,7 +648,7 @@ Toolbox.prototype = {
     }
   },
 
-  _onTargetDestroyed(type, targetFront, isTopLevel) {
+  _onTargetDestroyed({ type, targetFront, isTopLevel }) {
     if (isTopLevel) {
       this.detachTarget();
     } else {

devtools/client/inspector/changes/ChangesView.js

@@ -115,7 +115,7 @@ class ChangesView {
     changesFront.off("clear-changes", this.onClearChanges);
   }
 
-  async onTargetAvailable(type, targetFront, isTopLevel) {
+  async onTargetAvailable({ type, targetFront, isTopLevel }) {
     targetFront.watchFronts(
       "changes",
       this.onChangesFrontAvailable,
@@ -127,7 +127,7 @@ class ChangesView {
     }
   }
 
-  async onTargetDestroyed(type, targetFront, isTopLevel) {
+  async onTargetDestroyed({ type, targetFront, isTopLevel }) {
     targetFront.unwatchFronts(
       "changes",
       this.onChangesFrontAvailable,

devtools/client/inspector/inspector.js

@@ -249,7 +249,7 @@ Inspector.prototype = {
     return this._deferredOpen();
   },
 
-  async _onTargetAvailable(type, targetFront, isTopLevel) {
+  async _onTargetAvailable({ type, targetFront, isTopLevel }) {
     // Ignore all targets but the top level one
     if (!isTopLevel) {
       return;
@@ -283,7 +283,7 @@ Inspector.prototype = {
     }
   },
 
-  _onTargetDestroyed(type, targetFront, isTopLevel) {
+  _onTargetDestroyed({ type, targetFront, isTopLevel }) {
     // Ignore all targets but the top level one
     if (!isTopLevel) {
       return;

devtools/client/inspector/shared/walker-event-listener.js

@@ -41,11 +41,10 @@ class WalkerEventListener {
     const targets = this._inspector.toolbox.targetList.getAllTargets(
       this._inspector.toolbox.targetList.TYPES.FRAME
     );
-    for (const target of targets) {
-      this._onTargetDestroyed(
-        this._inspector.toolbox.targetList.TYPES.FRAME,
-        target
-      );
+    for (const targetFront of targets) {
+      this._onTargetDestroyed({
+        targetFront,
+      });
     }
 
     this._inspector = null;
@@ -60,7 +59,7 @@ class WalkerEventListener {
     );
   }
 
-  async _onTargetAvailable(type, targetFront, isTopLevel) {
+  async _onTargetAvailable({ type, targetFront, isTopLevel }) {
     const inspectorFront = await targetFront.getFront("inspector");
     const { walker } = inspectorFront;
     for (const [name, listener] of Object.entries(this._listenerMap)) {
@@ -68,7 +67,7 @@ class WalkerEventListener {
     }
   }
 
-  _onTargetDestroyed(type, targetFront, isTopLevel) {
+  _onTargetDestroyed({ type, targetFront, isTopLevel }) {
     const inspectorFront = targetFront.getCachedFront("inspector");
     if (inspectorFront) {
       const { walker } = inspectorFront;

devtools/client/webconsole/webconsole-ui.js

@@ -346,7 +346,7 @@ class WebConsoleUI {
    *        to an URL which has to be loaded in a distinct process.
    *        A new top level target is created.
    */
-  async _onTargetAvailable(type, targetFront, isTopLevel) {
+  async _onTargetAvailable({ type, targetFront, isTopLevel }) {
     // This is a top level target. It may update on process switches
     // when navigating to another domain.
     if (isTopLevel) {
@@ -379,7 +379,7 @@ class WebConsoleUI {
    * @private
    * See _onTargetAvailable for param's description.
    */
-  _onTargetDestroyed(type, targetFront, isTopLevel) {
+  _onTargetDestroyed({ type, targetFront, isTopLevel }) {
     if (isTopLevel) {
       this.proxy.disconnect();
       this.proxy = null;

devtools/shared/resources/target-list.js

@@ -248,22 +248,20 @@ class TargetList {
     const targetType = this._getTargetType(targetFront);
 
     // Notify the target front creation listeners
-    this._createListeners.emit(
-      targetType,
-      targetType,
+    this._createListeners.emit(targetType, {
+      type: targetType,
       targetFront,
-      targetFront == this.targetFront
-    );
+      isTopLevel: targetFront == this.targetFront,
+    });
   }
 
   _onTargetDestroyed(targetFront) {
     const targetType = this._getTargetType(targetFront);
-    this._destroyListeners.emit(
-      targetType,
-      targetType,
+    this._destroyListeners.emit(targetType, {
+      type: targetType,
       targetFront,
-      targetFront == this.targetFront
-    );
+      isTopLevel: targetFront == this.targetFront,
+    });
     this._targets.delete(targetFront);
   }
 
@@ -356,7 +354,7 @@ class TargetList {
    *        Callback fired when a target has been just created or was already available.
    *        The function is called with three arguments:
    *        - {String} type: The target type
-   *        - {TargetFront} target: The target Front
+   *        - {TargetFront} targetFront: The target Front
    *        - {Boolean} isTopLevel: Is this target the top level one?
    * @param {Function} onDestroy
    *        Callback fired in case of target front destruction.
@@ -377,13 +375,17 @@ class TargetList {
       }
 
       // Notify about already existing target of these types
-      for (const target of this._targets) {
-        if (this._matchTargetType(type, target)) {
+      for (const targetFront of this._targets) {
+        if (this._matchTargetType(type, targetFront)) {
           try {
             // Ensure waiting for eventual async create listeners
             // which may setup things regarding the existing targets
             // and listen callsite may care about the full initialization
-            await onAvailable(type, target, target == this.targetFront);
+            await onAvailable({
+              type,
+              targetFront,
+              isTopLevel: targetFront == this.targetFront,
+            });
           } catch (e) {
             // Prevent throwing when onAvailable handler throws on one target
             // so that it can try to register the other targets

devtools/shared/resources/tests/browser_target_list_frames.js

@@ -56,17 +56,17 @@ async function testBrowserFrames(mainRoot) {
 
   // Assert that watchTargets will call the create callback for all existing frames
   const targets = [];
-  const onAvailable = (type, newTarget, isTopLevel) => {
+  const onAvailable = ({ type, targetFront, isTopLevel }) => {
     is(
       type,
       TargetList.TYPES.FRAME,
       "We are only notified about frame targets"
     );
     ok(
-      newTarget == target ? isTopLevel : !isTopLevel,
+      targetFront == target ? isTopLevel : !isTopLevel,
       "isTopLevel argument is correct"
     );
-    targets.push(newTarget);
+    targets.push(targetFront);
   };
   await targetList.watchTargets([TargetList.TYPES.FRAME], onAvailable);
   is(
@@ -129,17 +129,17 @@ async function testTabFrames(mainRoot) {
 
   // Assert that watchTargets will call the create callback for all existing frames
   const targets = [];
-  const onAvailable = (type, newTarget, isTopLevel) => {
+  const onAvailable = ({ type, targetFront, isTopLevel }) => {
     is(
       type,
       TargetList.TYPES.FRAME,
       "We are only notified about frame targets"
     );
     ok(
-      newTarget == target ? isTopLevel : !isTopLevel,
+      targetFront == target ? isTopLevel : !isTopLevel,
       "isTopLevel argument is correct"
     );
-    targets.push(newTarget);
+    targets.push(targetFront);
   };
   await targetList.watchTargets([TargetList.TYPES.FRAME], onAvailable);
   is(

devtools/shared/resources/tests/browser_target_list_preffedoff.js

@@ -53,22 +53,22 @@ async function testPreffedOffMainProcess(mainRoot, mainProcess) {
   );
 
   const processTargets = [];
-  const onProcessAvailable = (type, newTarget, isTopLevel) => {
-    processTargets.push(newTarget);
+  const onProcessAvailable = ({ type, targetFront, isTopLevel }) => {
+    processTargets.push(targetFront);
   };
   await targetList.watchTargets([TargetList.TYPES.PROCESS], onProcessAvailable);
   is(processTargets.length, 0, "We get no process when preffed-off");
   targetList.unwatchTargets([TargetList.TYPES.PROCESS], onProcessAvailable);
 
   const frameTargets = [];
-  const onFrameAvailable = (type, newTarget, isTopLevel) => {
+  const onFrameAvailable = ({ type, targetFront, isTopLevel }) => {
     is(
       type,
       TargetList.TYPES.FRAME,
       "We are only notified about frame targets"
     );
     ok(isTopLevel, "We are only notified about the top level target");
-    frameTargets.push(newTarget);
+    frameTargets.push(targetFront);
   };
   await targetList.watchTargets([TargetList.TYPES.FRAME], onFrameAvailable);
   is(
@@ -110,22 +110,22 @@ async function testPreffedOffTab(mainRoot) {
   is(frames[0], target, "The target is the top level one via getAllTargets");
 
   const processTargets = [];
-  const onProcessAvailable = newTarget => {
-    processTargets.push(newTarget);
+  const onProcessAvailable = ({ type, targetFront }) => {
+    processTargets.push(targetFront);
   };
   await targetList.watchTargets([TargetList.TYPES.PROCESS], onProcessAvailable);
   is(processTargets.length, 0, "We get no process when preffed-off");
   targetList.unwatchTargets([TargetList.TYPES.PROCESS], onProcessAvailable);
 
   const frameTargets = [];
-  const onFrameAvailable = (type, newTarget, isTopLevel) => {
+  const onFrameAvailable = ({ type, targetFront, isTopLevel }) => {
     is(
       type,
       TargetList.TYPES.FRAME,
       "We are only notified about frame targets"
     );
     ok(isTopLevel, "We are only notified about the top level target");
-    frameTargets.push(newTarget);
+    frameTargets.push(targetFront);
   };
   await targetList.watchTargets([TargetList.TYPES.FRAME], onFrameAvailable);
   is(

devtools/shared/resources/tests/browser_target_list_processes.js

@@ -61,8 +61,8 @@ async function testProcesses(targetList, target) {
 
   // Assert that watchTargets will call the create callback for all existing frames
   const targets = new Set();
-  const onAvailable = (type, newTarget, isTopLevel) => {
-    if (targets.has(newTarget)) {
+  const onAvailable = ({ type, targetFront, isTopLevel }) => {
+    if (targets.has(targetFront)) {
       ok(false, "The same target is notified multiple times via onAvailable");
     }
     is(
@@ -71,13 +71,13 @@ async function testProcesses(targetList, target) {
       "We are only notified about process targets"
     );
     ok(
-      newTarget == target ? isTopLevel : !isTopLevel,
+      targetFront == target ? isTopLevel : !isTopLevel,
       "isTopLevel argument is correct"
     );
-    targets.add(newTarget);
+    targets.add(targetFront);
   };
-  const onDestroyed = (type, newTarget, isTopLevel) => {
-    if (!targets.has(newTarget)) {
+  const onDestroyed = ({ type, targetFront, isTopLevel }) => {
+    if (!targets.has(targetFront)) {
       ok(
         false,
         "A target is declared destroyed via onDestroyed without being notified via onAvailable"
@@ -92,7 +92,7 @@ async function testProcesses(targetList, target) {
       !isTopLevel,
       "We are never notified about the top level target destruction"
     );
-    targets.delete(newTarget);
+    targets.delete(targetFront);
   };
   await targetList.watchTargets(
     [TargetList.TYPES.PROCESS],
@@ -114,12 +114,12 @@ async function testProcesses(targetList, target) {
   const previousTargets = new Set(targets);
   // Assert that onAvailable is called for processes created *after* the call to watchTargets
   const onProcessCreated = new Promise(resolve => {
-    const onAvailable2 = (type, newTarget, isTopLevel) => {
-      if (previousTargets.has(newTarget)) {
+    const onAvailable2 = ({ type, targetFront, isTopLevel }) => {
+      if (previousTargets.has(targetFront)) {
         return;
       }
       targetList.unwatchTargets([TargetList.TYPES.PROCESS], onAvailable2);
-      resolve(newTarget);
+      resolve(targetFront);
     };
     targetList.watchTargets([TargetList.TYPES.PROCESS], onAvailable2);
   });
@@ -138,8 +138,8 @@ async function testProcesses(targetList, target) {
   // Assert that onDestroyed is called for destroyed processes
   const onProcessDestroyed = new Promise(resolve => {
     const onAvailable3 = () => {};
-    const onDestroyed3 = (type, newTarget, isTopLevel) => {
-      resolve(newTarget);
+    const onDestroyed3 = ({ type, targetFront, isTopLevel }) => {
+      resolve(targetFront);
       targetList.unwatchTargets(
         [TargetList.TYPES.PROCESS],
         onAvailable3,

devtools/shared/resources/tests/browser_target_list_switchToTarget.js

@@ -46,17 +46,17 @@ async function testSwitchToTarget(client) {
   const secondTarget = await mainRoot.getTab({ tab: gBrowser.selectedTab });
 
   const frameTargets = [];
-  const onFrameAvailable = (type, newTarget, isTopLevel) => {
+  const onFrameAvailable = ({ type, targetFront, isTopLevel }) => {
     is(
       type,
       TargetList.TYPES.FRAME,
       "We are only notified about frame targets"
     );
     ok(
-      newTarget == target ? isTopLevel : !isTopLevel,
+      targetFront == target ? isTopLevel : !isTopLevel,
       "isTopLevel argument is correct"
     );
-    frameTargets.push(newTarget);
+    frameTargets.push(targetFront);
   };
   await targetList.watchTargets([TargetList.TYPES.FRAME], onFrameAvailable);
 

devtools/shared/resources/tests/browser_target_list_watchTargets.js

@@ -44,8 +44,8 @@ async function testWatchTargets(mainRoot) {
     "Check that onAvailable is called for processes already created *before* the call to watchTargets"
   );
   const targets = new Set();
-  const onAvailable = (type, newTarget, isTopLevel) => {
-    if (targets.has(newTarget)) {
+  const onAvailable = ({ type, targetFront, isTopLevel }) => {
+    if (targets.has(targetFront)) {
       ok(false, "The same target is notified multiple times via onAvailable");
     }
     is(
@@ -54,13 +54,13 @@ async function testWatchTargets(mainRoot) {
       "We are only notified about process targets"
     );
     ok(
-      newTarget == target ? isTopLevel : !isTopLevel,
+      targetFront == target ? isTopLevel : !isTopLevel,
       "isTopLevel argument is correct"
     );
-    targets.add(newTarget);
+    targets.add(targetFront);
   };
-  const onDestroyed = (type, newTarget, isTopLevel) => {
-    if (!targets.has(newTarget)) {
+  const onDestroyed = ({ type, targetFront, isTopLevel }) => {
+    if (!targets.has(targetFront)) {
       ok(
         false,
         "A target is declared destroyed via onDestroyed without being notified via onAvailable"
@@ -75,7 +75,7 @@ async function testWatchTargets(mainRoot) {
       !isTopLevel,
       "We are not notified about the top level target destruction"
     );
-    targets.delete(newTarget);
+    targets.delete(targetFront);
   };
   await targetList.watchTargets(
     [TargetList.TYPES.PROCESS],
@@ -104,12 +104,12 @@ async function testWatchTargets(mainRoot) {
   );
   const previousTargets = new Set(targets);
   const onProcessCreated = new Promise(resolve => {
-    const onAvailable2 = (type, newTarget, isTopLevel) => {
-      if (previousTargets.has(newTarget)) {
+    const onAvailable2 = ({ type, targetFront, isTopLevel }) => {
+      if (previousTargets.has(targetFront)) {
         return;
       }
       targetList.unwatchTargets([TargetList.TYPES.PROCESS], onAvailable2);
-      resolve(newTarget);
+      resolve(targetFront);
     };
     targetList.watchTargets([TargetList.TYPES.PROCESS], onAvailable2);
   });
@@ -129,8 +129,8 @@ async function testWatchTargets(mainRoot) {
   // Assert that onDestroyed is called for destroyed processes
   const onProcessDestroyed = new Promise(resolve => {
     const onAvailable3 = () => {};
-    const onDestroyed3 = (type, newTarget, isTopLevel) => {
-      resolve(newTarget);
+    const onDestroyed3 = ({ type, targetFront, isTopLevel }) => {
+      resolve(targetFront);
       targetList.unwatchTargets(
         [TargetList.TYPES.PROCESS],
         onAvailable3,
@@ -184,8 +184,8 @@ async function testContentProcessTarget(mainRoot) {
   // as listening for additional target is only enable for the parent process target.
   // See bug 1593928.
   const targets = new Set();
-  const onAvailable = (type, newTarget, isTopLevel) => {
-    if (targets.has(newTarget)) {
+  const onAvailable = ({ type, targetFront, isTopLevel }) => {
+    if (targets.has(targetFront)) {
       // This may fail if the top level target is reported by LegacyImplementation
       // to TargetList and emits an available event for it.
       ok(false, "The same target is notified multiple times via onAvailable");
@@ -195,11 +195,11 @@ async function testContentProcessTarget(mainRoot) {
       TargetList.TYPES.PROCESS,
       "We are only notified about process targets"
     );
-    is(newTarget, target, "This is the existing top level target");
+    is(targetFront, target, "This is the existing top level target");
     ok(isTopLevel, "We are only notified about the top level target");
-    targets.add(newTarget);
+    targets.add(targetFront);
   };
-  const onDestroyed = (type, newTarget, isTopLevel) => {
+  const onDestroyed = ({ type, targetFront, isTopLevel }) => {
     ok(false, "onDestroyed should never be called in this test");
   };
   await targetList.watchTargets(
@@ -233,12 +233,12 @@ async function testThrowingInOnAvailable(mainRoot) {
   );
   const targets = new Set();
   let thrown = false;
-  const onAvailable = (type, newTarget, isTopLevel) => {
+  const onAvailable = ({ type, targetFront, isTopLevel }) => {
     if (!thrown) {
       thrown = true;
       throw new Error("Force an exception when processing the first target");
     }
-    targets.add(newTarget);
+    targets.add(targetFront);
   };
   await targetList.watchTargets([TargetList.TYPES.PROCESS], onAvailable);
   is(