From 61afb77264b64c2988843110865c17b592788c8b Mon Sep 17 00:00:00 2001 From: Hubert Boma Manilla Date: Thu, 2 Jun 2022 14:35:24 +0000 Subject: [PATCH] Bug 1764348 - Pause / Resume should toggle listening to network resources r=ochameau With this patch Pause and Resume, now stop and start listening to network requests Differential Revision: https://phabricator.services.mozilla.com/D146445 --- devtools/client/framework/toolbox.js | 4 +- .../client/netmonitor/src/connector/index.js | 96 +++++++++---------- .../netmonitor/test/browser_net_pause.js | 63 ++++++++++-- 3 files changed, 104 insertions(+), 59 deletions(-) diff --git a/devtools/client/framework/toolbox.js b/devtools/client/framework/toolbox.js index d8c330f2c323..8f06b41a6802 100644 --- a/devtools/client/framework/toolbox.js +++ b/devtools/client/framework/toolbox.js @@ -4015,7 +4015,9 @@ Toolbox.prototype = { this.resourceCommand.TYPES.DOCUMENT_EVENT, this.resourceCommand.TYPES.THREAD_STATE, ], - { onAvailable: this._onResourceAvailable } + { + onAvailable: this._onResourceAvailable, + } ); // Unregister buttons listeners diff --git a/devtools/client/netmonitor/src/connector/index.js b/devtools/client/netmonitor/src/connector/index.js index 22658ada8063..609256055c76 100644 --- a/devtools/client/netmonitor/src/connector/index.js +++ b/devtools/client/netmonitor/src/connector/index.js @@ -50,7 +50,6 @@ class Connector { this.updatePersist = this.updatePersist.bind(this); this.networkFront = null; - this.listenForNetworkEvents = true; } get currentTarget() { @@ -99,29 +98,13 @@ class Connector { }); const { TYPES } = this.toolbox.resourceCommand; - const targetResources = [ - TYPES.DOCUMENT_EVENT, - TYPES.NETWORK_EVENT, - TYPES.NETWORK_EVENT_STACKTRACE, - ]; - if (Services.prefs.getBoolPref("devtools.netmonitor.features.webSockets")) { - targetResources.push(TYPES.WEBSOCKET); - } - - if ( - Services.prefs.getBoolPref( - "devtools.netmonitor.features.serverSentEvents" - ) - ) { - targetResources.push(TYPES.SERVER_SENT_EVENT); - } - - await this.toolbox.resourceCommand.watchResources(targetResources, { + await this.toolbox.resourceCommand.watchResources([TYPES.DOCUMENT_EVENT], { onAvailable: this.onResourceAvailable, - onUpdated: this.onResourceUpdated, }); + await this.resume(false); + // Server side persistance of the data across reload is disabled by default. // Ensure enabling it, if the related frontend pref is true. if (Services.prefs.getBoolPref(DEVTOOLS_ENABLE_PERSISTENT_LOG_PREF)) { @@ -147,19 +130,11 @@ class Connector { }); const { TYPES } = this.toolbox.resourceCommand; - this.toolbox.resourceCommand.unwatchResources( - [ - TYPES.DOCUMENT_EVENT, - TYPES.NETWORK_EVENT, - TYPES.NETWORK_EVENT_STACKTRACE, - TYPES.WEBSOCKET, - TYPES.SERVER_SENT_EVENT, - ], - { - onAvailable: this.onResourceAvailable, - onUpdated: this.onResourceUpdated, - } - ); + this.toolbox.resourceCommand.unwatchResources([TYPES.DOCUMENT_EVENT], { + onAvailable: this.onResourceAvailable, + }); + + this.pause(); Services.prefs.removeObserver( DEVTOOLS_ENABLE_PERSISTENT_LOG_PREF, @@ -176,12 +151,47 @@ class Connector { this.dataProvider = null; } - async pause() { - this.listenForNetworkEvents = false; + pause() { + const { resourceCommand } = this.toolbox; + return resourceCommand.unwatchResources( + [ + resourceCommand.TYPES.NETWORK_EVENT, + resourceCommand.TYPES.NETWORK_EVENT_STACKTRACE, + resourceCommand.TYPES.WEBSOCKET, + resourceCommand.TYPES.SERVER_SENT_EVENT, + ], + { + onAvailable: this.onResourceAvailable, + onUpdated: this.onResourceUpdated, + } + ); } - async resume() { - this.listenForNetworkEvents = true; + resume(ignoreExistingResources = true) { + const { resourceCommand } = this.toolbox; + + const targetResources = [ + resourceCommand.TYPES.NETWORK_EVENT, + resourceCommand.TYPES.NETWORK_EVENT_STACKTRACE, + ]; + + if (Services.prefs.getBoolPref("devtools.netmonitor.features.webSockets")) { + targetResources.push(resourceCommand.TYPES.WEBSOCKET); + } + + if ( + Services.prefs.getBoolPref( + "devtools.netmonitor.features.serverSentEvents" + ) + ) { + targetResources.push(resourceCommand.TYPES.SERVER_SENT_EVENT); + } + + return resourceCommand.watchResources(targetResources, { + onAvailable: this.onResourceAvailable, + onUpdated: this.onResourceUpdated, + ignoreExistingResources, + }); } async onTargetAvailable({ targetFront, isTargetSwitching }) { @@ -207,10 +217,6 @@ class Connector { continue; } - if (!this.listenForNetworkEvents) { - continue; - } - if (resource.resourceType === TYPES.NETWORK_EVENT) { this.dataProvider.onNetworkResourceAvailable(resource); continue; @@ -279,13 +285,7 @@ class Connector { async onResourceUpdated(updates) { for (const { resource, update } of updates) { - if ( - resource.resourceType === - this.toolbox.resourceCommand.TYPES.NETWORK_EVENT && - this.listenForNetworkEvents - ) { - this.dataProvider.onNetworkResourceUpdated(resource, update); - } + this.dataProvider.onNetworkResourceUpdated(resource, update); } } diff --git a/devtools/client/netmonitor/test/browser_net_pause.js b/devtools/client/netmonitor/test/browser_net_pause.js index b21c1a1dfe22..9f09de7e979d 100644 --- a/devtools/client/netmonitor/test/browser_net_pause.js +++ b/devtools/client/netmonitor/test/browser_net_pause.js @@ -22,7 +22,7 @@ add_task(async function() { assertRequestCount(store, 0); // Load one request and assert it shows up in the list. - await performRequestAndWait(tab, monitor); + await performRequestAndWait(tab, monitor, SIMPLE_SJS + "?id=1"); assertRequestCount(store, 1); let noRequest = true; @@ -36,27 +36,69 @@ add_task(async function() { // Click pause, load second request and make sure they don't show up. EventUtils.sendMouseEvent({ type: "click" }, pauseButton); + await waitForPauseButtonToChange(document, true); + await performPausedRequest(tab, monitor, toolbox); + ok(noRequest, "There should be no activity when paused."); assertRequestCount(store, 1); // Click pause again to resume monitoring. Load a third request // and make sure they will show up. EventUtils.sendMouseEvent({ type: "click" }, pauseButton); - await performRequestAndWait(tab, monitor); + await waitForPauseButtonToChange(document, false); + + await performRequestAndWait(tab, monitor, SIMPLE_SJS + "?id=2"); + + ok(!noRequest, "There should be activity when resumed."); assertRequestCount(store, 2); // Click pause, reload the page and check that there are - // some requests. Page reload should auto-resume. + // some requests. EventUtils.sendMouseEvent({ type: "click" }, pauseButton); - const networkEvents = waitForNetworkEvents(monitor, 1); + await waitForPauseButtonToChange(document, true); + + // Page reload should auto-resume await reloadBrowser(); - await networkEvents; - assertRequestCount(store, 1); + await waitForPauseButtonToChange(document, false); + await performRequestAndWait(tab, monitor, SIMPLE_SJS + "?id=3"); + + ok(!noRequest, "There should be activity when resumed."); return teardown(monitor); }); +/** + * Wait until a request is visible in the request list + */ +function waitForRequest(doc, url) { + return waitUntil(() => + [ + ...doc.querySelectorAll(".request-list-item .requests-list-file"), + ].some(columns => columns.title.includes(url)) + ); +} + +/** + * Waits for the state of the paused/resume button to change. + */ +async function waitForPauseButtonToChange(doc, isPaused) { + await waitUntil( + () => + !!doc.querySelector( + `.requests-list-pause-button.devtools-${ + isPaused ? "play" : "pause" + }-icon` + ) + ); + ok( + true, + `The pause button is correctly in the ${ + isPaused ? "paused" : "resumed" + } state` + ); +} + /** * Asserts the number of requests in the network monitor. */ @@ -71,9 +113,9 @@ function assertRequestCount(store, count) { /** * Execute simple GET request and wait till it's done. */ -async function performRequestAndWait(tab, monitor) { - const wait = waitForNetworkEvents(monitor, 1); - await SpecialPowers.spawn(tab.linkedBrowser, [SIMPLE_SJS], async function( +async function performRequestAndWait(tab, monitor, requestURL) { + const wait = waitForRequest(monitor.panelWin.document, requestURL); + await SpecialPowers.spawn(tab.linkedBrowser, [requestURL], async function( url ) { await content.wrappedJSObject.performRequests(url); @@ -82,7 +124,8 @@ async function performRequestAndWait(tab, monitor) { } /** - * Execute simple GET request + * Execute simple GET request, and uses a one time listener to + * know when the resource is available. */ async function performPausedRequest(tab, monitor, toolbox) { const {