Bug 1834554 - Update PiP telemetry. r=pip-reviewers,kpatenio

Depends on D175048 Differential Revision: https://phabricator.services.mozilla.com/D178864
2024-12-03 18:47:53 +00:00 · 2023-05-30 22:07:26 +00:00 · 2023-05-30 22:07:26 +00:00 · 1c305eb10b
commit 1c305eb10b
parent cc6b936914
8 changed files with 133 additions and 45 deletions
--- a/browser/actors/ContextMenuChild.sys.mjs
+++ b/browser/actors/ContextMenuChild.sys.mjs
@ -119,18 +119,6 @@ export class ContextMenuChild extends JSWindowActorChild {
                    "contextmenu",
                    1
                  );
-                  let args = {
-                    firstTimeToggle: (!Services.prefs.getBoolPref(
-                      "media.videocontrols.picture-in-picture.video-toggle.has-used"
-                    )).toString(),
-                  };
-                  Services.telemetry.recordEvent(
-                    "pictureinpicture",
-                    "opened_method",
-                    "contextMenu",
-                    null,
-                    args
-                  );
                }
                let event = new this.contentWindow.CustomEvent(
                  "MozTogglePictureInPicture",
--- a/toolkit/actors/PictureInPictureChild.sys.mjs
+++ b/toolkit/actors/PictureInPictureChild.sys.mjs
@ -138,7 +138,7 @@ export class PictureInPictureLauncherChild extends JSWindowActorChild {
        if (event.isTrusted) {
          this.togglePictureInPicture({
            video: event.target,
-            reason: event.detail,
+            reason: event.detail?.reason,
          });
        }
        break;
@ -179,7 +179,7 @@ export class PictureInPictureLauncherChild extends JSWindowActorChild {
        "MozStopPictureInPicture",
        {
          bubbles: true,
-          detail: reason,
+          detail: { reason },
        }
      );
      video.dispatchEvent(stopPipEvent);
@ -225,6 +225,20 @@ export class PictureInPictureLauncherChild extends JSWindowActorChild {
      scrubberPosition,
      timestamp,
    });
+
+    let args = {
+      firstTimeToggle: (!Services.prefs.getBoolPref(
+        TOGGLE_HAS_USED_PREF
+      )).toString(),
+    };
+
+    Services.telemetry.recordEvent(
+      "pictureinpicture",
+      "opened_method",
+      reason,
+      null,
+      args
+    );
  }

  /**
@ -249,7 +263,7 @@ export class PictureInPictureLauncherChild extends JSWindowActorChild {
          listOfVideos.sort((a, b) => b.duration - a.duration)[0];
      }
      if (video) {
-        this.togglePictureInPicture({ video });
+        this.togglePictureInPicture({ video, reason: "shortcut" });
      }
    }
  }
@ -652,20 +666,6 @@ export class PictureInPictureToggleChild extends JSWindowActorChild {
      this.eligiblePipVideos
    )[0];
    if (video) {
-      if (!video.isCloningElementVisually) {
-        let args = {
-          firstTimeToggle: (!Services.prefs.getBoolPref(
-            "media.videocontrols.picture-in-picture.video-toggle.has-used"
-          )).toString(),
-        };
-        Services.telemetry.recordEvent(
-          "pictureinpicture",
-          "opened_method",
-          "urlBar",
-          null,
-          args
-        );
-      }
      let pipEvent = new this.contentWindow.CustomEvent(
        "MozTogglePictureInPicture",
        {
@ -1058,23 +1058,12 @@ export class PictureInPictureToggleChild extends JSWindowActorChild {
      "toggle",
      1
    );
-    let args = {
-      firstTimeToggle: (!Services.prefs.getBoolPref(
-        TOGGLE_HAS_USED_PREF
-      )).toString(),
-    };
-    Services.telemetry.recordEvent(
-      "pictureinpicture",
-      "opened_method",
-      "toggle",
-      null,
-      args
-    );

    let pipEvent = new this.contentWindow.CustomEvent(
      "MozTogglePictureInPicture",
      {
        bubbles: true,
+        detail: { reason: "toggle" },
      }
    );
    video.dispatchEvent(pipEvent);
--- a/toolkit/components/pictureinpicture/PictureInPicture.sys.mjs
+++ b/toolkit/components/pictureinpicture/PictureInPicture.sys.mjs
@ -450,6 +450,12 @@ export var PictureInPicture = {
      "media.videocontrols.picture-in-picture.respect-disablePictureInPicture",
      respectPipDisabled
    );
+
+    Services.telemetry.recordEvent(
+      "pictureinpicture",
+      "disrespect_disable",
+      "urlBar"
+    );
  },

  /**
@ -632,6 +638,13 @@ export var PictureInPicture = {
      );

      pipPanel.openPopup(anchor, "bottomright topright");
+      Services.telemetry.recordEvent(
+        "pictureinpicture",
+        "opened_method",
+        "urlBar",
+        null,
+        { disableDialog: "true" }
+      );
    } else {
      pipPanel.hidePopup();
    }
--- a/toolkit/components/pictureinpicture/content/player.js
+++ b/toolkit/components/pictureinpicture/content/player.js
@ -1062,7 +1062,7 @@ let Player = {
   *  Event context data object
   */
  onCommand(event) {
-    this.closePipWindow({ reason: "player-shortcut" });
+    this.closePipWindow({ reason: "shortcut" });
  },

  get controls() {
--- a/toolkit/components/pictureinpicture/tests/browser_keyboardShortcut.js
+++ b/toolkit/components/pictureinpicture/tests/browser_keyboardShortcut.js
@ -3,6 +3,26 @@

 "use strict";

+const { TelemetryTestUtils } = ChromeUtils.importESModule(
+  "resource://testing-common/TelemetryTestUtils.sys.mjs"
+);
+
+const PIP_SHORTCUT_OPEN_EVENTS = [
+  {
+    category: "pictureinpicture",
+    method: "opened_method",
+    object: "shortcut",
+  },
+];
+
+const PIP_SHORTCUT_CLOSE_EVENTS = [
+  {
+    category: "pictureinpicture",
+    method: "closed_method",
+    object: "shortcut",
+  },
+];
+
 /**
 * Tests that if the user keys in the keyboard shortcut for
 * Picture-in-Picture, then the first video on the currently
@ -15,6 +35,7 @@ add_task(async function test_pip_keyboard_shortcut() {
      gBrowser,
    },
    async browser => {
+      Services.telemetry.clearEvents();
      await ensureVideosReady(browser);

      // In test-page.html, the "with-controls" video is the first one that
@ -51,6 +72,21 @@ add_task(async function test_pip_keyboard_shortcut() {

      await ensureMessageAndClosePiP(browser, VIDEO_ID, pipWin, false);

+      let openFilter = {
+        category: "pictureinpicture",
+        method: "opened_method",
+        object: "shortcut",
+      };
+      await waitForTelemeryEvents(
+        openFilter,
+        PIP_SHORTCUT_OPEN_EVENTS.length,
+        "content"
+      );
+      TelemetryTestUtils.assertEvents(PIP_SHORTCUT_OPEN_EVENTS, openFilter, {
+        clear: true,
+        process: "content",
+      });
+
      // Reopen PiP Window
      pipWin = await triggerPictureInPicture(browser, VIDEO_ID);
      await videoReady;
@ -78,6 +114,21 @@ add_task(async function test_pip_keyboard_shortcut() {
      await BrowserTestUtils.windowClosed(pipWin);

      ok(pipWin.closed, "Picture-in-Picture window closed.");
+
+      let closeFilter = {
+        category: "pictureinpicture",
+        method: "closed_method",
+        object: "shortcut",
+      };
+      await waitForTelemeryEvents(
+        closeFilter,
+        PIP_SHORTCUT_CLOSE_EVENTS.length,
+        "parent"
+      );
+      TelemetryTestUtils.assertEvents(PIP_SHORTCUT_CLOSE_EVENTS, closeFilter, {
+        clear: true,
+        process: "parent",
+      });
    }
  );
 });
--- a/toolkit/components/pictureinpicture/tests/browser_urlbar_toggle.js
+++ b/toolkit/components/pictureinpicture/tests/browser_urlbar_toggle.js
@ -15,6 +15,20 @@ const PIP_URLBAR_EVENTS = [
  },
 ];

+const PIP_DISABLED_EVENTS = [
+  {
+    category: "pictureinpicture",
+    method: "opened_method",
+    object: "urlBar",
+    extra: { disableDialog: "true" },
+  },
+  {
+    category: "pictureinpicture",
+    method: "disrespect_disable",
+    object: "urlBar",
+  },
+];
+
 add_task(async function test_urlbar_toggle_multiple_contexts() {
  await BrowserTestUtils.withNewTab(
    {
@ -199,6 +213,7 @@ add_task(async function test_pipDisabled() {
      gBrowser,
    },
    async browser => {
+      Services.telemetry.clearEvents();
      await SpecialPowers.pushPrefEnv({
        set: [
          [
@ -262,6 +277,16 @@ add_task(async function test_pipDisabled() {
        return BrowserTestUtils.is_hidden(panel);
      });

+      let filter = {
+        category: "pictureinpicture",
+        object: "urlBar",
+      };
+      await waitForTelemeryEvents(filter, PIP_DISABLED_EVENTS.length, "parent");
+      TelemetryTestUtils.assertEvents(PIP_DISABLED_EVENTS, filter, {
+        clear: true,
+        process: "parent",
+      });
+
      // Confirm that the toggle is now visible because we no longer respect disablePictureInPicture
      await testToggleHelper(browser, VIDEO_ID, true);

--- a/toolkit/components/pictureinpicture/tests/head.js
+++ b/toolkit/components/pictureinpicture/tests/head.js
@ -1099,7 +1099,11 @@ async function waitForTelemeryEvents(filter, length, process) {
      info(JSON.stringify(filtered, null, 2));
      return filtered && filtered.length >= length;
    },
-    "Waiting for one create pictureinpicture telemetry event.",
+    `Waiting for ${length} pictureinpicture telemetry event(s) with filter ${JSON.stringify(
+      filter,
+      null,
+      2
+    )}`,
    200,
    100
  );
--- a/toolkit/components/telemetry/Events.yaml
+++ b/toolkit/components/telemetry/Events.yaml
@ -2906,11 +2906,12 @@ pictureinpicture:
    expiry_version: "never"
    release_channel_collection: opt-out
  opened_method:
-    objects: ["toggle", "contextMenu", "urlBar"]
+    objects: ["toggle", "contextMenu", "urlBar", "shortcut"]
    description: >
      Records the method for opening the Picture-in-Picture window.
    extra_keys:
      firstTimeToggle: If the user has used Picture-in-Picture before
+      disableDialog: True when urlbar button opens the disable PiP dialog
    notification_emails:
      - mconley@mozilla.com
    products:
@ -2918,10 +2919,11 @@ pictureinpicture:
    record_in_processes: ["main", "content"]
    bug_numbers:
      - 1749585
+      - 1834554
    expiry_version: "never"
    release_channel_collection: opt-out
  closed_method:
-    objects: ["closeButton", "unpip", "pagehide", "fullscreen", "setupFailure", "closePlayerShortcut", "contextMenu", "videoElRemove", "videoElEmptied", "urlBar"]
+    objects: ["closeButton", "unpip", "pagehide", "fullscreen", "setupFailure", "closePlayerShortcut", "contextMenu", "videoElRemove", "videoElEmptied", "urlBar", "shortcut"]
    description: >
      Records the method for closing the Picture-in-Picture window.
    notification_emails:
@ -2934,6 +2936,7 @@ pictureinpicture:
      - "main"
    bug_numbers:
      - 1756703
+      - 1834554
    expiry_version: "never"
    release_channel_collection: opt-out
  subtitles_shown:
@ -2970,6 +2973,21 @@ pictureinpicture:
      - 1822395
    expiry_version: "never"
    release_channel_collection: opt-out
+  disrespect_disable:
+    objects: ["urlBar"]
+    description: >
+      Recorded when the user chooses to enable PiP anyway on a PiP disabled video
+    notification_emails:
+      - mconley@mozilla.com
+      - mhowell@mozilla.com
+    products:
+      - "firefox"
+    record_in_processes:
+      - "main"
+    bug_numbers:
+      - 1834554
+    expiry_version: "never"
+    release_channel_collection: opt-out

 pictureinpicture.settings:
  enable: