Bug 1619720 - Request favicons directly from the tab descriptor r=ochameau,ladybenko

Depends on D69331

This changeset adds a new getFavicon API on the TabDescriptorActor and uses it from about:debugging to retrieve the favicons for the tabs.

Currently, the TabDescriptorActor (living in the parent process) modifies the form of its BrowsingContextTargetActor (living in the content process) to add a "favicon" property on it. The reason we do this is that the API to get favicons can only be used from the parent process. Before tab descriptors, we had "frame-prox(ies)" instead in the parent process and the client could not directly query those frame proxies (they were not actors, and had no corresponding front). That's why we used this workaround of modifying the target actor form.

But now clients can directly call APIs on the TabDescriptor, so we don't have to insert the favicon information in the form of the target actor anymore.

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

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

devtools/client/aboutdebugging/src/actions/debug-targets.js

@@ -206,9 +206,18 @@ function requestTabs() {
         DEBUG_TARGET_PANE.TAB
       );
       const tabs = isSupported
-        ? await clientWrapper.listTabs({ favicons: true })
+        ? await clientWrapper.listTabs({
+            // Backward compatibility: this is only used for FF75 or older.
+            // The argument can be dropped when FF76 hits the release channel.
+            favicons: true,
+          })
         : [];
 
+      // Fetch the favicon for all tabs.
+      await Promise.all(
+        tabs.map(async tab => (tab.favicon = await getTabFavicon(tab)))
+      );
+
       dispatch({ type: REQUEST_TABS_SUCCESS, tabs });
     } catch (e) {
       dispatch({ type: REQUEST_TABS_FAILURE, error: e });
@@ -216,6 +225,29 @@ function requestTabs() {
   };
 }
 
+async function getTabFavicon(targetFront) {
+  const { descriptorFront } = targetFront;
+  if (!descriptorFront || !descriptorFront.traits.getFavicon) {
+    // Backward compatibility for FF75 or older.
+    // The favicon used to be included directly on the target form.
+    // Starting with Firefox 76, consumers should retrieve the favicon
+    // using the getFavicon request.
+    return targetFront.favicon;
+  }
+
+  try {
+    const favicon = await descriptorFront.getFavicon();
+    return favicon;
+  } catch (e) {
+    // We might request the favicon for a tab which is going to be destroyed.
+    // In this case targetFront.actorID will be null. Otherwise log an error.
+    if (targetFront.actorID) {
+      console.error("Failed to retrieve the favicon for " + targetFront.url, e);
+    }
+    return "";
+  }
+}
+
 function requestExtensions() {
   return async (dispatch, getState) => {
     dispatch({ type: REQUEST_EXTENSIONS_START });

devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_addons_remote_runtime.js

@@ -117,7 +117,14 @@ async function testAddonsOnMockedRemoteClient(
   // tab. We assume that if the addon update mechanism had started, it would also be done
   // when the new tab was processed.
   info("Wait until the tab target for 'http://some.random/url.com' appears");
-  const testTab = { outerWindowID: 0, url: "http://some.random/url.com" };
+  const testTab = {
+    getFavicon: () => {},
+    outerWindowID: 0,
+    traits: {
+      getFavicon: true,
+    },
+    url: "http://some.random/url.com",
+  };
   remoteClient.listTabs = () => [testTab];
   remoteClient._eventEmitter.emit("tabListChanged");
   await waitUntil(() =>

devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_workers_remote_runtime.js

@@ -135,7 +135,14 @@ async function testWorkerOnMockedRemoteClient(
   // tab. We assume that if the worker update mechanism had started, it would also be done
   // when the new tab was processed.
   info("Wait until the tab target for 'http://some.random/url.com' appears");
-  const testTab = { outerWindowID: 0, url: "http://some.random/url.com" };
+  const testTab = {
+    getFavicon: () => {},
+    outerWindowID: 0,
+    traits: {
+      getFavicon: true,
+    },
+    url: "http://some.random/url.com",
+  };
   remoteClient.listTabs = () => [testTab];
   remoteClient._eventEmitter.emit("tabListChanged");
   await waitUntil(() =>

devtools/client/aboutdebugging/test/browser/mocks/helper-client-wrapper-mock.js

@@ -114,7 +114,11 @@ function createThisFirefoxClientMock() {
   // Create a fake about:debugging tab because our test helper openAboutDebugging
   // waits until about:debugging is displayed in the list of tabs.
   const mockAboutDebuggingTab = {
+    getFavicon: () => {},
     outerWindowID: 0,
+    traits: {
+      getFavicon: true,
+    },
     url: "about:debugging",
   };
 

devtools/client/fronts/descriptors/tab.js

@@ -28,6 +28,11 @@ class TabDescriptorFront extends FrontClassWithSpec(tabDescriptorSpec) {
     this._client = client;
   }
 
+  form(json) {
+    this.actorID = json.actor;
+    this.traits = json.traits || {};
+  }
+
   _createTabTarget(form, filter) {
     // Instanciate a specialized class for a local tab as it needs some more
     // client side integration with the Firefox frontend.

devtools/client/fronts/root.js

@@ -230,9 +230,12 @@ class RootFront extends FrontClassWithSpec(rootSpec) {
   /**
    * Override default listTabs request in order to return a list of
    * BrowsingContextTargetFronts while updating their selected state.
+   *
+   * Backward compatibility: favicons is only useful for FF75 or older.
+   * It can be removed when Firefox 76 hits the release channel.
    */
-  async listTabs(options) {
-    const { selected, tabs } = await super.listTabs(options);
+  async listTabs({ favicons } = {}) {
+    const { selected, tabs } = await super.listTabs({ favicons });
     const targets = [];
     for (const i in tabs) {
       if (!this.actorID) {

devtools/server/actors/descriptors/tab.js

@@ -15,7 +15,6 @@
 const {
   connectToFrame,
 } = require("devtools/server/connectors/frame-connector");
-
 loader.lazyImporter(
   this,
   "PlacesUtils",
@@ -51,6 +50,17 @@ const TabDescriptorActor = ActorClassWithSpec(tabDescriptorSpec, {
     this._formUpdateReject = null;
   },
 
+  form() {
+    return {
+      actor: this.actorID,
+      traits: {
+        // Backward compatibility for FF75 or older.
+        // Remove when FF76 is on the release channel.
+        getFavicon: true,
+      },
+    };
+  },
+
   async getTarget() {
     if (!this._conn) {
       return {
@@ -87,10 +97,6 @@ const TabDescriptorActor = ActorClassWithSpec(tabDescriptorSpec, {
         );
 
         const form = this._createTargetForm(connectForm);
-        if (this.options.favicons) {
-          form.favicon = await this.getFaviconData();
-        }
-
         this._form = form;
         resolve(form);
       } catch (e) {
@@ -117,7 +123,7 @@ const TabDescriptorActor = ActorClassWithSpec(tabDescriptorSpec, {
     );
   },
 
-  async getFaviconData() {
+  async getFavicon() {
     if (!AppConstants.MOZ_PLACES) {
       // PlacesUtils is not supported
       return null;
@@ -143,8 +149,8 @@ const TabDescriptorActor = ActorClassWithSpec(tabDescriptorSpec, {
     // If the child happens to be crashed/close/detach, it won't have _form set,
     // so only request form update if some code is still listening on the other
     // side.
-    if (this.exited) {
-      return this.getTarget();
+    if (!this._form) {
+      return;
     }
 
     // This function may be called if we are inspecting tabs and the actor proxy
@@ -170,11 +176,6 @@ const TabDescriptorActor = ActorClassWithSpec(tabDescriptorSpec, {
     });
 
     this._form = form;
-    if (this.options.favicons) {
-      this._form.favicon = await this.getFaviconData();
-    }
-
-    return this;
   },
 
   _isZombieTab() {

devtools/server/actors/webbrowser.js

@@ -338,7 +338,7 @@ BrowserTabList.prototype.getList = async function(browserActorOptions) {
 /**
  * @param browserActorOptions see options argument of TabDescriptorActor constructor.
  */
-BrowserTabList.prototype._getActorForBrowser = function(
+BrowserTabList.prototype._getActorForBrowser = async function(
   browser,
   browserActorOptions
 ) {
@@ -346,7 +346,8 @@ BrowserTabList.prototype._getActorForBrowser = function(
   let actor = this._actorByBrowser.get(browser);
   if (actor) {
     this._foundCount++;
-    return actor.update(browserActorOptions);
+    await actor.update();
+    return actor;
   }
 
   actor = new TabDescriptorActor(

devtools/shared/specs/descriptors/tab.js

@@ -15,6 +15,12 @@ const tabDescriptorSpec = generateActorSpec({
         frame: RetVal("json"),
       },
     },
+    getFavicon: {
+      request: {},
+      response: {
+        favicon: RetVal("string"),
+      },
+    },
   },
 });
 

devtools/shared/specs/root.js

@@ -50,6 +50,8 @@ const rootSpecPrototype = {
 
     listTabs: {
       request: {
+        // Backward compatibility: this is only used for FF75 or older.
+        // The argument can be dropped when FF76 hits the release channel.
         favicons: Option(0, "boolean"),
       },
       response: RetVal("root.listTabs"),