Bug 1796395, remove now unneeded downloads.helpertype telemetry probe, r=mconley

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

toolkit/components/telemetry/Events.yaml

@@ -480,21 +480,6 @@ downloads:
     record_in_processes: ["main"]
     release_channel_collection: opt-out
     expiry_version: never
-  helpertype:
-    description: >
-      Sent when a content type is handed off the external helper app service for handling because it cannot
-      be displayed directly.
-    bug_numbers: [1736864, 1754636]
-    extra_keys:
-      reason: The value "attachment", "sniffed" or "other"
-      type: The value "pdf", "octetstream" or "other"
-    notification_emails: ["rtestard@mozilla.com"]
-    objects: ["unknowntype"]
-    products:
-      - "firefox"
-    record_in_processes: ["main"]
-    release_channel_collection: opt-out
-    expiry_version: "109"
 
 form_autocomplete:
   show:

uriloader/exthandler/nsExternalHelperAppService.cpp

@@ -1565,7 +1565,6 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest* request) {
 
   if (!mForceSave && StaticPrefs::browser_download_enable_spam_prevention() &&
       IsDownloadSpam(aChannel)) {
-    RecordDownloadTelemetry(aChannel, "spam");
     return NS_OK;
   }
 
@@ -1577,7 +1576,6 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest* request) {
     // cancel the request so no ui knows about this.
     mCanceled = true;
     request->Cancel(NS_ERROR_ABORT);
-    RecordDownloadTelemetry(aChannel, "forbidden");
     return NS_OK;
   }
 
@@ -1655,8 +1653,6 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest* request) {
 
     SendStatusChange(kWriteError, transferError, request, path);
 
-    RecordDownloadTelemetry(aChannel, "savefailed");
-
     return NS_OK;
   }
 
@@ -1836,20 +1832,6 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest* request) {
   }
 #endif
 
-  nsAutoCString actionTelem;
-  if (alwaysAsk) {
-    actionTelem.AssignLiteral("ask");
-  } else if (shouldAutomaticallyHandleInternally) {
-    actionTelem.AssignLiteral("internal");
-  } else if (action == nsIMIMEInfo::useHelperApp ||
-             action == nsIMIMEInfo::useSystemDefault) {
-    actionTelem.AssignLiteral("external");
-  } else {
-    actionTelem.AssignLiteral("save");
-  }
-
-  RecordDownloadTelemetry(aChannel, actionTelem.get());
-
   if (alwaysAsk) {
     // Display the dialog
     mDialog = do_CreateInstance(NS_HELPERAPPLAUNCHERDLG_CONTRACTID, &rv);
@@ -1882,50 +1864,6 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest* request) {
   return NS_OK;
 }
 
-void nsExternalAppHandler::RecordDownloadTelemetry(nsIChannel* aChannel,
-                                                   const char* aAction) {
-  // Telemetry for helper app dialog
-
-  if (XRE_IsContentProcess()) {
-    return;
-  }
-
-  nsAutoCString reason;
-  switch (mReason) {
-    case nsIHelperAppLauncherDialog::REASON_SERVERREQUEST:
-      reason.AssignLiteral("attachment");
-      break;
-    case nsIHelperAppLauncherDialog::REASON_TYPESNIFFED:
-      reason.AssignLiteral("sniffed");
-      break;
-    case nsIHelperAppLauncherDialog::REASON_CANTHANDLE:
-    default:
-      reason.AssignLiteral("other");
-      break;
-  }
-
-  nsAutoCString contentTypeTelem;
-  nsAutoCString contentType;
-  aChannel->GetContentType(contentType);
-  if (contentType.EqualsIgnoreCase(APPLICATION_PDF)) {
-    contentTypeTelem.AssignLiteral("pdf");
-  } else if (contentType.EqualsIgnoreCase(APPLICATION_OCTET_STREAM) ||
-             contentType.EqualsIgnoreCase(BINARY_OCTET_STREAM)) {
-    contentTypeTelem.AssignLiteral("octetstream");
-  } else {
-    contentTypeTelem.AssignLiteral("other");
-  }
-
-  CopyableTArray<mozilla::Telemetry::EventExtraEntry> extra(1);
-  extra.AppendElement(
-      mozilla::Telemetry::EventExtraEntry{"type"_ns, contentTypeTelem});
-  extra.AppendElement(mozilla::Telemetry::EventExtraEntry{"reason"_ns, reason});
-
-  mozilla::Telemetry::RecordEvent(
-      mozilla::Telemetry::EventID::Downloads_Helpertype_Unknowntype,
-      mozilla::Some(aAction), mozilla::Some(extra));
-}
-
 bool nsExternalAppHandler::IsDownloadSpam(nsIChannel* aChannel) {
   nsCOMPtr<nsILoadInfo> loadInfo = aChannel->LoadInfo();
   nsCOMPtr<nsIPermissionManager> permissionManager =

uriloader/exthandler/nsExternalHelperAppService.h

@@ -313,9 +313,6 @@ class nsExternalAppHandler final : public nsIStreamListener,
   void SetShouldCloseWindow() { mShouldCloseWindow = true; }
 
  protected:
-  // Record telemetry about a download that was attempted.
-  void RecordDownloadTelemetry(nsIChannel* aChannel, const char* aAction);
-
   bool IsDownloadSpam(nsIChannel* aChannel);
 
   ~nsExternalAppHandler();

uriloader/exthandler/tests/mochitest/browser_download_open_with_internal_handler.js

@@ -12,10 +12,6 @@ const { DownloadIntegration } = ChromeUtils.importESModule(
   "resource://gre/modules/DownloadIntegration.sys.mjs"
 );
 
-const { TelemetryTestUtils } = ChromeUtils.import(
-  "resource://testing-common/TelemetryTestUtils.jsm"
-);
-
 const TEST_PATH = getRootDirectory(gTestPath).replace(
   "chrome://mochitests/content",
   "https://example.com"
@@ -78,28 +74,6 @@ function alwaysAskForHandlingTypes(typeExtensions, ask = true) {
   return mimeInfos;
 }
 
-function checkTelemetry(desc, expectedAction, expectedType, expectedReason) {
-  let events = Services.telemetry.snapshotEvents(
-    Ci.nsITelemetry.DATASET_PRERELEASE_CHANNELS,
-    true
-  );
-  events = (events.parent || []).filter(
-    e => e[1] == "downloads" && e[2] == "helpertype"
-  );
-
-  if (expectedAction == "none") {
-    is(events.length, 0, desc + " number of events");
-    return;
-  }
-
-  is(events.length, 1, desc + " number of events");
-
-  let event = events[0];
-  is(event[4], expectedAction, desc + " telemetry action");
-  is(event[5].type, expectedType, desc + " telemetry type");
-  is(event[5].reason, expectedReason, desc + " telemetry reason");
-}
-
 add_setup(async function() {
   // Remove the security delay for the dialog during the test.
   await SpecialPowers.pushPrefEnv({
@@ -136,8 +110,6 @@ add_setup(async function() {
  * is clicked from pdf.js.
  */
 add_task(async function test_check_open_with_internal_handler() {
-  Services.telemetry.clearEvents();
-
   const mimeInfosToRestore = alwaysAskForHandlingTypes({
     "application/pdf": "pdf",
     "binary/octet-stream": "pdf",
@@ -174,13 +146,6 @@ add_task(async function test_check_open_with_internal_handler() {
 
     await waitForAcceptButtonToGetEnabled(doc);
 
-    checkTelemetry(
-      "open " + file + " internal",
-      "ask",
-      file.includes("octet") ? "octetstream" : "pdf",
-      "attachment"
-    );
-
     ok(!internalHandlerRadio.hidden, "The option should be visible for PDF");
     ok(internalHandlerRadio.selected, "The option should be selected");
 
@@ -220,13 +185,6 @@ add_task(async function test_check_open_with_internal_handler() {
     let subDialogWindow = await subdialogPromise;
     let subDoc = subDialogWindow.document;
 
-    checkTelemetry(
-      "open " + file + " internal from current tab",
-      "ask",
-      file.includes("octet") ? "octetstream" : "pdf",
-      "attachment"
-    );
-
     // Prevent racing with initialization of the dialog and make sure that
     // the final state of the dialog has the correct visibility of the internal-handler option.
     await waitForAcceptButtonToGetEnabled(subDoc);
@@ -289,8 +247,6 @@ add_task(async function test_check_open_with_internal_handler() {
  * open the PDF into pdf.js
  */
 add_task(async function test_check_open_with_external_application() {
-  Services.telemetry.clearEvents();
-
   const mimeInfosToRestore = alwaysAskForHandlingTypes({
     "application/pdf": "pdf",
     "binary/octet-stream": "pdf",
@@ -327,13 +283,6 @@ add_task(async function test_check_open_with_external_application() {
       };
     });
 
-    checkTelemetry(
-      "open " + file + " external",
-      "ask",
-      file.includes("octet") ? "octetstream" : "pdf",
-      "attachment"
-    );
-
     let doc = dialogWindow.document;
     await waitForAcceptButtonToGetEnabled(doc);
     let dialog = doc.querySelector("#unknownContentType");
@@ -543,7 +492,6 @@ add_task(
     await SpecialPowers.pushPrefEnv({
       set: [["image.webp.enabled", true]],
     });
-    Services.telemetry.clearEvents();
 
     const mimeInfosToRestore = alwaysAskForHandlingTypes({
       "binary/octet-stream": "xml",
@@ -576,13 +524,6 @@ add_task(
       // the final state of the dialog has the correct visibility of the internal-handler option.
       await waitForAcceptButtonToGetEnabled(doc);
 
-      checkTelemetry(
-        "open " + file + " for viewable internal type",
-        "ask",
-        file.endsWith(".webp") ? "other" : "octetstream",
-        "attachment"
-      );
-
       let fileDesc = file.substring(file.lastIndexOf(".") + 1);
 
       ok(
@@ -611,8 +552,6 @@ add_task(
  * for non-PDF, non-viewable-internally types.
  */
 add_task(async function test_internal_handler_hidden_with_other_type() {
-  Services.telemetry.clearEvents();
-
   const mimeInfosToRestore = alwaysAskForHandlingTypes({
     "text/plain": "txt",
   });
@@ -636,13 +575,6 @@ add_task(async function test_internal_handler_hidden_with_other_type() {
   // the final state of the dialog has the correct visibility of the internal-handler option.
   await waitForAcceptButtonToGetEnabled(doc);
 
-  checkTelemetry(
-    "open file_txt_attachment_test.txt for non-viewable internal type",
-    "ask",
-    "other",
-    "attachment"
-  );
-
   let internalHandlerRadio = doc.querySelector("#handleInternally");
   ok(
     internalHandlerRadio.hidden,
@@ -753,9 +685,6 @@ add_task(
  * This test sets the action to internal. The files should open directly without asking.
  */
 add_task(async function test_check_open_with_internal_handler_noask() {
-  Services.telemetry.clearScalars();
-  Services.telemetry.clearEvents();
-
   const mimeInfosToRestore = alwaysAskForHandlingTypes(
     {
       "application/pdf": "pdf",
@@ -901,14 +830,6 @@ add_task(async function test_check_open_with_internal_handler_noask() {
           }
         );
 
-        let action = expectDialog ? "ask" : "internal";
-        checkTelemetry(
-          "open " + file + " internal",
-          openPDFDirectly ? "none" : action,
-          file.includes("octet") ? "octetstream" : "pdf",
-          "attachment"
-        );
-
         await BrowserTestUtils.removeTab(gBrowser.selectedTab);
       }
     );

uriloader/exthandler/tests/mochitest/browser_download_spam_permissions.js

@@ -52,8 +52,6 @@ add_setup(async function() {
     Services.prefs.clearUserPref("browser.download.dir");
     await IOUtils.remove(tempDir.path, { recursive: true });
   });
-
-  Services.telemetry.clearEvents();
 });
 
 add_task(async function check_download_spam_permissions() {
@@ -124,22 +122,6 @@ add_task(async function check_download_spam_permissions() {
     "The test URI should have blocked automatic downloads"
   );
 
-  let events = Services.telemetry.snapshotEvents(
-    Ci.nsITelemetry.DATASET_PRERELEASE_CHANNELS,
-    true
-  );
-  events = (events.parent || []).filter(
-    e => e[1] == "downloads" && e[2] == "helpertype"
-  );
-  is(events.length, 100, "should be 100 events");
-
-  let initialEvent = events.shift();
-  is(initialEvent[4], "save", "download is saved");
-
-  for (let event of events) {
-    is(event[4], "spam", "download is blocked");
-  }
-
   await savelink();
 });