Bug 1827770 - Remove QueryContext.view and pass the controller to onE…

…ngagement() instead. r=daleharvey Differential Revision: https://phabricator.services.mozilla.com/D182771
dothq · Jul 7, 2023 · 9804a51 · 9804a51
1 parent d555e56
commit 9804a51
Show file tree

Hide file tree

Showing 24 changed files with 150 additions and 145 deletions.
diff --git a/browser/components/urlbar/UrlbarController.sys.mjs b/browser/components/urlbar/UrlbarController.sys.mjs
@@ -795,7 +795,7 @@ class TelemetryEvent {
       "start",
       queryContext,
       {},
-      this._controller.browserWindow
+      this._controller
     );
   }
 
@@ -905,7 +905,7 @@ class TelemetryEvent {
           "discard",
           queryContext,
           {},
-          this._controller.browserWindow
+          this._controller
         );
       }
       return;
@@ -997,7 +997,7 @@ class TelemetryEvent {
         method,
         queryContext,
         details,
-        this._controller.browserWindow
+        this._controller
       );
       return;
     }
@@ -1053,11 +1053,11 @@ class TelemetryEvent {
 
     if (
       method === "engagement" &&
-      queryContext?.view?.visibleResults?.[0]?.autofill
+      this._controller.view?.visibleResults?.[0]?.autofill
     ) {
       // Record autofill impressions upon engagement.
       const type = lazy.UrlbarUtils.telemetryTypeFromResult(
-        queryContext.view.visibleResults[0]
+        this._controller.view.visibleResults[0]
       );
       Services.telemetry.scalarAdd(`urlbar.impression.${type}`, 1);
     }
@@ -1067,7 +1067,7 @@ class TelemetryEvent {
       method,
       queryContext,
       details,
-      this._controller.browserWindow
+      this._controller
     );
   }
 
@@ -1112,7 +1112,7 @@ class TelemetryEvent {
       searchMode
     );
     const search_mode = this.#getSearchMode(searchMode);
-    const currentResults = queryContext?.view?.visibleResults ?? [];
+    const currentResults = this._controller.view?.visibleResults ?? [];
     let numResults = currentResults.length;
     let groups = currentResults
       .map(r => lazy.UrlbarUtils.searchEngagementTelemetryGroup(r))
@@ -1133,7 +1133,7 @@ class TelemetryEvent {
           selectedElement
         );
 
-      if (selected_result === "input_field" && !queryContext?.view?.isOpen) {
+      if (selected_result === "input_field" && !this._controller.view?.isOpen) {
         numResults = 0;
         groups = "";
         results = "";

diff --git a/browser/components/urlbar/UrlbarInput.sys.mjs b/browser/components/urlbar/UrlbarInput.sys.mjs
@@ -1554,7 +1554,6 @@ export class UrlbarInput {
       isPrivate: this.isPrivate,
       maxResults: lazy.UrlbarPrefs.get("maxRichResults"),
       searchString,
-      view: this.view,
       userContextId:
         this.window.gBrowser.selectedBrowser.getAttribute("usercontextid"),
       currentPage: this.window.gBrowser.currentURI.spec,

diff --git a/browser/components/urlbar/UrlbarProviderContextualSearch.sys.mjs b/browser/components/urlbar/UrlbarProviderContextualSearch.sys.mjs
@@ -242,10 +242,10 @@ class ProviderContextualSearch extends UrlbarProvider {
     };
   }
 
-  onEngagement(isPrivate, state, queryContext, details, window) {
+  onEngagement(isPrivate, state, queryContext, details, controller) {
     let { result } = details;
     if (result?.providerName == this.name) {
-      this.#pickResult(result, window);
+      this.#pickResult(result, controller.browserWindow);
     }
   }
 

diff --git a/browser/components/urlbar/UrlbarProviderInterventions.sys.mjs b/browser/components/urlbar/UrlbarProviderInterventions.sys.mjs
@@ -711,10 +711,10 @@ class ProviderInterventions extends UrlbarProvider {
     }
   }
 
-  onEngagement(isPrivate, state, queryContext, details, window) {
+  onEngagement(isPrivate, state, queryContext, details, controller) {
     let { result } = details;
     if (result?.providerName == this.name) {
-      this.#pickResult(result, window);
+      this.#pickResult(result, controller.browserWindow);
     }
 
     if (["engagement", "abandonment"].includes(state)) {

diff --git a/browser/components/urlbar/UrlbarProviderOmnibox.sys.mjs b/browser/components/urlbar/UrlbarProviderOmnibox.sys.mjs
@@ -180,15 +180,15 @@ class ProviderOmnibox extends UrlbarProvider {
     );
   }
 
-  onEngagement(isPrivate, state, queryContext, details) {
+  onEngagement(isPrivate, state, queryContext, details, controller) {
     let { result } = details;
     if (result?.providerName != this.name) {
       return;
     }
 
     if (details.selType == "dismiss" && result.payload.isBlockable) {
       lazy.ExtensionSearchHandler.handleInputDeleted(result.payload.title);
-      queryContext.view.controller.removeResult(result);
+      controller.removeResult(result);
     }
   }
 }

diff --git a/browser/components/urlbar/UrlbarProviderPlaces.sys.mjs b/browser/components/urlbar/UrlbarProviderPlaces.sys.mjs
@@ -1479,7 +1479,7 @@ class ProviderPlaces extends UrlbarProvider {
     search.notifyResult(false);
   }
 
-  onEngagement(isPrivate, state, queryContext, details) {
+  onEngagement(isPrivate, state, queryContext, details, controller) {
     let { result } = details;
     if (result?.providerName != this.name) {
       return;
@@ -1492,14 +1492,14 @@ class ProviderPlaces extends UrlbarProvider {
           // from browsing history.
           let { url } = UrlbarUtils.getUrlFromResult(result);
           lazy.PlacesUtils.history.remove(url).catch(console.error);
-          queryContext.view.controller.removeResult(result);
+          controller.removeResult(result);
           break;
         case UrlbarUtils.RESULT_TYPE.URL:
           // Remove browsing history entries from Places.
           lazy.PlacesUtils.history
             .remove(result.payload.url)
             .catch(console.error);
-          queryContext.view.controller.removeResult(result);
+          controller.removeResult(result);
           break;
       }
     }

diff --git a/browser/components/urlbar/UrlbarProviderQuickActions.sys.mjs b/browser/components/urlbar/UrlbarProviderQuickActions.sys.mjs
@@ -246,24 +246,7 @@ class ProviderQuickActions extends UrlbarProvider {
     }
   }
 
-  /**
-   * Called when the user starts and ends an engagement with the urlbar.  For
-   * details on parameters, see UrlbarProvider.onEngagement().
-   *
-   * @param {boolean} isPrivate
-   *   True if the engagement is in a private context.
-   * @param {string} state
-   *   The state of the engagement, one of: start, engagement, abandonment,
-   *   discard
-   * @param {UrlbarQueryContext} queryContext
-   *   The engagement's query context.  This is *not* guaranteed to be defined
-   *   when `state` is "start".  It will always be defined for "engagement" and
-   *   "abandonment".
-   * @param {object} details
-   *   This is defined only when `state` is "engagement" or "abandonment", and
-   *   it describes the search string and picked result.
-   */
-  onEngagement(isPrivate, state, queryContext, details) {
+  onEngagement(isPrivate, state, queryContext, details, controller) {
     // Ignore engagements on other results that didn't end the session.
     if (details.result?.providerName != this.name && details.isSessionOngoing) {
       return;
@@ -275,7 +258,7 @@ class ProviderQuickActions extends UrlbarProvider {
       // visible result. Otherwise fall back to #getVisibleResultFromLastQuery.
       let { result } = details;
       if (result?.providerName != this.name) {
-        result = this.#getVisibleResultFromLastQuery(queryContext.view);
+        result = this.#getVisibleResultFromLastQuery(controller.view);
       }
 
       result?.payload.results.forEach(({ key }) => {

diff --git a/browser/components/urlbar/UrlbarProviderQuickSuggest.sys.mjs b/browser/components/urlbar/UrlbarProviderQuickSuggest.sys.mjs
@@ -208,24 +208,7 @@ class ProviderQuickSuggest extends UrlbarProvider {
     }
   }
 
-  /**
-   * Called when the user starts and ends an engagement with the urlbar.  For
-   * details on parameters, see UrlbarProvider.onEngagement().
-   *
-   * @param {boolean} isPrivate
-   *   True if the engagement is in a private context.
-   * @param {string} state
-   *   The state of the engagement, one of: start, engagement, abandonment,
-   *   discard
-   * @param {UrlbarQueryContext} queryContext
-   *   The engagement's query context.  This is *not* guaranteed to be defined
-   *   when `state` is "start".  It will always be defined for "engagement" and
-   *   "abandonment".
-   * @param {object} details
-   *   This is defined only when `state` is "engagement" or "abandonment", and
-   *   it describes the search string and picked result.
-   */
-  onEngagement(isPrivate, state, queryContext, details) {
+  onEngagement(isPrivate, state, queryContext, details, controller) {
     // Ignore engagements on other results that didn't end the session.
     if (details.result?.providerName != this.name && details.isSessionOngoing) {
       return;
@@ -246,7 +229,7 @@ class ProviderQuickSuggest extends UrlbarProvider {
       // visible result. Otherwise fall back to #getVisibleResultFromLastQuery.
       let { result } = details;
       if (result?.providerName != this.name) {
-        result = this.#getVisibleResultFromLastQuery(queryContext.view);
+        result = this.#getVisibleResultFromLastQuery(controller.view);
       }
 
       this.#recordEngagement(queryContext, isPrivate, result, details);
@@ -255,10 +238,10 @@ class ProviderQuickSuggest extends UrlbarProvider {
     if (details.result?.providerName == this.name) {
       let feature = this.#getFeatureByResult(details.result);
       if (feature?.handleCommand) {
-        feature.handleCommand(queryContext, details.result, details.selType);
+        feature.handleCommand(controller.view, details.result, details.selType);
       } else if (details.selType == "dismiss") {
         // Handle dismissals.
-        this.#dismissResult(queryContext, details.result);
+        this.#dismissResult(controller, details.result);
       }
     }
 
@@ -442,7 +425,7 @@ class ProviderQuickSuggest extends UrlbarProvider {
     return view?.visibleResults?.findLast(r => r.providerName == this.name);
   }
 
-  #dismissResult(queryContext, result) {
+  #dismissResult(controller, result) {
     if (!result.payload.isBlockable) {
       this.logger.info("Dismissals disabled, ignoring dismissal");
       return;
@@ -453,7 +436,7 @@ class ProviderQuickSuggest extends UrlbarProvider {
       // adM results have `originalUrl`, which contains timestamp templates.
       result.payload.originalUrl ?? result.payload.url
     );
-    queryContext.view.controller.removeResult(result);
+    controller.removeResult(result);
   }
 
   /**

diff --git a/browser/components/urlbar/UrlbarProviderSearchSuggestions.sys.mjs b/browser/components/urlbar/UrlbarProviderSearchSuggestions.sys.mjs
@@ -318,7 +318,7 @@ class ProviderSearchSuggestions extends UrlbarProvider {
     }
   }
 
-  onEngagement(isPrivate, state, queryContext, details) {
+  onEngagement(isPrivate, state, queryContext, details, controller) {
     let { result } = details;
     if (result?.providerName != this.name) {
       return;
@@ -332,7 +332,7 @@ class ProviderSearchSuggestions extends UrlbarProvider {
       }).catch(error =>
         console.error(`Removing form history failed: ${error}`)
       );
-      queryContext.view.controller.removeResult(result);
+      controller.removeResult(result);
     }
   }
 

diff --git a/browser/components/urlbar/UrlbarProviderSearchTips.sys.mjs b/browser/components/urlbar/UrlbarProviderSearchTips.sys.mjs
@@ -274,34 +274,15 @@ class ProviderSearchTips extends UrlbarProvider {
     lazy.UrlbarPrefs.set(`tipShownCount.${tip}`, MAX_SHOWN_COUNT);
   }
 
-  /**
-   * Called when the user starts and ends an engagement with the urlbar.  For
-   * details on parameters, see UrlbarProvider.onEngagement().
-   *
-   * @param {boolean} isPrivate
-   *   True if the engagement is in a private context.
-   * @param {string} state
-   *   The state of the engagement, one of: start, engagement, abandonment,
-   *   discard
-   * @param {UrlbarQueryContext} queryContext
-   *   The engagement's query context.  This is *not* guaranteed to be defined
-   *   when `state` is "start".  It will always be defined for "engagement" and
-   *   "abandonment".
-   * @param {object} details
-   *   This is defined only when `state` is "engagement" or "abandonment", and
-   *   it describes the search string and picked result.
-   * @param {window} window
-   *   The browser window where the engagement event took place.
-   */
-  onEngagement(isPrivate, state, queryContext, details, window) {
+  onEngagement(isPrivate, state, queryContext, details, controller) {
     // Ignore engagements on other results that didn't end the session.
     let { result } = details;
     if (result?.providerName != this.name && details.isSessionOngoing) {
       return;
     }
 
     if (result?.providerName == this.name) {
-      this.#pickResult(result, window);
+      this.#pickResult(result, controller.browserWindow);
     }
 
     this.showedTipTypeInCurrentEngagement = TIPS.NONE;

diff --git a/browser/components/urlbar/UrlbarProviderWeather.sys.mjs b/browser/components/urlbar/UrlbarProviderWeather.sys.mjs
@@ -418,7 +418,7 @@ class ProviderWeather extends UrlbarProvider {
     };
   }
 
-  onEngagement(isPrivate, state, queryContext, details) {
+  onEngagement(isPrivate, state, queryContext, details, controller) {
     // Ignore engagements on other results that didn't end the session.
     if (details.result?.providerName != this.name && details.isSessionOngoing) {
       return;
@@ -433,7 +433,7 @@ class ProviderWeather extends UrlbarProvider {
       // visible result. Otherwise fall back to #getVisibleResultFromLastQuery.
       let { result } = details;
       if (result?.providerName != this.name) {
-        result = this.#getVisibleResultFromLastQuery(queryContext.view);
+        result = this.#getVisibleResultFromLastQuery(controller.view);
       }
 
       if (result) {
@@ -448,7 +448,7 @@ class ProviderWeather extends UrlbarProvider {
     // Handle commands.
     if (details.result?.providerName == this.name) {
       this.#handlePossibleCommand(
-        queryContext,
+        controller.view,
         details.result,
         details.selType
       );
@@ -553,7 +553,7 @@ class ProviderWeather extends UrlbarProvider {
     );
   }
 
-  #handlePossibleCommand(queryContext, result, selType) {
+  #handlePossibleCommand(view, result, selType) {
     switch (selType) {
       case RESULT_MENU_COMMAND.HELP:
         // "help" is handled by UrlbarInput, no need to do anything here.
@@ -564,20 +564,20 @@ class ProviderWeather extends UrlbarProvider {
       case RESULT_MENU_COMMAND.NOT_RELEVANT:
         this.logger.info("Dismissing weather result");
         lazy.UrlbarPrefs.set("suggest.weather", false);
-        queryContext.view.acknowledgeDismissal(result);
+        view.acknowledgeDismissal(result);
         break;
       case RESULT_MENU_COMMAND.INACCURATE_LOCATION:
         // Currently the only way we record this feedback is in the Glean
         // engagement event. As with all commands, it will be recorded with an
         // `engagement_type` value that is the command's name, in this case
         // `inaccurate_location`.
-        queryContext.view.acknowledgeFeedback(result);
+        view.acknowledgeFeedback(result);
         break;
       case RESULT_MENU_COMMAND.SHOW_LESS_FREQUENTLY:
-        queryContext.view.acknowledgeFeedback(result);
+        view.acknowledgeFeedback(result);
         lazy.QuickSuggest.weather.incrementMinKeywordLength();
         if (!lazy.QuickSuggest.weather.canIncrementMinKeywordLength) {
-          queryContext.view.invalidateResultMenuCommands();
+          view.invalidateResultMenuCommands();
         }
         break;
     }

diff --git a/browser/components/urlbar/UrlbarProvidersManager.sys.mjs b/browser/components/urlbar/UrlbarProvidersManager.sys.mjs
@@ -342,18 +342,24 @@ class ProvidersManager {
    *   The engagement's query context, if available.
    * @param {object} details
    *   An object that describes the search string and the picked result, if any.
-   * @param {window} window
-   *   Browser window object associated with engagement
+   * @param {UrlbarController} controller
+   *   The controller associated with the engagement
    */
-  notifyEngagementChange(isPrivate, state, queryContext, details = {}, window) {
+  notifyEngagementChange(
+    isPrivate,
+    state,
+    queryContext,
+    details = {},
+    controller
+  ) {
     for (let provider of this.providers) {
       provider.tryMethod(
         "onEngagement",
         isPrivate,
         state,
         queryContext,
         details,
-        window
+        controller
       );
     }
   }