Bug 1691050 - Extend telemetry for search engine/add-on mismatches to check for name as well as parameters. r=mak
authorMark Banner <standard8@mozilla.com>
Wed, 10 Feb 2021 20:37:03 +0000
changeset 566876 423a59653735977cbcac4faac5cfe66d541b9113
parent 566875 8fe0f90430d40537a327faf0f8e732f0c1c3bd9e
child 566877 cfaa6bb3bb55e17d36abead05b42ca1c4b0c4a7c
push id38191
push userbtara@mozilla.com
push dateThu, 11 Feb 2021 05:02:45 +0000
treeherdermozilla-central@5cbcb80f72bd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1691050
milestone87.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1691050 - Extend telemetry for search engine/add-on mismatches to check for name as well as parameters. r=mak Differential Revision: https://phabricator.services.mozilla.com/D104384
toolkit/components/search/SearchService.jsm
toolkit/components/search/tests/xpcshell/test_webextensions_valid.js
toolkit/components/telemetry/Scalars.yaml
--- a/toolkit/components/search/SearchService.jsm
+++ b/toolkit/components/search/SearchService.jsm
@@ -1398,30 +1398,45 @@ SearchService.prototype = {
         );
         Services.telemetry.keyedScalarSet(
           "browser.searchinit.engine_invalid_webextension",
           engine._extensionID,
           2
         );
       } else {
         let policy = await this._getExtensionPolicy(engine._extensionID);
-        let manifest = policy.extension.manifest;
+        let providerSettings =
+          policy.extension.manifest?.chrome_settings_overrides?.search_provider;
 
-        if (
-          !engine.checkSearchUrlMatchesManifest(
-            manifest?.chrome_settings_overrides?.search_provider
-          )
-        ) {
+        if (!providerSettings) {
+          logConsole.debug(
+            `Add-on ${engine._extensionID} for search engine ${engine.name} no longer has an engine defined`
+          );
+          Services.telemetry.keyedScalarSet(
+            "browser.searchinit.engine_invalid_webextension",
+            engine._extensionID,
+            4
+          );
+        } else if (engine.name != providerSettings.name) {
+          logConsole.debug(
+            `Add-on ${engine._extensionID} for search engine ${engine.name} has a different name!`
+          );
+          Services.telemetry.keyedScalarSet(
+            "browser.searchinit.engine_invalid_webextension",
+            engine._extensionID,
+            5
+          );
+        } else if (!engine.checkSearchUrlMatchesManifest(providerSettings)) {
           logConsole.debug(
             `Add-on ${engine._extensionID} for search engine ${engine.name} has out-of-date manifest!`
           );
           Services.telemetry.keyedScalarSet(
             "browser.searchinit.engine_invalid_webextension",
             engine._extensionID,
-            3
+            6
           );
         }
       }
     }
     logConsole.debug("WebExtension engine check complete");
   },
 
   async getEngines() {
--- a/toolkit/components/search/tests/xpcshell/test_webextensions_valid.js
+++ b/toolkit/components/search/tests/xpcshell/test_webextensions_valid.js
@@ -54,35 +54,80 @@ add_task(async function test_valid_exten
 
   await Services.search.runBackgroundChecks();
 
   let scalars = TelemetryTestUtils.getProcessScalars("parent", true, true);
 
   Assert.deepEqual(scalars, {}, "Should not have recorded any issues");
 });
 
+add_task(async function test_different_name() {
+  Services.telemetry.clearScalars();
+
+  let engine = Services.search.getEngineByName("Example");
+
+  engine.wrappedJSObject._name = "Example Test";
+
+  await Services.search.runBackgroundChecks();
+
+  TelemetryTestUtils.assertKeyedScalar(
+    TelemetryTestUtils.getProcessScalars("parent", true, true),
+    "browser.searchinit.engine_invalid_webextension",
+    extension.id,
+    5
+  );
+
+  engine.wrappedJSObject._name = "Example";
+});
+
 add_task(async function test_different_url() {
   Services.telemetry.clearScalars();
 
   let engine = Services.search.getEngineByName("Example");
 
   engine.wrappedJSObject._urls = [];
   engine.wrappedJSObject._setUrls({
-    name: "Example",
     search_url: "https://example.com/123",
     search_url_get_params: "?q={searchTerms}",
   });
 
   await Services.search.runBackgroundChecks();
 
   TelemetryTestUtils.assertKeyedScalar(
     TelemetryTestUtils.getProcessScalars("parent", true, true),
     "browser.searchinit.engine_invalid_webextension",
     extension.id,
-    3
+    6
+  );
+});
+
+add_task(async function test_extension_no_longer_specifies_engine() {
+  Services.telemetry.clearScalars();
+
+  let extensionInfo = {
+    useAddonManager: "permanent",
+    manifest: {
+      version: "2.0",
+      applications: {
+        gecko: {
+          id: "example@tests.mozilla.org",
+        },
+      },
+    },
+  };
+
+  await extension.upgrade(extensionInfo);
+
+  await Services.search.runBackgroundChecks();
+
+  TelemetryTestUtils.assertKeyedScalar(
+    TelemetryTestUtils.getProcessScalars("parent", true, true),
+    "browser.searchinit.engine_invalid_webextension",
+    extension.id,
+    4
   );
 });
 
 add_task(async function test_disabled_extension() {
   // We don't clear scalars between tests to ensure the scalar gets set
   // to the new value, rather than added.
 
   // Disable the extension, this won't remove the search engine because we've
--- a/toolkit/components/telemetry/Scalars.yaml
+++ b/toolkit/components/telemetry/Scalars.yaml
@@ -5154,21 +5154,24 @@ browser.searchinit:
     release_channel_collection: opt-out
     products:
       - 'firefox'
     record_in_processes:
       - main
   engine_invalid_webextension:
     bug_numbers:
       - 1665938
+      - 1691050
     description: >
       Records the WebExtension ID of a search engine where the WebExtension
-      is not installed (= 1), disabled (= 2) or where the submission URL
-      is different between the search engine and the WebExtension (= 3)
-    expires: "89"
+      is not installed (= 1), disabled (= 2), search engine no longer specified (= 4),
+      a different name (= 5), where the submission URL is different between the
+      search engine and the WebExtension (= 6). The value '3' has been replaced
+      by '6' to distinguish newer entries.
+    expires: "94"
     keyed: true
     kind: uint
     notification_emails:
       - fx-search@mozilla.com
       - mbanner@mozilla.com
     products:
       - 'firefox'
     record_in_processes: