Bug 1597376 - Change the isPopup flag to a PageContext; r=julienw

The new about:profiling page creates some more complexity around what
the page context is. It is simpler to handle the different cases with
a union, rather than booleans.

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

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

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

@@ -96,6 +96,11 @@ export type RecordingState =
   // Profiling is not available when in private browsing mode.
   | "locked-by-private-browsing";
 
+// 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 = "popup" | "devtools" | "aboutprofiling";
+
 export interface State {
   recordingState: RecordingState;
   recordingUnexpectedlyStopped: boolean;
@@ -205,9 +210,8 @@ export interface InitializedValues {
   receiveProfile: ReceiveProfile;
   // A function to set the recording settings.
   setRecordingPreferences: SetRecordingPreferences;
-  // A boolean value that sets lets the UI know if it is in the popup window
-  // or inside of devtools.
-  isPopup: boolean;
+  // Determine the current page context.
+  pageContext: PageContext;
   // The popup and devtools panel use different codepaths for getting symbol tables.
   getSymbolTableGetter: (profile: object) => GetSymbolTableCallback;
   // The list of profiler features that the current target supports. Note that
@@ -260,7 +264,7 @@ export type Action =
       perfFront: PerfFront;
       receiveProfile: ReceiveProfile;
       setRecordingPreferences: SetRecordingPreferences;
-      isPopup: boolean;
+      pageContext: PageContext;
       recordingSettingsFromPreferences: RecordingStateFromPreferences;
       getSymbolTableGetter: (profile: object) => GetSymbolTableCallback;
       supportedFeatures: string[] | null;
@@ -270,7 +274,7 @@ export interface InitializeStoreValues {
   perfFront: PerfFront;
   receiveProfile: ReceiveProfile;
   setRecordingPreferences: SetRecordingPreferences;
-  isPopup: boolean;
+  pageContext: PageContext;
   recordingPreferences: RecordingStateFromPreferences;
   supportedFeatures: string[] | null;
   getSymbolTableGetter: (profile: object) => GetSymbolTableCallback;

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

@@ -11,7 +11,7 @@
 /**
  * @typedef {Object} StateProps
  * @property {boolean?} isSupportedPlatform
- * @property {boolean?} isPopup
+ * @property {PageContext} pageContext
  * @property {string | null} promptEnvRestart
  */
 
@@ -19,6 +19,7 @@
  * @typedef {StateProps} Props
  * @typedef {import("../@types/perf").State} StoreState
  * @typedef {import("../@types/perf").PanelWindow} PanelWindow
+ * @typedef {import("../@types/perf").PageContext} PageContext
  */
 
 "use strict";
@@ -72,17 +73,15 @@ class DevToolsAndPopup extends PureComponent {
   }
 
   render() {
-    const { isSupportedPlatform, isPopup, promptEnvRestart } = this.props;
+    const { isSupportedPlatform, pageContext, promptEnvRestart } = this.props;
 
     if (isSupportedPlatform === null) {
       // We don't know yet if this is a supported platform, wait for a response.
       return null;
     }
 
-    const additionalClassName = isPopup ? "perf-popup" : "perf-devtools";
-
     return div(
-      { className: `perf ${additionalClassName}` },
+      { className: `perf perf-${pageContext}` },
       promptEnvRestart
         ? div(
             { className: "perf-env-restart" },
@@ -106,7 +105,7 @@ class DevToolsAndPopup extends PureComponent {
         : null,
       RecordingButton(),
       Settings(),
-      isPopup ? null : Description()
+      pageContext === "devtools" ? Description() : null
     );
   }
 }
@@ -118,7 +117,7 @@ class DevToolsAndPopup extends PureComponent {
 function mapStateToProps(state) {
   return {
     isSupportedPlatform: selectors.getIsSupportedPlatform(state),
-    isPopup: selectors.getIsPopup(state),
+    pageContext: selectors.getPageContext(state),
     promptEnvRestart: selectors.getPromptEnvRestart(state),
   };
 }

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

@@ -13,7 +13,7 @@
  * @property {PerfFront} perfFront
  * @property {RecordingState} recordingState
  * @property {boolean?} isSupportedPlatform
- * @property {boolean?} isPopup
+ * @property {PageContext} pageContext
  * @property {string | null} promptEnvRestart
  */
 
@@ -29,6 +29,7 @@
  * @typedef {import("../@types/perf").PerfFront} PerfFront
  * @typedef {import("../@types/perf").RecordingState} RecordingState
  * @typedef {import("../@types/perf").State} StoreState
+ * @typedef {import("../@types/perf").PageContext} PageContext
  */
 
 /**
@@ -41,6 +42,7 @@ const { PureComponent } = require("devtools/client/shared/vendor/react");
 const { connect } = require("devtools/client/shared/vendor/react-redux");
 const actions = require("devtools/client/performance-new/store/actions");
 const selectors = require("devtools/client/performance-new/store/selectors");
+const { UnhandledCaseError } = require("devtools/client/performance-new/utils");
 
 /**
  * This component state changes for the performance recording. e.g. If the profiler
@@ -64,7 +66,7 @@ class ProfilerEventHandling extends PureComponent {
   }
 
   componentDidMount() {
-    const { perfFront, reportProfilerReady, isPopup } = this.props;
+    const { perfFront, reportProfilerReady, pageContext } = this.props;
 
     // Ask for the initial state of the profiler.
     Promise.all([
@@ -85,9 +87,22 @@ class ProfilerEventHandling extends PureComponent {
         if (isLockedForPrivateBrowsing) {
           recordingState = "locked-by-private-browsing";
         } else if (isActive) {
-          // The popup is a global control for the recording, so allow it to take
-          // control of it.
-          recordingState = isPopup ? "recording" : "other-is-recording";
+          switch (pageContext) {
+            case "popup":
+            case "aboutprofiling":
+              // These page contexts are in global control of the profiler, so allow it
+              // to take control of recording.
+              recordingState = "recording";
+              break;
+            case "devtools":
+              // The DevTools performance recording shouldn't take control of others
+              // use of the Gecko Profiler, since it's more interested in the current
+              // page.
+              recordingState = "other-is-recording";
+              break;
+            default:
+              throw new UnhandledCaseError(pageContext, "PageContext");
+          }
         } else {
           recordingState = "available-to-record";
         }
@@ -142,7 +157,7 @@ class ProfilerEventHandling extends PureComponent {
   }
 
   handleProfilerStarting() {
-    const { changeRecordingState, recordingState, isPopup } = this.props;
+    const { changeRecordingState, recordingState, pageContext } = this.props;
     switch (recordingState) {
       case "not-yet-known":
       // We couldn't have started it yet, so it must have been someone
@@ -153,14 +168,21 @@ class ProfilerEventHandling extends PureComponent {
       // We requested to stop the profiler, but someone else already started
       // it up. (fallthrough)
       case "request-to-get-profile-and-stop-profiler":
-        if (isPopup) {
-          // The profiler popup doesn't care who is recording. It will take control
-          // of it.
-          changeRecordingState("recording");
-        } else {
-          // Someone re-started the profiler while we were asking for the completed
-          // profile.
-          changeRecordingState("other-is-recording");
+        switch (pageContext) {
+          case "popup":
+          case "aboutprofiling":
+            // These page contexts are in global control of the profiler, so allow it
+            // to take control of recording.
+            changeRecordingState("recording");
+            break;
+          case "devtools":
+            // The DevTools performance recording shouldn't take control of others
+            // use of the Gecko Profiler, since it's more interested in the current
+            // page.
+            changeRecordingState("other-is-recording");
+            break;
+          default:
+            throw new UnhandledCaseError(pageContext, "PageContext");
         }
         break;
 
@@ -265,7 +287,7 @@ function mapStateToProps(state) {
     perfFront: selectors.getPerfFront(state),
     recordingState: selectors.getRecordingState(state),
     isSupportedPlatform: selectors.getIsSupportedPlatform(state),
-    isPopup: selectors.getIsPopup(state),
+    pageContext: selectors.getPageContext(state),
     promptEnvRestart: selectors.getPromptEnvRestart(state),
   };
 }

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

@@ -13,7 +13,7 @@
  * @property {RecordingState} recordingState
  * @property {boolean} isSupportedPlatform
  * @property {boolean} recordingUnexpectedlyStopped
- * @property {boolean} isPopup
+ * @property {PageContext} pageContext
  */
 
 /**
@@ -29,6 +29,7 @@
  * @typedef {StateProps & DispatchProps} Props
  * @typedef {import("../@types/perf").RecordingState} RecordingState
  * @typedef {import("../@types/perf").State} StoreState
+ * @typedef {import("../@types/perf").PageContext} PageContext
  */
 
 "use strict";
@@ -65,7 +66,7 @@ class RecordingButton extends PureComponent {
    *   onClick?: any,
    *   additionalMessage?: React.ReactNode,
    *   isPrimary?: boolean,
-   *   isPopup?: boolean,
+   *   pageContext?: PageContext,
    *   additionalButton?: {
    *     label: string,
    *     onClick: any,
@@ -79,12 +80,12 @@ class RecordingButton extends PureComponent {
       onClick,
       additionalMessage,
       isPrimary,
-      isPopup,
+      pageContext,
       additionalButton,
     } = buttonSettings;
 
     const nbsp = "\u00A0";
-    const showAdditionalMessage = isPopup && additionalMessage;
+    const showAdditionalMessage = pageContext === "popup" && additionalMessage;
     const buttonClass = isPrimary ? "primary" : "default";
 
     return div(
@@ -223,7 +224,7 @@ function mapStateToProps(state) {
     recordingUnexpectedlyStopped: selectors.getRecordingUnexpectedlyStopped(
       state
     ),
-    isPopup: selectors.getIsPopup(state),
+    pageContext: selectors.getPageContext(state),
   };
 }
 

devtools/client/performance-new/initializer.js

@@ -91,7 +91,7 @@ async function gInit(perfFront, preferenceFront) {
       receiveProfile,
       recordingPreferences,
       supportedFeatures,
-      isPopup: false,
+      pageContext: "devtools",
 
       // Go ahead and hide the implementation details for the component on how the
       // preference information is stored

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

@@ -106,7 +106,7 @@ async function gInit() {
       // The popup doesn't need to support remote symbol tables from the debuggee.
       // Only get the symbols from this browser.
       getSymbolTableGetter: () => getSymbolsFromThisBrowser,
-      isPopup: true,
+      pageContext: "popup",
     })
   );
 

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

@@ -107,7 +107,7 @@ exports.changeEntries = entries =>
 exports.changeFeatures = features => {
   return (dispatch, getState) => {
     let promptEnvRestart = null;
-    if (selectors.getIsPopup(getState())) {
+    if (selectors.getPageContext(getState()) === "popup") {
       // The popup supports checks to restart the browser for environment
       // variables.
       if (

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

@@ -152,7 +152,7 @@ function initializedValues(state = null, action) {
         perfFront: action.perfFront,
         receiveProfile: action.receiveProfile,
         setRecordingPreferences: action.setRecordingPreferences,
-        isPopup: Boolean(action.isPopup),
+        pageContext: action.pageContext,
         getSymbolTableGetter: action.getSymbolTableGetter,
         supportedFeatures: action.supportedFeatures,
       };

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

@@ -14,6 +14,7 @@
  * @typedef {import("../@types/perf").GetSymbolTableCallback} GetSymbolTableCallback
  * @typedef {import("../@types/perf").RestartBrowserWithEnvironmentVariable} RestartBrowserWithEnvironmentVariable
  * @typedef {import("../@types/perf").GetEnvironmentVariable} GetEnvironmentVariable
+ * @typedef {import("../@types/perf").PageContext} PageContext
  */
 /**
  * @template S
@@ -85,8 +86,8 @@ const getReceiveProfileFn = state => getInitializedValues(state).receiveProfile;
 const getSetRecordingPreferencesFn = state =>
   getInitializedValues(state).setRecordingPreferences;
 
-/** @type {Selector<boolean>} */
-const getIsPopup = state => getInitializedValues(state).isPopup;
+/** @type {Selector<PageContext>} */
+const getPageContext = state => getInitializedValues(state).pageContext;
 
 /** @type {Selector<(profile: Object) => GetSymbolTableCallback>} */
 const getSymbolTableGetter = state =>
@@ -114,7 +115,7 @@ module.exports = {
   getPerfFront,
   getReceiveProfileFn,
   getSetRecordingPreferencesFn,
-  getIsPopup,
+  getPageContext,
   getSymbolTableGetter,
   getPromptEnvRestart,
   getSupportedFeatures,

devtools/client/performance-new/test/chrome/head.js

@@ -224,7 +224,7 @@ function createPerfComponent() {
         recordingPreferences: getRecordingPreferencesFromBrowser(),
         setRecordingPreferences: recordingPreferencesMock,
         getSymbolTableGetter: () => noop,
-        isPopup: false,
+        pageContext: "devtools",
         supportedFeatures: perfFrontMock.getSupportedFeatures(),
       })
     );

devtools/client/performance-new/utils.js

@@ -247,10 +247,22 @@ function withCommonPathPrefixRemoved(pathArray) {
   );
 }
 
+class UnhandledCaseError extends Error {
+  /**
+   * @param {never} value - Check that
+   * @param {string} typeName - A friendly type name.
+   */
+  constructor(value, typeName) {
+    super(`There was an unhandled case for "${typeName}": ${value}`);
+    this.name = "UnhandledCaseError";
+  }
+}
+
 module.exports = {
   formatFileSize,
   makeExponentialScale,
   scaleRangeWithClamping,
   calculateOverhead,
   withCommonPathPrefixRemoved,
+  UnhandledCaseError,
 };