Bug 1880410 - Clean up parts of search telemetry - r=scunnane

Largely non-functional changes:
- The pagehide callback can live in the top level pagehide event.
- Moving the submitted check into the callback will make it easier to refactor
  the helper that adds event listeners.
- Changing the callbacks to use target as the name will make it easier to
  remember that the target is potentially overwritable.
- waitForPageWithAction can be used in future tests.

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

browser/actors/SearchSERPTelemetryChild.sys.mjs

@@ -834,11 +834,8 @@ class SearchAdImpression {
 
     for (let element of elements) {
       let clickCallback = () => {
-        if (clickAction == "submitted") {
-          documentToSubmitMap.set(document, true);
-        }
         callback({
-          type,
+          target: type,
           url,
           action: clickAction,
         });
@@ -847,11 +844,8 @@ class SearchAdImpression {
 
       let keydownCallback = event => {
         if (event.key == "Enter") {
-          if (keydownEnterAction == "submitted") {
-            documentToSubmitMap.set(document, true);
-          }
           callback({
-            type,
+            target: type,
             url,
             action: keydownEnterAction,
           });
@@ -865,20 +859,6 @@ class SearchAdImpression {
       });
     }
 
-    document.ownerGlobal.addEventListener(
-      "pagehide",
-      () => {
-        let callbacks = documentToRemoveEventListenersMap.get(document);
-        if (callbacks) {
-          for (let removeEventListenerCallback of callbacks) {
-            removeEventListenerCallback();
-          }
-          documentToRemoveEventListenersMap.delete(document);
-        }
-      },
-      { once: true }
-    );
-
     // The map might have entries from previous callers, so we must ensure
     // we don't discard existing event listener callbacks.
     if (documentToRemoveEventListenersMap.has(document)) {
@@ -1150,8 +1130,11 @@ export class SearchSERPTelemetryChild extends JSWindowActorChild {
       let timerId = Glean.serp.categorizationDuration.start();
 
       let pageActionCallback = info => {
+        if (info.action == "submitted") {
+          documentToSubmitMap.set(doc, true);
+        }
         this.sendAsyncMessage("SearchTelemetry:Action", {
-          type: info.type,
+          target: info.target,
           url: info.url,
           action: info.action,
         });
@@ -1288,6 +1271,13 @@ export class SearchSERPTelemetryChild extends JSWindowActorChild {
         break;
       }
       case "pagehide": {
+        let callbacks = documentToRemoveEventListenersMap.get(this.document);
+        if (callbacks) {
+          for (let removeEventListenerCallback of callbacks) {
+            removeEventListenerCallback();
+          }
+          documentToRemoveEventListenersMap.delete(this.document);
+        }
         this.#cancelCheck();
         break;
       }

browser/components/search/SearchSERPTelemetry.sys.mjs

@@ -1630,8 +1630,8 @@ class ContentHandler {
    *
    * @param {object} info
    *   The search provider infomation for the page.
-   * @param {string} info.type
-   *   The component type that was clicked on.
+   * @param {string} info.target
+   *   The target component that was interacted with.
    * @param {string} info.action
    *   The action taken on the page.
    * @param {object} browser
@@ -1644,22 +1644,23 @@ class ContentHandler {
     }
     let telemetryState = item.browserTelemetryStateMap.get(browser);
     let impressionId = telemetryState?.impressionId;
-    if (info.type && impressionId) {
+    if (info.target && impressionId) {
       lazy.logConsole.debug(`Recorded page action:`, {
         impressionId: telemetryState.impressionId,
-        type: info.type,
+        target: info.target,
         action: info.action,
       });
       Glean.serp.engagement.record({
         impression_id: impressionId,
         action: info.action,
-        target: info.type,
+        target: info.target,
       });
       impressionIdsWithoutEngagementsSet.delete(impressionId);
       // In-content searches are not be categorized with a type, so they will
       // not be picked up in the network processes.
       if (
-        info.type == SearchSERPTelemetryUtils.COMPONENTS.INCONTENT_SEARCHBOX &&
+        info.target ==
+          SearchSERPTelemetryUtils.COMPONENTS.INCONTENT_SEARCHBOX &&
         info.action == SearchSERPTelemetryUtils.ACTIONS.SUBMITTED
       ) {
         telemetryState.searchBoxSubmitted = true;
@@ -1668,6 +1669,7 @@ class ContentHandler {
           SearchSERPTelemetryUtils.INCONTENT_SOURCES.SEARCHBOX
         );
       }
+      Services.obs.notifyObservers(null, "reported-page-with-action");
     } else {
       lazy.logConsole.warn(
         "Expected to report a",

browser/components/search/test/browser/telemetry/head.js

@@ -451,6 +451,10 @@ function assertCategorizationValues(expectedResults) {
   }
 }
 
+function waitForPageWithAction() {
+  return TestUtils.topicObserved("reported-page-with-action");
+}
+
 function waitForPageWithAdImpressions() {
   return TestUtils.topicObserved("reported-page-with-ad-impressions");
 }