Backed out changeset 735f5c30d397 (bug 1064333) for XPC Bustage on a CLOSED TREE

--HG--
extra : amend_source : 8b12237f285b03acfe6e18cccc9f8480c6d385c2

services/common/utils.js

@@ -398,8 +398,9 @@ this.CommonUtils = {
    * @return a promise, as produced by OS.File.writeAtomic.
    */
   writeJSON: function(contents, path) {
-    let data = JSON.stringify(contents);
-    return OS.File.writeAtomic(path, data, {encoding: "utf-8", tmpPath: path + ".tmp"});
+    let encoder = new TextEncoder();
+    let array = encoder.encode(JSON.stringify(contents));
+    return OS.File.writeAtomic(path, array, {tmpPath: path + ".tmp"});
   },
 
 

services/datareporting/DataReportingService.js

@@ -9,9 +9,6 @@ const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 Cu.import("resource://gre/modules/Preferences.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://services-common/utils.js");
-Cu.import("resource://gre/modules/Promise.jsm");
-Cu.import("resource://gre/modules/Task.jsm");
-Cu.import("resource://gre/modules/osfile.jsm");
 
 
 const ROOT_BRANCH = "datareporting.";
@@ -64,13 +61,6 @@ this.DataReportingService = function () {
 
   this._os = Cc["@mozilla.org/observer-service;1"]
                .getService(Ci.nsIObserverService);
-
-  this._clientID = null;
-  this._loadClientIdTask = null;
-  this._saveClientIdTask = null;
-
-  this._stateDir = null;
-  this._stateFilePath = null;
 }
 
 DataReportingService.prototype = Object.freeze({
@@ -136,9 +126,6 @@ DataReportingService.prototype = Object.freeze({
           let policyPrefs = new Preferences(POLICY_BRANCH);
           this.policy = new DataReportingPolicy(policyPrefs, this._prefs, this);
 
-          this._stateDir = OS.Path.join(OS.Constants.Path.profileDir, "datareporting");
-          this._stateFilePath = OS.Path.join(this._stateDir, "state.json");
-
           this._os.addObserver(this, "sessionstore-windows-restored", true);
         } catch (ex) {
           Cu.reportError("Exception when initializing data reporting service: " +
@@ -297,109 +284,6 @@ DataReportingService.prototype = Object.freeze({
       this._prefs.set("service.firstRun", true);
     }.bind(this));
   },
-
-  _loadClientID: Task.async(function* () {
-    if (this._loadClientIdTask) {
-      return this._loadClientIdTask;
-    }
-
-    // Previously we had the stable client ID managed in FHR.
-    // As we want to start correlating FHR and telemetry data (and moving towards
-    // unifying the two), we moved the ID management to the datareporting
-    // service. Consequently, we try to import the FHR ID first, so we can keep
-    // using it.
-
-    // Try to load the client id from the DRS state file first.
-    try {
-      let state = yield CommonUtils.readJSON(this._stateFilePath);
-      if (state && 'clientID' in state && typeof(state.clientID) == 'string') {
-        this._clientID = state.clientID;
-        this._loadClientIdTask = null;
-        return this._clientID;
-      }
-    } catch (e) {
-      // fall through to next option
-    }
-
-    // If we dont have DRS state yet, try to import from the FHR state.
-    try {
-      let fhrStatePath = OS.Path.join(OS.Constants.Path.profileDir, "healthreport", "state.json");
-      let state = yield CommonUtils.readJSON(fhrStatePath);
-      if (state && 'clientID' in state && typeof(state.clientID) == 'string') {
-        this._clientID = state.clientID;
-        this._loadClientIdTask = null;
-        this._saveClientID();
-        return this._clientID;
-      }
-    } catch (e) {
-      // fall through to next option
-    }
-
-    // We dont have an id from FHR yet, generate a new ID.
-    this._clientID = CommonUtils.generateUUID();
-    this._loadClientIdTask = null;
-    this._saveClientIdTask = this._saveClientID();
-
-    // Wait on persisting the id. Otherwise failure to save the ID would result in
-    // the client creating and subsequently sending multiple IDs to the server.
-    // This would appear as multiple clients submitting similar data, which would
-    // result in orphaning.
-    yield this._saveClientIdTask;
-
-    return this._clientID;
-  }),
-
-  _saveClientID: Task.async(function* () {
-    let obj = { clientID: this._clientID };
-    yield OS.File.makeDir(this._stateDir);
-    yield CommonUtils.writeJSON(obj, this._stateFilePath);
-    this._saveClientIdTask = null;
-  }),
-
-  /**
-   * This returns a promise resolving to the the stable client ID we use for
-   * data reporting (FHR & Telemetry). Previously exising FHR client IDs are
-   * migrated to this.
-   *
-   * @return Promise<string> The stable client ID.
-   */
-  getClientID: function() {
-    if (this._loadClientIdTask) {
-      return this._loadClientIdTask;
-    }
-
-    if (!this._clientID) {
-      this._loadClientIdTask = this._loadClientID();
-      return this._loadClientIdTask;
-    }
-
-    return Promise.resolve(this._clientID);
-  },
-
-  /**
-   * Reset the stable client id.
-   *
-   * @return Promise<string> The new client ID.
-   */
-  resetClientID: Task.async(function* () {
-    yield this._loadClientIdTask;
-    yield this._saveClientIdTask;
-
-    this._clientID = CommonUtils.generateUUID();
-    this._saveClientIdTask = this._saveClientID();
-    yield this._saveClientIdTask;
-
-    return this._clientID;
-  }),
-
-  /*
-   * Simulate a restart of the service. This is for testing only.
-   */
-  _reset: Task.async(function* () {
-    yield this._loadClientIdTask;
-    yield this._saveClientIdTask;
-    this._clientID = null;
-  }),
 });
 
 this.NSGetFactory = XPCOMUtils.generateNSGetFactory([DataReportingService]);

services/datareporting/policy.jsm

@@ -817,8 +817,7 @@ this.DataReportingPolicy.prototype = Object.freeze({
     this._log.info("Requesting data submission. Will expire at " +
                    requestExpiresDate);
     try {
-      let promise = this._listener[handler](this._inProgressSubmissionRequest);
-      chained = chained.then(() => promise, null);
+      this._listener[handler](this._inProgressSubmissionRequest);
     } catch (ex) {
       this._log.warn("Exception when calling " + handler + ": " +
                      CommonUtils.exceptionStr(ex));

services/datareporting/tests/xpcshell/test_client_id.js

@@ -1,86 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
- * http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
-
-Cu.import("resource://gre/modules/Task.jsm");
-Cu.import("resource://services-common/utils.js");
-Cu.import("resource://gre/modules/osfile.jsm");
-Cu.import("resource://gre/modules/XPCOMUtils.jsm");
-
-XPCOMUtils.defineLazyGetter(this, "gDatareportingService",
-  () => Cc["@mozilla.org/datareporting/service;1"]
-          .getService(Ci.nsISupports)
-          .wrappedJSObject);
-
-function run_test() {
-  do_get_profile();
-
-  // Send the needed startup notifications to the datareporting service
-  // to ensure that it has been initialized.
-  gDatareportingService.observe(null, "app-startup", null);
-  gDatareportingService.observe(null, "profile-after-change", null);
-
-  run_next_test();
-}
-
-add_task(function* () {
-  const drsPath = gDatareportingService._stateFilePath;
-  const fhrDir  = OS.Path.join(OS.Constants.Path.profileDir, "healthreport");
-  const fhrPath = OS.Path.join(fhrDir, "state.json");
-  const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
-
-  yield OS.File.makeDir(fhrDir);
-
-  // Check that we are importing the FHR client ID.
-  let clientID = CommonUtils.generateUUID();
-  yield CommonUtils.writeJSON({clientID: clientID}, fhrPath);
-  Assert.equal(clientID, yield gDatareportingService.getClientID());
-
-  // We should persist the ID in DRS now and not pick up a differing ID from FHR.
-  yield gDatareportingService._reset();
-  yield CommonUtils.writeJSON({clientID: CommonUtils.generateUUID()}, fhrPath);
-  Assert.equal(clientID, yield gDatareportingService.getClientID());
-
-  // We should be guarded against broken FHR data.
-  yield gDatareportingService._reset();
-  yield OS.File.remove(drsPath);
-  yield CommonUtils.writeJSON({clientID: -1}, fhrPath);
-  clientID = yield gDatareportingService.getClientID();
-  Assert.equal(typeof(clientID), 'string');
-  Assert.ok(uuidRegex.test(clientID));
-
-  // We should be guarded against invalid FHR json.
-  yield gDatareportingService._reset();
-  yield OS.File.remove(drsPath);
-  yield OS.File.writeAtomic(fhrPath, "abcd", {encoding: "utf-8", tmpPath: fhrPath + ".tmp"});
-  clientID = yield gDatareportingService.getClientID();
-  Assert.equal(typeof(clientID), 'string');
-  Assert.ok(uuidRegex.test(clientID));
-
-  // We should be guarded against broken DRS data too and fall back to loading
-  // the FHR ID.
-  yield gDatareportingService._reset();
-  clientID = CommonUtils.generateUUID();
-  yield CommonUtils.writeJSON({clientID: clientID}, fhrPath);
-  yield CommonUtils.writeJSON({clientID: -1}, drsPath);
-  Assert.equal(clientID, yield gDatareportingService.getClientID());
-
-  // We should be guarded against invalid DRS json too.
-  yield gDatareportingService._reset();
-  yield OS.File.remove(fhrPath);
-  yield OS.File.writeAtomic(drsPath, "abcd", {encoding: "utf-8", tmpPath: drsPath + ".tmp"});
-  clientID = yield gDatareportingService.getClientID();
-  Assert.equal(typeof(clientID), 'string');
-  Assert.ok(uuidRegex.test(clientID));
-
-  // If both the FHR and DSR data are broken, we should end up with a new client ID.
-  yield gDatareportingService._reset();
-  yield CommonUtils.writeJSON({clientID: -1}, fhrPath);
-  yield CommonUtils.writeJSON({clientID: -1}, drsPath);
-  clientID = yield gDatareportingService.getClientID();
-  Assert.equal(typeof(clientID), 'string');
-  Assert.ok(uuidRegex.test(clientID));
-});

services/datareporting/tests/xpcshell/xpcshell.ini

@@ -5,4 +5,3 @@ skip-if = toolkit == 'android' || toolkit == 'gonk'
 
 [test_policy.js]
 [test_session_recorder.js]
-[test_client_id.js]

services/healthreport/healthreporter.jsm

@@ -130,18 +130,13 @@ HealthReporterState.prototype = Object.freeze({
     return Task.spawn(function* init() {
       yield OS.File.makeDir(this._stateDir);
 
-      let drs = Cc["@mozilla.org/datareporting/service;1"]
-                  .getService(Ci.nsISupports)
-                  .wrappedJSObject;
-      let drsClientID = yield drs.getClientID();
-
       let resetObjectState = function () {
         this._s = {
           // The payload version. This is bumped whenever there is a
           // backwards-incompatible change.
           v: 1,
           // The persistent client identifier.
-          clientID: drsClientID,
+          clientID: CommonUtils.generateUUID(),
           // Denotes the mechanism used to generate the client identifier.
           // 1: Random UUID.
           clientIDVersion: 1,
@@ -181,7 +176,22 @@ HealthReporterState.prototype = Object.freeze({
         // comes along and fixes us.
       }
 
-      this._s.clientID = drsClientID;
+      let regen = false;
+      if (!this._s.clientID) {
+        this._log.warn("No client ID stored. Generating random ID.");
+        regen = true;
+      }
+
+      if (typeof(this._s.clientID) != "string") {
+        this._log.warn("Client ID is not a string. Regenerating.");
+        regen = true;
+      }
+
+      if (regen) {
+        this._s.clientID = CommonUtils.generateUUID();
+        this._s.clientIDVersion = 1;
+        yield this.save();
+      }
 
       // Always look for preferences. This ensures that downgrades followed
       // by reupgrades don't result in excessive data loss.
@@ -245,18 +255,21 @@ HealthReporterState.prototype = Object.freeze({
 
   /**
    * Reset the client ID to something else.
-   * Returns a promise that is resolved when completed.
+   *
+   * This fails if remote IDs are stored because changing the client ID
+   * while there is remote data will create orphaned records.
    */
-  resetClientID: Task.async(function* () {
-    let drs = Cc["@mozilla.org/datareporting/service;1"]
-                .getService(Ci.nsISupports)
-                .wrappedJSObject;
-    yield drs.resetClientID();
-    this._s.clientID = yield drs.getClientID();
-    this._log.info("Reset client id to " + this._s.clientID + ".");
+  resetClientID: function () {
+    if (this.remoteIDs.length) {
+      throw new Error("Cannot reset client ID while remote IDs are stored.");
+    }
 
-    yield this.save();
-  }),
+    this._log.warn("Resetting client ID.");
+    this._s.clientID = CommonUtils.generateUUID();
+    this._s.clientIDVersion = 1;
+
+    return this.save();
+  },
 
   _migratePrefs: function () {
     let prefs = this._reporter._prefs;

services/healthreport/modules-testing/utils.jsm

@@ -193,14 +193,12 @@ this.getHealthReporter = function (name, uri=DUMMY_URI, inspected=false) {
   let policyPrefs = new Preferences(branch + "policy.");
   let listener = new MockPolicyListener();
   listener.onRequestDataUpload = function (request) {
-    let promise = reporter.requestDataUpload(request);
+    reporter.requestDataUpload(request);
     MockPolicyListener.prototype.onRequestDataUpload.call(this, request);
-    return promise;
   }
   listener.onRequestRemoteDelete = function (request) {
-    let promise = reporter.deleteRemoteData(request);
+    reporter.deleteRemoteData(request);
     MockPolicyListener.prototype.onRequestRemoteDelete.call(this, request);
-    return promise;
   }
   let policy = new DataReportingPolicy(policyPrefs, prefs, listener);
   let type = inspected ? InspectedHealthReporter : HealthReporter;

services/healthreport/tests/xpcshell/test_healthreporter.js

@@ -21,12 +21,6 @@ Cu.import("resource://testing-common/services/common/bagheeraserver.js");
 Cu.import("resource://testing-common/services/metrics/mocks.jsm");
 Cu.import("resource://testing-common/services/healthreport/utils.jsm");
 Cu.import("resource://testing-common/AppData.jsm");
-Cu.import("resource://gre/modules/XPCOMUtils.jsm");
-
-XPCOMUtils.defineLazyGetter(this, "gDatareportingService",
-  () => Cc["@mozilla.org/datareporting/service;1"]
-          .getService(Ci.nsISupports)
-          .wrappedJSObject);
 
 
 const DUMMY_URI = "http://localhost:62013/";
@@ -114,13 +108,6 @@ function ensureUserNotified (reporter) {
 }
 
 function run_test() {
-  do_get_profile();
-
-  // Send the needed startup notifications to the datareporting service
-  // to ensure that it has been initialized.
-  gDatareportingService.observe(null, "app-startup", null);
-  gDatareportingService.observe(null, "profile-after-change", null);
-
   run_next_test();
 }
 
@@ -758,7 +745,7 @@ add_task(function test_request_remote_data_deletion() {
     do_check_false(reporter.haveRemoteData());
     do_check_false(server.hasDocument(reporter.serverNamespace, id));
 
-     // Client ID should be updated.
+    // Client ID should be updated.
     do_check_neq(reporter._state.clientID, null);
     do_check_neq(reporter._state.clientID, clientID);
     do_check_eq(reporter._state.clientIDVersion, 1);
@@ -1184,6 +1171,38 @@ add_task(function test_state_downgrade_upgrade() {
   }
 });
 
+// Missing client ID in state should be created on state load.
+add_task(function* test_state_create_client_id() {
+  let reporter = getHealthReporter("state_create_client_id");
+
+  yield CommonUtils.writeJSON({
+    v: 1,
+    remoteIDs: ["id1", "id2"],
+    lastPingTime: Date.now(),
+    removeOutdatedLastPayload: true,
+  }, reporter._state._filename);
+
+  try {
+    yield reporter.init();
+
+    do_check_eq(reporter.lastSubmitID, "id1");
+    do_check_neq(reporter._state.clientID, null);
+    do_check_eq(reporter._state.clientID.length, 36);
+    do_check_eq(reporter._state.clientIDVersion, 1);
+
+    let clientID = reporter._state.clientID;
+
+    // The client ID should be persisted as soon as it is created.
+    reporter._shutdown();
+
+    reporter = getHealthReporter("state_create_client_id");
+    yield reporter.init();
+    do_check_eq(reporter._state.clientID, clientID);
+  } finally {
+    reporter._shutdown();
+  }
+});
+
 // Invalid stored client ID is reset automatically.
 add_task(function* test_empty_client_id() {
   let reporter = getHealthReporter("state_empty_client_id");

services/healthreport/tests/xpcshell/test_provider_appinfo.js

@@ -3,29 +3,16 @@
 
 "use strict";
 
-const {interfaces: Ci, results: Cr, utils: Cu, classes: Cc} = Components;
+const {interfaces: Ci, results: Cr, utils: Cu} = Components;
 
 Cu.import("resource://gre/modules/Metrics.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/services/healthreport/providers.jsm");
 Cu.import("resource://testing-common/services/healthreport/utils.jsm");
-Cu.import("resource://gre/modules/XPCOMUtils.jsm");
-
-XPCOMUtils.defineLazyGetter(this, "gDatareportingService",
-  () => Cc["@mozilla.org/datareporting/service;1"]
-          .getService(Ci.nsISupports)
-          .wrappedJSObject);
 
 
 function run_test() {
-  do_get_profile();
-
-  // Send the needed startup notifications to the datareporting service
-  // to ensure that it has been initialized.
-  gDatareportingService.observe(null, "app-startup", null);
-  gDatareportingService.observe(null, "profile-after-change", null);
-
   run_next_test();
 }