From 6c430374e4ab36fae7b9a44d47d0637fc6876e54 Mon Sep 17 00:00:00 2001 From: Alexandre Poirot Date: Wed, 8 Jul 2020 22:57:36 +0000 Subject: [PATCH] Bug 1649910 - Emit "switched-target" only after we fully attached to the new target. r=jdescottes The previous code, emitting the event from Toolbox.onTargetAvailable, wasn't waiting for the call to TargetList.startListening which is done from TargetList.onTargetAvailable. Differential Revision: https://phabricator.services.mozilla.com/D82664 --- .../client/dom/test/browser_dom_fission_target_switching.js | 2 +- .../framework/test/browser_toolbox_remoteness_change.js | 2 +- devtools/client/framework/toolbox.js | 2 -- .../test/browser/browser_memory_fission_switch_target.js | 2 +- devtools/client/shared/test/shared-head.js | 2 +- .../test/browser_styleeditor_fission_switch_target.js | 2 +- .../browser_webconsole_sandbox_update_after_navigation.js | 2 +- devtools/shared/resources/target-list.js | 6 +++++- 8 files changed, 11 insertions(+), 9 deletions(-) diff --git a/devtools/client/dom/test/browser_dom_fission_target_switching.js b/devtools/client/dom/test/browser_dom_fission_target_switching.js index b3e68d56334b..6d9359a79411 100644 --- a/devtools/client/dom/test/browser_dom_fission_target_switching.js +++ b/devtools/client/dom/test/browser_dom_fission_target_switching.js @@ -31,7 +31,7 @@ add_task(async function() { const onPropertiesFetched = waitForDispatch(panel, "FETCH_PROPERTIES"); // Also wait for the toolbox to switch to the new target, to avoid hanging requests when // the test ends. - const onTargetSwitched = toolbox.once("switched-target"); + const onTargetSwitched = toolbox.targetList.once("switched-target"); await toolbox.target.navigateTo({ url: CONTENT_PROCESS_URI }); diff --git a/devtools/client/framework/test/browser_toolbox_remoteness_change.js b/devtools/client/framework/test/browser_toolbox_remoteness_change.js index 9118e4b4fbf3..8d0407c1d39a 100644 --- a/devtools/client/framework/test/browser_toolbox_remoteness_change.js +++ b/devtools/client/framework/test/browser_toolbox_remoteness_change.js @@ -39,7 +39,7 @@ async function navigateBetweenProcesses(enableTargetSwitching) { const onToolboxDestroyed = toolbox.once("destroyed"); const onToolboxCreated = gDevTools.once("toolbox-created"); - const onToolboxSwitchedToTarget = toolbox.once("switched-target"); + const onToolboxSwitchedToTarget = toolbox.targetList.once("switched-target"); info("Navigate to a URL supporting remote process"); if (enableTargetSwitching) { diff --git a/devtools/client/framework/toolbox.js b/devtools/client/framework/toolbox.js index 4694a810467e..babfa20170cb 100644 --- a/devtools/client/framework/toolbox.js +++ b/devtools/client/framework/toolbox.js @@ -727,8 +727,6 @@ Toolbox.prototype = { // expect the target to be attached. await this._listFrames(); await this.initPerformance(); - - this.emit("switched-target", targetFront); } }, diff --git a/devtools/client/memory/test/browser/browser_memory_fission_switch_target.js b/devtools/client/memory/test/browser/browser_memory_fission_switch_target.js index dc349705275a..1c22fbfd8b0a 100644 --- a/devtools/client/memory/test/browser/browser_memory_fission_switch_target.js +++ b/devtools/client/memory/test/browser/browser_memory_fission_switch_target.js @@ -80,7 +80,7 @@ function getNodeNames(snapshot) { } async function navigateTo(uri, toolbox, tab) { - const onSwitched = once(toolbox, "switched-target"); + const onSwitched = toolbox.targetList.once("switched-target"); const onLoaded = BrowserTestUtils.browserLoaded(tab.linkedBrowser); await BrowserTestUtils.loadURI(tab.linkedBrowser, uri); await onLoaded; diff --git a/devtools/client/shared/test/shared-head.js b/devtools/client/shared/test/shared-head.js index a5e1cef375e9..158d693d99de 100644 --- a/devtools/client/shared/test/shared-head.js +++ b/devtools/client/shared/test/shared-head.js @@ -409,7 +409,7 @@ async function navigateTo(uri, { isErrorPage = false } = {}) { // Navigating from/to pages loaded in the parent process, like about:robots, // also spawn new targets. // (If target-switching pref is false, the toolbox will reboot) - const onTargetSwitched = toolbox.once("switched-target"); + const onTargetSwitched = toolbox.targetList.once("switched-target"); // Otherwise, if we don't switch target, it is safe to wait for navigate event. const onNavigate = target.once("navigate"); diff --git a/devtools/client/styleeditor/test/browser_styleeditor_fission_switch_target.js b/devtools/client/styleeditor/test/browser_styleeditor_fission_switch_target.js index ad6d447a8d74..93dacdd149c6 100644 --- a/devtools/client/styleeditor/test/browser_styleeditor_fission_switch_target.js +++ b/devtools/client/styleeditor/test/browser_styleeditor_fission_switch_target.js @@ -21,7 +21,7 @@ add_task(async function() { info("Navigate to a page that runs in the child process"); const onEditorReady = ui.editors[0].getSourceEditor(); - const onTargetSwitched = toolbox.once("switched-target"); + const onTargetSwitched = toolbox.targetList.once("switched-target"); await navigateToAndWaitForStyleSheets(CONTENT_PROCESS_URI, ui); // We also have to wait for the toolbox to complete the target switching // in order to avoid pending requests during test teardown. diff --git a/devtools/client/webconsole/test/browser/browser_webconsole_sandbox_update_after_navigation.js b/devtools/client/webconsole/test/browser/browser_webconsole_sandbox_update_after_navigation.js index e2818a3310d2..3ed76b2e576d 100644 --- a/devtools/client/webconsole/test/browser/browser_webconsole_sandbox_update_after_navigation.js +++ b/devtools/client/webconsole/test/browser/browser_webconsole_sandbox_update_after_navigation.js @@ -46,7 +46,7 @@ add_task(async function() { // 1270234). const promises = [hud.ui.once("messages-cleared")]; if (isTargetSwitchingEnabled()) { - promises.push(hud.toolbox.once("switched-target")); + promises.push(hud.targetList.once("switched-target")); } gBrowser.goBack(); diff --git a/devtools/shared/resources/target-list.js b/devtools/shared/resources/target-list.js index ee2be64204c8..08b3a7d41e48 100644 --- a/devtools/shared/resources/target-list.js +++ b/devtools/shared/resources/target-list.js @@ -33,7 +33,7 @@ loader.lazyRequireGetter( true ); -class TargetList { +class TargetList extends EventEmitter { /** * This class helps managing, iterating over and listening for Targets. * @@ -53,6 +53,8 @@ class TargetList { * this may be replaced by a new one over time. */ constructor(rootFront, targetFront) { + super(); + this.rootFront = rootFront; // Once we have descriptor for all targets we create a toolbox for, @@ -506,6 +508,8 @@ class TargetList { // Notify about this new target to creation listeners await this._onTargetAvailable(newTarget, true); + + this.emit("switched-target", newTarget); } isTargetRegistered(targetFront) {