diff --git a/devtools/client/responsive/components/App.js b/devtools/client/responsive/components/App.js index 42b8f82e26fc..da9bec0527d7 100644 --- a/devtools/client/responsive/components/App.js +++ b/devtools/client/responsive/components/App.js @@ -78,7 +78,6 @@ class App extends PureComponent { super(props); this.onAddCustomDevice = this.onAddCustomDevice.bind(this); - this.onBrowserContextMenu = this.onBrowserContextMenu.bind(this); this.onChangeDevice = this.onChangeDevice.bind(this); this.onChangeNetworkThrottling = this.onChangeNetworkThrottling.bind(this); this.onChangePixelRatio = this.onChangePixelRatio.bind(this); @@ -103,21 +102,10 @@ class App extends PureComponent { this.onUpdateDeviceModal = this.onUpdateDeviceModal.bind(this); } - componentWillUnmount() { - this.browser.removeEventListener("contextmenu", this.onContextMenu); - this.browser = null; - } - onAddCustomDevice(device) { this.props.dispatch(addCustomDevice(device)); } - onBrowserContextMenu() { - // Update the position of remote browser so that makes the context menu to show at - // proper position before showing. - this.browser.frameLoader.requestUpdatePosition(); - } - onChangeDevice(id, device, deviceType) { // Resize the viewport first. this.doResizeViewport(id, device.width, device.height); diff --git a/devtools/client/responsive/index.js b/devtools/client/responsive/index.js index 5df50fa665b8..84bdf16e12fb 100644 --- a/devtools/client/responsive/index.js +++ b/devtools/client/responsive/index.js @@ -73,6 +73,9 @@ const bootstrap = { destroy() { window.removeEventListener("unload", this.destroy, { once: true }); + // unmount to stop async action and renders after destroy + ReactDOM.unmountComponentAtNode(this._root); + this.store = null; this.telemetry.toolClosed("responsive", this); diff --git a/devtools/client/responsive/manager.js b/devtools/client/responsive/manager.js index 04657302ab8d..2491b7c67ff4 100644 --- a/devtools/client/responsive/manager.js +++ b/devtools/client/responsive/manager.js @@ -124,8 +124,6 @@ class ResponsiveUIManager { */ async openIfNeeded(window, tab, options = {}) { if (!this.isActiveForTab(tab)) { - await gDevToolsBrowser.loadBrowserStyleSheet(window); - this.initMenuCheckListenerFor(window); const ui = new ResponsiveUI(this, window, tab); @@ -134,6 +132,7 @@ class ResponsiveUIManager { // Explicitly not await on telemetry to avoid delaying RDM opening this.recordTelemetryOpen(window, tab, options); + await gDevToolsBrowser.loadBrowserStyleSheet(window); await this.setMenuCheckFor(tab, window); await ui.inited; this.emit("on", { tab }); diff --git a/devtools/client/responsive/test/browser/browser.toml b/devtools/client/responsive/test/browser/browser.toml index cbc5d211fb59..d71267d360a0 100644 --- a/devtools/client/responsive/test/browser/browser.toml +++ b/devtools/client/responsive/test/browser/browser.toml @@ -73,6 +73,9 @@ tags = "devtools webextensions" ["browser_in_rdm_pane.js"] +["browser_many_toggles.js"] +skip-if = ["verify"] # Too many exceptions happening on test teardown + ["browser_max_touchpoints.js"] ["browser_menu_item_01.js"] diff --git a/devtools/client/responsive/test/browser/browser_many_toggles.js b/devtools/client/responsive/test/browser/browser_many_toggles.js new file mode 100644 index 000000000000..a0fe19dba4eb --- /dev/null +++ b/devtools/client/responsive/test/browser/browser_many_toggles.js @@ -0,0 +1,51 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// Verify RDM can be toggled many times in a raw + +const TEST_URL = "https://example.com/"; + +addRDMTask( + null, + async function () { + const tab = await addTab(TEST_URL); + + // Record all currently active RDMs, there may not be one created on each loop + let opened = 0; + ResponsiveUIManager.on("on", () => opened++); + ResponsiveUIManager.on("off", () => opened--); + + for (let i = 0; i < 10; i++) { + info(`Toggling RDM #${i + 1}`); + // This may throw when we were just closing is still ongoing, + // ignore any exception. + openRDM(tab).catch(e => {}); + // Sometime pause in order to cover both full synchronous opening and close + // but also the same but with some pause between each operation. + if (i % 2 == 0) { + info("Wait a bit after open"); + await wait(250); + } + closeRDM(tab); + if (i % 3 == 0) { + info("Wait a bit after close"); + await wait(250); + } + } + + // Wait a bit so that we can receive the very last ResponsiveUIManager `on` event, + // and properly wait for its closing. + await wait(1000); + + // This is important to wait for all destruction as closing the tab while RDM + // is still closing may lead to exception because of pending cleanup RDP requests. + info("Wait for all opened RDM to be closed before closing the tab"); + await waitFor(() => opened == 0); + info("All RDM are closed"); + + await removeTab(tab); + }, + { onlyPrefAndTask: true } +); diff --git a/devtools/client/responsive/ui.js b/devtools/client/responsive/ui.js index 1a76879f59a3..713158d65494 100644 --- a/devtools/client/responsive/ui.js +++ b/devtools/client/responsive/ui.js @@ -266,7 +266,7 @@ class ResponsiveUI { // gracefully, but that shouldn't be a problem since the tab will go away. // So, skip any waiting when we're about to close the tab. const isTabDestroyed = - !this.tab.linkedBrowser || this.responsiveFront.isDestroyed(); + !this.tab.linkedBrowser || this.responsiveFront?.isDestroyed(); const isWindowClosing = options?.reason === "unload" || isTabDestroyed; const isTabContentDestroying = isWindowClosing || options?.reason === "TabClose";