Bug 1453014 - Persist the recording settings to preferences; r?julienw draft
authorGreg Tatum <gtatum@mozilla.com>
Wed, 18 Apr 2018 11:15:59 -0500
changeset 787987 6a4eceb97b5e3ce9dbc766be8356f1a1ec836661
parent 787860 7f6a582f00bfb5d0acb8d8bf7f8c79ca37c99b65
child 788082 a04cda40abc870804e9c8a9774c52337075a7375
child 788083 90ede1d32116dffca22a355e4fc82d74b9def89d
child 788579 81f53c135cd73cd8a1e8816d7c3fa955a1f82269
child 788671 ad5dd642b251ae977d4c48704edbab975c42a01f
push id107866
push usergtatum@mozilla.com
push dateWed, 25 Apr 2018 19:37:27 +0000
reviewersjulienw
bugs1453014
milestone61.0a1
Bug 1453014 - Persist the recording settings to preferences; r?julienw MozReview-Commit-ID: JvPGx2ZstiA
devtools/client/performance-new/browser.js
devtools/client/performance-new/components/Description.js
devtools/client/performance-new/initializer.js
devtools/client/performance-new/moz.build
devtools/client/performance-new/panel.js
devtools/client/performance-new/store/actions.js
devtools/client/performance-new/store/reducers.js
devtools/client/performance-new/store/selectors.js
devtools/client/performance-new/test/chrome/head.js
new file mode 100644
--- /dev/null
+++ b/devtools/client/performance-new/browser.js
@@ -0,0 +1,150 @@
+/* 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/. */
+"use strict";
+const Services = require("Services");
+
+/**
+ * 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.
+ */
+
+/**
+ * Once a profile is received from the actor, it needs to be opened up in perf.html
+ * to be analyzed. This function opens up perf.html into a new browser tab, and injects
+ * the profile via a frame script.
+ *
+ * @param {object} profile - The Gecko profile.
+ */
+function receiveProfile(profile) {
+  // Find the most recently used window, as the DevTools client could be in a variety
+  // of hosts.
+  const win = Services.wm.getMostRecentWindow("navigator:browser");
+  if (!win) {
+    throw new Error("No browser window");
+  }
+  const browser = win.gBrowser;
+  Services.focus.activeWindow = win;
+
+  const tab = browser.addTab("https://perf-html.io/from-addon");
+  browser.selectedTab = tab;
+  const mm = tab.linkedBrowser.messageManager;
+  mm.loadFrameScript(
+    "chrome://devtools/content/performance-new/frame-script.js",
+    false
+  );
+  mm.sendAsyncMessage("devtools:perf-html-transfer-profile", profile);
+}
+
+/**
+ * Don't trust that the user has stored the correct value in preferences, or that it
+ * even exists. Gracefully handle malformed data or missing data. Ensure that this
+ * function always returns a valid array of strings.
+ * @param {PreferenceFront} preferenceFront
+ * @param {string} prefName
+ * @param {array of string} defaultValue
+ */
+async function _getArrayOfStringsPref(preferenceFront, prefName, defaultValue) {
+  let array;
+  try {
+    const text = await preferenceFront.getCharPref(prefName);
+    array = JSON.parse(text);
+  } catch (error) {
+    return defaultValue;
+  }
+
+  if (Array.isArray(array) && array.every(feature => typeof feature === "string")) {
+    return array;
+  }
+
+  return defaultValue;
+}
+
+/**
+ * Attempt to get a int preference value from the debuggee.
+ *
+ * @param {PreferenceFront} preferenceFront
+ * @param {string} prefName
+ * @param {number} defaultValue
+ */
+async function _getIntPref(preferenceFront, prefName, defaultValue) {
+  try {
+    return await preferenceFront.getIntPref(prefName);
+  } catch (error) {
+    return defaultValue;
+  }
+}
+
+/**
+ * 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.
+ */
+async function getRecordingPreferences(preferenceFront, defaultSettings = {}) {
+  const [ entries, interval, features, threads ] = await Promise.all([
+    _getIntPref(
+      preferenceFront,
+      `devtools.performance.recording.entries`,
+      defaultSettings.entries
+    ),
+    _getIntPref(
+      preferenceFront,
+      `devtools.performance.recording.interval`,
+      defaultSettings.interval
+    ),
+    _getArrayOfStringsPref(
+      preferenceFront,
+      `devtools.performance.recording.features`,
+      defaultSettings.features
+    ),
+    _getArrayOfStringsPref(
+      preferenceFront,
+      `devtools.performance.recording.threads`,
+      defaultSettings.threads
+    ),
+  ]);
+
+  return { entries, interval, features, threads };
+}
+
+/**
+ * Take the recording settings, as defined by the getRecordingSettings selector, and
+ * persist them to preferences.
+ *
+ * @param {PreferenceFront} preferenceFront
+ * @param {object} defaultSettings See the getRecordingSettings selector for the shape
+ *                                 of the object and how it gets defined.
+ */
+async function setRecordingPreferences(preferenceFront, settings) {
+  await Promise.all([
+    preferenceFront.setIntPref(
+      `devtools.performance.recording.entries`,
+      settings.entries
+    ),
+    preferenceFront.setIntPref(
+      `devtools.performance.recording.interval`,
+      settings.interval
+    ),
+    preferenceFront.setCharPref(
+      `devtools.performance.recording.features`,
+      JSON.stringify(settings.features)
+    ),
+    preferenceFront.setCharPref(
+      `devtools.performance.recording.threads`,
+      JSON.stringify(settings.threads)
+    )
+  ]);
+}
+
+module.exports = {
+  receiveProfile,
+  getRecordingPreferences,
+  setRecordingPreferences
+};
--- a/devtools/client/performance-new/components/Description.js
+++ b/devtools/client/performance-new/components/Description.js
@@ -1,16 +1,16 @@
 /* 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/. */
 "use strict";
 
 const { PureComponent } = require("devtools/client/shared/vendor/react");
 const { div, button, p } = require("devtools/client/shared/vendor/react-dom-factories");
-const { openLink } = require("devtools/client/shared/link");
+const { openWebLink } = require("devtools/client/shared/link");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const { connect } = require("devtools/client/shared/vendor/react-redux");
 const selectors = require("devtools/client/performance-new/store/selectors");
 
 /**
  * This component provides a helpful description for what is going on in the component
  * and provides some external links.
  */
@@ -23,17 +23,17 @@ class Description extends PureComponent 
   }
 
   constructor(props) {
     super(props);
     this.handleLinkClick = this.handleLinkClick.bind(this);
   }
 
   handleLinkClick(event) {
-    openLink(event.target.value, this.props.toolbox);
+    openWebLink(event.target.value, this.props.toolbox);
   }
 
   /**
    * Implement links as buttons to avoid any risk of loading the link in the
    * the panel.
    */
   renderLink(href, text) {
     return button(
--- a/devtools/client/performance-new/initializer.js
+++ b/devtools/client/performance-new/initializer.js
@@ -7,63 +7,58 @@
 
 const BrowserLoaderModule = {};
 ChromeUtils.import("resource://devtools/client/shared/browser-loader.js", BrowserLoaderModule);
 const { require } = BrowserLoaderModule.BrowserLoader({
   baseURI: "resource://devtools/client/memory/",
   window
 });
 const Perf = require("devtools/client/performance-new/components/Perf");
-const Services = require("Services");
 const ReactDOM = require("devtools/client/shared/vendor/react-dom");
 const React = require("devtools/client/shared/vendor/react");
 const createStore = require("devtools/client/shared/redux/create-store")();
+const selectors = require("devtools/client/performance-new/store/selectors");
 const reducers = require("devtools/client/performance-new/store/reducers");
 const actions = require("devtools/client/performance-new/store/actions");
 const { Provider } = require("devtools/client/shared/vendor/react-redux");
+const {
+  receiveProfile,
+  getRecordingPreferences,
+  setRecordingPreferences
+} = require("devtools/client/performance-new/browser");
 
 /**
  * Initialize the panel by creating a redux store, and render the root component.
  *
  * @param toolbox - The toolbox
  * @param perfFront - The Perf actor's front. Used to start and stop recordings.
  */
-function gInit(toolbox, perfFront) {
+async function gInit(toolbox, perfFront, preferenceFront) {
   const store = createStore(reducers);
+
+  // Do some initialization, especially with privileged things that are part of the
+  // the browser.
   store.dispatch(actions.initializeStore({
     toolbox,
     perfFront,
-    /**
-     * This function uses privileged APIs in order to take the profile, open up a new
-     * tab, and then inject it into perf.html. In order to provide a clear separation
-     * in the codebase between privileged and non-privileged code, this function is
-     * defined in initializer.js, and injected into the the normal component. All of
-     * the React components and Redux store behave as normal unprivileged web components.
-     */
-    receiveProfile: profile => {
-      // Open up a new tab and send a message with the profile.
-      let browser = top.gBrowser;
-      if (!browser) {
-        // Current isn't browser window. Looking for the recent browser.
-        const win = Services.wm.getMostRecentWindow("navigator:browser");
-        if (!win) {
-          throw new Error("No browser window");
-        }
-        browser = win.gBrowser;
-        Services.focus.activeWindow = win;
-      }
-      const tab = browser.addTab("https://perf-html.io/from-addon");
-      browser.selectedTab = tab;
-      const mm = tab.linkedBrowser.messageManager;
-      mm.loadFrameScript(
-        "chrome://devtools/content/performance-new/frame-script.js",
-        false
-      );
-      mm.sendAsyncMessage("devtools:perf-html-transfer-profile", profile);
-    }
+    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.
+    recordingSettingsFromPreferences: await getRecordingPreferences(
+      preferenceFront,
+      selectors.getRecordingSettings(store.getState())
+    ),
+    // Go ahead and hide the implementation details for the component on how the
+    // preference information is stored
+    setRecordingPreferences: () => setRecordingPreferences(
+      preferenceFront,
+      selectors.getRecordingSettings(store.getState())
+    )
   }));
 
   ReactDOM.render(
     React.createElement(
       Provider,
       { store },
       React.createElement(Perf)
     ),
--- a/devtools/client/performance-new/moz.build
+++ b/devtools/client/performance-new/moz.build
@@ -4,16 +4,17 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 DIRS += [
     'components',
     'store',
 ]
 
 DevToolsModules(
+    'browser.js',
     'panel.js',
     'utils.js',
 )
 
 MOCHITEST_CHROME_MANIFESTS += ['test/chrome/chrome.ini']
 
 with Files('**'):
     BUG_COMPONENT = ('Firefox', 'Developer Tools: Performance Tools (Profiler/Timeline)')
--- a/devtools/client/performance-new/panel.js
+++ b/devtools/client/performance-new/panel.js
@@ -1,15 +1,15 @@
 /* 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/. */
 "use strict";
 
 const { PerfFront } = require("devtools/shared/fronts/perf");
-
+const { getPreferenceFront } = require("devtools/shared/fronts/preference");
 loader.lazyRequireGetter(this, "EventEmitter", "devtools/shared/event-emitter");
 
 class PerformancePanel {
   constructor(iframeWindow, toolbox) {
     this.panelWin = iframeWindow;
     this.toolbox = toolbox;
 
     EventEmitter.decorate(this);
@@ -27,20 +27,21 @@ class PerformancePanel {
   }
 
   async _doOpen() {
     this.panelWin.gToolbox = this.toolbox;
     this.panelWin.gTarget = this.target;
 
     const rootForm = await this.target.root;
     const perfFront = new PerfFront(this.target.client, rootForm);
+    const preferenceFront = getPreferenceFront(this.target.client, rootForm);
 
     this.isReady = true;
     this.emit("ready");
-    this.panelWin.gInit(this.toolbox, perfFront);
+    this.panelWin.gInit(this.toolbox, perfFront, preferenceFront);
     return this;
   }
 
   // DevToolPanel API:
 
   get target() {
     return this.toolbox.target;
   }
--- a/devtools/client/performance-new/store/actions.js
+++ b/devtools/client/performance-new/store/actions.js
@@ -31,59 +31,78 @@ const changeRecordingState = exports.cha
  */
 exports.reportProfilerReady = (isSupportedPlatform, recordingState) => ({
   type: "REPORT_PROFILER_READY",
   isSupportedPlatform,
   recordingState,
 });
 
 /**
+ * Dispatch the given action, and then update the recording settings.
+ * @param {object} action
+ */
+function _dispatchAndUpdatePreferences(action) {
+  return (dispatch, getState) => {
+    if (typeof action !== "object") {
+      throw new Error(
+        "This function assumes that the dispatched action is a simple object and " +
+        "synchronous."
+      );
+    }
+    dispatch(action);
+    const setRecordingPreferences = selectors.getSetRecordingPreferencesFn(getState());
+    const recordingSettings = selectors.getRecordingSettings(getState());
+    setRecordingPreferences(recordingSettings);
+  };
+}
+
+/**
  * Updates the recording settings for the interval.
  * @param {number} interval
  */
-exports.changeInterval = interval => ({
+exports.changeInterval = interval => _dispatchAndUpdatePreferences({
   type: "CHANGE_INTERVAL",
   interval
 });
 
 /**
  * Updates the recording settings for the entries.
  * @param {number} entries
  */
-exports.changeEntries = entries => ({
+exports.changeEntries = entries => _dispatchAndUpdatePreferences({
   type: "CHANGE_ENTRIES",
   entries
 });
 
 /**
  * Updates the recording settings for the features.
  * @param {object} features
  */
-exports.changeFeatures = features => ({
+exports.changeFeatures = features => _dispatchAndUpdatePreferences({
   type: "CHANGE_FEATURES",
   features
 });
 
 /**
  * Updates the recording settings for the threads.
  * @param {array} threads
  */
-exports.changeThreads = threads => ({
+exports.changeThreads = threads => _dispatchAndUpdatePreferences({
   type: "CHANGE_THREADS",
   threads
 });
 
 /**
  * Receive the values to intialize the store. See the reducer for what values
  * are expected.
  * @param {object} threads
  */
 exports.initializeStore = values => ({
   type: "INITIALIZE_STORE",
-  values
+  ...values
 });
 
 /**
  * Start a new recording with the perfFront and update the internal recording state.
  */
 exports.startRecording = () => {
   return (dispatch, getState) => {
     const recordingSettings = selectors.getRecordingSettings(getState());
--- a/devtools/client/performance-new/store/reducers.js
+++ b/devtools/client/performance-new/store/reducers.js
@@ -57,74 +57,88 @@ function isSupportedPlatform(state = nul
 /**
  * The setting for the recording interval.
  * @param {number} state
  */
 function interval(state = 1, action) {
   switch (action.type) {
     case "CHANGE_INTERVAL":
       return action.interval;
+    case "INITIALIZE_STORE":
+      return action.recordingSettingsFromPreferences.interval;
     default:
       return state;
   }
 }
 
 /**
  * The number of entries in the profiler's circular buffer. Defaults to 90mb.
  * @param {number} state
  */
 function entries(state = 10000000, 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.
  * @param {array} state
  */
 function features(state = ["js", "stackwalk"], action) {
   switch (action.type) {
     case "CHANGE_FEATURES":
       return action.features;
+    case "INITIALIZE_STORE":
+      return action.recordingSettingsFromPreferences.features;
     default:
       return state;
   }
 }
 
 /**
  * The current threads list.
  * @param {array of strings} state
  */
 function threads(state = ["GeckoMain", "Compositor"], action) {
   switch (action.type) {
     case "CHANGE_THREADS":
       return action.threads;
+    case "INITIALIZE_STORE":
+      return action.recordingSettingsFromPreferences.threads;
     default:
       return state;
   }
 }
 
 /**
  * These are all the values used to initialize the profiler. They should never change
  * once added to the store.
  *
  * state = {
  *   toolbox - The current toolbox.
  *   perfFront - The current Front to the Perf actor.
  *   receiveProfile - A function to receive the profile and open it into a new window.
+ *   setRecordingPreferences - A function to set the recording settings.
  * }
  */
 function initializedValues(state = null, action) {
   switch (action.type) {
     case "INITIALIZE_STORE":
-      return action.values;
+      return {
+        toolbox: action.toolbox,
+        perfFront: action.perfFront,
+        receiveProfile: action.receiveProfile,
+        setRecordingPreferences: action.setRecordingPreferences
+      };
     default:
       return state;
   }
 }
 
 module.exports = combineReducers({
   recordingState,
   recordingUnexpectedlyStopped,
--- a/devtools/client/performance-new/store/selectors.js
+++ b/devtools/client/performance-new/store/selectors.js
@@ -27,24 +27,27 @@ const getInitializedValues = state => {
     throw new Error("The store must be initialized before it can be used.");
   }
   return values;
 };
 
 const getPerfFront = state => getInitializedValues(state).perfFront;
 const getToolbox = state => getInitializedValues(state).toolbox;
 const getReceiveProfileFn = state => getInitializedValues(state).receiveProfile;
+const getSetRecordingPreferencesFn =
+  state => getInitializedValues(state).setRecordingPreferences;
 
 module.exports = {
   getRecordingState,
   getRecordingUnexpectedlyStopped,
   getIsSupportedPlatform,
   getInterval,
   getEntries,
   getFeatures,
   getThreads,
   getThreadsString,
   getRecordingSettings,
   getInitializedValues,
   getPerfFront,
   getToolbox,
   getReceiveProfileFn,
+  getSetRecordingPreferencesFn
 };
--- a/devtools/client/performance-new/test/chrome/head.js
+++ b/devtools/client/performance-new/test/chrome/head.js
@@ -154,16 +154,18 @@ function createPerfComponent() {
     receiveProfileCalls.push(profile);
   }
 
   const mountComponent = () => {
     store.dispatch(actions.initializeStore({
       toolbox: toolboxMock,
       perfFront,
       receiveProfile: receiveProfileMock,
+      recordingSettingsFromPreferences: selectors.getRecordingSettings(store.getState()),
+      setRecordingPreferences: () => {}
     }));
 
     return ReactDOM.render(
       React.createElement(
         ReactRedux.Provider,
         { store },
         React.createElement(Perf)
       ),