Bug 1466490 - Add multi-process xpcshell test for scalar semantics. r?dexter draft
authorJan-Erik Rediger <jrediger@mozilla.com>
Thu, 14 Jun 2018 16:05:27 -0700
changeset 808657 b01733ba6d5e957baaf43a20f0f357261f55b38b
parent 808656 a7b3ea1036ffce66a66e85ba45047fac41cdfe4b
push id113453
push userbmo:jrediger@mozilla.com
push dateWed, 20 Jun 2018 09:16:34 +0000
reviewersdexter
bugs1466490
milestone62.0a1
Bug 1466490 - Add multi-process xpcshell test for scalar semantics. r?dexter This follows the documentation example: * Scalar deserialization is started * “test” scalar is incremented by “10” by the application -> The operation [test, add, 10] is recorded into the list. * The state of the “test” scalar is loaded off the persistence file, and the value “14” is set. * Deserialization is finished and the pending operations are applied. * The “test” scalar is incremented by “10”, the value is now “24” It does that in both the parent and the content process. MozReview-Commit-ID: CnzDcJ5o7jJ
toolkit/components/telemetry/Scalars.yaml
toolkit/components/telemetry/TelemetryScalar.h
toolkit/components/telemetry/geckoview/TelemetryGeckoViewTesting.cpp
toolkit/components/telemetry/geckoview/nsITelemetryGeckoViewTesting.idl
toolkit/components/telemetry/tests/unit/head_GeckoView.js
toolkit/components/telemetry/tests/unit/test_GeckoView_ScalarSemantics.js
toolkit/components/telemetry/tests/unit/test_GeckoView_content_scalars.js
toolkit/components/telemetry/tests/unit/xpcshell.ini
--- a/toolkit/components/telemetry/Scalars.yaml
+++ b/toolkit/components/telemetry/Scalars.yaml
@@ -1876,16 +1876,28 @@ telemetry.test:
     expires: never
     kind: uint
     keyed: true
     notification_emails:
       - telemetry-client-dev@mozilla.com
     record_in_processes:
       - 'main'
 
+  child_keyed_unsigned_int:
+    bug_numbers:
+      - 1466490
+    description: A testing keyed uint scalar; not meant to be touched.
+    expires: never
+    kind: uint
+    keyed: true
+    notification_emails:
+      - telemetry-client-dev@mozilla.com
+    record_in_processes:
+      - 'content'
+
   keyed_boolean_kind:
     bug_numbers:
       - 1277806
     description: A testing keyed boolean scalar; not meant to be touched.
     expires: never
     kind: boolean
     keyed: true
     notification_emails:
--- a/toolkit/components/telemetry/TelemetryScalar.h
+++ b/toolkit/components/telemetry/TelemetryScalar.h
@@ -1,16 +1,17 @@
 /* -*-  Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2; -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef TelemetryScalar_h__
 #define TelemetryScalar_h__
 
+#include "nsTArray.h"
 #include "mozilla/TelemetryScalarEnums.h"
 #include "mozilla/TelemetryProcessEnums.h"
 
 // This module is internal to Telemetry. It encapsulates Telemetry's
 // scalar accumulation and storage logic. It should only be used by
 // Telemetry.cpp. These functions should not be used anywhere else.
 // For the public interface to Telemetry functionality, see Telemetry.h.
 
--- a/toolkit/components/telemetry/geckoview/TelemetryGeckoViewTesting.cpp
+++ b/toolkit/components/telemetry/geckoview/TelemetryGeckoViewTesting.cpp
@@ -1,14 +1,15 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "TelemetryGeckoViewTesting.h"
 #include "TelemetryGeckoViewPersistence.h"
+#include "TelemetryScalar.h"
 
 namespace TelemetryGeckoViewTesting {
   // This is defined in TelemetryGeckoViewPersistence.cpp
   void TestDispatchPersist();
 } // TelemetryGeckoViewTesting
 
 NS_IMPL_ISUPPORTS(TelemetryGeckoViewTestingImpl, nsITelemetryGeckoViewTesting)
 
@@ -36,8 +37,15 @@ TelemetryGeckoViewTestingImpl::ClearPers
 }
 
 NS_IMETHODIMP
 TelemetryGeckoViewTestingImpl::ForcePersist(JSContext*)
 {
   TelemetryGeckoViewTesting::TestDispatchPersist();
   return NS_OK;
 }
+
+NS_IMETHODIMP
+TelemetryGeckoViewTestingImpl::DeserializationStarted(JSContext*)
+{
+  TelemetryScalar::DeserializationStarted();
+  return NS_OK;
+}
--- a/toolkit/components/telemetry/geckoview/nsITelemetryGeckoViewTesting.idl
+++ b/toolkit/components/telemetry/geckoview/nsITelemetryGeckoViewTesting.idl
@@ -27,9 +27,18 @@ interface nsITelemetryGeckoViewTesting :
   void clearPersistenceData();
 
   /**
    * Enqueues a persist action into the Telemetry persistence thread:
    * measurements might be written to the disk after it returns.
    */
   [implicit_jscontext]
   void forcePersist();
+
+  /**
+   * The following method maps to the function with the same name from TelemetryScalar.cpp.
+   *
+   * It marks deserialization as in progress.
+   * After this, all scalar operations are recorded into the pending operations list.
+   */
+  [implicit_jscontext]
+  void deserializationStarted();
 };
--- a/toolkit/components/telemetry/tests/unit/head_GeckoView.js
+++ b/toolkit/components/telemetry/tests/unit/head_GeckoView.js
@@ -56,11 +56,29 @@ async function waitForHistogramSnapshotD
       ? Telemetry.snapshotKeyedHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false)
       : Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false);
 
     return (aProcessName in data)
            && (aHistogramName in data[aProcessName]);
   });
 }
 
+/**
+ * This function waits until the desired scalar is reported into the
+ * snapshot of the relevant process.
+ * @param aScalarName - The name of the scalar to look for.
+ * @param aProcessName - The name of the process to look in.
+ * @param aKeyed - Whether or not to look in keyed snapshots.
+ */
+async function waitForScalarSnapshotData(aScalarName, aProcessName, aKeyed) {
+  await ContentTaskUtils.waitForCondition(() => {
+    const data = aKeyed
+      ? Telemetry.snapshotKeyedScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false)
+      : Telemetry.snapshotScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false);
+
+    return (aProcessName in data)
+           && (aScalarName in data[aProcessName]);
+  });
+}
+
 if (runningInParent) {
   Services.prefs.setBoolPref(TelemetryUtils.Preferences.OverridePreRelease, true);
 }
new file mode 100644
--- /dev/null
+++ b/toolkit/components/telemetry/tests/unit/test_GeckoView_ScalarSemantics.js
@@ -0,0 +1,131 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/
+*/
+"use strict";
+
+ChromeUtils.import("resource://gre/modules/PromiseUtils.jsm", this);
+ChromeUtils.import("resource://gre/modules/Services.jsm", this);
+ChromeUtils.import("resource://testing-common/ContentTaskUtils.jsm", this);
+
+const CONTENT_ONLY_UINT_SCALAR = "telemetry.test.content_only_uint";
+const ALL_PROCESSES_UINT_SCALAR = "telemetry.test.all_processes_uint";
+const DEFAULT_PRODUCTS_SCALAR = "telemetry.test.default_products";
+const CHILD_KEYED_UNSIGNED_INT = "telemetry.test.child_keyed_unsigned_int";
+
+add_task(async function setup() {
+  // Init the profile.
+  let profileDir = do_get_profile(true);
+
+  // Set geckoview mode.
+  Services.prefs.setBoolPref("toolkit.telemetry.isGeckoViewMode", true);
+
+  // Set the ANDROID_DATA_DIR to the profile dir.
+  let env = Cc["@mozilla.org/process/environment;1"].getService(Ci.nsIEnvironment);
+  env.set("MOZ_ANDROID_DATA_DIR", profileDir.path);
+});
+
+add_task(async function test_MultiprocessScalarSemantics() {
+  /**
+   * To mitigate races during deserialization of persisted data
+   * scalar operations will be recorded and applied after the deserialization is finished.
+   *
+   * This test ensures it works acording to the semantics and follows the documentation example:
+   *
+   *  * Scalar deserialization is started
+   *  * “test” scalar is incremented by “10” by the application -> The operation [test, add, 10] is recorded into the list.
+   *  * The state of the “test” scalar is loaded off the persistence file, and the value “14” is set.
+   *  * Deserialization is finished and the pending operations are applied.
+   *  * The “test” scalar is incremented by “10”, the value is now “24”
+   */
+
+  Telemetry.clearScalars();
+
+  let loadPromise = waitGeckoViewLoadComplete();
+  TelemetryGeckoView.initPersistence();
+  await loadPromise;
+
+  // Set something in the parent
+  Telemetry.scalarSet(ALL_PROCESSES_UINT_SCALAR, 34);
+
+  // Set scalars in the child process.
+  // The child will then wait for a signal to continue.
+  let child = run_in_child("test_GeckoView_content_scalars.js");
+
+  // Wait for the data to be collected by the parent process.
+  await waitForScalarSnapshotData(ALL_PROCESSES_UINT_SCALAR, "parent", false /* aKeyed */);
+  await waitForScalarSnapshotData(CONTENT_ONLY_UINT_SCALAR, "content", false /* aKeyed */);
+  await waitForScalarSnapshotData(ALL_PROCESSES_UINT_SCALAR, "content", false /* aKeyed */);
+  await waitForScalarSnapshotData(CHILD_KEYED_UNSIGNED_INT, "content", true /* aKeyed */);
+
+  let snapshot = Telemetry.snapshotScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false /* clear */);
+  let keyedSnapshot = Telemetry.snapshotKeyedScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false /* clear */);
+  Assert.equal(snapshot.content[CONTENT_ONLY_UINT_SCALAR], 14,
+            `The ${CONTENT_ONLY_UINT_SCALAR} scalar must have the expected value in the content section.`);
+  Assert.equal(snapshot.content[ALL_PROCESSES_UINT_SCALAR], 24,
+            `The ${ALL_PROCESSES_UINT_SCALAR} scalar must have the expected value in the content section.`);
+  Assert.equal(snapshot.parent[ALL_PROCESSES_UINT_SCALAR], 34,
+            `The ${ALL_PROCESSES_UINT_SCALAR} scalar must have the expected value in the parent section.`);
+  Assert.equal(keyedSnapshot.content[CHILD_KEYED_UNSIGNED_INT].chewbacca, 44,
+            `The ${CHILD_KEYED_UNSIGNED_INT} keyed scalar must have the expected value in the content section.`);
+
+  // Force persisting the measurements to file.
+  TelemetryGeckoView.forcePersist();
+  TelemetryGeckoView.deInitPersistence();
+
+  // Clear all data from memory.
+  Telemetry.clearScalars();
+
+  // Mark deserialization as in progress, following operations are recorded and not applied.
+  TelemetryGeckoView.deserializationStarted();
+
+  // Modify a scalar in the parent process.
+  Telemetry.scalarAdd(ALL_PROCESSES_UINT_SCALAR, 10);
+
+  // Let child know to progress and wait for it to finish.
+  do_send_remote_message("child-scalar-semantics");
+  await child;
+
+  // Start the persistence system again, to unpersist the data.
+  loadPromise = waitGeckoViewLoadComplete();
+  TelemetryGeckoView.initPersistence();
+  // Wait for the load to finish.
+  await loadPromise;
+
+  // Wait for the data to be collected by the parent process.
+  // We only wait for the new data, as the rest will be in there from the persistence load.
+  await waitForScalarSnapshotData(DEFAULT_PRODUCTS_SCALAR, "content", false /* aKeyed */);
+
+  // Validate the snapshot data.
+  snapshot =
+    Telemetry.snapshotScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false /* clear */);
+  keyedSnapshot =
+    Telemetry.snapshotKeyedScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false /* clear */);
+
+  Assert.ok("parent" in snapshot, "The snapshot object must have a 'content' entry.");
+  Assert.ok("content" in snapshot, "The snapshot object must have a 'content' entry.");
+  Assert.ok("content" in keyedSnapshot, "The keyed snapshot object must have a 'content' entry.");
+
+  Assert.ok(ALL_PROCESSES_UINT_SCALAR in snapshot.parent,
+            `The ${ALL_PROCESSES_UINT_SCALAR} scalar must exist in the parent section.`);
+  Assert.ok(CONTENT_ONLY_UINT_SCALAR in snapshot.content,
+            `The ${CONTENT_ONLY_UINT_SCALAR} scalar must exist in the content section.`);
+  Assert.ok(ALL_PROCESSES_UINT_SCALAR in snapshot.content,
+            `The ${ALL_PROCESSES_UINT_SCALAR} scalar must exist in the content section.`);
+
+  Assert.equal(snapshot.content[CONTENT_ONLY_UINT_SCALAR], 24,
+               `The ${CONTENT_ONLY_UINT_SCALAR} must have the expected value in the content section.`);
+  Assert.equal(snapshot.content[ALL_PROCESSES_UINT_SCALAR], 34,
+               `The ${ALL_PROCESSES_UINT_SCALAR} must have the expected value in the content section.`);
+
+  Assert.equal(snapshot.parent[ALL_PROCESSES_UINT_SCALAR], 44,
+               `The ${ALL_PROCESSES_UINT_SCALAR} must have the expected value in the parent section.`);
+
+  Assert.equal(keyedSnapshot.content[CHILD_KEYED_UNSIGNED_INT].chewbacca, 54,
+            `The ${CHILD_KEYED_UNSIGNED_INT} keyed scalar must have the expected value in the content section.`);
+
+  TelemetryGeckoView.deInitPersistence();
+});
+
+add_task(async function cleanup() {
+  Services.prefs.clearUserPref("toolkit.telemetry.isGeckoViewMode");
+});
new file mode 100644
--- /dev/null
+++ b/toolkit/components/telemetry/tests/unit/test_GeckoView_content_scalars.js
@@ -0,0 +1,30 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/
+*/
+"use strict";
+
+ChromeUtils.import("resource://gre/modules/Services.jsm", this);
+
+const CONTENT_ONLY_UINT_SCALAR = "telemetry.test.content_only_uint";
+const ALL_PROCESSES_UINT_SCALAR = "telemetry.test.all_processes_uint";
+const DEFAULT_PRODUCTS_SCALAR = "telemetry.test.default_products";
+const CHILD_KEYED_UNSIGNED_INT = "telemetry.test.child_keyed_unsigned_int";
+
+// Note: this test file is only supposed to be run by
+// test_GeckoView_ScalarSemantics.js.
+// It assumes to be in the content process.
+add_task(async function run_child() {
+  // Set initial values in content process.
+  Services.telemetry.scalarSet(CONTENT_ONLY_UINT_SCALAR, 14);
+  Services.telemetry.scalarSet(ALL_PROCESSES_UINT_SCALAR, 24);
+  Services.telemetry.keyedScalarSet(CHILD_KEYED_UNSIGNED_INT, "chewbacca", 44);
+
+  // Wait for the parent to inform us to continue.
+  await do_await_remote_message("child-scalar-semantics");
+
+  // Modifications to probes will be recorded and applied later.
+  Services.telemetry.scalarAdd(CONTENT_ONLY_UINT_SCALAR, 10);
+  Services.telemetry.scalarAdd(ALL_PROCESSES_UINT_SCALAR, 10);
+  Services.telemetry.keyedScalarAdd(CHILD_KEYED_UNSIGNED_INT, "chewbacca", 10);
+  Services.telemetry.scalarSet(DEFAULT_PRODUCTS_SCALAR, 1);
+});
--- a/toolkit/components/telemetry/tests/unit/xpcshell.ini
+++ b/toolkit/components/telemetry/tests/unit/xpcshell.ini
@@ -22,16 +22,21 @@ generated-files =
   system.xpi
   restartless.xpi
 
 [test_GeckoView.js]
 skip-if = os == "android" # Disabled due to crashes (see bug 1331366)
 head = head_GeckoView.js
 support-files =
   test_GeckoView_content_histograms.js
+[test_GeckoView_ScalarSemantics.js]
+skip-if = os == "android" # Disabled due to crashes (see bug 1331366)
+head = head_GeckoView.js
+support-files =
+  test_GeckoView_content_scalars.js
 [test_MigratePendingPings.js]
 [test_TelemetryHistograms.js]
 [test_TelemetryStorage.js]
 [test_SubsessionChaining.js]
 tags = addons
 [test_TelemetryEnvironment.js]
 skip-if = os == "android"
 tags = addons