Bug 1597388 - Hook about:profiling up to remote debugging; r=julienw,jdescottes

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

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

devtools/client/aboutdebugging/src/actions/ui.js

@@ -24,6 +24,7 @@ const {
   SELECT_PAGE_SUCCESS,
   SELECTED_RUNTIME_ID_UPDATED,
   SHOW_PROFILER_DIALOG,
+  SWITCH_PROFILER_CONTEXT,
   USB_RUNTIMES_SCAN_START,
   USB_RUNTIMES_SCAN_SUCCESS,
 } = require("devtools/client/aboutdebugging/src/constants");
@@ -109,6 +110,14 @@ function showProfilerDialog() {
   return { type: SHOW_PROFILER_DIALOG };
 }
 
+/**
+ * The profiler can switch between "devtools-remote" and "aboutprofiling-remote"
+ * page contexts.
+ */
+function switchProfilerContext(profilerContext) {
+  return { type: SWITCH_PROFILER_CONTEXT, profilerContext };
+}
+
 function hideProfilerDialog() {
   return { type: HIDE_PROFILER_DIALOG };
 }
@@ -182,6 +191,7 @@ module.exports = {
   scanUSBRuntimes,
   selectPage,
   showProfilerDialog,
+  switchProfilerContext,
   uninstallAdbAddon,
   updateAdbAddonStatus,
   updateAdbReady,

devtools/client/aboutdebugging/src/components/ProfilerDialog.css

@@ -34,11 +34,19 @@
 .profiler-dialog__inner {
   background-color: var(--white-100);
   display: grid;
-  height: calc(var(--base-unit) * 150); /* 600px */
   grid-template-rows: max-content auto;
   max-height: calc(100% - calc(var(--base-unit) * 25)); /* 100% - 100px */
   position: fixed;
+}
+
+.profiler-dialog__inner--medium {
   width: calc(var(--base-unit) * 150); /* 600px */
+  height: calc(var(--base-unit) * 150); /* 600px */
+}
+
+.profiler-dialog__inner--large {
+  width: calc(var(--base-unit) * 200); /* 800px */
+  height: calc(var(--base-unit) * 175); /* 700px */
 }
 
 .profiler-dialog__mask {

devtools/client/aboutdebugging/src/components/ProfilerDialog.js

@@ -4,6 +4,7 @@
 
 "use strict";
 
+const { connect } = require("devtools/client/shared/vendor/react-redux");
 const {
   createFactory,
   PureComponent,
@@ -16,24 +17,80 @@ const Localized = createFactory(FluentReact.Localized);
 
 const Actions = require("devtools/client/aboutdebugging/src/actions/index");
 const Types = require("devtools/client/aboutdebugging/src/types/index");
+const {
+  PROFILER_PAGE_CONTEXT,
+} = require("devtools/client/aboutdebugging/src/constants");
 
 /**
- * This component a modal dialog containing the performance profiler UI.
+ * This component is a modal dialog containing the performance profiler UI. It uses
+ * the simplified DevTools panel located in devtools/client/performance-new. When
+ * using a custom preset, and editing the settings, the page context switches
+ * to about:profiling, which receives the PerfFront of the remote debuggee.
  */
 class ProfilerDialog extends PureComponent {
   static get propTypes() {
     return {
       runtimeDetails: Types.runtimeDetails.isRequired,
-      dispatch: PropTypes.func.isRequired,
+      profilerContext: PropTypes.string.isRequired,
+      hideProfilerDialog: PropTypes.func.isRequired,
+      switchProfilerContext: PropTypes.func.isRequired,
     };
   }
 
   hide() {
-    this.props.dispatch(Actions.hideProfilerDialog());
+    this.props.hideProfilerDialog();
+  }
+
+  /**
+   * The profiler iframe can either be the simplified devtools recording panel,
+   * or the more detailed about:profiling settings page.
+   */
+  renderProfilerIframe() {
+    const {
+      runtimeDetails: { clientWrapper },
+      switchProfilerContext,
+      profilerContext,
+    } = this.props;
+
+    let src, onLoad;
+
+    switch (profilerContext) {
+      case PROFILER_PAGE_CONTEXT.DEVTOOLS_REMOTE:
+        src = clientWrapper.getPerformancePanelUrl();
+        onLoad = e => {
+          clientWrapper.loadPerformanceProfiler(e.target.contentWindow, () => {
+            switchProfilerContext(PROFILER_PAGE_CONTEXT.ABOUTPROFILING_REMOTE);
+          });
+        };
+        break;
+
+      case PROFILER_PAGE_CONTEXT.ABOUTPROFILING_REMOTE:
+        src = "about:profiling#remote";
+        onLoad = e => {
+          clientWrapper.loadAboutProfiling(e.target.contentWindow, () => {
+            switchProfilerContext(PROFILER_PAGE_CONTEXT.DEVTOOLS_REMOTE);
+          });
+        };
+        break;
+
+      default:
+        throw new Error(`Unhandled profiler context: "${profilerContext}"`);
+    }
+
+    return dom.iframe({
+      key: profilerContext,
+      className: "profiler-dialog__frame",
+      src,
+      onLoad,
+    });
   }
 
   render() {
-    const { clientWrapper } = this.props.runtimeDetails;
+    const { profilerContext, switchProfilerContext } = this.props;
+    const dialogSizeClassName =
+      profilerContext === PROFILER_PAGE_CONTEXT.DEVTOOLS_REMOTE
+        ? "profiler-dialog__inner--medium"
+        : "profiler-dialog__inner--large";
 
     return dom.div(
       {
@@ -42,7 +99,7 @@ class ProfilerDialog extends PureComponent {
       },
       dom.article(
         {
-          className: "profiler-dialog__inner qa-profiler-dialog",
+          className: `profiler-dialog__inner ${dialogSizeClassName} qa-profiler-dialog`,
           onClick: e => e.stopPropagation(),
         },
         dom.header(
@@ -63,23 +120,37 @@ class ProfilerDialog extends PureComponent {
           dom.button(
             {
               className: "ghost-button qa-profiler-dialog-close",
-              onClick: () => this.hide(),
+              onClick: () => {
+                if (profilerContext === PROFILER_PAGE_CONTEXT.DEVTOOLS_REMOTE) {
+                  this.hide();
+                } else {
+                  switchProfilerContext(PROFILER_PAGE_CONTEXT.DEVTOOLS_REMOTE);
+                }
+              },
             },
             dom.img({
               src: "chrome://devtools/skin/images/close.svg",
             })
           )
         ),
-        dom.iframe({
-          className: "profiler-dialog__frame",
-          src: clientWrapper.getPerformancePanelUrl(),
-          onLoad: e => {
-            clientWrapper.loadPerformanceProfiler(e.target.contentWindow);
-          },
-        })
+        this.renderProfilerIframe()
       )
     );
   }
 }
 
-module.exports = ProfilerDialog;
+const mapStateToProps = state => {
+  return {
+    profilerContext: state.ui.profilerContext,
+  };
+};
+
+const mapDispatchToProps = {
+  hideProfilerDialog: Actions.hideProfilerDialog,
+  switchProfilerContext: Actions.switchProfilerContext,
+};
+
+module.exports = connect(
+  mapStateToProps,
+  mapDispatchToProps
+)(ProfilerDialog);

devtools/client/aboutdebugging/src/constants.js

@@ -28,6 +28,7 @@ const actionTypes = {
   DISCONNECT_RUNTIME_START: "DISCONNECT_RUNTIME_START",
   DISCONNECT_RUNTIME_SUCCESS: "DISCONNECT_RUNTIME_SUCCESS",
   HIDE_PROFILER_DIALOG: "HIDE_PROFILER_DIALOG",
+  SWITCH_PROFILER_CONTEXT: "SWITCH_PROFILER_CONTEXT",
   NETWORK_LOCATIONS_UPDATE_FAILURE: "NETWORK_LOCATIONS_UPDATE_FAILURE",
   NETWORK_LOCATIONS_UPDATE_START: "NETWORK_LOCATIONS_UPDATE_START",
   NETWORK_LOCATIONS_UPDATE_SUCCESS: "NETWORK_LOCATIONS_UPDATE_SUCCESS",
@@ -150,6 +151,16 @@ const USB_STATES = {
   UPDATING_USB: "UPDATING_USB",
 };
 
+/**
+ * These constants reference the performance-new's concept of a PageContext.
+ * These are defined in devtools/client/performance-new/@types/perf.d.ts
+ * about:debugging only uses the remote variants of the PageContexts.
+ */
+const PROFILER_PAGE_CONTEXT = {
+  DEVTOOLS_REMOTE: "devtools-remote",
+  ABOUTPROFILING_REMOTE: "aboutprofiling-remote",
+};
+
 // flatten constants
 module.exports = Object.assign(
   {},
@@ -165,6 +176,7 @@ module.exports = Object.assign(
     SERVICE_WORKER_FETCH_STATES,
     SERVICE_WORKER_STATUSES,
     USB_STATES,
+    PROFILER_PAGE_CONTEXT,
   },
   actionTypes
 );

devtools/client/aboutdebugging/src/modules/client-wrapper.js

@@ -174,10 +174,22 @@ class ClientWrapper {
     return "chrome://devtools/content/performance-new/index.xhtml";
   }
 
-  async loadPerformanceProfiler(win) {
+  /**
+   * @param {Window} win - The window of the dialog window.
+   * @param {Function} openAboutProfiling
+   */
+  async loadPerformanceProfiler(win, openAboutProfiling) {
     const perfFront = await this.getFront("perf");
-    const perfActorVersion = this.client.mainRoot.traits.perfActorVersion;
-    win.gInit(perfFront, "devtools-remote", perfActorVersion);
+    win.gInit(perfFront, "devtools-remote", openAboutProfiling);
+  }
+
+  /**
+   * @param {Window} win - The window of the dialog window.
+   * @param {Function} openRemoteDevTools
+   */
+  async loadAboutProfiling(win, openRemoteDevTools) {
+    const perfFront = await this.getFront("perf");
+    win.gInit(perfFront, "aboutprofiling-remote", openRemoteDevTools);
   }
 }
 

devtools/client/aboutdebugging/src/reducers/ui-state.js

@@ -10,8 +10,10 @@ const {
   DEBUG_TARGET_COLLAPSIBILITY_UPDATED,
   HIDE_PROFILER_DIALOG,
   NETWORK_LOCATIONS_UPDATE_SUCCESS,
+  PROFILER_PAGE_CONTEXT,
   SELECT_PAGE_SUCCESS,
   SHOW_PROFILER_DIALOG,
+  SWITCH_PROFILER_CONTEXT,
   TEMPORARY_EXTENSION_INSTALL_FAILURE,
   TEMPORARY_EXTENSION_INSTALL_SUCCESS,
   USB_RUNTIMES_SCAN_START,
@@ -29,6 +31,7 @@ function UiState(
     isAdbReady: false,
     isScanningUsb: false,
     networkLocations: locations,
+    profilerContext: PROFILER_PAGE_CONTEXT.DEVTOOLS_REMOTE,
     selectedPage: null,
     showProfilerDialog: false,
     showHiddenAddons,
@@ -68,13 +71,22 @@ function uiReducer(state = UiState(), action) {
     }
 
     case SHOW_PROFILER_DIALOG: {
-      return Object.assign({}, state, { showProfilerDialog: true });
+      return Object.assign({}, state, {
+        showProfilerDialog: true,
+        // Always start in the devtools-remote view.
+        profilerContext: "devtools-remote",
+      });
     }
 
     case HIDE_PROFILER_DIALOG: {
       return Object.assign({}, state, { showProfilerDialog: false });
     }
 
+    case SWITCH_PROFILER_CONTEXT: {
+      const { profilerContext } = action;
+      return Object.assign({}, state, { profilerContext });
+    }
+
     case USB_RUNTIMES_SCAN_START: {
       return Object.assign({}, state, { isScanningUsb: true });
     }

devtools/client/performance-new/@types/perf.d.ts

@@ -58,6 +58,7 @@ export interface PerfFront {
   isLockedForPrivateBrowsing: () => MaybePromise<boolean>;
   on: (type: string, listener: () => void) => void;
   off: (type: string, listener: () => void) => void;
+  destroy: () => void,
   /**
    * This method was was added in Firefox 72.
    */
@@ -97,7 +98,11 @@ export type RecordingState =
 // We are currently migrating to a new UX workflow with about:profiling.
 // This type provides an easy way to change the implementation based
 // on context.
-export type PageContext = "devtools" | "devtools-remote" | "aboutprofiling";
+export type PageContext =
+  | "devtools"
+  | "devtools-remote"
+  | "aboutprofiling"
+  | "aboutprofiling-remote";
 
 export interface State {
   recordingState: RecordingState;
@@ -221,6 +226,11 @@ export interface InitializedValues {
   // by the actor system. This compatibility can be required when the ESR version
   // is running at least Firefox 72.
   supportedFeatures: string[] | null
+  // Allow different devtools contexts to open about:profiling with different methods.
+  // e.g. via a new tab, or page navigation.
+  openAboutProfiling?: () => void,
+  // Allow about:profiling to switch back to the remote devtools panel.
+  openRemoteDevTools?: () => void,
 }
 
 /**
@@ -268,6 +278,8 @@ export type Action =
       setRecordingPreferences: SetRecordingPreferences;
       presets: Presets;
       pageContext: PageContext;
+      openAboutProfiling?: () => void,
+      openRemoteDevTools?: () => void,
       recordingSettingsFromPreferences: RecordingStateFromPreferences;
       getSymbolTableGetter: (profile: object) => GetSymbolTableCallback;
       supportedFeatures: string[] | null;
@@ -287,6 +299,8 @@ export interface InitializeStoreValues {
   recordingPreferences: RecordingStateFromPreferences;
   supportedFeatures: string[] | null;
   getSymbolTableGetter: (profile: object) => GetSymbolTableCallback;
+  openAboutProfiling?: () => void;
+  openRemoteDevTools?: () => void;
 }
 
 export type PopupBackgroundFeatures = { [feature: string]: boolean };

devtools/client/performance-new/aboutprofiling/initializer.js

@@ -6,6 +6,8 @@
  * @typedef {import("../@types/perf").InitializeStoreValues} InitializeStoreValues
  * @typedef {import("../@types/perf").PopupWindow} PopupWindow
  * @typedef {import("../@types/perf").RecordingStateFromPreferences} RecordingStateFromPreferences
+ * @typedef {import("../@types/perf").PerfFront} PerfFront
+ * @typedef {import("../@types/perf").PageContext} PageContext
  */
 "use strict";
 
@@ -71,33 +73,37 @@ const {
 
 /**
  * Initialize the panel by creating a redux store, and render the root component.
+ *
+ * @param {PerfFront} perfFront - The Perf actor's front. Used to start and stop recordings.
+ * @param {PageContext} pageContext - The context that the UI is being loaded in under.
+ * @param {(() => void)} [openRemoteDevTools] Optionally provide a way to go back to
+ *                                            the remote devtools page.
  */
-
-document.addEventListener("DOMContentLoaded", async () => {
+async function gInit(perfFront, pageContext, openRemoteDevTools) {
   const store = createStore(reducers);
-  const perfFrontInterface = new ActorReadyGeckoProfilerInterface();
-  const supportedFeatures = await perfFrontInterface.getSupportedFeatures();
+  const supportedFeatures = await perfFront.getSupportedFeatures();
 
   // Do some initialization, especially with privileged things that are part of the
   // the browser.
   store.dispatch(
     actions.initializeStore({
-      perfFront: perfFrontInterface,
+      perfFront,
       receiveProfile,
       supportedFeatures,
       presets,
       // Get the preferences from the current browser
-      recordingPreferences: getRecordingPreferences("aboutprofiling"),
+      recordingPreferences: getRecordingPreferences(pageContext),
       /**
        * @param {RecordingStateFromPreferences} newRecordingPreferences
        */
       setRecordingPreferences: newRecordingPreferences =>
-        setRecordingPreferences("aboutprofiling", newRecordingPreferences),
+        setRecordingPreferences(pageContext, newRecordingPreferences),
 
       // The popup doesn't need to support remote symbol tables from the debuggee.
       // Only get the symbols from this browser.
       getSymbolTableGetter: () => getSymbolsFromThisBrowser,
-      pageContext: "aboutprofiling",
+      pageContext,
+      openRemoteDevTools,
     })
   );
 
@@ -116,8 +122,20 @@ document.addEventListener("DOMContentLoaded", async () => {
   );
 
   window.addEventListener("unload", function() {
-    // The perf front interface needs to be unloaded in order to remove event handlers.
-    // Not doing so leads to leaks.
-    perfFrontInterface.destroy();
+    // Do not destroy the perf front if working remotely, about:debugging will do
+    // this for us.
+    if (pageContext !== "aboutprofiling-remote") {
+      // The perf front interface needs to be unloaded in order to remove event handlers.
+      // Not doing so leads to leaks.
+      perfFront.destroy();
+    }
   });
-});
+}
+
+// Automatically initialize the page if it's not a remote connection, otherwise
+// the page will be initialized by about:debugging.
+if (window.location.hash !== "#remote") {
+  document.addEventListener("DOMContentLoaded", () => {
+    gInit(new ActorReadyGeckoProfilerInterface(), "aboutprofiling");
+  });
+}

devtools/client/performance-new/components/AboutProfiling.js

@@ -13,6 +13,7 @@
  * @property {boolean?} isSupportedPlatform
  * @property {PageContext} pageContext
  * @property {string | null} promptEnvRestart
+ * @property {(() => void) | undefined} openRemoteDevTools
  */
 
 /**
@@ -70,7 +71,12 @@ class AboutProfiling extends PureComponent {
   }
 
   render() {
-    const { isSupportedPlatform, pageContext, promptEnvRestart } = this.props;
+    const {
+      isSupportedPlatform,
+      pageContext,
+      promptEnvRestart,
+      openRemoteDevTools,
+    } = this.props;
 
     if (isSupportedPlatform === null) {
       // We don't know yet if this is a supported platform, wait for a response.
@@ -100,6 +106,21 @@ class AboutProfiling extends PureComponent {
             )
           )
         : null,
+
+      openRemoteDevTools
+        ? div(
+            { className: "perf-back" },
+            button(
+              {
+                className: "perf-back-button",
+                type: "button",
+                onClick: openRemoteDevTools,
+              },
+              "Save settings and go back"
+            )
+          )
+        : null,
+
       div(
         { className: "perf-intro" },
         h1({ className: "perf-intro-title" }, "Profiler Settings"),
@@ -130,6 +151,7 @@ function mapStateToProps(state) {
     isSupportedPlatform: selectors.getIsSupportedPlatform(state),
     pageContext: selectors.getPageContext(state),
     promptEnvRestart: selectors.getPromptEnvRestart(state),
+    openRemoteDevTools: selectors.getOpenRemoteDevTools(state),
   };
 }
 

devtools/client/performance-new/components/DevToolsPresetSelection.js

@@ -14,6 +14,7 @@
  * @property {number} interval
  * @property {string[]} threads
  * @property {string[]} features
+ * @property {() => void} openAboutProfiling
  * @property {import("../@types/perf").Presets} presets
  */
 
@@ -61,7 +62,6 @@ class DevToolsPresetSelection extends PureComponent {
   constructor(props) {
     super(props);
     this.onPresetChange = this.onPresetChange.bind(this);
-    this.handleLinkClick = this.handleLinkClick.bind(this);
 
     /**
      * Create an object map to easily look up feature description.
@@ -82,13 +82,8 @@ class DevToolsPresetSelection extends PureComponent {
     this.props.changePreset(presets, event.target.value);
   }
 
-  handleLinkClick() {
-    const { openTrustedLink } = require("devtools/client/shared/link");
-    openTrustedLink("about:profiling", {});
-  }
-
   render() {
-    const { presetName, presets } = this.props;
+    const { presetName, presets, openAboutProfiling } = this.props;
 
     let presetDescription;
     const currentPreset = presets[presetName];
@@ -124,7 +119,7 @@ class DevToolsPresetSelection extends PureComponent {
           })
         ),
         button(
-          { className: "perf-external-link", onClick: this.handleLinkClick },
+          { className: "perf-external-link", onClick: openAboutProfiling },
           "Edit Settings…"
         )
       );
@@ -170,6 +165,7 @@ function mapStateToProps(state) {
     interval: selectors.getInterval(state),
     threads: selectors.getThreads(state),
     features: selectors.getFeatures(state),
+    openAboutProfiling: selectors.getOpenAboutProfiling(state),
   };
 }
 

devtools/client/performance-new/initializer.js

@@ -71,8 +71,9 @@ const {
  *
  * @param {PerfFront} perfFront - The Perf actor's front. Used to start and stop recordings.
  * @param {PageContext} pageContext - The context that the UI is being loaded in under.
+ * @param {(() => void)?} openAboutProfiling - Optional call to open about:profiling
  */
-async function gInit(perfFront, pageContext) {
+async function gInit(perfFront, pageContext, openAboutProfiling) {
   const store = createStore(reducers);
   // Get the supported features from the debuggee. If the debuggee is before
   // Firefox 72, then return null, as the actor does not support that feature.
@@ -86,6 +87,13 @@ async function gInit(perfFront, pageContext) {
     perfFront.getSupportedFeatures()
   ).catch(() => null);
 
+  if (!openAboutProfiling) {
+    openAboutProfiling = () => {
+      const { openTrustedLink } = require("devtools/client/shared/link");
+      openTrustedLink("about:profiling", {});
+    };
+  }
+
   // Do some initialization, especially with privileged things that are part of the
   // the browser.
   store.dispatch(
@@ -95,6 +103,7 @@ async function gInit(perfFront, pageContext) {
       recordingPreferences: getRecordingPreferences(pageContext),
       presets,
       supportedFeatures,
+      openAboutProfiling,
       pageContext: "devtools",
 
       // Go ahead and hide the implementation details for the component on how the

devtools/client/performance-new/popup/background.jsm.js

@@ -330,6 +330,7 @@ function getPrefPostfix(pageContext) {
       // Don't use any postfix on the prefs.
       return "";
     case "devtools-remote":
+    case "aboutprofiling-remote":
       return ".remote";
     default: {
       const { UnhandledCaseError } = lazyUtils();

devtools/client/performance-new/store/reducers.js

@@ -185,6 +185,8 @@ function initializedValues(state = null, action) {
         pageContext: action.pageContext,
         getSymbolTableGetter: action.getSymbolTableGetter,
         supportedFeatures: action.supportedFeatures,
+        openAboutProfiling: action.openAboutProfiling,
+        openRemoteDevTools: action.openRemoteDevTools,
       };
     default:
       return state;

devtools/client/performance-new/store/selectors.js

@@ -56,6 +56,28 @@ const getPresets = state => getInitializedValues(state).presets;
 /** @type {Selector<string>} */
 const getPresetName = state => state.presetName;
 
+/**
+ * When remote profiling, there will be a back button to the settings.
+ *
+ * @type {Selector<(() => void) | undefined>}
+ */
+const getOpenRemoteDevTools = state =>
+  getInitializedValues(state).openRemoteDevTools;
+
+/**
+ * Get the functon to open about:profiling. This assumes that the function exists,
+ * otherwise it will throw an error.
+ *
+ * @type {Selector<() => void>}
+ */
+const getOpenAboutProfiling = state => {
+  const { openAboutProfiling } = getInitializedValues(state);
+  if (!openAboutProfiling) {
+    throw new Error("Expected to get an openAboutProfiling function.");
+  }
+  return openAboutProfiling;
+};
+
 /**
  * Warning! This function returns a new object on every run, and so should not
  * be used directly as a React prop.
@@ -138,6 +160,8 @@ module.exports = {
   getObjdirs,
   getPresets,
   getPresetName,
+  getOpenRemoteDevTools,
+  getOpenAboutProfiling,
   getRecordingSettings,
   getInitializedValues,
   getPerfFront,

devtools/client/themes/aboutprofiling.css

@@ -196,3 +196,13 @@ h2 {
   display: flex;
   align-items: start;
 }
+
+.perf-back {
+  margin-block-end: 13px;
+}
+
+.perf-back-button {
+  /* Remove the inherited margins */
+  margin-left: 0;
+  margin-right: 0;
+}