Bug 1833877 - Don't check page for components related to the impression if components don't exist - r=scunnane

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

browser/actors/SearchSERPTelemetryChild.sys.mjs

@@ -918,7 +918,7 @@ export class SearchSERPTelemetryChild extends JSWindowActorChild {
 
     if (
       lazy.serpEventsEnabled &&
-      providerInfo?.components &&
+      providerInfo?.components.length &&
       (eventType == "load" || eventType == "pageshow")
     ) {
       searchAdImpression.pageUrl = new URL(url);
@@ -956,19 +956,20 @@ export class SearchSERPTelemetryChild extends JSWindowActorChild {
   #checkForPageImpressionComponents() {
     let url = this.document.documentURI;
     let providerInfo = this._getProviderInfoForUrl(url);
-    searchAdImpression.providerInfo = providerInfo;
-
-    let start = Cu.now();
-    let hasShoppingTab = searchAdImpression.hasShoppingTab(this.document);
-    ChromeUtils.addProfilerMarker(
-      "SearchSERPTelemetryChild.#recordImpression",
-      start,
-      "Checked for shopping tab"
-    );
-    this.sendAsyncMessage("SearchTelemetry:PageImpression", {
-      url,
-      hasShoppingTab,
-    });
+    if (providerInfo.components?.length) {
+      searchAdImpression.providerInfo = providerInfo;
+      let start = Cu.now();
+      let hasShoppingTab = searchAdImpression.hasShoppingTab(this.document);
+      ChromeUtils.addProfilerMarker(
+        "SearchSERPTelemetryChild.#recordImpression",
+        start,
+        "Checked for shopping tab"
+      );
+      this.sendAsyncMessage("SearchTelemetry:PageImpression", {
+        url,
+        hasShoppingTab,
+      });
+    }
   }
 
   /**

browser/components/search/SearchSERPTelemetry.sys.mjs

@@ -373,7 +373,11 @@ class TelemetryHandler {
     // from something that happened in content. We keep this separate from
     // source because legacy telemetry should not change its reporting.
     let inContentSource;
-    if (lazy.serpEventsEnabled && this.#browserContentSourceMap.has(browser)) {
+    if (
+      lazy.serpEventsEnabled &&
+      info.hasComponents &&
+      this.#browserContentSourceMap.has(browser)
+    ) {
       inContentSource = this.#browserContentSourceMap.get(browser);
       this.#browserContentSourceMap.delete(browser);
     }
@@ -386,7 +390,7 @@ class TelemetryHandler {
     }
 
     let impressionId;
-    if (lazy.serpEventsEnabled) {
+    if (lazy.serpEventsEnabled && info.hasComponents) {
       // The UUID generated by Services.uuid contains leading and trailing braces.
       // Need to trim them first.
       impressionId = Services.uuid
@@ -402,7 +406,7 @@ class TelemetryHandler {
     let item = this._browserInfoByURL.get(url);
 
     let impressionInfo;
-    if (lazy.serpEventsEnabled) {
+    if (lazy.serpEventsEnabled && info.hasComponents) {
       let partnerCode = "";
       if (info.code != "none" && info.code != null) {
         partnerCode = info.code;
@@ -725,14 +729,21 @@ class TelemetryHandler {
       }
     }
     let isShoppingPage = false;
-    if (lazy.serpEventsEnabled && searchProviderInfo.shoppingTab?.regexp) {
-      isShoppingPage = searchProviderInfo.shoppingTab.regexp.test(url);
+    let hasComponents = false;
+    if (lazy.serpEventsEnabled) {
+      if (searchProviderInfo.shoppingTab?.regexp) {
+        isShoppingPage = searchProviderInfo.shoppingTab.regexp.test(url);
+      }
+      if (searchProviderInfo.components?.length) {
+        hasComponents = true;
+      }
     }
     return {
       provider: searchProviderInfo.telemetryId,
       type,
       code,
       isShoppingPage,
+      hasComponents,
     };
   }
 

browser/components/search/test/browser/browser_search_telemetry_abandonment.js

@@ -194,3 +194,51 @@ add_task(async function test_click_ad() {
 
   BrowserTestUtils.removeTab(tab);
 });
+
+add_task(async function test_without_components() {
+  // Mock a provider that doesn't have components.
+  let providerInfo = [
+    {
+      ...TEST_PROVIDER_INFO[0],
+      components: [],
+    },
+  ];
+  SearchSERPTelemetry.overrideSearchTelemetryForTests(providerInfo);
+  await waitForIdle();
+  resetTelemetry();
+
+  let tab = await BrowserTestUtils.openNewForegroundTab(
+    gBrowser,
+    getSERPUrl("searchTelemetryAd.html")
+  );
+
+  // We shouldn't expect a SERP impression, so instead wait roughly
+  // around how long it would usually take to receive an impression following
+  // a page load.
+  await promiseWaitForAdLinkCheck();
+  Assert.equal(
+    !!Glean.serp.impression.testGetValue(),
+    false,
+    "Should not have any impression events."
+  );
+
+  let browserLoadedPromise = BrowserTestUtils.browserLoaded(
+    gBrowser,
+    false,
+    "https://www.example.com/"
+  );
+  BrowserTestUtils.loadURIString(gBrowser, "https://www.example.com");
+  await browserLoadedPromise;
+
+  Assert.equal(
+    !!Glean.serp.abandonment.testGetValue(),
+    false,
+    "Should not have any abandonment events."
+  );
+
+  BrowserTestUtils.removeTab(tab);
+
+  // Allow subsequent tests to use the default provider.
+  SearchSERPTelemetry.overrideSearchTelemetryForTests(TEST_PROVIDER_INFO);
+  await waitForIdle();
+});

browser/components/search/test/unit/test_urlTelemetry_generic.js

@@ -8,6 +8,7 @@ const { XPCOMUtils } = ChromeUtils.importESModule(
 ChromeUtils.defineESModuleGetters(this, {
   BrowserSearchTelemetry: "resource:///modules/BrowserSearchTelemetry.sys.mjs",
   SearchSERPTelemetry: "resource:///modules/SearchSERPTelemetry.sys.mjs",
+  SearchSERPTelemetryUtils: "resource:///modules/SearchSERPTelemetry.sys.mjs",
   SearchUtils: "resource://gre/modules/SearchUtils.sys.mjs",
   TelemetryTestUtils: "resource://testing-common/TelemetryTestUtils.sys.mjs",
   sinon: "resource://testing-common/Sinon.sys.mjs",
@@ -31,6 +32,12 @@ const TEST_PROVIDER_INFO = [
     shoppingTab: {
       regexp: "&site=shop",
     },
+    components: [
+      {
+        type: SearchSERPTelemetryUtils.COMPONENTS.AD_LINK,
+        default: true,
+      },
+    ],
   },
   {
     telemetryId: "example2",
@@ -42,6 +49,12 @@ const TEST_PROVIDER_INFO = [
     organicCodes: ["foo"],
     followOnParamNames: ["a"],
     extraAdServersRegexps: [/^https:\/\/www\.example\.com\/ad2/],
+    components: [
+      {
+        type: SearchSERPTelemetryUtils.COMPONENTS.AD_LINK,
+        default: true,
+      },
+    ],
   },
 ];