Bug 1776145 - [devtools] Remove unnecessary WebConsoleUI additionalProxies. r=ochameau,devtools-backward-compat-reviewers,jdescottes.

`WebConsoleConnectionProxy` is now only used for:
- listening to `lastPrivateContextExited`, which can be done from the top-level
  console front instead
- setting `NetworkMonitor.saveRequestAndResponseBodies` for toolbox we don't have
  network resource support for, which would be better done in `startWatchingNetworkResources`.

This means we don't need to keep a Map of additional proxies.
The only impact is on `clearMessagesCache`, where we now fetch all fronts with
`getAllFronts` instead of looping through the Map of additional proxies.
This change highlighted some race condition in tests, as it's now slightly slower.
So we add a new `clearMessagesCacheAsync` function, which is doing the same thing
as `clearMessagesCache`, except it's not `oneway`, so we can know when the cache
was indeed cleared, and we emit an event to indicate when the cache was cleared.
We can't simply remove `oneway` from `clearMessagesCache` as it causes backward
compatibility issues we can't avoid.
This also highlighted an issue with the cache not being cleared on `console.clear`
when it is batched, so we fix this (a test was failing without it).

The next patch in the queue will completely remove `WebConsoleConnectionProxy`.

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

devtools/client/webconsole/browser-console.js

@@ -93,9 +93,9 @@ class BrowserConsole extends WebConsole {
 
       this.commands.targetCommand.destroy();
       // Wait for any pending connection initialization.
-      await Promise.all(
-        this.ui.getAllProxies().map(proxy => proxy.getConnectionPromise())
-      );
+      if (this.ui.proxy) {
+        await this.ui.proxy.getConnectionPromise();
+      }
 
       await super.destroy();
       await this.currentTarget.destroy();

devtools/client/webconsole/enhancers/message-cache-clearing.js

@@ -14,9 +14,12 @@ const { MESSAGES_CLEAR } = require("devtools/client/webconsole/constants");
 function enableMessagesCacheClearing(webConsoleUI) {
   return next => (reducer, initialState, enhancer) => {
     function messagesCacheClearingEnhancer(state, action) {
+      const storeHadMessages =
+        state?.messages?.mutableMessagesById &&
+        state.messages.mutableMessagesById.size > 0;
       state = reducer(state, action);
 
-      if (webConsoleUI && action.type === MESSAGES_CLEAR) {
+      if (storeHadMessages && webConsoleUI && action.type === MESSAGES_CLEAR) {
         webConsoleUI.clearMessagesCache();
 
         // cleans up all the network data provider internal state

devtools/client/webconsole/store.js

@@ -108,9 +108,11 @@ function configureStore(webConsoleUI, options = {}) {
     compose(
       middleware,
       enableActorReleaser(webConsoleUI),
-      enableBatching(),
       enableMessagesCacheClearing(webConsoleUI),
-      ensureCSSErrorReportingEnabled(webConsoleUI)
+      ensureCSSErrorReportingEnabled(webConsoleUI),
+      // ⚠️ Keep this one last so it will be executed before all the other ones. This is
+      // needed so batched actions can be "unbatched" and handled in the other enhancers.
+      enableBatching()
     )
   );
 }

devtools/client/webconsole/test/browser/browser_webconsole_clear_cache.js

@@ -27,8 +27,9 @@ add_task(async function() {
   info(
     "Click the clear output button and wait until there's no messages in the output"
   );
+  let onMessagesCacheCleared = hud.ui.once("messages-cache-cleared");
   hud.ui.window.document.querySelector(".devtools-clear-icon").click();
-  await waitFor(() => findAllMessages(hud).length === 0);
+  await onMessagesCacheCleared;
 
   info("Close and re-open the console");
   await closeToolbox();
@@ -52,6 +53,7 @@ add_task(async function() {
   await logTextToConsole(hud, NEW_CACHED_MESSAGE);
 
   info("Send a console.clear() from the content page");
+  onMessagesCacheCleared = hud.ui.once("messages-cache-cleared");
   const onConsoleCleared = waitForMessageByType(
     hud,
     "Console was cleared",
@@ -60,7 +62,7 @@ add_task(async function() {
   SpecialPowers.spawn(gBrowser.selectedBrowser, [], () => {
     content.wrappedJSObject.console.clear();
   });
-  await onConsoleCleared;
+  await Promise.all([onConsoleCleared, onMessagesCacheCleared]);
 
   info("Close and re-open the console");
   await closeToolbox();

devtools/client/webconsole/test/browser/browser_webconsole_console_logging_workers_api.js

@@ -83,12 +83,14 @@ async function testWorkerMessage(directConnectionToWorkerThread = false) {
   }
 
   info("Click on the clear button and wait for messages to be removed");
+  const onMessagesCacheCleared = hud.ui.once("messages-cache-cleared");
   hud.ui.window.document.querySelector(".devtools-clear-icon").click();
   await waitFor(
     () =>
       !findConsoleAPIMessage(hud, "initial-message-from-worker") &&
       !findConsoleAPIMessage(hud, "log-from-worker")
   );
+  await onMessagesCacheCleared;
   ok(true, "Messages were removed");
 
   info("Close and reopen the console to check messages were cleared properly");

devtools/client/webconsole/test/node/helpers.js

@@ -140,7 +140,6 @@ function getWebConsoleUiMock(hud, proxyOverrides) {
     emit: () => {},
     emitForTests: () => {},
     hud,
-    getAllProxies: () => [proxy],
     proxy,
     clearNetworkRequests: () => {},
     clearMessagesCache: () => {},

devtools/client/webconsole/webconsole-ui.js

@@ -108,34 +108,6 @@ class WebConsoleUI {
     return this.proxy;
   }
 
-  /**
-   * Return all the proxies we're currently managing (i.e. the "main" one, and the
-   * possible additional ones).
-   *
-   * @param {Boolean} filterDisconnectedProxies: True by default, if false, this
-   *   function also returns not-already-connected or already disconnected proxies.
-   *
-   * @returns {Array<WebConsoleConnectionProxy>}
-   */
-  getAllProxies(filterDisconnectedProxies = true) {
-    let proxies = [this.getProxy()];
-
-    if (this.additionalProxies) {
-      proxies = proxies.concat([...this.additionalProxies.values()]);
-    }
-
-    // Ignore Fronts that are already destroyed
-    if (filterDisconnectedProxies) {
-      proxies = proxies.filter(proxy => {
-        return (
-          proxy && proxy.webConsoleFront && !!proxy.webConsoleFront.actorID
-        );
-      });
-    }
-
-    return proxies;
-  }
-
   /**
    * Initialize the WebConsoleUI instance.
    * @return object
@@ -213,7 +185,7 @@ class WebConsoleUI {
     this.hud.commands.targetCommand.unwatchTargets({
       types: this.hud.commands.targetCommand.ALL_TYPES,
       onAvailable: this._onTargetAvailable,
-      onDestroyed: this._onTargetDestroy,
+      onDestroyed: this._onTargetDestroyed,
     });
 
     const resourceCommand = this.hud.resourceCommand;
@@ -233,11 +205,10 @@ class WebConsoleUI {
 
     this.stopWatchingNetworkResources();
 
-    for (const proxy of this.getAllProxies()) {
-      proxy.disconnect();
+    if (this.proxy) {
+      this.proxy.disconnect();
+      this.proxy = null;
     }
-    this.proxy = null;
-    this.additionalProxies = null;
 
     this.networkDataProvider.destroy();
     this.networkDataProvider = null;
@@ -257,7 +228,7 @@ class WebConsoleUI {
    * @param object event
    *        If the event exists, calls preventDefault on it.
    */
-  clearOutput(clearStorage, event) {
+  async clearOutput(clearStorage, event) {
     if (event) {
       event.preventDefault();
     }
@@ -266,14 +237,39 @@ class WebConsoleUI {
     }
 
     if (clearStorage) {
-      this.clearMessagesCache();
+      await this.clearMessagesCache();
     }
     this.emitForTests("messages-cleared");
   }
 
-  clearMessagesCache() {
-    for (const proxy of this.getAllProxies()) {
-      proxy.webConsoleFront.clearMessagesCache();
+  async clearMessagesCache() {
+    if (!this.hud) {
+      return;
+    }
+
+    const {
+      hasWebConsoleClearMessagesCacheAsync,
+    } = this.hud.commands.client.mainRoot.traits;
+
+    // This can be called during console destruction and getAllFronts would reject in such case.
+    try {
+      const consoleFronts = await this.hud.commands.targetCommand.getAllFronts(
+        this.hud.commands.targetCommand.ALL_TYPES,
+        "console"
+      );
+      const promises = [];
+      for (const consoleFront of consoleFronts) {
+        // @backward-compat { version 104 } clearMessagesCacheAsync was added in 104
+        promises.push(
+          hasWebConsoleClearMessagesCacheAsync
+            ? consoleFront.clearMessagesCacheAsync()
+            : consoleFront.clearMessagesCache()
+        );
+      }
+      await Promise.all(promises);
+      this.emitForTests("messages-cache-cleared");
+    } catch (e) {
+      console.warn("Exception in clearMessagesCache", e);
     }
   }
 
@@ -325,8 +321,6 @@ class WebConsoleUI {
    *         A promise object that is resolved/reject based on the proxies connections.
    */
   async _attachTargets() {
-    this.additionalProxies = new Map();
-
     const { commands, resourceCommand } = this.hud;
     this.networkDataProvider = new FirefoxDataProvider({
       commands,
@@ -346,7 +340,7 @@ class WebConsoleUI {
     await commands.targetCommand.watchTargets({
       types: this.hud.commands.targetCommand.ALL_TYPES,
       onAvailable: this._onTargetAvailable,
-      onDestroyed: this._onTargetDestroy,
+      onDestroyed: this._onTargetDestroyed,
     });
 
     await resourceCommand.watchResources(
@@ -544,42 +538,13 @@ class WebConsoleUI {
    *        composed of a WindowGlobalTargetFront or ContentProcessTargetFront.
    */
   async _onTargetAvailable({ targetFront }) {
-    // This is a top level target. It may update on process switches
-    // when navigating to another domain.
-    if (targetFront.isTopLevel) {
-      this.proxy = new WebConsoleConnectionProxy(this, targetFront);
-      await this.proxy.connect();
+    // Only handle new top level target
+    if (!targetFront.isTopLevel) {
       return;
     }
 
-    // Allow frame, but only in content toolbox, i.e. still ignore them in
-    // the context of the browser toolbox as we inspect messages via the process targets
-    const listenForFrames = this.hud.commands.descriptorFront.isLocalTab;
-
-    const { TYPES } = this.hud.commands.targetCommand;
-    const isWorkerTarget =
-      targetFront.targetType == TYPES.WORKER ||
-      targetFront.targetType == TYPES.SHARED_WORKER ||
-      targetFront.targetType == TYPES.SERVICE_WORKER;
-
-    const acceptTarget =
-      // Unconditionally accept all process targets, this should only happens in the
-      // multiprocess browser toolbox/console
-      targetFront.targetType == TYPES.PROCESS ||
-      (targetFront.targetType == TYPES.FRAME && listenForFrames) ||
-      // Accept worker targets if the platform dispatching of worker messages to the main
-      // thread is disabled (e.g. we get them directly from the worker target).
-      (isWorkerTarget &&
-        !this.hud.commands.targetCommand.rootFront.traits
-          .workerConsoleApiMessagesDispatchedToMainThread);
-
-    if (!acceptTarget) {
-      return;
-    }
-
-    const proxy = new WebConsoleConnectionProxy(this, targetFront);
-    this.additionalProxies.set(targetFront, proxy);
-    await proxy.connect();
+    this.proxy = new WebConsoleConnectionProxy(this, targetFront);
+    await this.proxy.connect();
   }
 
   /**
@@ -589,14 +554,13 @@ class WebConsoleUI {
    * See _onTargetAvailable for param's description.
    */
   _onTargetDestroyed({ targetFront }) {
-    if (targetFront.isTopLevel) {
-      this.proxy.disconnect();
-      this.proxy = null;
-    } else {
-      const proxy = this.additionalProxies.get(targetFront);
-      proxy.disconnect();
-      this.additionalProxies.delete(targetFront);
+    // Only handle new top level target
+    if (!targetFront.isTopLevel || !this.proxy) {
+      return;
     }
+
+    this.proxy.disconnect();
+    this.proxy = null;
   }
 
   _initUI() {

devtools/docs/user/web_console/remoting/index.rst

@@ -487,6 +487,7 @@ The response packet:
 
 
 There's also the ``clearMessagesCache`` request packet that has no response. This clears the console API calls cache and should clear the page errors cache - see `bug 717611 <https://bugzilla.mozilla.org/show_bug.cgi?id=717611>`_.
+An alternate version was added in Firefox 104, ``clearMessagesCacheAsync``, which does exactly the same thing but resolves when the cache was actually cleared.
 
 
 Network logging

devtools/server/actors/root.js

@@ -129,6 +129,8 @@ exports.RootActor = protocol.ActorClassWithSpec(rootSpec, {
     }
 
     this.traits = {
+      // @backward-compat { version 104 } clearMessagesCacheAsync was added in 104
+      hasWebConsoleClearMessagesCacheAsync: true,
       networkMonitor: true,
       resources: supportedResources,
       // @backward-compat { version 103 } Clear resources not supported by old servers

devtools/server/actors/webconsole.js

@@ -1473,9 +1473,9 @@ const WebConsoleActor = ActorClassWithSpec(webconsoleSpec, {
   },
 
   /**
-   * The "clearMessagesCache" request handler.
+   * The "clearMessagesCacheAsync" request handler.
    */
-  clearMessagesCache: function() {
+  clearMessagesCacheAsync: function() {
     if (isWorker) {
       // Defined on WorkerScope
       clearConsoleEvents();

devtools/shared/commands/README.md

@@ -30,7 +30,6 @@ This layer already exists in some panels, but we are using slightly different na
   we could instead pass the netmonitor command object.
 * Web Console has:
   * WebConsoleConnectionProxy, but this is probably going to disappear and doesn't do much.
-  * WebConsoleUI, which does dispatch to many target's actor, via getAllProxies (see clearMessagesCache)
   * See devtools/client/webconsole/actions/input.js:handleHelperResult(), where we have to put some code, which is a duplicate of Netmonitor Connector,
     and could be shared via a netmonitor command class.
 * Inspector is probably the panel doing the most dispatch to many target's actor.

devtools/shared/commands/target/target-command.js

@@ -907,9 +907,12 @@ class TargetCommand extends EventEmitter {
       ? this.getAllTargets(targetTypes)
       : await this.getAllTargetsInSelectedTargetTree(targetTypes);
     for (const target of targets) {
-      // For still-attaching worker targets, the threadFront may not yet be available,
+      // For still-attaching worker targets, the thread or console front may not yet be available,
       // whereas TargetMixin.getFront will throw if the actorID isn't available in targetForm.
-      if (frontType == "thread" && !target.targetForm.threadActor) {
+      if (
+        (frontType == "thread" && !target.targetForm.threadActor) ||
+        (frontType == "console" && !target.targetForm.consoleActor)
+      ) {
         continue;
       }
 

devtools/shared/specs/webconsole.js

@@ -189,9 +189,18 @@ const webconsoleSpecPrototype = {
     /**
      * Clear the cache of messages (page errors and console API calls) expects no response.
      */
+    // @backward-compat { version 104 } This can be removed once older server have clearMessagesCacheAsync.
     clearMessagesCache: {
       oneway: true,
     },
+
+    /**
+     * Same as clearMessagesCache, but wait for the server response.
+     */
+    clearMessagesCacheAsync: {
+      request: {},
+    },
+
     /**
      * Get Web Console-related preferences on the server.
      *