Bug 1587117 - Fix profiler popup shortcuts to use the correct preferences; r=canaltinova
authorGreg Tatum <gtatum@mozilla.com>
Thu, 10 Oct 2019 18:00:05 +0000
changeset 497273 7fe7debdb0f60b80757434a3e12f2d7d59ea041e
parent 497272 0f96b33c8efdde9423b068e9da3db4e229a65c05
child 497274 8813a475c23796f0e83305c1c71e97eb4cb2c88c
push id36681
push usercbrindusan@mozilla.com
push dateFri, 11 Oct 2019 21:50:12 +0000
treeherdermozilla-central@c5e6477c3a24 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscanaltinova
bugs1587117
milestone71.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 1587117 - Fix profiler popup shortcuts to use the correct preferences; r=canaltinova Differential Revision: https://phabricator.services.mozilla.com/D48562
devtools/client/performance-new/@types/perf.d.ts
devtools/client/performance-new/browser.js
devtools/client/performance-new/initializer.js
devtools/client/performance-new/popup/background.jsm.js
devtools/client/performance-new/popup/initializer.js
devtools/client/performance-new/popup/menu-button.jsm.js
devtools/client/performance-new/store/reducers.js
devtools/client/performance-new/store/selectors.js
devtools/client/performance-new/test/chrome/head.js
devtools/client/performance-new/test/chrome/test_perf-settings-features.html
devtools/client/performance-new/test/chrome/test_perf-settings-interval.html
devtools/client/performance-new/test/xpcshell/test_popup_initial_state.js
--- a/devtools/client/performance-new/@types/perf.d.ts
+++ b/devtools/client/performance-new/@types/perf.d.ts
@@ -146,16 +146,18 @@ interface GeckoProfilerFrameScriptInterf
 }
 
 export interface RecordingStateFromPreferences {
   entries: number;
   interval: number;
   features: string[];
   threads: string[];
   objdirs: string[];
+  // The duration is currently not wired up to the UI yet. See Bug 1587165.
+  duration: number;
 }
 
 /**
  * A Redux Reducer that knows about the performance-new client's Actions.
  */
 export type Reducer<S> = (state: S | undefined, action: Action) => S;
 
 export interface InitializedValues {
@@ -220,23 +222,77 @@ export type Action =
     };
 
 export type PopupBackgroundFeatures = { [feature: string]: boolean };
 
 /**
  * The state of the profiler popup.
  */
 export interface PopupBackgroundState {
-  isRunning: boolean;
-  settingsOpen: boolean;
   features: PopupBackgroundFeatures;
   buffersize: number;
   windowLength: number;
   interval: number;
   threads: string;
 }
 
 // TS-TODO - Stub
 export interface ContentFrameMessageManager {
   addMessageListener: (event: string, listener: (event: any) => void) => void;
   addEventListener: (event: string, listener: (event: any) => void) => void;
   sendAsyncMessage: (name: string, data: any) => void;
 }
+
+/**
+ * This interface serves as documentation for all of the prefs used by the
+ * performance-new client. Each preference access string access can be coerced to
+ * one of the properties of this interface.
+ */
+export interface PerformancePref {
+  /**
+   * Stores the total number of entries to be used in the profile buffer.
+   */
+  Entries: "devtools.performance.recording.entries";
+  /**
+   * The recording interval, stored in microseconds. Note that the StartProfiler
+   * interface uses milliseconds, but this lets us store higher precision numbers
+   * inside of an integer preference store.
+   */
+  Interval: "devtools.performance.recording.interval";
+  /**
+   * The features enabled for the profiler, stored as a comma-separated list.
+   */
+  Features: "devtools.performance.recording.features";
+  /**
+   * The threads to profile, stored as a comma-separated list.
+   */
+  Threads: "devtools.performance.recording.threads";
+  /**
+   * The location of the objdirs to use, stored as a comma-separated list.
+   */
+  ObjDirs: "devtools.performance.recording.objdirs";
+  /**
+   * The duration of the profiling window to use in seconds. Setting this to 0
+   * will cause no profile window to be used, and the values will naturally roll
+   * off from the profiling buffer.
+   *
+   * This is currently not hooked up to any UI. See Bug 1587165.
+   */
+  Duration: "devtools.performance.recording.duration";
+  /**
+   * Normally this defaults to https://profiler.firefox.com, but this can be overridden
+   * to point the profiler to a different URL, such as http://localhost:4242/ for
+   * local development workflows.
+   */
+  UIBaseUrl: "devtools.performance.recording.ui-base-url";
+  /**
+   * This pref allows tests to override the /from-addon in order to more easily
+   * test the profile injection mechanism.
+   */
+  UIBaseUrlPathPref: "devtools.performance.recording.ui-base-url-path";
+  /**
+   * The profiler's menu button and its popup can be enabled/disabled by the user.
+   * This pref controls whether the user has turned it on or not. Note that this
+   * preference is also used outside of the type-checked files, so make sure
+   * and update it elsewhere.
+   */
+  PopupEnabled: "devtools.performance.popup.enabled";
+}
--- a/devtools/client/performance-new/browser.js
+++ b/devtools/client/performance-new/browser.js
@@ -7,16 +7,18 @@
 /**
  * @typedef {import("./@types/perf").Action} Action
  * @typedef {import("./@types/perf").Library} Library
  * @typedef {import("./@types/perf").PerfFront} PerfFront
  * @typedef {import("./@types/perf").SymbolTableAsTuple} SymbolTableAsTuple
  * @typedef {import("./@types/perf").RecordingState} RecordingState
  * @typedef {import("./@types/perf").GetSymbolTableCallback} GetSymbolTableCallback
  * @typedef {import("./@types/perf").PreferenceFront} PreferenceFront
+ * @typedef {import("./@types/perf").PerformancePref} PerformancePref
+ * @typedef {import("./@types/perf").RecordingStateFromPreferences} RecordingStateFromPreferences
  */
 
 /**
  * TS-TODO
  *
  * This function replaces lazyRequireGetter, and TypeScript can understand it. It's
  * currently duplicated until we have consensus that TypeScript is a good idea.
  *
@@ -42,21 +44,35 @@ const lazyOS = requireLazy(() => require
 
 const lazyProfilerGetSymbols = requireLazy(() =>
   require("resource://gre/modules/ProfilerGetSymbols.jsm")
 );
 
 const TRANSFER_EVENT = "devtools:perf-html-transfer-profile";
 const SYMBOL_TABLE_REQUEST_EVENT = "devtools:perf-html-request-symbol-table";
 const SYMBOL_TABLE_RESPONSE_EVENT = "devtools:perf-html-reply-symbol-table";
+
+/** @type {PerformancePref["Entries"]} */
+const ENTRIES_PREF = "devtools.performance.recording.entries";
+/** @type {PerformancePref["Interval"]} */
+const INTERVAL_PREF = "devtools.performance.recording.interval";
+/** @type {PerformancePref["Features"]} */
+const FEATURES_PREF = "devtools.performance.recording.features";
+/** @type {PerformancePref["Threads"]} */
+const THREADS_PREF = "devtools.performance.recording.threads";
+
+/** @type {PerformancePref["ObjDirs"]} */
+const OBJDIRS_PREF = "devtools.performance.recording.objdirs";
+/** @type {PerformancePref["UIBaseUrl"]} */
 const UI_BASE_URL_PREF = "devtools.performance.recording.ui-base-url";
+/** @type {PerformancePref["UIBaseUrlPathPref"]} */
 const UI_BASE_URL_PATH_PREF = "devtools.performance.recording.ui-base-url-path";
+
 const UI_BASE_URL_DEFAULT = "https://profiler.firefox.com";
 const UI_BASE_URL_PATH_DEFAULT = "/from-addon";
-const OBJDIRS_PREF = "devtools.performance.recording.objdirs";
 
 /**
  * This file contains all of the privileged browser-specific functionality. This helps
  * keep a clear separation between the privileged and non-privileged client code. It
  * is also helpful in being able to mock out browser behavior for tests, without
  * worrying about polluting the browser environment.
  */
 
@@ -206,22 +222,21 @@ async function _getIntPref(preferenceFro
 
 /**
  * Get the recording settings from the preferences. These settings are stored once
  * for local debug targets, and another set of settings for remote targets. This
  * is helpful for configuring for remote targets like Android phones that may require
  * different features or configurations.
  *
  * @param {PreferenceFront} preferenceFront
- * @param {object} defaultSettings See the getRecordingSettings selector for the shape
- *                                 of the object and how it gets defined.
+ * @param {RecordingStateFromPreferences} defaultSettings
  */
 async function getRecordingPreferencesFromDebuggee(
   preferenceFront,
-  defaultSettings = {}
+  defaultSettings
 ) {
   const [entries, interval, features, threads, objdirs] = await Promise.all([
     _getIntPref(
       preferenceFront,
       `devtools.performance.recording.entries`,
       defaultSettings.entries
     ),
     _getIntPref(
@@ -248,39 +263,29 @@ async function getRecordingPreferencesFr
 }
 
 /**
  * Take the recording settings, as defined by the getRecordingSettings selector, and
  * persist them to preferences. Some of these prefs get persisted on the debuggee,
  * and some of them on the host browser instance.
  *
  * @param {PreferenceFront} preferenceFront
- * @param {object} settings See the getRecordingSettings selector for the shape
- *                                 of the object and how it gets defined.
+ * @param {RecordingStateFromPreferences} settings
  */
 async function setRecordingPreferencesOnDebuggee(preferenceFront, settings) {
   const { Services } = lazyServices();
   await Promise.all([
-    preferenceFront.setIntPref(
-      `devtools.performance.recording.entries`,
-      settings.entries
-    ),
-    preferenceFront.setIntPref(
-      `devtools.performance.recording.interval`,
-      // The pref stores the value in usec.
-      settings.interval * 1000
-    ),
+    preferenceFront.setIntPref(ENTRIES_PREF, settings.entries),
+    // The interval pref stores the value in usec.
+    preferenceFront.setIntPref(INTERVAL_PREF, settings.interval * 1000),
     preferenceFront.setCharPref(
-      `devtools.performance.recording.features`,
+      FEATURES_PREF,
       JSON.stringify(settings.features)
     ),
-    preferenceFront.setCharPref(
-      `devtools.performance.recording.threads`,
-      JSON.stringify(settings.threads)
-    ),
+    preferenceFront.setCharPref(THREADS_PREF, JSON.stringify(settings.threads)),
     Services.prefs.setCharPref(OBJDIRS_PREF, JSON.stringify(settings.objdirs)),
   ]);
 }
 
 /**
  * Returns a function getDebugPathFor(debugName, breakpadId) => Library which
  * resolves a (debugName, breakpadId) pair to the library's information, which
  * contains the absolute paths on the file system where the binary and its
--- a/devtools/client/performance-new/initializer.js
+++ b/devtools/client/performance-new/initializer.js
@@ -27,16 +27,20 @@ const actions = require("devtools/client
 const { Provider } = require("devtools/client/shared/vendor/react-redux");
 const {
   receiveProfile,
   getRecordingPreferencesFromDebuggee,
   setRecordingPreferencesOnDebuggee,
   createMultiModalGetSymbolTableFn,
 } = require("devtools/client/performance-new/browser");
 
+const { getDefaultRecordingSettings } = ChromeUtils.import(
+  "resource://devtools/client/performance-new/popup/background.jsm.js"
+);
+
 /**
  * This file initializes the DevTools Panel UI. It is in charge of initializing
  * the DevTools specific environment, and then passing those requirements into
  * the UI.
  */
 
 /**
  * Initialize the panel by creating a redux store, and render the root component.
@@ -48,23 +52,23 @@ async function gInit(perfFront, preferen
   const store = createStore(reducers);
 
   // Do some initialization, especially with privileged things that are part of the
   // the browser.
   store.dispatch(
     actions.initializeStore({
       perfFront,
       receiveProfile,
-      // Pull the default recording settings from the reducer, and update them according
-      // to what's in the target's preferences. This way the preferences are stored
-      // on the target. This could be useful for something like Android where you might
-      // want to tweak the settings.
+      // Pull the default recording settings from the background.jsm module. Update them
+      // according to what's in the target's preferences. This way the preferences are
+      // stored on the target. This could be useful for something like Android where you
+      // might want to tweak the settings.
       recordingSettingsFromPreferences: await getRecordingPreferencesFromDebuggee(
         preferenceFront,
-        selectors.getRecordingSettings(store.getState())
+        getDefaultRecordingSettings()
       ),
 
       // Go ahead and hide the implementation details for the component on how the
       // preference information is stored
       setRecordingPreferences: () =>
         setRecordingPreferencesOnDebuggee(
           preferenceFront,
           selectors.getRecordingSettings(store.getState())
--- a/devtools/client/performance-new/popup/background.jsm.js
+++ b/devtools/client/performance-new/popup/background.jsm.js
@@ -19,18 +19,32 @@ const { Services } = ChromeUtils.import(
 const { AppConstants } = ChromeUtils.import(
   "resource://gre/modules/AppConstants.jsm"
 );
 
 /**
  * @typedef {import("../@types/perf").RecordingStateFromPreferences} RecordingStateFromPreferences
  * @typedef {import("../@types/perf").PopupBackgroundFeatures} PopupBackgroundFeatures
  * @typedef {import("../@types/perf").SymbolTableAsTuple} SymbolTableAsTuple
+ * @typedef {import("../@types/perf").PerformancePref} PerformancePref
  */
 
+/** @type {PerformancePref["Entries"]} */
+const ENTRIES_PREF = "devtools.performance.recording.entries";
+/** @type {PerformancePref["Interval"]} */
+const INTERVAL_PREF = "devtools.performance.recording.interval";
+/** @type {PerformancePref["Features"]} */
+const FEATURES_PREF = "devtools.performance.recording.features";
+/** @type {PerformancePref["Threads"]} */
+const THREADS_PREF = "devtools.performance.recording.threads";
+/** @type {PerformancePref["ObjDirs"]} */
+const OBJDIRS_PREF = "devtools.performance.recording.objdirs";
+/** @type {PerformancePref["Duration"]} */
+const DURATION_PREF = "devtools.performance.recording.duration";
+
 // The following utilities are lazily loaded as they are not needed when controlling the
 // global state of the profiler, and only are used during specific funcationality like
 // symbolication or capturing a profile.
 
 /**
  * TS-TODO
  *
  * This function replaces lazyRequireGetter, and TypeScript can understand it. It's
@@ -64,286 +78,127 @@ const lazyReceiveProfile = requireLazy((
   const { require } = ChromeUtils.import(
     "resource://devtools/shared/Loader.jsm"
   );
   /** @type {import("devtools/client/performance-new/browser")} */
   const browserModule = require("devtools/client/performance-new/browser");
   return browserModule.receiveProfile;
 });
 
-// This pref contains the JSON serialization of the popup's profiler state with
-// a string key based off of the debug name and breakpad id.
-const PROFILER_STATE_PREF = "devtools.performance.popup";
-const DEFAULT_WINDOW_LENGTH = 20; // 20sec
-const DEFAULT_INTERVAL = 1; // 1ms
-const DEFAULT_BUFFER_SIZE = 10000000; // 90MB
-const DEFAULT_THREADS = "GeckoMain,Compositor";
-const DEFAULT_STACKWALK_FEATURE = true;
-
-/**
- * This Map caches the symbols from the shared libraries.
- * @type {Map<string, { path: string, debugPath: string }>}
- */
-const symbolCache = new Map();
-
-/**
- * @typedef {{
- *   path: string,
- *   debugName: string,
- *   debugPath: string,
- *   breakpadId: string
- * }} Libs
- *
- * @type {(libs: Libs[]) => void}
- */
-const primeSymbolStore = libs => {
-  for (const { path, debugName, debugPath, breakpadId } of libs) {
-    symbolCache.set(`${debugName}/${breakpadId}`, { path, debugPath });
-  }
-};
-
-/**
- * @typedef {import("../@types/perf").PopupBackgroundState} PopupBackgroundState
- */
-
-/**
- * @type {PopupBackgroundState}
- */
-const state = initializeState();
-
-const forTestsOnly = {
-  DEFAULT_BUFFER_SIZE,
-  DEFAULT_STACKWALK_FEATURE,
-  initializeState,
-  adjustState,
-  getState() {
-    return state;
-  },
-  revertPrefs() {
-    Services.prefs.clearUserPref(PROFILER_STATE_PREF);
-  },
-};
-
-/**
- * @param {Partial<PopupBackgroundState>} newState
- */
-function adjustState(newState) {
-  // Deep clone the object, since this can be called through popup.xhtml,
-  // which can be unloaded thus leaving this object dead.
-  newState = JSON.parse(JSON.stringify(newState));
-  Object.assign(state, newState);
-
-  try {
-    Services.prefs.setStringPref(PROFILER_STATE_PREF, JSON.stringify(state));
-  } catch (error) {
-    console.error("Unable to save the profiler state for the popup.");
-    throw error;
-  }
-}
-
-/**
- * TS-TODO - Fix any.
- * @type {(debugName: string, breakpadId: string) => Promise<SymbolTableAsTuple>}
- */
-async function getSymbolsFromThisBrowser(debugName, breakpadId) {
-  if (symbolCache.size === 0) {
-    primeSymbolStore(Services.profiler.sharedLibraries);
-  }
-
-  const cachedLibInfo = symbolCache.get(`${debugName}/${breakpadId}`);
-  if (!cachedLibInfo) {
-    throw new Error(
-      `The library ${debugName} ${breakpadId} is not in the ` +
-        "Services.profiler.sharedLibraries list, so the local path for it is not known " +
-        "and symbols for it can not be obtained. This usually happens if a content " +
-        "process uses a library that's not used in the parent process - " +
-        "Services.profiler.sharedLibraries only knows about libraries in the " +
-        "parent process."
-    );
-  }
-
-  const { path, debugPath } = cachedLibInfo;
-  const { OS } = lazyOS();
-  if (!OS.Path.split(path).absolute) {
-    throw new Error(
-      "Services.profiler.sharedLibraries did not contain an absolute path for " +
-        `the library ${debugName} ${breakpadId}, so symbols for this library can not ` +
-        "be obtained."
-    );
-  }
-
-  const { ProfilerGetSymbols } = lazyProfilerGetSymbols();
-  return ProfilerGetSymbols.getSymbolTable(path, debugPath, breakpadId);
-}
-
 /**
  * @type {() => Promise<void>}
  */
 async function captureProfile() {
-  if (!state.isRunning) {
+  if (!Services.profiler.IsActive()) {
     // The profiler is not active, ignore this shortcut.
     return;
   }
   // Pause profiler before we collect the profile, so that we don't capture
   // more samples while the parent process waits for subprocess profiles.
   Services.profiler.PauseSampling();
 
   const profile = await Services.profiler
     .getProfileDataAsGzippedArrayBuffer()
     .catch(
       /** @type {(e: any) => {}} */ e => {
         console.error(e);
         return {};
       }
     );
 
+  // This Map caches the symbols from the shared libraries.
+  const _symbolCache = new Map();
+
   const receiveProfile = lazyReceiveProfile();
-  receiveProfile(profile, getSymbolsFromThisBrowser);
+
+  receiveProfile(profile, async function getSymbols(debugName, breakpadId) {
+    if (_symbolCache.size === 0) {
+      // Prime the symbols cache.
+      for (const lib of Services.profiler.sharedLibraries) {
+        _symbolCache.set(`${lib.debugName}/${lib.breakpadId}`, {
+          path: lib.path,
+          debugPath: lib.debugPath,
+        });
+      }
+    }
+
+    const cachedLibInfo = _symbolCache.get(`${debugName}/${breakpadId}`);
+    if (!cachedLibInfo) {
+      throw new Error(
+        `The library ${debugName} ${breakpadId} is not in the ` +
+          "Services.profiler.sharedLibraries list, so the local path for it is not known " +
+          "and symbols for it can not be obtained. This usually happens if a content " +
+          "process uses a library that's not used in the parent process - " +
+          "Services.profiler.sharedLibraries only knows about libraries in the " +
+          "parent process."
+      );
+    }
+
+    const { path, debugPath } = cachedLibInfo;
+    const { OS } = lazyOS();
+    if (!OS.Path.split(path).absolute) {
+      throw new Error(
+        "Services.profiler.sharedLibraries did not contain an absolute path for " +
+          `the library ${debugName} ${breakpadId}, so symbols for this library can not ` +
+          "be obtained."
+      );
+    }
+
+    const { ProfilerGetSymbols } = lazyProfilerGetSymbols();
+
+    return ProfilerGetSymbols.getSymbolTable(path, debugPath, breakpadId);
+  });
 
   Services.profiler.StopProfiler();
 }
 
-/**
- * Not all features are supported on every version of Firefox. Get the list of checked
- * features, add a few defaults, and filter for what is actually supported.
- * @param {PopupBackgroundFeatures} features
- * @param {string[]} threads
- * @returns {string[]}
- */
-function getEnabledFeatures(features, threads) {
-  const enabledFeatures = Object.keys(features).filter(f => features[f]);
-  if (threads.length > 0) {
-    enabledFeatures.push("threads");
-  }
-  const supportedFeatures = Services.profiler.GetFeatures([]);
-  return enabledFeatures.filter(feature => supportedFeatures.includes(feature));
-}
-
 function startProfiler() {
-  const threads = state.threads.split(",");
-  const features = getEnabledFeatures(state.features, threads);
-  const windowLength =
-    state.windowLength !== state.infiniteWindowLength ? state.windowLength : 0;
-
-  const { buffersize, interval } = state;
-
-  Services.profiler.StartProfiler(
-    buffersize,
+  const {
+    entries,
     interval,
     features,
     threads,
-    windowLength
+    duration,
+  } = getRecordingPreferencesFromBrowser();
+
+  Services.profiler.StartProfiler(
+    entries,
+    interval,
+    features,
+    threads,
+    duration
   );
 }
 
 /**
  * @type {() => void}
  */
 function stopProfiler() {
   Services.profiler.StopProfiler();
 }
 
 /**
  * @type {() => void}
  */
 function toggleProfiler() {
-  if (state.isRunning) {
+  if (Services.profiler.IsActive()) {
     stopProfiler();
   } else {
     startProfiler();
   }
 }
 
 /**
  * @type {() => void}
  */
 function restartProfiler() {
   stopProfiler();
   startProfiler();
 }
 
-// This running observer was adapted from the web extension.
-const isRunningObserver = {
-  _observers: new Set(),
-
-  /**
-   * @param {string} subject
-   * @param {string} topic
-   * @param {unknown} data
-   */
-  observe(subject, topic, data) {
-    switch (topic) {
-      case "profiler-started":
-      case "profiler-stopped":
-        // Make the observer calls asynchronous.
-        const isRunningPromise = Promise.resolve(topic === "profiler-started");
-        for (const observer of this._observers) {
-          isRunningPromise.then(observer);
-        }
-        break;
-    }
-  },
-
-  _startListening() {
-    Services.obs.addObserver(this, "profiler-started");
-    Services.obs.addObserver(this, "profiler-stopped");
-  },
-
-  _stopListening() {
-    Services.obs.removeObserver(this, "profiler-started");
-    Services.obs.removeObserver(this, "profiler-stopped");
-  },
-
-  /**
-   * @param {(isActive: boolean) => any} observer
-   */
-  addObserver(observer) {
-    if (this._observers.size === 0) {
-      this._startListening();
-    }
-
-    this._observers.add(observer);
-    // Notify the observers the current state asynchronously.
-    Promise.resolve(Services.profiler.IsActive()).then(observer);
-  },
-
-  /**
-   * @param {(isActive: boolean) => any} observer
-   */
-  removeObserver(observer) {
-    if (this._observers.delete(observer) && this._observers.size === 0) {
-      this._stopListening();
-    }
-  },
-};
-
-/**
- * @returns {PopupBackgroundState | null}
- */
-function getStoredStateOrNull() {
-  // Pull out the stored state from preferences, it is a raw string.
-  const storedStateString = Services.prefs.getStringPref(
-    PROFILER_STATE_PREF,
-    ""
-  );
-  if (storedStateString === "") {
-    return null;
-  }
-
-  try {
-    // Attempt to parse the results.
-    return JSON.parse(storedStateString);
-  } catch (error) {
-    console.error(
-      `Could not parse the stored state for the profile in the ` +
-        `preferences ${PROFILER_STATE_PREF}`
-    );
-  }
-  return null;
-}
 /**
  * @param {string} prefName
  * @param {string[]} defaultValue
  * @return {string[]}
  */
 function _getArrayOfStringsPref(prefName, defaultValue) {
   let array;
   try {
@@ -386,169 +241,113 @@ function _getArrayOfStringsHostPref(pref
   ) {
     return array;
   }
 
   return defaultValue;
 }
 
 /**
- * @param {RecordingStateFromPreferences} defaultSettings
- * @return {RecordingStateFromPreferences}
+ * A simple cache for the recording settings.
+ * @type {RecordingStateFromPreferences}
+ */
+let _defaultSettings;
+
+/**
+ * This function contains the canonical defaults for both the popup and panel's
+ * recording settings.
+ */
+function getDefaultRecordingSettings() {
+  if (!_defaultSettings) {
+    _defaultSettings = {
+      entries: 10000000, // ~80mb,
+      // Do not expire markers, let them roll off naturally from the circular buffer.
+      duration: 0,
+      interval: 1, // milliseconds
+      features: ["js", "leaf", "responsiveness", "stackwalk"],
+      threads: ["GeckoMain", "Compositor"],
+      objdirs: [],
+    };
+
+    if (AppConstants.platform === "android") {
+      // Java profiling is only meaningful on android.
+      _defaultSettings.features.push("java");
+    }
+  }
+
+  return _defaultSettings;
+}
+
+/**
+ * @returns {RecordingStateFromPreferences}
  */
-function getRecordingPreferencesFromBrowser(defaultSettings) {
-  const [entries, interval, features, threads, objdirs] = [
-    Services.prefs.getIntPref(
-      `devtools.performance.recording.entries`,
-      defaultSettings.entries
-    ),
-    Services.prefs.getIntPref(
-      `devtools.performance.recording.interval`,
-      defaultSettings.interval
-    ),
-    _getArrayOfStringsPref(
-      `devtools.performance.recording.features`,
-      defaultSettings.features
-    ),
-    _getArrayOfStringsPref(
-      `devtools.performance.recording.threads`,
-      defaultSettings.threads
-    ),
-    _getArrayOfStringsHostPref(
-      "devtools.performance.recording.objdirs",
-      defaultSettings.objdirs
-    ),
-  ];
+function getRecordingPreferencesFromBrowser() {
+  const defaultSettings = getDefaultRecordingSettings();
 
-  // The pref stores the value in usec.
-  const newInterval = interval / 1000;
-  return { entries, interval: newInterval, features, threads, objdirs };
+  const entries = Services.prefs.getIntPref(
+    ENTRIES_PREF,
+    defaultSettings.entries
+  );
+  const interval = Services.prefs.getIntPref(
+    INTERVAL_PREF,
+    defaultSettings.interval
+  );
+  const features = _getArrayOfStringsPref(
+    FEATURES_PREF,
+    defaultSettings.features
+  );
+  const threads = _getArrayOfStringsPref(THREADS_PREF, defaultSettings.threads);
+  const objdirs = _getArrayOfStringsHostPref(
+    OBJDIRS_PREF,
+    defaultSettings.objdirs
+  );
+  const duration = Services.prefs.getIntPref(
+    DURATION_PREF,
+    defaultSettings.duration
+  );
+
+  const supportedFeatures = new Set(Services.profiler.GetFeatures());
+
+  return {
+    entries,
+    // The pref stores the value in usec.
+    interval: interval / 1000,
+    // Validate the features before passing them to the profiler.
+    features: features.filter(feature => supportedFeatures.has(feature)),
+    threads,
+    objdirs,
+    duration,
+  };
 }
 
 /**
  * @param {RecordingStateFromPreferences} settings
  */
 function setRecordingPreferencesOnBrowser(settings) {
-  Services.prefs.setIntPref(
-    `devtools.performance.recording.entries`,
-    settings.entries
-  );
-  Services.prefs.setIntPref(
-    `devtools.performance.recording.interval`,
-    // The pref stores the value in usec.
-    settings.interval * 1000
-  );
-  Services.prefs.setCharPref(
-    `devtools.performance.recording.features`,
-    JSON.stringify(settings.features)
-  );
-  Services.prefs.setCharPref(
-    `devtools.performance.recording.threads`,
-    JSON.stringify(settings.threads)
-  );
-  Services.prefs.setCharPref(
-    "devtools.performance.recording.objdirs",
-    JSON.stringify(settings.objdirs)
-  );
+  Services.prefs.setIntPref(ENTRIES_PREF, settings.entries);
+  // The interval pref stores the value in microseconds for extra precision.
+  Services.prefs.setIntPref(INTERVAL_PREF, settings.interval * 1000);
+  Services.prefs.setCharPref(FEATURES_PREF, JSON.stringify(settings.features));
+  Services.prefs.setCharPref(THREADS_PREF, JSON.stringify(settings.threads));
+  Services.prefs.setCharPref(OBJDIRS_PREF, JSON.stringify(settings.objdirs));
 }
 
-/**
- * @returns {PopupBackgroundState}
- */
-function initializeState() {
-  const features = {
-    java: false,
-    js: true,
-    leaf: true,
-    mainthreadio: false,
-    privacy: false,
-    responsiveness: true,
-    screenshots: false,
-    seqstyle: false,
-    stackwalk: DEFAULT_STACKWALK_FEATURE,
-    tasktracer: false,
-    trackopts: false,
-    jstracer: false,
-    preferencereads: false,
-    jsallocations: false,
-    nativeallocations: false,
-  };
-
-  if (AppConstants.platform === "android") {
-    // Java profiling is only meaningful on android.
-    features.java = true;
-  }
-
-  const storedState = getStoredStateOrNull();
-
-  if (storedState && storedState.features) {
-    const storedFeatures = storedState.features;
-    // Validate the stored state. It's possible a feature was added or removed
-    // since the profiler was last run.
-    for (const key of Object.keys(features)) {
-      /** @type {{[key: string]: boolean}} */
-      const featureAsObjMap = features;
-      featureAsObjMap[key] =
-        key in storedFeatures
-          ? Boolean(storedFeatures[key])
-          : featureAsObjMap[key];
-    }
-  }
-
-  /**
-   *  This function is created inline to make it easy to validate
-   * the stored state using the captured storedState value.
-   * @template Value
-   * @type {(key: string, type: string, defaultValue: Value) => Value}
-   */
-  function validateStoredState(key, type, defaultValue) {
-    if (!storedState) {
-      return defaultValue;
-    }
-    /** @type {object} */
-    const storedStateAsObjMap = storedState;
-    const storedValue = storedStateAsObjMap[key];
-    return typeof storedValue === type ? storedValue : defaultValue;
-  }
-
-  return {
-    // These values are stale, and need to be re-generated.
-    isRunning: Services.profiler.IsActive(),
-    settingsOpen: false,
-    features,
-
-    // Look these up from stored state.
-    buffersize: validateStoredState(
-      "buffersize",
-      "number",
-      DEFAULT_BUFFER_SIZE
-    ),
-    windowLength: validateStoredState(
-      "windowLength",
-      "number",
-      DEFAULT_WINDOW_LENGTH
-    ),
-    interval: validateStoredState("interval", "number", DEFAULT_INTERVAL),
-    threads: validateStoredState("threads", "string", DEFAULT_THREADS),
-  };
-}
-
-isRunningObserver.addObserver(isRunning => {
-  adjustState({ isRunning });
-});
-
 const platform = AppConstants.platform;
 
+/**
+ * @type {() => void}
+ */
+function revertRecordingPreferences() {
+  setRecordingPreferencesOnBrowser(getDefaultRecordingSettings());
+}
+
 var EXPORTED_SYMBOLS = [
-  "adjustState",
   "captureProfile",
-  "state",
   "startProfiler",
   "stopProfiler",
   "restartProfiler",
   "toggleProfiler",
-  "isRunningObserver",
   "platform",
+  "getDefaultRecordingSettings",
   "getRecordingPreferencesFromBrowser",
   "setRecordingPreferencesOnBrowser",
-  "forTestsOnly",
-  "getSymbolsFromThisBrowser",
+  "revertRecordingPreferences",
 ];
--- a/devtools/client/performance-new/popup/initializer.js
+++ b/devtools/client/performance-new/popup/initializer.js
@@ -69,21 +69,18 @@ async function gInit() {
   const perfFrontInterface = new ActorReadyGeckoProfilerInterface();
 
   // Do some initialization, especially with privileged things that are part of the
   // the browser.
   store.dispatch(
     actions.initializeStore({
       perfFront: perfFrontInterface,
       receiveProfile,
-      // Pull the default recording settings from the reducer, and update them according
-      // to what's in the browser's preferences.
-      recordingSettingsFromPreferences: getRecordingPreferencesFromBrowser(
-        selectors.getRecordingSettings(store.getState())
-      ),
+      // Get the preferences from the current browser
+      recordingSettingsFromPreferences: getRecordingPreferencesFromBrowser(),
       // In the popup, the preferences are stored directly on the current browser.
       setRecordingPreferences: () =>
         setRecordingPreferencesOnBrowser(
           selectors.getRecordingSettings(store.getState())
         ),
       // The popup doesn't need to support remote symbol tables from the debuggee.
       // Only get the symbols from this browser.
       getSymbolTableGetter: () => getSymbolsFromThisBrowser,
--- a/devtools/client/performance-new/popup/menu-button.jsm.js
+++ b/devtools/client/performance-new/popup/menu-button.jsm.js
@@ -1,15 +1,18 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 // @ts-check
 "use strict";
 
 /**
+ * @typedef {import("../@types/perf").PerformancePref} PerformancePref
+ * */
+/**
  * This file controls the enabling and disabling of the menu button for the profiler.
  * Care should be taken to keep it minimal as it can be run with browser initialization.
  */
 
 /**
  * TS-TODO
  *
  * This function replaces lazyRequireGetter, and TypeScript can understand it. It's
@@ -36,21 +39,17 @@ const lazyServices = requireLazy(() =>
 const lazyCustomizableUI = requireLazy(() =>
   /** @type {import("resource:///modules/CustomizableUI.jsm")} */
   ChromeUtils.import("resource:///modules/CustomizableUI.jsm")
 );
 const lazyCustomizableWidgets = requireLazy(() =>
   /** @type {import("resource:///modules/CustomizableWidgets.jsm")} */
   ChromeUtils.import("resource:///modules/CustomizableWidgets.jsm")
 );
-
-// The profiler's menu button and its popup can be enabled/disabled by the user.
-// This is the pref to control whether the user has turned it on or not.
-// This pref is repeated across many files in order to avoid loading this file if
-// it's not needed. Make sure and search the rest of the codebase for other uses.
+/** @type {PerformancePref["PopupEnabled"]} */
 const BUTTON_ENABLED_PREF = "devtools.performance.popup.enabled";
 const WIDGET_ID = "profiler-button";
 
 /**
  * @return {boolean}
  */
 function isEnabled() {
   const { Services } = lazyServices();
--- a/devtools/client/performance-new/store/reducers.js
+++ b/devtools/client/performance-new/store/reducers.js
@@ -75,50 +75,50 @@ function interval(state = 1000, action) 
     case "INITIALIZE_STORE":
       return action.recordingSettingsFromPreferences.interval;
     default:
       return state;
   }
 }
 
 /**
- * The number of entries in the profiler's circular buffer. Defaults to 90mb.
+ * The number of entries in the profiler's circular buffer.
  * @type {Reducer<number>}
  */
-function entries(state = 10000000, action) {
+function entries(state = 0, action) {
   switch (action.type) {
     case "CHANGE_ENTRIES":
       return action.entries;
     case "INITIALIZE_STORE":
       return action.recordingSettingsFromPreferences.entries;
     default:
       return state;
   }
 }
 
 /**
  * The features that are enabled for the profiler.
  * @type {Reducer<string[]>}
  */
-function features(state = ["js", "stackwalk", "responsiveness"], action) {
+function features(state = [], action) {
   switch (action.type) {
     case "CHANGE_FEATURES":
       return action.features;
     case "INITIALIZE_STORE":
       return action.recordingSettingsFromPreferences.features;
     default:
       return state;
   }
 }
 
 /**
  * The current threads list.
  * @type {Reducer<string[]>}
  */
-function threads(state = ["GeckoMain", "Compositor"], action) {
+function threads(state = [], action) {
   switch (action.type) {
     case "CHANGE_THREADS":
       return action.threads;
     case "INITIALIZE_STORE":
       return action.recordingSettingsFromPreferences.threads;
     default:
       return state;
   }
--- a/devtools/client/performance-new/store/selectors.js
+++ b/devtools/client/performance-new/store/selectors.js
@@ -54,16 +54,18 @@ const getObjdirs = state => state.objdir
  */
 const getRecordingSettings = state => {
   return {
     entries: getEntries(state),
     interval: getInterval(state),
     features: getFeatures(state),
     threads: getThreads(state),
     objdirs: getObjdirs(state),
+    // The client doesn't implement durations yet. See Bug 1587165.
+    duration: 0,
   };
 };
 
 /** @type {Selector<InitializedValues>} */
 const getInitializedValues = state => {
   const values = state.initializedValues;
   if (!values) {
     throw new Error("The store must be initialized before it can be used.");
--- a/devtools/client/performance-new/test/chrome/head.js
+++ b/devtools/client/performance-new/test/chrome/head.js
@@ -182,16 +182,19 @@ function createPerfComponent() {
   const Perf = require("devtools/client/performance-new/components/Perf");
   const React = require("devtools/client/shared/vendor/react");
   const ReactDOM = require("devtools/client/shared/vendor/react-dom");
   const ReactRedux = require("devtools/client/shared/vendor/react-redux");
   const createStore = require("devtools/client/shared/redux/create-store");
   const reducers = require("devtools/client/performance-new/store/reducers");
   const actions = require("devtools/client/performance-new/store/actions");
   const selectors = require("devtools/client/performance-new/store/selectors");
+  const { getDefaultRecordingSettings } = ChromeUtils.import(
+    "resource://devtools/client/performance-new/popup/background.jsm.js"
+  );
 
   const perfFrontMock = new MockPerfFront();
   const store = createStore(reducers);
   const container = document.querySelector("#container");
   const receiveProfileCalls = [];
   const recordingPreferencesCalls = [];
 
   function receiveProfileMock(profile, getSymbolTableCallback) {
@@ -204,19 +207,17 @@ function createPerfComponent() {
 
   const noop = () => {};
 
   function mountComponent() {
     store.dispatch(
       actions.initializeStore({
         perfFront: perfFrontMock,
         receiveProfile: receiveProfileMock,
-        recordingSettingsFromPreferences: selectors.getRecordingSettings(
-          store.getState()
-        ),
+        recordingSettingsFromPreferences: getDefaultRecordingSettings(),
         setRecordingPreferences: recordingPreferencesMock,
         getSymbolTableGetter: () => noop,
       })
     );
 
     return ReactDOM.render(
       React.createElement(
         ReactRedux.Provider,
--- a/devtools/client/performance-new/test/chrome/test_perf-settings-features.html
+++ b/devtools/client/performance-new/test/chrome/test_perf-settings-features.html
@@ -29,43 +29,43 @@
           recordingPreferencesCalls,
         } = createPerfComponent();
 
         await mountAndInitializeComponent();
 
         // Open up the features summary.
         document.querySelector("#perf-settings-features-summary").click();
 
-        is(selectors.getFeatures(getState()).join(","), "js,stackwalk,responsiveness",
+        is(selectors.getFeatures(getState()).join(","), "js,leaf,responsiveness,stackwalk",
           "The features starts out with the default");
         is(recordingPreferencesCalls.length, 0,
           "No calls have been made to set preferences");
 
         // Click the "features checkbox.
         document.querySelector("#perf-settings-feature-checkbox-js").click();
 
-        is(selectors.getFeatures(getState()).join(","), "stackwalk,responsiveness",
+        is(selectors.getFeatures(getState()).join(","), "leaf,responsiveness,stackwalk",
           "The feature has been removed.");
         is(recordingPreferencesCalls.length, 1,
           "The preferences have been updated.");
-        is(recordingPreferencesCalls[0].features.join(","), "stackwalk,responsiveness",
+        is(recordingPreferencesCalls[0].features.join(","), "leaf,responsiveness,stackwalk",
           "The preferences have been updated.");
 
         // Enable a feature
-        document.querySelector("#perf-settings-feature-checkbox-leaf").click();
-        is(selectors.getFeatures(getState()).join(","), "leaf,stackwalk,responsiveness",
+        document.querySelector("#perf-settings-feature-checkbox-screenshots").click();
+        is(selectors.getFeatures(getState()).join(","), "screenshots,leaf,responsiveness,stackwalk",
           "Another feature was added");
 
         // Start the profiler by clicking the start button, and flushing the async
         // calls out to the mock perf front.
         document.querySelector("button").click();
         await perfFrontMock._flushAsyncQueue();
 
         is(perfFrontMock._startProfilerCalls.length, 1,
           "Start profiler was called once");
         is(perfFrontMock._startProfilerCalls[0].features.join(","),
-          "leaf,stackwalk,responsiveness",
+          "screenshots,leaf,responsiveness,stackwalk",
           "Start profiler was called with the correct features");
       });
     </script>
   </pre>
 </body>
 </html>
--- a/devtools/client/performance-new/test/chrome/test_perf-settings-interval.html
+++ b/devtools/client/performance-new/test/chrome/test_perf-settings-interval.html
@@ -26,18 +26,18 @@
           mountAndInitializeComponent,
           selectors,
           getState,
           recordingPreferencesCalls,
         } = createPerfComponent();
 
         await mountAndInitializeComponent();
 
-        is(selectors.getInterval(getState()), 1000,
-          "The interval starts out as 1000");
+        is(selectors.getInterval(getState()), 1,
+          "The interval starts out as 1ms");
         is(recordingPreferencesCalls.length, 0,
           "No calls have been made");
 
         const inputValue = 75;
         const scaledValue = 10;
         const input = document.querySelector("#perf-range-interval");
         setReactFriendlyInputValue(input, inputValue);
 
--- a/devtools/client/performance-new/test/xpcshell/test_popup_initial_state.js
+++ b/devtools/client/performance-new/test/xpcshell/test_popup_initial_state.js
@@ -3,65 +3,70 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
 /**
  * Tests the initial state of the background script for the popup.
  */
 
 function setupBackgroundJsm() {
-  const background = ChromeUtils.import(
+  return ChromeUtils.import(
     "resource://devtools/client/performance-new/popup/background.jsm.js"
   );
-  return background;
 }
 
 add_task(function test() {
   info("Test that we get the default values from state.");
   const {
-    getState,
-    revertPrefs,
-    DEFAULT_BUFFER_SIZE,
-    DEFAULT_STACKWALK_FEATURE,
-  } = setupBackgroundJsm().forTestsOnly;
+    getRecordingPreferencesFromBrowser,
+    revertRecordingPreferences,
+    getDefaultRecordingSettings,
+  } = setupBackgroundJsm();
 
   Assert.equal(
-    getState().buffersize,
-    DEFAULT_BUFFER_SIZE,
+    getRecordingPreferencesFromBrowser().buffersize,
+    getDefaultRecordingSettings().buffersize,
     "The initial state has the default buffersize."
   );
   Assert.equal(
-    getState().features.stackwalk,
-    DEFAULT_STACKWALK_FEATURE,
+    getRecordingPreferencesFromBrowser().features.includes("stackwalk"),
+    getDefaultRecordingSettings().features.includes("stackwalk"),
     "The stackwalk feature is initialized to the default."
   );
-  revertPrefs();
+  revertRecordingPreferences();
 });
 
 add_task(function test() {
-  info("Test that the state and features are properly validated.");
+  info(
+    "Test that the state and features are properly validated. This ensures that as " +
+      "we add and remove features, the stored preferences do not cause the Gecko " +
+      "Profiler interface to crash with invalid values."
+  );
   const {
-    getState,
-    adjustState,
-    revertPrefs,
-    initializeState,
-    DEFAULT_STACKWALK_FEATURE,
-  } = setupBackgroundJsm().forTestsOnly;
+    getRecordingPreferencesFromBrowser,
+    setRecordingPreferencesOnBrowser,
+    revertRecordingPreferences,
+  } = setupBackgroundJsm();
 
-  info("Manipulate the state.");
-  const state = getState();
-  state.features.stackwalk = !DEFAULT_STACKWALK_FEATURE;
-  state.features.UNKNOWN_FEATURE_FOR_TESTS = true;
-  adjustState(state);
-  adjustState(initializeState());
+  Assert.ok(
+    getRecordingPreferencesFromBrowser().features.includes("stackwalk"),
+    "The stackwalk preference is present initially."
+  );
 
-  Assert.equal(
-    getState().features.UNKNOWN_FEATURE_FOR_TESTS,
-    undefined,
+  const settings = getRecordingPreferencesFromBrowser();
+  settings.features = settings.features.filter(
+    feature => feature !== "stackwalk"
+  );
+  settings.features.push("UNKNOWN_FEATURE_FOR_TESTS");
+  setRecordingPreferencesOnBrowser(settings);
+
+  Assert.ok(
+    !getRecordingPreferencesFromBrowser().features.includes(
+      "UNKNOWN_FEATURE_FOR_TESTS"
+    ),
     "The unknown feature is removed."
   );
-  Assert.equal(
-    getState().features.stackwalk,
-    !DEFAULT_STACKWALK_FEATURE,
+  Assert.ok(
+    !getRecordingPreferencesFromBrowser().features.includes("stackwalk"),
     "The stackwalk preference is still flipped from the default."
   );
-  revertPrefs();
+  revertRecordingPreferences();
 });