Bug 1542286 - Share runtime info with DebugTargetInfo component via remote-client-manager r=daisuke
authorJulian Descottes <jdescottes@mozilla.com>
Tue, 07 May 2019 15:21:22 +0000
changeset 531737 1d326e6840189ef74100cdf109eec8107ae6c689
parent 531736 27b582600447f02b895c1f6b920e129a06d21b95
child 531738 796149a6d1b6e28ca83307dc6ad10f8882c3ca67
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdaisuke
bugs1542286
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1542286 - Share runtime info with DebugTargetInfo component via remote-client-manager r=daisuke Depends on D29462 Differential Revision: https://phabricator.services.mozilla.com/D29485
devtools/client/aboutdebugging-new/src/reducers/runtimes-state.js
devtools/client/framework/components/DebugTargetInfo.js
devtools/client/framework/components/ToolboxToolbar.js
devtools/client/framework/test/browser_about-devtools-toolbox_reload.js
devtools/client/framework/test/jest/components/debug-target-info.test.js
devtools/client/framework/toolbox.js
devtools/client/shared/remote-debugging/remote-client-manager.js
devtools/client/shared/remote-debugging/test/unit/test_remote_client_manager.js
--- a/devtools/client/aboutdebugging-new/src/reducers/runtimes-state.js
+++ b/devtools/client/aboutdebugging-new/src/reducers/runtimes-state.js
@@ -100,17 +100,22 @@ function runtimesReducer(state = Runtime
         isConnectionNotResponding: false,
         isConnectionTimeout: true,
       };
       return _updateRuntimeById(id, updatedState, state);
     }
 
     case CONNECT_RUNTIME_SUCCESS: {
       const { id, runtimeDetails, type } = action.runtime;
-      remoteClientManager.setClient(id, type, runtimeDetails.clientWrapper.client);
+
+      // Update the remoteClientManager with the connected runtime.
+      const client = runtimeDetails.clientWrapper.client;
+      const runtimeInfo = runtimeDetails.info;
+      remoteClientManager.setClient(id, type, client, runtimeInfo);
+
       const updatedState = {
         isConnecting: false,
         isConnectionFailed: false,
         isConnectionNotResponding: false,
         isConnectionTimeout: false,
         runtimeDetails,
       };
       return _updateRuntimeById(id, updatedState, state);
--- a/devtools/client/framework/components/DebugTargetInfo.js
+++ b/devtools/client/framework/components/DebugTargetInfo.js
@@ -13,20 +13,20 @@ const { CONNECTION_TYPES, DEBUG_TARGET_T
  * This is header that should be displayed on top of the toolbox when using
  * about:devtools-toolbox.
  */
 class DebugTargetInfo extends PureComponent {
   static get propTypes() {
     return {
       debugTargetData: PropTypes.shape({
         connectionType: PropTypes.oneOf(Object.values(CONNECTION_TYPES)).isRequired,
-        deviceDescription: PropTypes.shape({
-          brandName: PropTypes.string.isRequired,
-          channel: PropTypes.string.isRequired,
+        runtimeInfo: PropTypes.shape({
           deviceName: PropTypes.string,
+          icon: PropTypes.string.isRequired,
+          name: PropTypes.string.isRequired,
           version: PropTypes.string.isRequired,
         }).isRequired,
         targetType: PropTypes.oneOf(Object.values(DEBUG_TARGET_TYPES)).isRequired,
       }).isRequired,
       L10N: PropTypes.object.isRequired,
       toolbox: PropTypes.object.isRequired,
     };
   }
@@ -53,22 +53,22 @@ class DebugTargetInfo extends PureCompon
         targetTypeStr,
         title
       );
     }
   }
 
   getRuntimeText() {
     const { debugTargetData, L10N } = this.props;
-    const { brandName, version } = debugTargetData.deviceDescription;
+    const { name, version } = debugTargetData.runtimeInfo;
     const { connectionType } = debugTargetData;
 
     return (connectionType === CONNECTION_TYPES.THIS_FIREFOX)
       ? L10N.getFormatStr("toolbox.debugTargetInfo.runtimeLabel.thisFirefox", version)
-      : L10N.getFormatStr("toolbox.debugTargetInfo.runtimeLabel", brandName, version);
+      : L10N.getFormatStr("toolbox.debugTargetInfo.runtimeLabel", name, version);
   }
 
   getAssetsForConnectionType() {
     const { connectionType } = this.props.debugTargetData;
 
     switch (connectionType) {
       case CONNECTION_TYPES.USB:
         return {
@@ -138,28 +138,30 @@ class DebugTargetInfo extends PureCompon
         className: "iconized-label qa-connection-info",
       },
       dom.img({ src: image, alt: `${connectionType} icon`}),
       this.props.L10N.getStr(l10nId),
     );
   }
 
   renderRuntime() {
-    const { channel, deviceName } = this.props.debugTargetData.deviceDescription;
+    if (!this.props.debugTargetData.runtimeInfo) {
+      // Skip the runtime render if no runtimeInfo is available.
+      // Runtime info is retrieved from the remote-client-manager, which might not be
+      // setup if about:devtools-toolbox was not opened from about:debugging.
+      return null;
+    }
 
-    const channelIcon =
-      (channel === "release" || channel === "beta" || channel === "aurora") ?
-      `chrome://devtools/skin/images/aboutdebugging-firefox-${ channel }.svg` :
-      "chrome://devtools/skin/images/aboutdebugging-firefox-nightly.svg";
+    const { icon, deviceName } = this.props.debugTargetData.runtimeInfo;
 
     return dom.span(
       {
         className: "iconized-label",
       },
-      dom.img({ src: channelIcon, className: "channel-icon" }),
+      dom.img({ src: icon, className: "channel-icon" }),
       dom.b({ className: "devtools-ellipsis-text" }, this.getRuntimeText()),
       dom.span({ className: "devtools-ellipsis-text" }, deviceName),
     );
   }
 
   renderTarget() {
     const title = this.props.toolbox.target.name;
     const url = this.props.toolbox.target.url;
--- a/devtools/client/framework/components/ToolboxToolbar.js
+++ b/devtools/client/framework/components/ToolboxToolbar.js
@@ -93,17 +93,17 @@ class ToolboxToolbar extends Component {
       onTabsOrderUpdated: PropTypes.func.isRequired,
       // Count of visible toolbox buttons which is used by ToolboxTabs component
       // to recognize that the visibility of toolbox buttons were changed.
       // Because in the component we cannot compare the visibility since the
       // button definition instance in toolboxButtons will be unchanged.
       visibleToolboxButtonCount: PropTypes.number,
       // Data to show debug target info, if needed
       debugTargetData: PropTypes.shape({
-        deviceDescription: PropTypes.object.isRequired,
+        runtimeInfo: PropTypes.object.isRequired,
         targetType: PropTypes.string.isRequired,
       }),
     };
   }
 
   constructor(props) {
     super(props);
 
--- a/devtools/client/framework/test/browser_about-devtools-toolbox_reload.js
+++ b/devtools/client/framework/test/browser_about-devtools-toolbox_reload.js
@@ -8,17 +8,17 @@
  * client instance.
  */
 add_task(async function() {
   const debuggerClient = await createLocalClient();
 
   info("Preload a local DebuggerClient as this-firefox in the remoteClientManager");
   const { remoteClientManager } =
     require("devtools/client/shared/remote-debugging/remote-client-manager");
-  remoteClientManager.setClient("this-firefox", "this-firefox", debuggerClient);
+  remoteClientManager.setClient("this-firefox", "this-firefox", debuggerClient, {});
   registerCleanupFunction(() => {
     remoteClientManager.removeAllClients();
   });
 
   info("Create a dummy target tab");
   const targetTab = await addTab("data:text/html,somehtml");
 
   const { tab } = await openAboutToolbox({
--- a/devtools/client/framework/test/jest/components/debug-target-info.test.js
+++ b/devtools/client/framework/test/jest/components/debug-target-info.test.js
@@ -36,52 +36,52 @@ const TEST_TOOLBOX = {
 
 const TEST_TOOLBOX_NO_NAME = {
   target: {
     url: "http://some.target/without/a/name",
   },
 };
 
 const USB_DEVICE_DESCRIPTION = {
-  brandName: "usbRuntimeBrandName",
-  channel: "release",
   deviceName: "usbDeviceName",
+  icon: "chrome://devtools/skin/images/aboutdebugging-firefox-release.svg",
+  name: "usbRuntimeBrandName",
   version: "1.0.0",
 };
 
 const THIS_FIREFOX_DEVICE_DESCRIPTION = {
-  brandName: "thisFirefoxRuntimeBrandName",
-  channel: "release",
+  icon: "chrome://devtools/skin/images/aboutdebugging-firefox-release.svg",
   version: "1.0.0",
+  name: "thisFirefoxRuntimeBrandName",
 };
 
 const USB_TARGET_INFO = {
   debugTargetData: {
     connectionType: CONNECTION_TYPES.USB,
-    deviceDescription: USB_DEVICE_DESCRIPTION,
+    runtimeInfo: USB_DEVICE_DESCRIPTION,
     targetType: DEBUG_TARGET_TYPES.TAB,
   },
   toolbox: TEST_TOOLBOX,
   L10N: stubL10N,
 };
 
 const THIS_FIREFOX_TARGET_INFO = {
   debugTargetData: {
     connectionType: CONNECTION_TYPES.THIS_FIREFOX,
-    deviceDescription: THIS_FIREFOX_DEVICE_DESCRIPTION,
+    runtimeInfo: THIS_FIREFOX_DEVICE_DESCRIPTION,
     targetType: DEBUG_TARGET_TYPES.TAB,
   },
   toolbox: TEST_TOOLBOX,
   L10N: stubL10N,
 };
 
 const THIS_FIREFOX_NO_NAME_TARGET_INFO = {
   debugTargetData: {
     connectionType: CONNECTION_TYPES.THIS_FIREFOX,
-    deviceDescription: THIS_FIREFOX_DEVICE_DESCRIPTION,
+    runtimeInfo: THIS_FIREFOX_DEVICE_DESCRIPTION,
     targetType: DEBUG_TARGET_TYPES.TAB,
   },
   toolbox: TEST_TOOLBOX_NO_NAME,
   L10N: stubL10N,
 };
 
 describe("DebugTargetInfo component", () => {
   describe("Connection info", () => {
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -458,17 +458,17 @@ Toolbox.prototype = {
         // Update the URL so that onceDOMReady watch for the right url.
         this._URL = this.win.location.href;
       }
 
       if (this.hostType === Toolbox.HostType.PAGE) {
         // Displays DebugTargetInfo which shows the basic information of debug target,
         // if `about:devtools-toolbox` URL opens directly.
         // DebugTargetInfo requires this._debugTargetData to be populated
-        this._debugTargetData = await this._getDebugTargetData();
+        this._debugTargetData = this._getDebugTargetData();
       }
 
       const domHelper = new DOMHelpers(this.win);
       const domReady = new Promise(resolve => {
         domHelper.onceDOMReady(() => {
           resolve();
         }, this._URL);
       });
@@ -789,30 +789,29 @@ Toolbox.prototype = {
 
   _removeWindowHostShortcuts: function() {
     if (this._windowHostShortcuts) {
       this._windowHostShortcuts.destroy();
       this._windowHostShortcuts = null;
     }
   },
 
-  _getDebugTargetData: async function() {
+  _getDebugTargetData: function() {
     const url = new URL(this.win.location);
     const searchParams = new this.win.URLSearchParams(url.search);
 
     const targetType = searchParams.get("type") || DEBUG_TARGET_TYPES.TAB;
 
-    const deviceFront = await this.target.client.mainRoot.getFront("device");
-    const deviceDescription = await deviceFront.getDescription();
     const remoteId = searchParams.get("remoteId");
+    const runtimeInfo = remoteClientManager.getRuntimeInfoByRemoteId(remoteId);
     const connectionType = remoteClientManager.getConnectionTypeByRemoteId(remoteId);
 
     return {
       connectionType,
-      deviceDescription,
+      runtimeInfo,
       targetType,
     };
   },
 
   _onTargetClosed: async function() {
     const win = this.win; // .destroy() will set this.win to null
 
     // clean up the toolbox
--- a/devtools/client/shared/remote-debugging/remote-client-manager.js
+++ b/devtools/client/shared/remote-debugging/remote-client-manager.js
@@ -8,31 +8,37 @@ const { CONNECTION_TYPES } = require("de
 
 /**
  * This class is designed to be a singleton shared by all DevTools to get access to
  * existing clients created for remote debugging.
  */
 class RemoteClientManager {
   constructor() {
     this._clients = new Map();
+    this._runtimeInfoMap = new Map();
     this._onClientClosed = this._onClientClosed.bind(this);
   }
 
   /**
    * Store a remote client that is already connected.
    *
    * @param {String} id
    *        Remote runtime id (see devtools/client/aboutdebugging-new/src/types).
    * @param {String} type
    *        Remote runtime type (see devtools/client/aboutdebugging-new/src/types).
    * @param {DebuggerClient} client
+   * @param {Object} runtimeInfo
+   *        See runtimeInfo type from client/aboutdebugging-new/src/types/runtime.js
    */
-  setClient(id, type, client) {
+  setClient(id, type, client, runtimeInfo) {
     const key = this._getKey(id, type);
     this._clients.set(key, client);
+    if (runtimeInfo) {
+      this._runtimeInfoMap.set(key, runtimeInfo);
+    }
     client.addOneTimeListener("closed", this._onClientClosed);
   }
 
   // See JSDoc for id, type from setClient.
   hasClient(id, type) {
     return this._clients.has(this._getKey(id, type));
   }
 
@@ -61,47 +67,65 @@ class RemoteClientManager {
     return encodeURIComponent(this._getKey(id, type));
   }
 
   /**
    * Retrieve a managed client for a remote id. The remote id should have been generated
    * using getRemoteId.
    */
   getClientByRemoteId(remoteId) {
-    const key = decodeURIComponent(remoteId);
+    const key = this._getKeyByRemoteId(remoteId);
     return this._clients.get(key);
   }
 
   /**
+   * Retrieve the runtime info for a remote id. To display metadata about a runtime, such
+   * as name, device name, version... this runtimeInfo should be used rather than calling
+   * APIs on the client.
+   */
+  getRuntimeInfoByRemoteId(remoteId) {
+    const key = this._getKeyByRemoteId(remoteId);
+    return this._runtimeInfoMap.get(key);
+  }
+
+  /**
    * Retrieve a managed client for a remote id. The remote id should have been generated
    * using getRemoteId.
    */
   getConnectionTypeByRemoteId(remoteId) {
-    if (!remoteId) {
-      return CONNECTION_TYPES.THIS_FIREFOX;
-    }
-
-    const key = decodeURIComponent(remoteId);
+    const key = this._getKeyByRemoteId(remoteId);
     for (const type of Object.values(CONNECTION_TYPES)) {
       if (key.endsWith(type)) {
         return type;
       }
     }
     return CONNECTION_TYPES.UNKNOWN;
   }
 
   _getKey(id, type) {
     return id + "-" + type;
   }
 
+  _getKeyByRemoteId(remoteId) {
+    if (!remoteId) {
+      // If no remote id was provided, return the key corresponding to the local
+      // this-firefox runtime.
+      const { THIS_FIREFOX } = CONNECTION_TYPES;
+      return this._getKey(THIS_FIREFOX, THIS_FIREFOX);
+    }
+
+    return decodeURIComponent(remoteId);
+  }
+
   _removeClientByKey(key) {
     const client = this._clients.get(key);
     if (client) {
       client.removeListener("closed", this._onClientClosed);
       this._clients.delete(key);
+      this._runtimeInfoMap.delete(key);
     }
   }
 
   /**
    * Cleanup all closed clients when a "closed" notification is received from a client.
    */
   _onClientClosed() {
     const closedClientKeys = [...this._clients.keys()].filter(key => {
--- a/devtools/client/shared/remote-debugging/test/unit/test_remote_client_manager.js
+++ b/devtools/client/shared/remote-debugging/test/unit/test_remote_client_manager.js
@@ -6,46 +6,72 @@
 const { remoteClientManager } =
   require("devtools/client/shared/remote-debugging/remote-client-manager");
 const { CONNECTION_TYPES } =
   require("devtools/client/shared/remote-debugging/constants");
 
 add_task(async function testRemoteClientManager() {
   for (const type of Object.values(CONNECTION_TYPES)) {
     const fakeClient = createFakeClient();
+    const runtimeInfo = {};
     const clientId = "clientId";
     const remoteId = remoteClientManager.getRemoteId(clientId, type);
 
     const connectionType = remoteClientManager.getConnectionTypeByRemoteId(remoteId);
     equal(connectionType, type,
       `[${type}]: Correct connection type was returned by getConnectionTypeByRemoteId`);
 
     equal(remoteClientManager.hasClient(clientId, type), false,
       `[${type}]: hasClient returns false if no client was set`);
     equal(remoteClientManager.getClient(clientId, type), null,
       `[${type}]: getClient returns null if no client was set`);
     equal(remoteClientManager.getClientByRemoteId(remoteId), null,
       `[${type}]: getClientByRemoteId returns null if no client was set`);
+    equal(remoteClientManager.getRuntimeInfoByRemoteId(remoteId), null,
+      `[${type}]: getRuntimeInfoByRemoteId returns null if no client was set`);
 
-    remoteClientManager.setClient(clientId, type, fakeClient);
+    remoteClientManager.setClient(clientId, type, fakeClient, runtimeInfo);
     equal(remoteClientManager.hasClient(clientId, type), true,
       `[${type}]: hasClient returns true`);
     equal(remoteClientManager.getClient(clientId, type), fakeClient,
       `[${type}]: getClient returns the correct client`);
     equal(remoteClientManager.getClientByRemoteId(remoteId), fakeClient,
       `[${type}]: getClientByRemoteId returns the correct client`);
+    equal(remoteClientManager.getRuntimeInfoByRemoteId(remoteId), runtimeInfo,
+      `[${type}]: getRuntimeInfoByRemoteId returns the correct object`);
 
     remoteClientManager.removeClient(clientId, type);
     equal(remoteClientManager.hasClient(clientId, type), false,
       `[${type}]: hasClient returns false after removing the client`);
     equal(remoteClientManager.getClient(clientId, type), null,
       `[${type}]: getClient returns null after removing the client`);
     equal(remoteClientManager.getClientByRemoteId(remoteId), null,
       `[${type}]: getClientByRemoteId returns null after removing the client`);
+    equal(remoteClientManager.getRuntimeInfoByRemoteId(), null,
+      `[${type}]: getRuntimeInfoByRemoteId returns null after removing the client`);
   }
+
+  // Test various fallback scenarios for APIs relying on remoteId, when called without a
+  // remoteId, we expect to get the information for the local this-firefox runtime.
+  const { THIS_FIREFOX } = CONNECTION_TYPES;
+  const thisFirefoxClient = createFakeClient();
+  const thisFirefoxInfo = {};
+  remoteClientManager.setClient(THIS_FIREFOX, THIS_FIREFOX, thisFirefoxClient,
+    thisFirefoxInfo);
+
+  equal(remoteClientManager.getClientByRemoteId(), thisFirefoxClient,
+    `[fallback]: getClientByRemoteId returns this-firefox if remoteId is null`);
+  equal(remoteClientManager.getRuntimeInfoByRemoteId(), thisFirefoxInfo,
+    `[fallback]: getRuntimeInfoByRemoteId returns this-firefox if remoteId is null`);
+
+  const otherRemoteId = remoteClientManager.getRemoteId("clientId", CONNECTION_TYPES.USB);
+  equal(remoteClientManager.getClientByRemoteId(otherRemoteId), null,
+    `[fallback]: getClientByRemoteId does not fallback if remoteId is non-null`);
+  equal(remoteClientManager.getRuntimeInfoByRemoteId(otherRemoteId), null,
+    `[fallback]: getRuntimeInfoByRemoteId does not fallback if remoteId is non-null`);
 });
 
 add_task(async function testRemoteClientManagerWithUnknownType() {
   const remoteId = remoteClientManager.getRemoteId("someClientId", "NotARealType");
   const connectionType = remoteClientManager.getConnectionTypeByRemoteId(remoteId);
   equal(connectionType, CONNECTION_TYPES.UNKNOWN,
     `Connection type UNKNOWN was returned by getConnectionTypeByRemoteId`);
 });