Bug 1554038 - Make UrlbarInput::pickResult and UrlbarController::recordSelectedResult take a UrlbarResult instead of an index and remove UrlbarView::getResult. r=adw

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

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

browser/components/urlbar/UrlbarController.jsm

@@ -162,7 +162,7 @@ class UrlbarController {
       }
       // The first time we receive results try to connect to the heuristic
       // result.
-      this.speculativeConnect(queryContext, 0, "resultsadded");
+      this.speculativeConnect(queryContext.results[0], queryContext, "resultsadded");
     }
 
     this._notify("onQueryResults", queryContext);
@@ -356,20 +356,19 @@ class UrlbarController {
   /**
    * Tries to initialize a speculative connection on a result.
    * Speculative connections are only supported for a subset of all the results.
+   * @param {UrlbarResult} result Tthe result to speculative connect to.
    * @param {UrlbarQueryContext} context The queryContext
-   * @param {number} resultIndex index of the result to speculative connect to.
    * @param {string} reason Reason for the speculative connect request.
    * @note speculative connect to:
    *  - Search engine heuristic results
    *  - autofill results
    *  - http/https results
    */
-  speculativeConnect(context, resultIndex, reason) {
+  speculativeConnect(result, context, reason) {
     // Never speculative connect in private contexts.
     if (!this.input || context.isPrivate || context.results.length == 0) {
       return;
     }
-    let result = context.results[resultIndex];
     let {url} = UrlbarUtils.getUrlFromResult(result);
     if (!url) {
       return;
@@ -378,7 +377,7 @@ class UrlbarController {
     switch (reason) {
       case "resultsadded": {
         // We should connect to an heuristic result, if it exists.
-        if ((resultIndex == 0 && context.preselected) || result.autofill) {
+        if ((result == context.results[0] && context.preselected) || result.autofill) {
           if (result.type == UrlbarUtils.RESULT_TYPE.SEARCH) {
             // Speculative connect only if search suggestions are enabled.
             if (UrlbarPrefs.get("suggest.searches") &&
@@ -426,19 +425,17 @@ class UrlbarController {
    *
    * @param {Event} event
    *   The event which triggered the result to be selected.
-   * @param {number} resultIndex
-   *   The index of the result.
+   * @param {UrlbarResult} result
+   *   The selected result.
    */
-  recordSelectedResult(event, resultIndex) {
-    let result;
+  recordSelectedResult(event, result) {
+    let resultIndex = result ? result.uiIndex : -1;
     let selectedResult = -1;
-
     if (resultIndex >= 0) {
-      result = this.view.getResult(resultIndex);
       // Except for the history popup, the urlbar always has a selection.  The
       // first result at index 0 is the "heuristic" result that indicates what
       // will happen when you press the Enter key.  Treat it as no selection.
-      selectedResult = resultIndex > 0 || !result.heuristic ? resultIndex : -1;
+      selectedResult = (resultIndex > 0 || !result.heuristic) ? resultIndex : -1;
     }
     BrowserUsageTelemetry.recordUrlbarSelectedResultMethod(
       event, selectedResult, this._userSelectionBehavior);

browser/components/urlbar/UrlbarInput.jsm

@@ -356,9 +356,9 @@ class UrlbarInput {
 
     // Use the selected result if we have one; this is usually the case
     // when the view is open.
-    let index = this.view.selectedIndex;
-    if (!selectedOneOff && index != -1) {
-      this.pickResult(event, index);
+    let result = this.view.selectedResult;
+    if (!selectedOneOff && result) {
+      this.pickResult(result, event);
       return;
     }
 
@@ -366,7 +366,7 @@ class UrlbarInput {
     if (selectedOneOff) {
       // If there's a selected one-off button then load a search using
       // the button's engine.
-      let result = this._resultForCurrentValue;
+      result = this._resultForCurrentValue;
       let searchString =
         (result && (result.payload.suggestion || result.payload.query)) ||
         this._lastSearchString;
@@ -384,12 +384,10 @@ class UrlbarInput {
       return;
     }
 
+    this.controller.recordSelectedResult(event, result || this.view.selectedResult);
+
     let where = openWhere || this._whereToOpen(event);
-
     openParams.allowInheritPrincipal = false;
-
-    this.controller.recordSelectedResult(event, index);
-
     url = this._maybeCanonizeURL(event, url) || url.trim();
 
     try {
@@ -423,11 +421,10 @@ class UrlbarInput {
   /**
    * Called by the view when a result is picked.
    *
+   * @param {UrlbarResult} result The result that was picked.
    * @param {Event} event The event that picked the result.
-   * @param {resultIndex} resultIndex The index of the result that was picked.
    */
-  pickResult(event, resultIndex) {
-    let result = this.view.getResult(resultIndex);
+  pickResult(result, event) {
     let isCanonized = this.setValueFromResult(result, event);
     let where = this._whereToOpen(event);
     let openParams = {
@@ -437,7 +434,8 @@ class UrlbarInput {
     if (!result.payload.isKeywordOffer) {
       this.view.close();
     }
-    this.controller.recordSelectedResult(event, resultIndex);
+
+    this.controller.recordSelectedResult(event, result);
 
     if (isCanonized) {
       this._loadURL(this.value, where, openParams);

browser/components/urlbar/UrlbarResult.jsm

@@ -54,6 +54,9 @@ class UrlbarResult {
     }
     this.source = resultSource;
 
+    // UrlbarView is responsible for updating this.
+    this.uiIndex = -1;
+
     // May be used to indicate an heuristic result. Heuristic results can bypass
     // source filters in the ProvidersManager, that otherwise may skip them.
     this.heuristic = false;

browser/components/urlbar/UrlbarView.jsm

@@ -80,7 +80,7 @@ class UrlbarView {
     if (!this.isOpen || !this._selected) {
       return -1;
     }
-    return parseInt(this._selected.getAttribute("resultIndex"));
+    return this._selected.result.uiIndex;
   }
 
   set selectedIndex(val) {
@@ -113,19 +113,6 @@ class UrlbarView {
     return this._selected.result;
   }
 
-  /**
-   * Gets the result for the index.
-   * @param {number} index
-   *   The index to look up.
-   * @returns {UrlbarResult}
-   */
-  getResult(index) {
-    if (index < 0 || index > this._queryContext.results.length) {
-      throw new Error(`UrlbarView: Index ${index} is out of bounds`);
-    }
-    return this._queryContext.results[index];
-  }
-
   /**
    * Moves the view selection forward or backward.
    *
@@ -263,15 +250,11 @@ class UrlbarView {
    * @param {number} index The index of the result that has been removed.
    */
   onQueryResultRemoved(index) {
-    // Change the index for any rows above the removed index.
-    for (let i = index + 1; i < this._rows.children.length; i++) {
-      let child = this._rows.children[i];
-      child.setAttribute("resultIndex", child.getAttribute("resultIndex") - 1);
-    }
-
     let rowToRemove = this._rows.children[index];
     rowToRemove.remove();
 
+    this._updateIndices();
+
     if (rowToRemove != this._selected) {
       return;
     }
@@ -482,6 +465,8 @@ class UrlbarView {
       }
       this._rows.appendChild(row);
     }
+
+    this._updateIndices();
   }
 
   _createRow() {
@@ -532,11 +517,8 @@ class UrlbarView {
   }
 
   _updateRow(item, result) {
-    let resultIndex = this._queryContext.results.indexOf(result);
     item.result = result;
     item.removeAttribute("stale");
-    item.id = "urlbarView-row-" + resultIndex;
-    item.setAttribute("resultIndex", resultIndex);
 
     if (result.type == UrlbarUtils.RESULT_TYPE.SEARCH &&
         !result.payload.isKeywordOffer) {
@@ -634,6 +616,14 @@ class UrlbarView {
     item._elements.get("titleSeparator").hidden = !action && !setURL;
   }
 
+  _updateIndices() {
+    for (let i = 0; i < this._rows.children.length; i++) {
+      let item = this._rows.children[i];
+      item.result.uiIndex = i;
+      item.id = "urlbarView-row-" + i;
+    }
+  }
+
   _removeStaleRows() {
     let row = this._rows.lastElementChild;
     while (row) {
@@ -799,7 +789,7 @@ class UrlbarView {
       row = row.parentNode;
     }
     this._selectItem(row, { updateInput: false });
-    this.controller.speculativeConnect(this._queryContext, this.selectedIndex, "mousedown");
+    this.controller.speculativeConnect(this.selectedResult, this._queryContext, "mousedown");
   }
 
   _on_mouseup(event) {
@@ -812,7 +802,7 @@ class UrlbarView {
     while (!row.classList.contains("urlbarView-row")) {
       row = row.parentNode;
     }
-    this.input.pickResult(event, parseInt(row.getAttribute("resultIndex")));
+    this.input.pickResult(row.result, event);
   }
 
   _on_overflow(event) {