Backed out changeset 519aa7ff2ca6 (bug 1888905) for causing bc failures on browser_glean_telemetry_reenter.js CLOSED TREE

browser/components/urlbar/UrlbarController.sys.mjs

@@ -795,6 +795,13 @@ class TelemetryEvent {
       interactionType: this._getStartInteractionType(event, searchString),
       searchString,
     };
+
+    this._controller.manager.notifyEngagementChange(
+      "start",
+      queryContext,
+      {},
+      this._controller
+    );
   }
 
   /**
@@ -862,6 +869,15 @@ class TelemetryEvent {
     const startEventInfo = this._startEventInfo;
 
     if (!this._category || !startEventInfo) {
+      if (this._discarded && this._category && details?.selType !== "dismiss") {
+        let { queryContext } = this._controller._lastQueryContextWrapper || {};
+        this._controller.manager.notifyEngagementChange(
+          "discard",
+          queryContext,
+          {},
+          this._controller
+        );
+      }
       return;
     }
     if (

browser/components/urlbar/UrlbarProviderClipboard.sys.mjs

@@ -144,6 +144,9 @@ class ProviderClipboard extends UrlbarProvider {
   }
 
   onEngagement(state, queryContext, details, controller) {
+    if (!["engagement", "abandonment"].includes(state)) {
+      return;
+    }
     const visibleResults = controller.view?.visibleResults ?? [];
     for (const result of visibleResults) {
       if (

browser/components/urlbar/UrlbarProviderInterventions.sys.mjs

@@ -714,10 +714,11 @@ class ProviderInterventions extends UrlbarProvider {
       this.#pickResult(result, controller.browserWindow);
     }
 
-    for (let tip of this.tipsShownInCurrentEngagement) {
-      Services.telemetry.keyedScalarAdd("urlbar.tips", `${tip}-shown`, 1);
+    if (["engagement", "abandonment"].includes(state)) {
+      for (let tip of this.tipsShownInCurrentEngagement) {
+        Services.telemetry.keyedScalarAdd("urlbar.tips", `${tip}-shown`, 1);
+      }
     }
-
     this.tipsShownInCurrentEngagement.clear();
   }
 

browser/components/urlbar/UrlbarProviderQuickSuggest.sys.mjs

@@ -237,7 +237,7 @@ class ProviderQuickSuggest extends UrlbarProvider {
 
     // Reset the Merino session ID when a session ends. By design for the user's
     // privacy, we don't keep it around between engagements.
-    if (!details.isSessionOngoing) {
+    if (state != "start" && !details.isSessionOngoing) {
       this.#merino?.resetSession();
     }
 

browser/components/urlbar/UrlbarProviderTopSites.sys.mjs

@@ -334,7 +334,11 @@ class ProviderTopSites extends UrlbarProvider {
   }
 
   onEngagement(state, queryContext) {
-    if (!queryContext.isPrivate && this.sponsoredSites) {
+    if (
+      !queryContext.isPrivate &&
+      this.sponsoredSites &&
+      ["engagement", "abandonment"].includes(state)
+    ) {
       for (let site of this.sponsoredSites) {
         Services.telemetry.keyedScalarAdd(
           SCALAR_CATEGORY_TOPSITES,

browser/components/urlbar/UrlbarProvidersManager.sys.mjs

@@ -337,7 +337,8 @@ class ProvidersManager {
    * urlbar.  For details on parameters, see UrlbarProvider.onEngagement().
    *
    * @param {string} state
-   *   The state of the engagement, one of: engagement, abandonment
+   *   The state of the engagement, one of: start, engagement, abandonment,
+   *   discard
    * @param {UrlbarQueryContext} queryContext
    *   The engagement's query context, if available.
    * @param {object} details

browser/components/urlbar/UrlbarUtils.sys.mjs

@@ -2433,12 +2433,21 @@ export class UrlbarProvider {
    * @param {string} _state
    *   The state of the engagement, one of the following strings:
    *
+   *   start
+   *       A new query has started in the urlbar.
    *   engagement
    *       The user picked a result in the urlbar or used paste-and-go.
    *   abandonment
    *       The urlbar was blurred (i.e., lost focus).
+   *   discard
+   *       This doesn't correspond to a user action, but it means that the
+   *       urlbar has discarded the engagement for some reason, and the
+   *       `onEngagement` implementation should ignore it.
+   *
    * @param {UrlbarQueryContext} _queryContext
-   *   The engagement's query context.
+   *   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 object is non-empty only when `state` is "engagement" or
    *   "abandonment", and it describes the search string and engaged result.

browser/components/urlbar/tests/browser/browser_engagement.js

@@ -110,21 +110,32 @@ async function doTest({
   let provider = new TestProvider();
   UrlbarProvidersManager.registerProvider(provider);
 
+  let startPromise = provider.promiseEngagement();
   await UrlbarTestUtils.promiseAutocompleteResultPopup({
     window: win,
     value: "test",
     fireInputEvent: true,
   });
 
+  let [state, queryContext, details, controller] = await startPromise;
+  Assert.equal(
+    controller.input.isPrivate,
+    expectedIsPrivate,
+    "Start isPrivate"
+  );
+  Assert.equal(state, "start", "Start state");
+
+  // `queryContext` isn't always defined for `start`, and `onEngagement`
+  // shouldn't rely on it being defined on start, but there's no good reason to
+  // assert that it's not defined here.
+
+  // Similarly, `details` is never defined for `start`, but there's no good
+  // reason to assert that it's not defined.
+
   let endPromise = provider.promiseEngagement();
   let { result, element } = (await endEngagement()) ?? {};
 
-  let [state, queryContext, details, controller] = await endPromise;
-
-  Assert.ok(
-    ["engagement", "abandonment"].includes(state),
-    "State should be either 'engagement' or 'abandonment'"
-  );
+  [state, queryContext, details, controller] = await endPromise;
   Assert.equal(controller.input.isPrivate, expectedIsPrivate, "End isPrivate");
   Assert.equal(state, expectedEndState, "End state");
   Assert.ok(queryContext, "End queryContext");