Bug 1499289 - Bug 1513244 - Extract fetchAutocompleteProperties from JsTerm into a Redux action; r=Honza.
☠☠ backed out by 6e8a03d07b12 ☠ ☠
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Thu, 13 Dec 2018 14:37:20 +0000
changeset 450418 3fa4474f947cbe2601a6b4451776d80b14b20a44
parent 450417 79c280bcb1d258e416a070c37c77b242bc71a772
child 450419 8aabc712969fb01572cbf4175ce1f1561be7fa80
push id74646
push usernchevobbe@mozilla.com
push dateThu, 13 Dec 2018 14:38:32 +0000
treeherderautoland@8aabc712969f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1499289, 1513244
milestone66.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 1499289 - Bug 1513244 - Extract fetchAutocompleteProperties from JsTerm into a Redux action; r=Honza. The functions needed to build the autocomplete request are moved to the serviceContainer so it can be called from everywhere in the code, and not only from the JsTerm. Differential Revision: https://phabricator.services.mozilla.com/D12939
devtools/client/webconsole/actions/autocomplete.js
devtools/client/webconsole/components/JSTerm.js
devtools/client/webconsole/store.js
devtools/client/webconsole/webconsole-output-wrapper.js
testing/talos/talos/tests/devtools/addon/content/tests/webconsole/autocomplete.js
testing/talos/talos/tests/devtools/addon/content/tests/webconsole/typing.js
--- a/devtools/client/webconsole/actions/autocomplete.js
+++ b/devtools/client/webconsole/actions/autocomplete.js
@@ -9,36 +9,31 @@ const {
   AUTOCOMPLETE_DATA_RECEIVE,
   AUTOCOMPLETE_PENDING_REQUEST,
   AUTOCOMPLETE_RETRIEVE_FROM_CACHE,
 } = require("devtools/client/webconsole/constants");
 
 /**
  * Update the data used for the autocomplete popup in the console input (JsTerm).
  *
- * @param {Object} Object of the following shape:
- *        - {String} inputValue: the expression to complete.
- *        - {Int} cursor: The position of the cursor in the inputValue.
- *        - {WebConsoleClient} client: The webconsole client.
- *        - {String} frameActorId: The id of the frame we want to autocomplete in.
- *        - {Boolean} force: True to force a call to the server (as opposed to retrieve
- *                           from the cache).
- *        - {String} selectedNodeActor: Actor id of the selected node in the inspector.
+ * @param {Boolean} force: True to force a call to the server (as opposed to retrieve
+ *                         from the cache).
  */
-function autocompleteUpdate({
-  inputValue,
-  cursor,
-  client,
-  frameActorId,
-  force,
-  selectedNodeActor,
-}) {
-  return ({dispatch, getState}) => {
-    const {cache} = getState().autocomplete;
+function autocompleteUpdate(force) {
+  return ({dispatch, getState, services}) => {
+    if (services.inputHasSelection()) {
+      return dispatch(autocompleteClear());
+    }
 
+    const inputValue = services.getInputValue();
+    const frameActorId = services.getFrameActor();
+    const cursor = services.getInputCursor();
+
+    const state = getState().autocomplete;
+    const { cache } = state;
     if (!force && (
       !inputValue ||
       /^[a-zA-Z0-9_$]/.test(inputValue.substring(cursor))
     )) {
       return dispatch(autocompleteClear());
     }
 
     const input = inputValue.substring(0, cursor);
@@ -51,18 +46,17 @@ function autocompleteUpdate({
 
     if (retrieveFromCache) {
       return dispatch(autoCompleteDataRetrieveFromCache(input));
     }
 
     return dispatch(autocompleteDataFetch({
       input,
       frameActorId,
-      client,
-      selectedNodeActor,
+      client: services.getWebConsoleClient(),
     }));
   };
 }
 
 /**
  * Called when the autocompletion data should be cleared.
  */
 function autocompleteClear() {
@@ -91,25 +85,24 @@ function generateRequestId() {
 
 /**
  * Action that fetch autocompletion data from the server.
  *
  * @param {Object} Object of the following shape:
  *        - {String} input: the expression that we want to complete.
  *        - {String} frameActorId: The id of the frame we want to autocomplete in.
  *        - {WebConsoleClient} client: The webconsole client.
- *        - {String} selectedNodeActor: Actor id of the selected node in the inspector.
  */
 function autocompleteDataFetch({
   input,
   frameActorId,
   client,
-  selectedNodeActor,
 }) {
-  return ({dispatch}) => {
+  return ({dispatch, services}) => {
+    const selectedNodeActor = services.getSelectedNodeActor();
     const id = generateRequestId();
     dispatch({type: AUTOCOMPLETE_PENDING_REQUEST, id});
     client.autocomplete(input, undefined, frameActorId, selectedNodeActor).then(res => {
       dispatch(autocompleteDataReceive(id, input, frameActorId, res));
     }).catch(e => {
       console.error("failed autocomplete", e);
       dispatch(autocompleteClear());
     });
--- a/devtools/client/webconsole/components/JSTerm.js
+++ b/devtools/client/webconsole/components/JSTerm.js
@@ -97,18 +97,16 @@ class JSTerm extends Component {
     this.hudId = this.hud.hudId;
 
     this._keyPress = this._keyPress.bind(this);
     this._inputEventHandler = this._inputEventHandler.bind(this);
     this._blurEventHandler = this._blurEventHandler.bind(this);
     this.onContextMenu = this.onContextMenu.bind(this);
     this.imperativeUpdate = this.imperativeUpdate.bind(this);
 
-    this.SELECTED_FRAME = -1;
-
     /**
      * Last input value.
      * @type string
      */
     this.lastInputValue = "";
 
     this.autocompletePopup = null;
     this.inputNode = null;
@@ -345,17 +343,17 @@ class JSTerm extends Component {
                 this.clearCompletion();
               }
 
               return "CodeMirror.Pass";
             },
 
             "Ctrl-Space": () => {
               if (!this.autocompletePopup.isOpen) {
-                this.fetchAutocompletionProperties(true);
+                this.props.autocompleteUpdate(true);
                 return null;
               }
 
               return "CodeMirror.Pass";
             },
 
             "Esc": false,
             "Cmd-F": false,
@@ -573,17 +571,16 @@ class JSTerm extends Component {
       mappedExpressionRes = await this.hud.owner.getMappedExpression(executeString);
     } catch (e) {
       console.warn("Error when calling getMappedExpression", e);
     }
 
     executeString = mappedExpressionRes ? mappedExpressionRes.expression : executeString;
 
     const options = {
-      frame: this.SELECTED_FRAME,
       selectedNodeActor,
       mapped: mappedExpressionRes ? mappedExpressionRes.mapped : null,
     };
 
     // Even if requestEvaluation rejects (because of webConsoleClient.evaluateJSAsync),
     // we still need to pass the error response to executeResultCallback.
     const onEvaluated = this.requestEvaluation(executeString, options)
       .then(res => res, res => res);
@@ -597,45 +594,34 @@ class JSTerm extends Component {
    * @param string str
    *        String to execute.
    * @param object [options]
    *        Options for evaluation:
    *        - bindObjectActor: tells the ObjectActor ID for which you want to do
    *        the evaluation. The Debugger.Object of the OA will be bound to
    *        |_self| during evaluation, such that it's usable in the string you
    *        execute.
-   *        - frame: tells the stackframe depth to evaluate the string in. If
-   *        the jsdebugger is paused, you can pick the stackframe to be used for
-   *        evaluation. Use |this.SELECTED_FRAME| to always pick th;
-   *        user-selected stackframe.
-   *        If you do not provide a |frame| the string will be evaluated in the
-   *        global content window.
    *        - selectedNodeActor: tells the NodeActor ID of the current selection
    *        in the Inspector, if such a selection exists. This is used by
    *        helper functions that can evaluate on the current selection.
    *        - mapped: basically getMappedExpression().mapped. An object that indicates
    *        which modifications were done to the input entered by the user.
    * @return object
    *         A promise object that is resolved when the server response is
    *         received.
    */
   requestEvaluation(str, options = {}) {
     // Send telemetry event. If we are in the browser toolbox we send -1 as the
     // toolbox session id.
     this.props.serviceContainer.recordTelemetryEvent("execute_js", {
       "lines": str.split(/\n/).length,
     });
 
-    let frameActor = null;
-    if ("frame" in options) {
-      frameActor = this.getFrameActor(options.frame);
-    }
-
     return this.webConsoleClient.evaluateJSAsync(str, null, {
-      frameActor,
+      frameActor: this.props.serviceContainer.getFrameActor(options.frame),
       ...options,
     });
   }
 
   /**
    * Copy the object/variable by invoking the server
    * which invokes the `copy(variable)` command and makes it
    * available in the clipboard
@@ -646,40 +632,16 @@ class JSTerm extends Component {
    *         received.
    */
   copyObject(evalString, evalOptions) {
     return this.webConsoleClient.evaluateJSAsync(`copy(${evalString})`,
       null, evalOptions);
   }
 
   /**
-   * Retrieve the FrameActor ID given a frame depth.
-   *
-   * @param number frame
-   *        Frame depth.
-   * @return string|null
-   *         The FrameActor ID for the given frame depth.
-   */
-  getFrameActor(frame) {
-    const state = this.hud.owner.getDebuggerFrames();
-    if (!state) {
-      return null;
-    }
-
-    let grip;
-    if (frame == this.SELECTED_FRAME) {
-      grip = state.frames[state.selected];
-    } else {
-      grip = state.frames[frame];
-    }
-
-    return grip ? grip.actor : null;
-  }
-
-  /**
    * Updates the size of the input field (command line) to fit its contents.
    *
    * @returns void
    */
   resizeInput() {
     if (this.props.codeMirrorEnabled || !this.inputNode) {
       return;
     }
@@ -781,17 +743,17 @@ class JSTerm extends Component {
   /**
    * The inputNode "input" and "keyup" event handler.
    * @private
    */
   _inputEventHandler() {
     const value = this.getInputValue();
     if (this.lastInputValue !== value) {
       this.resizeInput();
-      this.fetchAutocompletionProperties();
+      this.props.autocompleteUpdate();
       this.lastInputValue = value;
     }
   }
 
   /**
    * The window "blur" event handler.
    * @private
    */
@@ -868,17 +830,17 @@ class JSTerm extends Component {
           this.clearCompletion();
           break;
         default:
           break;
       }
 
       if (event.key === " " && !this.autocompletePopup.isOpen) {
         // Open the autocompletion popup on Ctrl-Space (if it wasn't displayed).
-        this.fetchAutocompletionProperties(true);
+        this.props.autocompleteUpdate(true);
         event.preventDefault();
       }
 
       if (event.keyCode === KeyCodes.DOM_VK_LEFT &&
         (this.autocompletePopup.isOpen || this.getAutoCompletionText())
       ) {
         this.clearCompletion();
       }
@@ -1122,57 +1084,16 @@ class JSTerm extends Component {
       return false;
     }
 
     return node.selectionStart == node.value.length ? true :
            node.selectionStart == 0 && !multiline;
   }
 
   /**
-   * Retrieves properties maching the current input for the selected frame, either from
-   * the server or from a cache if possible.
-   * Will bail-out if there's some text selection in the input.
-   *
-   * @param {Boolean} force: True to not perform any check before trying to show the
-   *                         autocompletion popup. Defaults to false.
-   * @fires autocomplete-updated
-   * @returns void
-   */
-  async fetchAutocompletionProperties(force = false) {
-    const inputValue = this.getInputValue();
-    const frameActorId = this.getFrameActor(this.SELECTED_FRAME);
-    const cursor = this.getSelectionStart();
-
-    const {editor, inputNode} = this;
-    if (
-      (inputNode && inputNode.selectionStart != inputNode.selectionEnd) ||
-      (editor && editor.getSelection())
-    ) {
-      this.clearCompletion();
-      this.emit("autocomplete-updated");
-      return;
-    }
-
-    let selectedNodeActor = null;
-    const inspectorSelection = this.hud.owner.getInspectorSelection();
-    if (inspectorSelection && inspectorSelection.nodeFront) {
-      selectedNodeActor = inspectorSelection.nodeFront.actorID;
-    }
-
-    this.props.autocompleteUpdate({
-      inputValue,
-      cursor,
-      frameActorId,
-      force,
-      client: this.webConsoleClient,
-      selectedNodeActor,
-    });
-  }
-
-  /**
    * Takes the data returned by the server and update the autocomplete popup state (i.e.
    * its visibility and items).
    *
    * @param {Object} data
    *        The autocompletion data as returned by the webconsole actor's autocomplete
    *        service. Should be of the following shape:
    *        {
    *          matches: {Array} array of the properties matching the input,
@@ -1316,16 +1237,17 @@ class JSTerm extends Component {
           this.inputNode.blur();
         }
         this.autocompletePopup.once("popup-closed", () => {
           this.focus();
         });
         this.autocompletePopup.hidePopup();
       }
     }
+    this.emit("autocomplete-updated");
   }
 
   /**
    * Accept the proposed input completion.
    */
   acceptProposedCompletion() {
     let completionText = this.getAutoCompletionText();
     let numberOfCharsToReplaceCharsBeforeCursor;
@@ -1631,21 +1553,18 @@ function mapStateToProps(state) {
     history: getHistory(state),
     getValueFromHistory: (direction) => getHistoryValue(state, direction),
     autocompleteData: getAutocompleteState(state),
   };
 }
 
 function mapDispatchToProps(dispatch) {
   return {
-
     appendToHistory: (expr) => dispatch(historyActions.appendToHistory(expr)),
     clearHistory: () => dispatch(historyActions.clearHistory()),
     updateHistoryPosition: (direction, expression) =>
       dispatch(historyActions.updateHistoryPosition(direction, expression)),
-    autocompleteUpdate: options => dispatch(
-      autocompleteActions.autocompleteUpdate(options)
-    ),
+    autocompleteUpdate: force => dispatch(autocompleteActions.autocompleteUpdate(force)),
     autocompleteBailOut: () => dispatch(autocompleteActions.autocompleteBailOut()),
   };
 }
 
 module.exports = connect(mapStateToProps, mapDispatchToProps)(JSTerm);
--- a/devtools/client/webconsole/store.js
+++ b/devtools/client/webconsole/store.js
@@ -71,18 +71,29 @@ function configureStore(hud, options = {
     ui: UiState({
       filterBarVisible: getBoolPref(PREFS.UI.FILTER_BAR),
       networkMessageActiveTabId: "headers",
       persistLogs: getBoolPref(PREFS.UI.PERSIST),
     }),
   };
 
   // Prepare middleware.
+  const services = (options.services || {});
+
   const middleware = applyMiddleware(
-    thunk.bind(null, {prefsService, client: (options.services || {})}),
+    thunk.bind(null, {
+      prefsService,
+      services,
+      // Needed for the ObjectInspector
+      client: {
+        createObjectClient: services.createObjectClient,
+        createLongStringClient: services.createLongStringClient,
+        releaseActor: services.releaseActor,
+      },
+    }),
     historyPersistence,
     eventTelemetry.bind(null, options.telemetry, options.sessionId),
   );
 
   return createStore(
     createRootReducer(),
     initialState,
     compose(
--- a/devtools/client/webconsole/webconsole-output-wrapper.js
+++ b/devtools/client/webconsole/webconsole-output-wrapper.js
@@ -103,16 +103,63 @@ WebConsoleOutputWrapper.prototype = {
 
         releaseActor: (actor) => {
           if (!actor) {
             return null;
           }
 
           return debuggerClient.release(actor);
         },
+
+        getWebConsoleClient: () => {
+          return hud.webConsoleClient;
+        },
+
+        /**
+         * Retrieve the FrameActor ID given a frame depth, or the selected one if no
+         * frame depth given.
+         *
+         * @param {Number} frame: optional frame depth.
+         * @return {String|null}: The FrameActor ID for the given frame depth (or the
+         *                        selected frame if it exists).
+         */
+        getFrameActor: (frame = null) => {
+          const state = this.owner.getDebuggerFrames();
+          if (!state) {
+            return null;
+          }
+
+          const grip = Number.isInteger(frame)
+            ? state.frames[frame]
+            : state.frames[state.selected];
+          return grip ? grip.actor : null;
+        },
+
+        inputHasSelection: () => {
+          const {editor, inputNode} = hud.jsterm || {};
+          return editor
+            ? !!editor.getSelection()
+            : (inputNode && inputNode.selectionStart !== inputNode.selectionEnd);
+        },
+
+        getInputValue: () => {
+          return hud.jsterm && hud.jsterm.getInputValue();
+        },
+
+        getInputCursor: () => {
+          return hud.jsterm && hud.jsterm.getSelectionStart();
+        },
+
+        getSelectedNodeActor: () => {
+          const inspectorSelection = this.owner.getInspectorSelection();
+          if (inspectorSelection && inspectorSelection.nodeFront) {
+            return inspectorSelection.nodeFront.actorID;
+          }
+          return null;
+        },
       };
 
       // Set `openContextMenu` this way so, `serviceContainer` variable
       // is available in the current scope and we can pass it into
       // `createContextMenu` method.
       serviceContainer.openContextMenu = (e, message) => {
         const { screenX, screenY, target } = e;
 
--- a/testing/talos/talos/tests/devtools/addon/content/tests/webconsole/autocomplete.js
+++ b/testing/talos/talos/tests/devtools/addon/content/tests/webconsole/autocomplete.js
@@ -64,13 +64,13 @@ async function triggerAutocompletePopup(
 function hideAutocompletePopup(jsterm) {
   let onPopUpClosed = jsterm.autocompletePopup.once("popup-closed");
   setJsTermValueForCompletion(jsterm, "");
   return onPopUpClosed;
 }
 
 function setJsTermValueForCompletion(jsterm, value) {
   // setInputValue does not trigger the autocompletion;
-  // we need to call `fetchAutocompletionProperties` in order to display the popup.
+  // we need to call the `autocompleteUpdate` action in order to display the popup.
   jsterm.setInputValue(value);
-  jsterm.fetchAutocompletionProperties();
+  jsterm.props.autocompleteUpdate();
 }
 
--- a/testing/talos/talos/tests/devtools/addon/content/tests/webconsole/typing.js
+++ b/testing/talos/talos/tests/devtools/addon/content/tests/webconsole/typing.js
@@ -39,17 +39,17 @@ module.exports = async function() {
 
   const test = runTest(TEST_NAME);
 
   // Simulate typing in the input.
   for (const char of Array.from(input)) {
     const onPopupOpened = jsterm.autocompletePopup.once("popup-opened");
     jsterm.insertStringAtCursor(char);
     // We need to trigger autocompletion update to show the popup.
-    jsterm.fetchAutocompletionProperties();
+    jsterm.props.autocompleteUpdate();
     await onPopupOpened;
   }
 
   test.done();
 
   const onPopupClosed = jsterm.autocompletePopup.once("popup-closed");
   jsterm.clearCompletion();
   await onPopupClosed;