Bug 1497457 - Remove transportDetails from runtimeDetails;r=daisuke,ladybenko
☠☠ backed out by a6eccac5baa2 ☠ ☠
authorJulian Descottes <jdescottes@mozilla.com>
Mon, 26 Nov 2018 19:57:35 +0000
changeset 504546 bd490139b395d47c6f1b4af6cf34f572a1902fe2
parent 504545 b2830500918c4bcef6bbbc794d1d70c0626b784b
child 504547 f3849030a93c44b14343bae1cc70709b8a56fa71
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_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_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));
   }
 
@@ -65,28 +62,27 @@ class RemoteClientManager {
   }
 
   _getKey(id, type) {
     return id + "-" + type;
   }
 
   _removeClientByKey(key) {
     if (this.hasClient(key)) {
-      this.getClient(key).client.removeListener("closed", this._onClientClosed);
+      this.getClient(key).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);
     }
   }
 }