Bug 1970720 - Change tabId to browsingContext.topWindowContext.innerWindowId, and make general improvements a=dmeehan DONTBUILD

This patch unfortunately got mixed up with other stuff, so here's a list of changes this patch does.

- Fixing the tab id issue
- Ignoring invisible captchas for hCaptcha and Google reCAPTCHA
- Fixing double counting issue with multiple iframe captchas (again, hCaptcha and Google reCAPTCHA)
- Removes arkoselabs_ps(_pbm). We never recorded it, and `arkoselabs_ps === arkoselabs_oc`, so it is safe to remove it.

Original Revision: https://phabricator.services.mozilla.com/D252749

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

toolkit/components/captchadetection/CaptchaDetectionChild.sys.mjs

@@ -21,20 +21,20 @@ class CaptchaHandler {
   /**
    * @param {CaptchaDetectionChild} actor - The window actor.
    * @param {Event} _event - The initial event that created the actor.
+   * @param {boolean} skipConstructedNotif - Whether to skip the constructed notification. Used for captchas with multiple frames.
    */
-  constructor(actor, _event) {
+  constructor(actor, _event, skipConstructedNotif = false) {
     /** @type {CaptchaDetectionChild} */
     this.actor = actor;
-    this.tabId = this.actor.docShell.browserChild.tabId;
+    if (!skipConstructedNotif) {
-    this.isPBM = this.actor.browsingContext.usePrivateBrowsing;
+      this.notifyConstructed();
-    this.notifyConstructed();
+    }
   }
 
   notifyConstructed() {
     lazy.console.debug(`CaptchaHandler constructed: ${this.constructor.type}`);
     this.actor.sendAsyncMessage("CaptchaHandler:Constructed", {
       type: this.constructor.type,
-      isPBM: this.isPBM,
     });
   }
 
@@ -43,11 +43,7 @@ class CaptchaHandler {
   }
 
   updateState(state) {
-    this.actor.sendAsyncMessage("CaptchaState:Update", {
+    this.actor.sendAsyncMessage("CaptchaState:Update", state);
-      tabId: this.tabId,
-      isPBM: this.isPBM,
-      state,
-    });
   }
 
   onActorDestroy() {
@@ -87,7 +83,15 @@ class GoogleRecaptchaV2Handler extends CaptchaHandler {
   static type = "g-recaptcha-v2";
 
   constructor(actor, event) {
-    super(actor, event);
+    super(
+      actor,
+      event,
+      actor.document.location.pathname.endsWith("/bframe") ||
+        (Cu.isInAutomation &&
+          actor.document.location.pathname.endsWith(
+            "g_recaptcha_v2_checkbox.html"
+          ))
+    );
     this.#enabled = true;
     this.#mutationObserver = new this.actor.contentWindow.MutationObserver(
       this.#mutationHandler.bind(this)
@@ -109,10 +113,13 @@ class GoogleRecaptchaV2Handler extends CaptchaHandler {
       );
     }
 
-    return [
+    return (
-      "https://www.google.com/recaptcha/api2/",
+      [
-      "https://www.google.com/recaptcha/enterprise/",
+        "https://www.google.com/recaptcha/api2/",
-    ].some(match => document.location.href.startsWith(match));
+        "https://www.google.com/recaptcha/enterprise/",
+      ].some(match => document.location.href.startsWith(match)) &&
+      !document.location.search.includes("size=invisible")
+    );
   }
 
   #mutationHandler(_mutations, observer) {
@@ -327,17 +334,19 @@ class HCaptchaHandler extends CaptchaHandler {
   static type = "hCaptcha";
 
   constructor(actor, event) {
-    super(actor, event);
-
     let params = null;
     try {
-      params = new URLSearchParams(this.actor.document.location.hash.slice(1));
+      params = new URLSearchParams(actor.document.location.hash.slice(1));
     } catch {
       // invalid URL
+      super(actor, event, true);
       return;
     }
 
     const frameType = params.get("frame");
+
+    super(actor, event, frameType === "challenge");
+
     if (frameType === "challenge") {
       this.#initChallengeHandler();
     } else if (frameType === "checkbox") {
@@ -357,7 +366,10 @@ class HCaptchaHandler extends CaptchaHandler {
     return (
       document.location.href.startsWith(
         "https://newassets.hcaptcha.com/captcha/v1/"
-      ) && document.location.pathname.endsWith("/static/hcaptcha.html")
+      ) &&
+      document.location.pathname.endsWith("/static/hcaptcha.html") &&
+      !document.location.hash.includes("size=invisible") &&
+      !document.location.hash.includes("frame=checkbox-invisible")
     );
   }
 
@@ -507,9 +519,7 @@ export class CaptchaDetectionChild extends JSWindowActorChild {
     }
 
     if (event.type === "pagehide") {
-      this.sendAsyncMessage("TabState:Closed", {
+      this.sendAsyncMessage("Page:Hide");
-        tabId: this.docShell.browserChild.tabId,
-      });
     }
 
     this.handler?.handleEvent(event);

toolkit/components/captchadetection/CaptchaDetectionCommunicationChild.sys.mjs

@@ -18,7 +18,6 @@ ChromeUtils.defineLazyGetter(lazy, "console", () => {
 export class CaptchaDetectionCommunicationChild extends JSWindowActorChild {
   actorCreated() {
     lazy.console.debug("actorCreated()");
-    this.tabId = this.docShell.browserChild.tabId;
     this.addedMessageListener = false;
   }
 
@@ -52,23 +51,15 @@ export class CaptchaDetectionCommunicationChild extends JSWindowActorChild {
 
       if (data.eventType === "load" && data.hasOwnProperty("responseType")) {
         this.sendAsyncMessage("CaptchaState:Update", {
-          tabId: this.tabId,
+          type: "datadome",
-          isPBM: this.browsingContext.usePrivateBrowsing,
+          event: "load",
-          state: {
+          captchaShown: data.responseType === "captcha",
-            type: "datadome",
+          blocked: data.responseType === "hardblock",
-            event: "load",
-            captchaShown: data.responseType === "captcha",
-            blocked: data.responseType === "hardblock",
-          },
         });
       } else if (data.eventType === "passed") {
         this.sendAsyncMessage("CaptchaState:Update", {
-          tabId: this.tabId,
+          type: "datadome",
-          isPBM: this.browsingContext.usePrivateBrowsing,
+          event: "passed",
-          state: {
-            type: "datadome",
-            event: "passed",
-          },
         });
       }
     });

toolkit/components/captchadetection/CaptchaDetectionParent.sys.mjs

@@ -20,11 +20,12 @@ ChromeUtils.defineESModuleGetters(lazy, {
 });
 
 /**
- * Holds the state of each tab.
+ * Holds the state of captchas for each top document.
+ * Currently, only used by google reCAPTCHA v2 and hCaptcha.
  * The state is an object with the following structure:
- * [key: tabId]: typeof ReturnType<TabState.#defaultValue()>
+ * [key: topBrowsingContextId]: typeof ReturnType<TopDocState.#defaultValue()>
  */
-class TabState {
+class DocCaptchaState {
   #state;
 
   constructor() {
@@ -32,11 +33,11 @@ class TabState {
   }
 
   /**
-   * @param {number} tabId - The tab id.
+   * @param {number} topId - The top bc id.
-   * @returns {Map<any, any>} - The state of the tab.
+   * @returns {Map<any, any>} - The state of the top bc.
    */
-  get(tabId) {
+  get(topId) {
-    return this.#state.get(tabId);
+    return this.#state.get(topId);
   }
 
   static #defaultValue() {
@@ -44,25 +45,25 @@ class TabState {
   }
 
   /**
-   * @param {number} tabId - The tab id.
+   * @param {number} topId - The top bc id.
-   * @param {(state: ReturnType<TabState['get']>) => void} updateFunction - The function to update the state.
+   * @param {(state: ReturnType<DocCaptchaState['get']>) => void} updateFunction - The function to update the state.
    */
-  update(tabId, updateFunction) {
+  update(topId, updateFunction) {
-    if (!this.#state.has(tabId)) {
+    if (!this.#state.has(topId)) {
-      this.#state.set(tabId, TabState.#defaultValue());
+      this.#state.set(topId, DocCaptchaState.#defaultValue());
     }
-    updateFunction(this.#state.get(tabId));
+    updateFunction(this.#state.get(topId));
   }
 
   /**
-   * @param {number} tabId - The tab id.
+   * @param {number} topId - The top doc id.
    */
-  clear(tabId) {
+  clear(topId) {
-    this.#state.delete(tabId);
+    this.#state.delete(topId);
   }
 }
 
-const tabState = new TabState();
+const docState = new DocCaptchaState();
 
 /**
  * This actor parent is responsible for recording the state of captchas
@@ -78,17 +79,18 @@ class CaptchaDetectionParent extends JSWindowActorParent {
   actorDestroy() {
     lazy.console.debug("actorDestroy()");
 
-    if (this.#responseObserver) {
+    this.#onPageHidden();
-      this.#responseObserver.unregister();
-    }
   }
 
   /** @type {CaptchaStateUpdateFunction} */
-  #updateGRecaptchaV2State({ tabId, isPBM, state: { type, changes } }) {
+  #updateGRecaptchaV2State({ changes, type }) {
     lazy.console.debug("updateGRecaptchaV2State", changes);
 
+    const topId = this.#topInnerWindowId;
+    const isPBM = this.browsingContext.usePrivateBrowsing;
+
     if (changes === "ImagesShown") {
-      tabState.update(tabId, state => {
+      docState.update(topId, state => {
         state.set(type + changes, true);
       });
 
@@ -99,28 +101,29 @@ class CaptchaDetectionParent extends JSWindowActorParent {
       const shownMetric = "googleRecaptchaV2Ps" + (isPBM ? "Pbm" : "");
       Glean.captchaDetection[shownMetric].add(1);
     } else if (changes === "GotCheckmark") {
-      const autoCompleted = !tabState.get(tabId)?.has(type + "ImagesShown");
+      const autoCompleted = !docState.get(topId)?.has(type + "ImagesShown");
-      lazy.console.debug(
-        "GotCheckmark" +
-          (autoCompleted ? " (auto-completed)" : " (manually-completed)")
-      );
       const resultMetric =
         "googleRecaptchaV2" +
         (autoCompleted ? "Ac" : "Pc") +
         (isPBM ? "Pbm" : "");
       Glean.captchaDetection[resultMetric].add(1);
+      lazy.console.debug("Incremented metric", resultMetric);
+      docState.clear(topId);
       this.#onMetricSet();
     }
   }
 
   /** @type {CaptchaStateUpdateFunction} */
-  #recordCFTurnstileResult({ isPBM, state: { result } }) {
+  #recordCFTurnstileResult({ result }) {
     lazy.console.debug("recordCFTurnstileResult", result);
+
+    const isPBM = this.browsingContext.usePrivateBrowsing;
     const resultMetric =
       "cloudflareTurnstile" +
       (result === "Succeeded" ? "Cc" : "Cf") +
       (isPBM ? "Pbm" : "");
     Glean.captchaDetection[resultMetric].add(1);
+    lazy.console.debug("Incremented metric", resultMetric);
     this.#onMetricSet();
   }
 
@@ -147,30 +150,42 @@ class CaptchaDetectionParent extends JSWindowActorParent {
   }
 
   /** @type {CaptchaStateUpdateFunction} */
-  #recordDatadomeEvent({ isPBM, state: { event, ...payload } }) {
+  #recordDatadomeEvent({ event, ...payload }) {
-    lazy.console.debug("recordDatadomeEvent", event, payload);
+    lazy.console.debug("recordDatadomeEvent", { event, payload });
-    const suffix = isPBM ? "Pbm" : "";
+
+    const suffix = this.browsingContext.usePrivateBrowsing ? "Pbm" : "";
+    let metricName = "datadome";
     if (event === "load") {
       if (payload.captchaShown) {
-        Glean.captchaDetection["datadomePs" + suffix].add(1);
+        metricName += "Ps";
       } else if (payload.blocked) {
-        Glean.captchaDetection["datadomeBl" + suffix].add(1);
+        metricName += "Bl";
       }
     } else if (event === "passed") {
-      Glean.captchaDetection["datadomePc" + suffix].add(1);
+      metricName += "Pc";
+    } else {
+      lazy.console.error("Unknown Datadome event", event);
+      return;
     }
 
+    metricName += suffix;
+    Glean.captchaDetection[metricName].add(1);
+    lazy.console.debug("Incremented metric", metricName);
+
     this.#onMetricSet(0);
   }
 
   /** @type {CaptchaStateUpdateFunction} */
-  #recordHCaptchaState({ isPBM, tabId, state: { type, changes } }) {
+  #recordHCaptchaState({ changes, type }) {
     lazy.console.debug("recordHCaptchaEvent", changes);
 
+    const topId = this.#topInnerWindowId;
+    const isPBM = this.browsingContext.usePrivateBrowsing;
+
     if (changes === "shown") {
       // I don't think HCaptcha supports auto-completion, but we act
       // as if it does just in case.
-      tabState.update(tabId, state => {
+      docState.update(topId, state => {
         state.set(type + changes, true);
       });
 
@@ -180,38 +195,40 @@ class CaptchaDetectionParent extends JSWindowActorParent {
       // received, or when the daily maybeSubmitPing is called.
       const shownMetric = "hcaptchaPs" + (isPBM ? "Pbm" : "");
       Glean.captchaDetection[shownMetric].add(1);
+      lazy.console.debug("Incremented metric", shownMetric);
     } else if (changes === "passed") {
-      const autoCompleted = !tabState.get(tabId)?.has(type + "shown");
+      const autoCompleted = !docState.get(topId)?.has(type + "shown");
       const resultMetric =
         "hcaptcha" + (autoCompleted ? "Ac" : "Pc") + (isPBM ? "Pbm" : "");
       Glean.captchaDetection[resultMetric].add(1);
+      lazy.console.debug("Incremented metric", resultMetric);
+      docState.clear(topId);
       this.#onMetricSet();
     }
   }
 
   /** @type {CaptchaStateUpdateFunction} */
-  #recordArkoseLabsEvent({
+  #recordArkoseLabsEvent({ event, solved, solutionsSubmitted }) {
-    isPBM,
+    lazy.console.debug("recordArkoseLabsEvent", {
-    state: { event, solved, solutionsSubmitted },
+      event,
-  }) {
+      solved,
-    if (event === "shown") {
+      solutionsSubmitted,
-      // We don't call maybeSubmitPing here because we might end up
+    });
-      // submitting the ping without the "completed" event.
-      // maybeSubmitPing will be called when "completed" event is
-      // received, or when the daily maybeSubmitPing is called.
-      const shownMetric = "arkoselabsPs" + (isPBM ? "Pbm" : "");
-      Glean.captchaDetection[shownMetric].add(1);
-    } else if (event === "completed") {
-      const suffix = isPBM ? "Pbm" : "";
-      const resultMetric = "arkoselabs" + (solved ? "Pc" : "Pf") + suffix;
-      Glean.captchaDetection[resultMetric].add(1);
 
-      const solutionsRequiredMetric =
+    const isPBM = this.browsingContext.usePrivateBrowsing;
-        Glean.captchaDetection["arkoselabsSolutionsRequired" + suffix];
-      solutionsRequiredMetric.accumulateSingleSample(solutionsSubmitted);
 
-      this.#onMetricSet();
+    const suffix = isPBM ? "Pbm" : "";
-    }
+    const resultMetric = "arkoselabs" + (solved ? "Pc" : "Pf") + suffix;
+    Glean.captchaDetection[resultMetric].add(1);
+    lazy.console.debug("Incremented metric", resultMetric);
+
+    const metricName = "arkoselabsSolutionsRequired" + suffix;
+    Glean.captchaDetection[metricName].accumulateSingleSample(
+      solutionsSubmitted
+    );
+    lazy.console.debug("Sampled", metricName, "with", solutionsSubmitted);
+
+    this.#onMetricSet();
   }
 
   async #arkoseLabsInit() {
@@ -259,17 +276,14 @@ class CaptchaDetectionParent extends JSWindowActorParent {
         }
 
         solutionsSubmitted++;
-        if (body.solved === null) {
+        if (typeof body.solved !== "boolean") {
           return;
         }
 
         this.#recordArkoseLabsEvent({
-          isPBM: this.browsingContext.usePrivateBrowsing,
+          event: "completed",
-          state: {
+          solved: body.solved,
-            event: "completed",
+          solutionsSubmitted,
-            solved: body.solved,
-            solutionsSubmitted,
-          },
         });
 
         solutionsSubmitted = 0;
@@ -278,8 +292,12 @@ class CaptchaDetectionParent extends JSWindowActorParent {
     this.#responseObserver.register();
   }
 
-  #onTabClosed(tabId) {
+  get #topInnerWindowId() {
-    tabState.clear(tabId);
+    return this.browsingContext.topWindowContext.innerWindowId;
+  }
+
+  #onPageHidden() {
+    docState.clear(this.#topInnerWindowId);
 
     if (this.#responseObserver) {
       this.#responseObserver.unregister();
@@ -330,7 +348,9 @@ class CaptchaDetectionParent extends JSWindowActorParent {
     await actor.sendQuery("Testing:MetricIsSet");
   }
 
-  recordCaptchaHandlerConstructed({ isPBM, type }) {
+  recordCaptchaHandlerConstructed({ type }) {
+    lazy.console.debug("recordCaptchaHandlerConstructed", type);
+
     let metric = "";
     switch (type) {
       case "g-recaptcha-v2":
@@ -349,8 +369,9 @@ class CaptchaDetectionParent extends JSWindowActorParent {
         metric = "arkoselabsOc";
         break;
     }
-    metric += isPBM ? "Pbm" : "";
+    metric += this.browsingContext.usePrivateBrowsing ? "Pbm" : "";
     Glean.captchaDetection[metric].add(1);
+    lazy.console.debug("Incremented metric", metric);
   }
 
   async receiveMessage(message) {
@@ -358,7 +379,7 @@ class CaptchaDetectionParent extends JSWindowActorParent {
 
     switch (message.name) {
       case "CaptchaState:Update":
-        switch (message.data.state.type) {
+        switch (message.data.type) {
           case "g-recaptcha-v2":
             this.#updateGRecaptchaV2State(message.data);
             break;
@@ -376,17 +397,14 @@ class CaptchaDetectionParent extends JSWindowActorParent {
       case "CaptchaHandler:Constructed":
         // message.name === "CaptchaHandler:Constructed"
         // => message.data = {
-        //   isPBM: bool,
         //   type: string,
         // }
         this.recordCaptchaHandlerConstructed(message.data);
         break;
-      case "TabState:Closed":
+      case "Page:Hide":
         // message.name === "TabState:Closed"
-        // => message.data = {
+        // => message.data = undefined
-        //   tabId: number,
+        this.#onPageHidden();
-        // }
-        this.#onTabClosed(message.data.tabId);
         break;
       case "CaptchaDetection:Init":
         // message.name === "CaptchaDetection:Init"
@@ -422,10 +440,8 @@ export {
 
 /**
  * @typedef CaptchaStateUpdateMessageData
- * @property {number} tabId - The tab id.
+ * @type {object}
- * @property {boolean} isPBM - Whether the tab is in PBM.
+ * @property {string} type - The type of the captcha.
- * @property {object} state - The state of the captcha.
- * @property {string} state.type - The type of the captcha.
  *
  * @typedef {(message: CaptchaStateUpdateMessageData) => void} CaptchaStateUpdateFunction
  */

toolkit/components/captchadetection/metrics.yaml

@@ -279,22 +279,6 @@ captcha_detection:
     data_sensitivity:
       - interaction
 
-  arkoselabs_ps:
-    type: counter
-    description: >
-      How many times the ArkoseLabs challenge was shown.
-    bugs:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1913421
-    data_reviews:
-      - https://phabricator.services.mozilla.com/D226021#7994299
-    notification_emails:
-      - tritter@mozilla.com
-    send_in_pings:
-      - captcha-detection
-    expires: never
-    data_sensitivity:
-      - interaction
-
   arkoselabs_pc:
     type: counter
     description: >
@@ -588,22 +572,6 @@ captcha_detection:
     data_sensitivity:
       - interaction
 
-  arkoselabs_ps_pbm:
-    type: counter
-    description: >
-      How many times the ArkoseLabs challenge was shown.
-    bugs:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1913421
-    data_reviews:
-      - https://phabricator.services.mozilla.com/D226021#7994299
-    notification_emails:
-      - tritter@mozilla.com
-    send_in_pings:
-      - captcha-detection
-    expires: never
-    data_sensitivity:
-      - interaction
-
   arkoselabs_pc_pbm:
     type: counter
     description: >

toolkit/components/captchadetection/tests/mochitest/test_g_recaptcha_v2_ac.html

@@ -40,7 +40,7 @@
 
         const occurredCount =
           await GleanTest.captchaDetection.googleRecaptchaV2Oc.testGetValue();
-        is(occurredCount, 2, "We should have detected the occurrence");
+        is(occurredCount, 1, "We should have detected the occurrence");
 
         await CaptchaTestingUtils.clearPrefs();
       });

toolkit/components/captchadetection/tests/mochitest/test_g_recaptcha_v2_pc.html

@@ -46,7 +46,7 @@
 
         const occurredCount =
           await GleanTest.captchaDetection.googleRecaptchaV2Oc.testGetValue();
-        is(occurredCount, 2, "We should have detected the occurrence");
+        is(occurredCount, 1, "We should have detected the occurrence");
 
         await CaptchaTestingUtils.clearPrefs();
       });

toolkit/components/captchadetection/tests/mochitest/test_hcaptcha_ac.html

@@ -40,7 +40,7 @@
 
         const occurredCount =
           await GleanTest.captchaDetection.hcaptchaOc.testGetValue();
-        is(occurredCount, 2, "We should have detected the occurrence");
+        is(occurredCount, 1, "We should have detected the occurrence");
 
         await CaptchaTestingUtils.clearPrefs();
       });

toolkit/components/captchadetection/tests/mochitest/test_hcaptcha_pc.html

@@ -44,7 +44,7 @@
 
         const occurredCount =
           await GleanTest.captchaDetection.hcaptchaOc.testGetValue();
-        is(occurredCount, 2, "We should have detected the occurrence");
+        is(occurredCount, 1, "We should have detected the occurrence");
 
         await CaptchaTestingUtils.clearPrefs();
       });