Bug 1497457 - Remove transportDetails from runtimeDetails;r=daisuke,ladybenko
authorJulian Descottes <jdescottes@mozilla.com>
Tue, 27 Nov 2018 10:23:24 +0000
changeset 504662 99da1452e409ddf36bcdc827aa13c6241092cbc0
parent 504661 5cda18843d7b6fe9bfbc20b9bfb0a5b3e0bfa84b
child 504663 a9127ac945bd49e22fd62cdb0c1305a866c2b2ce
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdaisuke, ladybenko
bugs1497457
milestone65.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 1497457 - Remove transportDetails from runtimeDetails;r=daisuke,ladybenko Depends on D12095. Now that we pass a remoteId to open about:devtools-toolbox, we actually don't need transportDetails Differential Revision: https://phabricator.services.mozilla.com/D12097
devtools/client/aboutdebugging-new/src/actions/runtimes.js
devtools/client/aboutdebugging-new/src/modules/runtime-client-factory.js
devtools/client/aboutdebugging-new/src/reducers/runtimes-state.js
devtools/client/aboutdebugging-new/src/types/runtime.js
devtools/client/aboutdebugging-new/test/browser/browser_aboutdebugging_system_addons.js
devtools/client/aboutdebugging-new/test/browser/browser_aboutdebugging_thisfirefox_runtime_info.js
devtools/client/aboutdebugging-new/test/browser/mocks/head-usb-mocks.js
devtools/client/framework/target-from-url.js
devtools/client/shared/remote-debugging/remote-client-manager.js
--- a/devtools/client/aboutdebugging-new/src/actions/runtimes.js
+++ b/devtools/client/aboutdebugging-new/src/actions/runtimes.js
@@ -66,26 +66,25 @@ function onUSBDebuggerClientClosed() {
   window.AboutDebugging.store.dispatch(Actions.scanUSBRuntimes());
 }
 
 function connectRuntime(id) {
   return async (dispatch, getState) => {
     dispatch({ type: CONNECT_RUNTIME_START });
     try {
       const runtime = findRuntimeById(id, getState().runtimes);
-      const { clientWrapper, transportDetails } = await createClientForRuntime(runtime);
+      const clientWrapper = await createClientForRuntime(runtime);
       const info = await getRuntimeInfo(runtime, clientWrapper);
 
       const promptPrefName = RUNTIME_PREFERENCE.CONNECTION_PROMPT;
       const connectionPromptEnabled = await clientWrapper.getPreference(promptPrefName);
       const runtimeDetails = {
         clientWrapper,
         connectionPromptEnabled,
         info,
-        transportDetails,
       };
 
       if (runtime.type === RUNTIMES.USB) {
         // `closed` event will be emitted when disabling remote debugging
         // on the connected USB runtime.
         clientWrapper.addOneTimeListener("closed", onUSBDebuggerClientClosed);
       }
 
--- a/devtools/client/aboutdebugging-new/src/modules/runtime-client-factory.js
+++ b/devtools/client/aboutdebugging-new/src/modules/runtime-client-factory.js
@@ -13,40 +13,39 @@ const { remoteClientManager } =
 
 const { RUNTIMES } = require("../constants");
 
 async function createLocalClient() {
   DebuggerServer.init();
   DebuggerServer.registerAllActors();
   const client = new DebuggerClient(DebuggerServer.connectPipe());
   await client.connect();
-  return { clientWrapper: new ClientWrapper(client) };
+  return new ClientWrapper(client);
 }
 
 async function createNetworkClient(host, port) {
-  const transportDetails = { host, port };
-  const transport = await DebuggerClient.socketConnect(transportDetails);
+  const transport = await DebuggerClient.socketConnect({ host, port });
   const client = new DebuggerClient(transport);
   await client.connect();
-  return { clientWrapper: new ClientWrapper(client), transportDetails };
+  return new ClientWrapper(client);
 }
 
 async function createUSBClient(socketPath) {
   const port = await ADB.prepareTCPConnection(socketPath);
   return createNetworkClient("localhost", port);
 }
 
 async function createClientForRuntime(runtime) {
   const { extra, id, type } = runtime;
 
   if (type === RUNTIMES.THIS_FIREFOX) {
     return createLocalClient();
   } else if (remoteClientManager.hasClient(id, type)) {
-    const { client, transportDetails } = remoteClientManager.getClient(id, type);
-    return { clientWrapper: new ClientWrapper(client), transportDetails };
+    const client = remoteClientManager.getClient(id, type);
+    return new ClientWrapper(client);
   } else if (type === RUNTIMES.NETWORK) {
     const { host, port } = extra.connectionParameters;
     return createNetworkClient(host, port);
   } else if (type === RUNTIMES.USB) {
     const { socketPath } = extra.connectionParameters;
     return createUSBClient(socketPath);
   }
 
--- a/devtools/client/aboutdebugging-new/src/reducers/runtimes-state.js
+++ b/devtools/client/aboutdebugging-new/src/reducers/runtimes-state.js
@@ -69,20 +69,17 @@ function _updateRuntimeById(runtimeId, u
   });
   return Object.assign({}, state, { [key]: updatedRuntimes });
 }
 
 function runtimesReducer(state = RuntimesState(), action) {
   switch (action.type) {
     case CONNECT_RUNTIME_SUCCESS: {
       const { id, runtimeDetails, type } = action.runtime;
-      remoteClientManager.setClient(id, type, {
-        client: runtimeDetails.clientWrapper.client,
-        transportDetails: runtimeDetails.transportDetails,
-      });
+      remoteClientManager.setClient(id, type, runtimeDetails.clientWrapper.client);
       return _updateRuntimeById(id, { runtimeDetails }, state);
     }
 
     case DISCONNECT_RUNTIME_SUCCESS: {
       const { id, type } = action.runtime;
       remoteClientManager.removeClient(id, type);
       return _updateRuntimeById(id, { runtimeDetails: null }, state);
     }
--- a/devtools/client/aboutdebugging-new/src/types/runtime.js
+++ b/devtools/client/aboutdebugging-new/src/types/runtime.js
@@ -17,37 +17,25 @@ const runtimeInfo = {
 
   // name of runtime such as "Firefox Nightly"
   name: PropTypes.string.isRequired,
 
   // version of runtime
   version: PropTypes.string.isRequired,
 };
 
-const runtimeTransportDetails = {
-  // host name of tcp connection to debugger server
-  host: PropTypes.string.isRequired,
-
-  // port number of tcp connection to debugger server
-  port: PropTypes.number.isRequired,
-};
-
 const runtimeDetails = {
   // ClientWrapper built using a DebuggerClient for the runtime
   clientWrapper: PropTypes.instanceOf(ClientWrapper).isRequired,
 
   // reflect devtools.debugger.prompt-connection preference of this runtime
   connectionPromptEnabled: PropTypes.bool.isRequired,
 
   // runtime information
   info: PropTypes.shape(runtimeInfo).isRequired,
-
-  // tcp connection information,
-  // unavailable on this-firefox runtime
-  transportDetails: PropTypes.shape(runtimeTransportDetails),
 };
 
 const networkRuntimeConnectionParameter = {
   // host name of debugger server to connect
   host: PropTypes.string.isRequired,
 
   // port number of debugger server to connect
   port: PropTypes.number.isRequired,
--- a/devtools/client/aboutdebugging-new/test/browser/browser_aboutdebugging_system_addons.js
+++ b/devtools/client/aboutdebugging-new/test/browser/browser_aboutdebugging_system_addons.js
@@ -54,17 +54,17 @@ add_task(async function testShowSystemAd
 // Create a basic mock for this-firefox client, and setup a runtime-client-factory mock
 // to return our mock client when needed.
 function setupThisFirefoxMock() {
   const runtimeClientFactoryMock = createRuntimeClientFactoryMock();
   const thisFirefoxClient = createThisFirefoxClientMock();
   runtimeClientFactoryMock.createClientForRuntime = runtime => {
     const { RUNTIMES } = require("devtools/client/aboutdebugging-new/src/constants");
     if (runtime.id === RUNTIMES.THIS_FIREFOX) {
-      return { clientWrapper: thisFirefoxClient };
+      return thisFirefoxClient;
     }
     throw new Error("Unexpected runtime id " + runtime.id);
   };
 
   info("Enable mocks");
   enableRuntimeClientFactoryMock(runtimeClientFactoryMock);
   registerCleanupFunction(() => {
     disableRuntimeClientFactoryMock();
--- a/devtools/client/aboutdebugging-new/test/browser/browser_aboutdebugging_thisfirefox_runtime_info.js
+++ b/devtools/client/aboutdebugging-new/test/browser/browser_aboutdebugging_thisfirefox_runtime_info.js
@@ -18,17 +18,17 @@ Services.scriptloader.loadSubScript(
 add_task(async function() {
   // Setup a mock for our runtime client factory to return the default THIS_FIREFOX client
   // when the client for the this-firefox runtime is requested.
   const runtimeClientFactoryMock = createRuntimeClientFactoryMock();
   const thisFirefoxClient = createThisFirefoxClientMock();
   runtimeClientFactoryMock.createClientForRuntime = runtime => {
     const { RUNTIMES } = require("devtools/client/aboutdebugging-new/src/constants");
     if (runtime.id === RUNTIMES.THIS_FIREFOX) {
-      return { clientWrapper: thisFirefoxClient };
+      return thisFirefoxClient;
     }
     throw new Error("Unexpected runtime id " + runtime.id);
   };
 
   info("Enable mocks");
   enableRuntimeClientFactoryMock(runtimeClientFactoryMock);
   registerCleanupFunction(() => {
     disableRuntimeClientFactoryMock();
--- a/devtools/client/aboutdebugging-new/test/browser/mocks/head-usb-mocks.js
+++ b/devtools/client/aboutdebugging-new/test/browser/mocks/head-usb-mocks.js
@@ -36,17 +36,17 @@ class UsbMocks {
 
     // Prepare a fake observer to be able to emit events from this mock.
     this._observerMock = addObserverMock(this.usbRuntimesMock);
 
     // Setup the runtime-client-factory mock to rely on the internal _clients map.
     this.runtimeClientFactoryMock = createRuntimeClientFactoryMock();
     this._clients = {};
     this.runtimeClientFactoryMock.createClientForRuntime = runtime => {
-      return { clientWrapper: this._clients[runtime.id] };
+      return this._clients[runtime.id];
     };
 
     // Add a client for THIS_FIREFOX, since about:debugging will start on the This Firefox
     // page.
     this._thisFirefoxClient = createThisFirefoxClientMock();
     this._clients[RUNTIMES.THIS_FIREFOX] = this._thisFirefoxClient;
   }
 
--- a/devtools/client/framework/target-from-url.js
+++ b/devtools/client/framework/target-from-url.js
@@ -133,17 +133,17 @@ exports.targetFromURL = async function t
  * @return a promise that resolves a DebuggerClient object
  */
 async function clientFromURL(url) {
   const params = url.searchParams;
 
   // If a remote id was provided we should already have a connected client available.
   const remoteId = params.get("remoteId");
   if (remoteId) {
-    return remoteClientManager.getClientByRemoteId(remoteId).client;
+    return remoteClientManager.getClientByRemoteId(remoteId);
   }
 
   const host = params.get("host");
   const port = params.get("port");
   const webSocket = !!params.get("ws");
 
   let transport;
   if (port) {
--- a/devtools/client/shared/remote-debugging/remote-client-manager.js
+++ b/devtools/client/shared/remote-debugging/remote-client-manager.js
@@ -16,24 +16,21 @@ class RemoteClientManager {
 
   /**
    * 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 {Object}
-   *        - client: {DebuggerClient}
-   *        - transportDetails: {Object} typically a host object ({hostname, port}) that
-   *          allows consumers to easily find the connection information for this client.
+   * @param {DebuggerClient} client
    */
-  setClient(id, type, { client, transportDetails }) {
+  setClient(id, type, client) {
     const key = this._getKey(id, type);
-    this._clients.set(key, { client, transportDetails });
+    this._clients.set(key, client);
     client.addOneTimeListener("closed", this._onClientClosed);
   }
 
   // See JSDoc for id, type from setClient.
   hasClient(id, type) {
     return this._clients.has(this._getKey(id, type));
   }
 
@@ -71,30 +68,29 @@ class RemoteClientManager {
     return this._clients.get(key);
   }
 
   _getKey(id, type) {
     return id + "-" + type;
   }
 
   _removeClientByKey(key) {
-    const client = this._clients.get(key).client;
+    const client = this._clients.get(key);
     if (client) {
       client.removeListener("closed", this._onClientClosed);
       this._clients.delete(key);
     }
   }
 
   /**
    * Cleanup all closed clients when a "closed" notification is received from a client.
    */
   _onClientClosed() {
     const closedClientKeys = [...this._clients.keys()].filter(key => {
-      const clientInfo = this._clients.get(key);
-      return clientInfo.client._closed;
+      return this._clients.get(key)._closed;
     });
 
     for (const key of closedClientKeys) {
       this._removeClientByKey(key);
     }
   }
 }