From b45f0d53f209ddebda0a8fa08fce0f4ff1fb435e Mon Sep 17 00:00:00 2001
From: Karandeep <klubana@mozilla.com>
Date: Fri, 19 Jul 2024 19:41:51 +0000
Subject: [PATCH] Bug 1897245 - Convert the Weather provider onLegacyEngagement
 to use the new notifications. r=adw,urlbar-reviewers

Differential Revision: https://phabricator.services.mozilla.com/D216687
---
 .../urlbar/UrlbarProviderWeather.sys.mjs      | 68 ++++---------------
 .../tests/quicksuggest/unit/test_weather.js   | 23 +++++--
 2 files changed, 31 insertions(+), 60 deletions(-)

diff --git a/browser/components/urlbar/UrlbarProviderWeather.sys.mjs b/browser/components/urlbar/UrlbarProviderWeather.sys.mjs
index 6dc38e7aedd4..d662e9453cdb 100644
--- a/browser/components/urlbar/UrlbarProviderWeather.sys.mjs
+++ b/browser/components/urlbar/UrlbarProviderWeather.sys.mjs
@@ -78,8 +78,6 @@ class ProviderWeather extends UrlbarProvider {
    * @returns {boolean} Whether this provider should be invoked for the search.
    */
   isActive(queryContext) {
-    this.#resultFromLastQuery = null;
-
     // When Rust is enabled and keywords are not defined in Nimbus, weather
     // results are created by the quick suggest provider, not this one.
     if (
@@ -141,7 +139,6 @@ class ProviderWeather extends UrlbarProvider {
       result.payload.source = weather.suggestion.source;
       result.payload.provider = weather.suggestion.provider;
       addCallback(this, result);
-      this.#resultFromLastQuery = result;
     }
   }
 
@@ -162,58 +159,26 @@ class ProviderWeather extends UrlbarProvider {
     return lazy.QuickSuggest.weather.getViewUpdate(result);
   }
 
-  onLegacyEngagement(state, queryContext, details, controller) {
-    // Ignore engagements on other results that didn't end the session.
-    if (details.result?.providerName != this.name && details.isSessionOngoing) {
-      return;
-    }
-
-    // Impression and clicked telemetry are both recorded on engagement. We
-    // define "impression" to mean a weather result was present in the view when
-    // any result was picked.
-    if (state == "engagement" && queryContext) {
-      // Get the result that's visible in the view. `details.result` is the
-      // engaged result, if any; if it's from this provider, then that's the
-      // visible result. Otherwise fall back to #getVisibleResultFromLastQuery.
-      let { result } = details;
-      if (result?.providerName != this.name) {
-        result = this.#getVisibleResultFromLastQuery(controller.view);
-      }
-
-      if (result) {
-        this.#recordEngagementTelemetry(
-          result,
-          controller.input.isPrivate,
-          details.result == result ? details.selType : ""
-        );
-      }
-    }
+  onEngagement(queryContext, controller, details) {
+    this.#recordEngagementTelemetry(
+      details.result,
+      controller.input.isPrivate,
+      details.selType
+    );
 
     // Handle commands.
-    if (details.result?.providerName == this.name) {
-      this.#handlePossibleCommand(
-        controller.view,
-        details.result,
-        details.selType
-      );
-    }
-
-    this.#resultFromLastQuery = null;
+    this.#handlePossibleCommand(
+      controller.view,
+      details.result,
+      details.selType
+    );
   }
 
-  #getVisibleResultFromLastQuery(view) {
-    let result = this.#resultFromLastQuery;
-
-    if (
-      result?.rowIndex >= 0 &&
-      view?.visibleResults?.[result.rowIndex] == result
-    ) {
-      // The result was visible.
-      return result;
+  onImpression(state, queryContext, controller, providerVisibleResults) {
+    for (let i = 0; i < providerVisibleResults.length; i++) {
+      const { result } = providerVisibleResults[i];
+      this.#recordEngagementTelemetry(result, controller.input.isPrivate, "");
     }
-
-    // Find a visible result.
-    return view?.visibleResults?.find(r => r.providerName == this.name);
   }
 
   /**
@@ -295,9 +260,6 @@ class ProviderWeather extends UrlbarProvider {
   #handlePossibleCommand(view, result, selType) {
     lazy.QuickSuggest.weather.handleCommand(view, result, selType);
   }
-
-  // The result we added during the most recent query.
-  #resultFromLastQuery = null;
 }
 
 export var UrlbarProviderWeather = new ProviderWeather();
diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_weather.js b/browser/components/urlbar/tests/quicksuggest/unit/test_weather.js
index 8479b9721065..6cd5d5cb2095 100644
--- a/browser/components/urlbar/tests/quicksuggest/unit/test_weather.js
+++ b/browser/components/urlbar/tests/quicksuggest/unit/test_weather.js
@@ -723,16 +723,25 @@ add_tasks_with_rust(async function block() {
   let result = context.results[0];
   let provider = UrlbarProvidersManager.getProvider(result.providerName);
   Assert.ok(provider, "Sanity check: Result provider found");
-  provider.onLegacyEngagement(
-    "engagement",
-    context,
-    {
+
+  if (result.providerName === "UrlbarProviderQuickSuggest") {
+    provider.onLegacyEngagement(
+      "engagement",
+      context,
+      {
+        result,
+        selType: "dismiss",
+        selIndex: context.results[0].rowIndex,
+      },
+      controller
+    );
+  } else {
+    provider.onEngagement(context, controller, {
       result,
       selType: "dismiss",
       selIndex: context.results[0].rowIndex,
-    },
-    controller
-  );
+    });
+  }
   Assert.ok(
     !UrlbarPrefs.get("suggest.weather"),
     "suggest.weather is false after blocking the result"