Address review feedback & add xpcshell tests draft
authorJared Hirsch <ohai@6a68.net>
Tue, 20 Feb 2018 15:47:59 -0800
changeset 760680 e6aabd1ac383358e3ecb1eb786ad033bd9528949
parent 760479 8da5aaecf9365da7dbb5376ed2ff9b2aa45bb16b
push id100717
push userbmo:jhirsch@mozilla.com
push dateTue, 27 Feb 2018 23:37:55 +0000
milestone60.0a1
Address review feedback & add xpcshell tests MozReview-Commit-ID: 2QCzvsvwfIg
toolkit/components/extensions/ext-telemetry.js
toolkit/components/extensions/schemas/telemetry.json
toolkit/components/extensions/test/xpcshell/test_ext_telemetry.js
toolkit/components/extensions/test/xpcshell/xpcshell-common.ini
--- a/toolkit/components/extensions/ext-telemetry.js
+++ b/toolkit/components/extensions/ext-telemetry.js
@@ -1,60 +1,49 @@
 "use strict";
 
-ChromeUtils.import("resource://gre/modules/TelemetryController.jsm");
-ChromeUtils.import("resource://gre/modules/TelemetryUtils.jsm");
-ChromeUtils.import("resource://gre/modules/Services.jsm");
+ChromeUtils.defineModuleGetter(this, "TelemetryController",
+                               "resource://gre/modules/TelemetryController.jsm");
+ChromeUtils.defineModuleGetter(this, "TelemetryUtils",
+                               "resource://gre/modules/TelemetryUtils.jsm");
+ChromeUtils.defineModuleGetter(this, "Services",
+                               "resource://gre/modules/Services.jsm");
 
 this.telemetry = class extends ExtensionAPI {
   getAPI(context) {
-    let {extension} = context;
     return {
       telemetry: {
-        send(message, options = {
-          addClientId: true,
-          addEnvironment: true
-        }) {
+        submitPing(type, message, options) {
           TelemetryController.submitExternalPing(
-            extension.id,
+            type,
             message,
             options
           );
         },
         canUpload() {
           // Note: remove the ternary and direct pref check when
-          // TelemetryController.canUpload() is implemented.
+          // TelemetryController.canUpload() is implemented (bug 1440089).
           return ('canUpload' in TelemetryController) ?
             TelemetryController.canUpload() :
             Services.prefs.getBoolPref(TelemetryUtils.Preferences.FhrUploadEnabled, false);
         },
-        // TODO: type check the name and value of scalar*, so that any console
-        // warnings are emitted from this API, not the underlying Telemetry API?
-        // or is that automatically handled by the schema?
         scalarAdd(name, value) {
-          Services.telemetry.scalarAdd(name, value);
+          return Services.telemetry.scalarAdd(name, value);
         },
         scalarSet(name, value) {
-          Services.telemetry.scalarSet(name, value);
+          return Services.telemetry.scalarSet(name, value);
         },
         scalarSetMaximum(name, value) {
-          Services.telemetry.scalarSetMaximum(name, value);
+          return Services.telemetry.scalarSetMaximum(name, value);
         },
         recordEvent(category, method, object, value, extra) {
-          try {
-            // TODO: what's the right way to invoke optional args? spread / rest?
-            Services.telemetry.recordEvent(category, method, object, value, extra);
-          } catch (ex) {
-            // TODO: how do we want to handle exceptions thrown by the underlying Telemetry APIs?
-            // Is there an internal API that WebExtensions use to handle extensions?
-            throw ex;
-          }
+          return Services.telemetry.recordEvent(category, method, object, value, extra);
         },
         registerScalars(category, data) {
-          Services.telemetry.registerScalars(category, data);
+          return Services.telemetry.registerScalars(category, data);
         },
         registerEvents(category, data) {
-          Services.telemetry.registerEvents(category, data);
+          return Services.telemetry.registerEvents(category, data);
         }
       },
     };
   }
 };
--- a/toolkit/components/extensions/schemas/telemetry.json
+++ b/toolkit/components/extensions/schemas/telemetry.json
@@ -1,87 +1,119 @@
 [
   {
+    "namespace": "manifest",
+    "types": [{
+      "$extend": "Permission",
+      "choices": [{
+        "type": "string",
+        "enum": [
+          "telemetry"
+        ]
+      }]
+    }]
+  },
+  {
     "namespace": "telemetry",
     "description": "Use the browser.telemetry API to send telemetry data to a telemetry service.",
-    "permissions": ["telemetry", "mozillaAddons"],
+    "permissions": [
+      "telemetry",
+      "mozillaAddons"
+    ],
     "functions": [{
       "name": "submitPing",
       "type": "function",
       "description": "Submits a custom ping to the Telemetry back-end.",
+      "async": true,
       "parameters": [
         {
+          "name": "type",
+          "type": "string",
+          "description": "The type of the ping. Alphanumeric only, with dashes allowed inside the string."
+        },
+        {
           "name": "message",
-          "type": "object"
+          "type": "any"
         },
         {
           "name": "options",
-          "type": "object"
+          "type": "any",
+          "optional": true,
+          "description": "Options object passed to `TelemetryController.submitExternalPing`. See Telemetry docs for more information.",
+          "default": {
+            "addClientId": true,
+            "addEnvironment": true
+          }
         }
       ]
     },
     {
       "name": "canUpload",
       "type": "function",
       "description": "Checks if Telemetry is enabled.",
       "parameters": [],
+      "async": true,
       "returns": {
         "type": "boolean",
         "description": "True if Telemetry is enabled"
       }
     },
     {
       "name": "scalarAdd",
       "type": "function",
       "description": "TODO",
+      "async": true,
       "parameters": [
         {
           "name": "name",
           "type": "string"
         },
         {
           "name": "value",
           "type": "integer"
         }
       ]
     },
     {
       "name": "scalarSet",
       "type": "function",
       "description": "TODO",
+      "async": true,
       "parameters": [
         {
           "name": "name",
           "type": "string"
         },
         {
           "name": "value",
-          "type": "integer"
+          "type": "any"
         }
       ]
     },
     {
       "name": "scalarSetMaximum",
       "type": "function",
       "description": "TODO",
+      "async": true,
       "parameters": [
         {
           "name": "name",
           "type": "string"
         },
         {
           "name": "value",
           "type": "integer"
         }
       ]
     },
     {
       "name": "recordEvent",
       "type": "function",
       "description": "TODO",
+      "async": true,
       "parameters": [
         {
           "name": "category",
           "type": "string"
         },
         {
           "name": "method",
           "type": "string"
@@ -101,36 +133,38 @@
           "type": "object"
         }
       ]
     },
     {
       "name": "registerScalars",
       "type": "function",
       "description": "TODO",
+      "async": true,
       "parameters": [
         {
           "name": "category",
           "type": "string"
         },
         {
           "name": "data",
-          "type": "object"
+          "type": "any"
         }
       ]
     },
     {
       "name": "registerEvents",
       "type": "function",
       "description": "TODO",
+      "async": true,
       "parameters": [
         {
           "name": "category",
           "type": "string"
         },
         {
           "name": "data",
-          "type": "object"
+          "type": "any"
         }
       ]
     }]
   }
 ]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_telemetry.js
@@ -0,0 +1,220 @@
+"use strict";
+
+ChromeUtils.import("resource://gre/modules/TelemetryArchive.jsm", this);
+
+function createExtension(backgroundScript, permissions) {
+  let extensionData = {
+    background: backgroundScript,
+    manifest: { permissions }
+  };
+  return ExtensionTestUtils.loadExtension(extensionData);
+}
+
+async function run(test) {
+  let extension = createExtension(test.backgroundScript, test.permissions || ["telemetry"]);
+  await extension.startup();
+  await extension.awaitFinish(test.doneSignal);
+  await extension.unload();
+}
+
+add_task(async function test_telemetry_without_telemetry_permission() {
+  await run({
+    backgroundScript: () => {
+      browser.test.assertTrue(!browser.telemetry, "'telemetry' permission is required");
+      browser.test.notifyPass("telemetry_permission");
+    },
+    permissions: [],
+    doneSignal: "telemetry_permission"
+  });
+});
+
+add_task(async function test_telemetry_scalar_add() {
+  await run({
+    backgroundScript: () => {
+      browser.telemetry.scalarAdd("telemetry.test.unsigned_int_kind", 1);
+      browser.test.notifyPass("scalar_add");
+    },
+    doneSignal: "scalar_add"
+  });
+});
+
+add_task(async function test_telemetry_scalar_add_unknown_name() {
+  let {messages} = await promiseConsoleOutput(async function() {
+    await run({
+      backgroundScript: () => {
+        browser.telemetry.scalarAdd("telemetry.test.does_not_exist", 1);
+        browser.test.notifyPass("scalar_add_unknown_name");
+      },
+      doneSignal: "scalar_add_unknown_name"
+    });
+  });
+
+  messages = messages.filter(msg => /Unknown scalar/);
+  equal(messages.length, 1, "Telemetry should throw if an unknown scalar is incremented");
+});
+
+add_task(async function test_telemetry_scalar_add_illegal_value() {
+  await run({
+    backgroundScript: () => {
+      browser.test.assertThrows(
+        () => browser.telemetry.scalarAdd("telemetry.test.unsigned_int_kind", "string"),
+        /Incorrect argument types for telemetry.scalarAdd/,
+        "The second 'value' argument to scalarAdd must be a scalar");
+      browser.test.notifyPass("scalar_add_illegal_value");
+    },
+    doneSignal: "scalar_add_illegal_value"
+  });
+});
+
+add_task(async function test_telemetry_scalar_add_invalid_keyed_scalar() {
+  let {messages} = await promiseConsoleOutput(async function() {
+    await run({
+      backgroundScript: () => {
+        browser.telemetry.scalarAdd("telemetry.test.keyed_unsigned_int", 1);
+        browser.test.notifyPass("scalar_add_invalid_keyed_scalar");
+      },
+      doneSignal: "scalar_add_invalid_keyed_scalar"
+    });
+  });
+
+  messages = messages.filter(msg => /Attempting to manage a keyed scalar as a scalar/);
+  equal(messages.length, 1, "Telemetry should throw if a keyed scalar is incremented");
+});
+
+add_task(async function test_telemetry_scalar_set() {
+  await run({
+    backgroundScript: () => {
+      browser.telemetry.scalarSet("telemetry.test.unsigned_int_kind", 1);
+      browser.telemetry.scalarSet("telemetry.test.boolean_kind", true);
+      browser.test.notifyPass("scalar_set");
+    },
+    doneSignal: "scalar_set"
+  });
+});
+
+add_task(async function test_telemetry_scalar_set_unknown_name() {
+  let {messages} = await promiseConsoleOutput(async function() {
+    await run({
+      backgroundScript: () => {
+        browser.telemetry.scalarSet("telemetry.test.does_not_exist", 1);
+        browser.test.notifyPass("scalar_set_unknown_name");
+      },
+      doneSignal: "scalar_set_unknown_name",
+    });
+  });
+
+  messages = messages.filter(msg => /Unknown scalar/);
+  equal(messages.length, 1, "Telemetry should throw if an unknown scalar is set");
+});
+
+add_task(async function test_telemetry_scalar_set_maximum() {
+  await run({
+    backgroundScript: () => {
+      browser.telemetry.scalarSetMaximum("telemetry.test.unsigned_int_kind", 123);
+      browser.test.notifyPass("scalar_set_maximum");
+    },
+    doneSignal: "scalar_set_maximum"
+  });
+});
+
+add_task(async function test_telemetry_scalar_set_maximum_unknown_name() {
+  let {messages} = await promiseConsoleOutput(async function() {
+    await run({
+      backgroundScript: () => {
+        browser.telemetry.scalarSetMaximum("telemetry.test.does_not_exist", 1);
+        browser.test.notifyPass("scalar_set_maximum_unknown_name");
+      },
+      doneSignal: "scalar_set_maximum_unknown_name"
+    });
+  });
+
+  messages = messages.filter(msg => /Unknown scalar/);
+  equal(messages.length, 1, "Telemetry should throw if an unknown scalar is set");
+});
+
+add_task(async function test_telemetry_scalar_set_maximum_illegal_value() {
+  await run({
+    backgroundScript: () => {
+      browser.test.assertThrows(
+        () => browser.telemetry.scalarSetMaximum("telemetry.test.unsigned_int_kind", "string"),
+        /Incorrect argument types for telemetry.scalarSetMaximum/,
+        "The second 'value' argument to scalarSetMaximum must be a scalar");
+      browser.test.notifyPass("scalar_set_maximum_illegal_value");
+    },
+    doneSignal: "scalar_set_maximum_illegal_value"
+  });
+});
+
+add_task(async function test_telemetry_record_event() {
+  Services.telemetry.setEventRecordingEnabled("telemetry.test", true);
+
+  await run({
+    backgroundScript: () => {
+      browser.telemetry.recordEvent("telemetry.test", "test1", "object1");
+      browser.test.notifyPass("record_event_ok");
+    },
+    doneSignal: "record_event_ok"
+  });
+
+  let events = Services.telemetry.snapshotEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, true);
+  equal(events.parent.length, 1);
+  equal(events.parent[0][1], "telemetry.test");
+
+  Services.telemetry.clearEvents();
+});
+
+add_task(async function test_telemetry_register_scalars() {
+  await run({
+    backgroundScript: () => {
+      browser.telemetry.registerScalars("telemetry.test.dynamic", {
+        "webext": {
+          kind: 0, // Ci.nsITelemetry.SCALAR_TYPE_COUNT
+          keyed: false,
+          record_on_release: true
+        }
+      });
+      browser.telemetry.scalarSet("telemetry.test.dynamic.webext", 123);
+      browser.test.notifyPass("register_scalars");
+    },
+    doneSignal: "register_scalars"
+  });
+
+  const scalars = Services.telemetry.snapshotScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, true);
+  equal(scalars["dynamic"]["telemetry.test.dynamic.webext"], 123);
+});
+
+add_task(async function test_telemetry_snapshot_scalars() {
+  await run({
+    backgroundScript: () => {
+      browser.telemetry.registerEvents("telemetry.test.dynamic", {
+        "test1": {
+          methods: ["test1"],
+          objects: ["object1"]
+        }
+      });
+      browser.telemetry.recordEvent("telemetry.test.dynamic", "test1", "object1");
+      browser.test.notifyPass("snapshot_scalars");
+    },
+    doneSignal: "snapshot_scalars"
+  });
+
+  let events = Services.telemetry.snapshotEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, true);
+  let expected = [ ["telemetry.test.dynamic", "test1", "object1"] ];
+  equal(events.dynamic.length, expected.length);
+  deepEqual(events.dynamic.map(e => e.slice(1)), expected);
+});
+
+add_task(async function test_telemetry_submit_ping() {
+  await run({
+    backgroundScript: () => {
+      browser.telemetry.submitPing("webext-test", {});
+      browser.test.notifyPass("submit_ping");
+    },
+    doneSignal: "submit_ping"
+  });
+
+  // TODO: is this ok? Or do we need to start up the mock server?
+  let pings = await TelemetryArchive.promiseArchivedPingList();
+  equal(pings.length, 1);
+  equal(pings[0].type, "webext-test");
+});
--- a/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini
+++ b/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini
@@ -69,16 +69,18 @@ skip-if = os == "android"
 [test_ext_storage_managed.js]
 skip-if = os == "android"
 [test_ext_storage_sync.js]
 head = head.js head_sync.js
 skip-if = os == "android"
 [test_ext_storage_sync_crypto.js]
 skip-if = os == "android"
 [test_ext_storage_telemetry.js]
+skip-if = os == "android"
+[test_ext_telemetry.js]
 skip-if = os == "android" # checking for telemetry needs to be updated: 1384923
 [test_ext_trustworthy_origin.js]
 [test_ext_topSites.js]
 skip-if = os == "android"
 [test_native_manifests.js]
 subprocess = true
 skip-if = os == "android"
 [test_ext_permissions.js]