Bug 1151666 - Fix intermittent orange by reducing verified timer intervals and always using mock storage. r=zaach, a=test-only
authorMark Hammond <mhammond@skippinet.com.au>
Tue, 14 Apr 2015 10:35:48 +1000
changeset 258465 87f3453f6cc0
parent 258464 d1e3ce033c7a
child 258466 5ca4e237b259
push id4676
push userryanvm@gmail.com
push date2015-04-15 02:06 +0000
treeherdermozilla-beta@91df81e2edac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerszaach, test-only
bugs1151666
milestone38.0
Bug 1151666 - Fix intermittent orange by reducing verified timer intervals and always using mock storage. r=zaach, a=test-only
services/fxaccounts/FxAccounts.jsm
services/fxaccounts/tests/xpcshell/test_accounts.js
--- a/services/fxaccounts/FxAccounts.jsm
+++ b/services/fxaccounts/FxAccounts.jsm
@@ -335,16 +335,21 @@ function FxAccountsInternal() {
  */
 FxAccountsInternal.prototype = {
 
   /**
    * The current data format's version number.
    */
   version: DATA_FORMAT_VERSION,
 
+  // The timeout (in ms) we use to poll for a verified mail for the first 2 mins.
+  VERIFICATION_POLL_TIMEOUT_INITIAL: 5000, // 5 seconds
+  // And how often we poll after the first 2 mins.
+  VERIFICATION_POLL_TIMEOUT_SUBSEQUENT: 15000, // 15 seconds.
+
   _fxAccountsClient: null,
 
   get fxAccountsClient() {
     if (!this._fxAccountsClient) {
       this._fxAccountsClient = new FxAccountsClient();
     }
     return this._fxAccountsClient;
   },
@@ -854,17 +859,18 @@ FxAccountsInternal.prototype = {
         currentState.whenVerifiedDeferred.reject(error);
         delete currentState.whenVerifiedDeferred;
       }
       log.debug("polling session exceeded, giving up");
       return;
     }
     if (timeoutMs === undefined) {
       let currentMinute = Math.ceil(ageMs / 60000);
-      timeoutMs = 1000 * (currentMinute <= 2 ? 5 : 15);
+      timeoutMs = currentMinute <= 2 ? this.VERIFICATION_POLL_TIMEOUT_INITIAL
+                                     : this.VERIFICATION_POLL_TIMEOUT_SUBSEQUENT;
     }
     log.debug("polling with timeout = " + timeoutMs);
     this.currentTimer = setTimeout(() => {
       this.pollEmailStatus(currentState, sessionToken, "timer");
     }, timeoutMs);
   },
 
   _requireHttps: function() {
--- a/services/fxaccounts/tests/xpcshell/test_accounts.js
+++ b/services/fxaccounts/tests/xpcshell/test_accounts.js
@@ -20,17 +20,18 @@ initTestLogging("Trace");
 
 // XXX until bug 937114 is fixed
 Cu.importGlobalProperties(['atob']);
 
 let log = Log.repository.getLogger("Services.FxAccounts.test");
 log.level = Log.Level.Debug;
 
 // See verbose logging from FxAccounts.jsm
-Services.prefs.setCharPref("identity.fxaccounts.loglevel", "DEBUG");
+Services.prefs.setCharPref("identity.fxaccounts.loglevel", "Trace");
+Log.repository.getLogger("FirefoxAccounts").level = Log.Level.Trace;
 
 // The oauth server is mocked, but set these prefs to pass param checks
 Services.prefs.setCharPref("identity.fxaccounts.remote.oauth.uri", "https://example.com/v1");
 Services.prefs.setCharPref("identity.fxaccounts.oauth.client_id", "abc123");
 
 
 function run_test() {
   run_next_test();
@@ -48,25 +49,20 @@ function MockFxAccountsClient() {
   this._email = "nobody@example.com";
   this._verified = false;
   this._deletedOnServer = false; // for testing accountStatus
 
   // mock calls up to the auth server to determine whether the
   // user account has been verified
   this.recoveryEmailStatus = function (sessionToken) {
     // simulate a call to /recovery_email/status
-    let deferred = Promise.defer();
-
-    let response = {
+    return Promise.resolve({
       email: this._email,
       verified: this._verified
-    };
-    deferred.resolve(response);
-
-    return deferred.promise;
+    });
   };
 
   this.accountStatus = function(uid) {
     let deferred = Promise.defer();
     deferred.resolve(!!uid && (!this._deletedOnServer));
     return deferred.promise;
   };
 
@@ -114,16 +110,18 @@ MockStorage.prototype = Object.freeze({
 /*
  * We need to mock the FxAccounts module's interfaces to external
  * services, such as storage and the FxAccounts client.  We also
  * mock the now() method, so that we can simulate the passing of
  * time and verify that signatures expire correctly.
  */
 function MockFxAccounts() {
   return new FxAccounts({
+    VERIFICATION_POLL_TIMEOUT_INITIAL: 100, // 100ms
+
     _getCertificateSigned_calls: [],
     _d_signCertificate: Promise.defer(),
     _now_is: new Date(),
     signedInUserStorage: new MockStorage(),
     now: function () {
       return this._now_is;
     },
     getCertificateSigned: function (sessionToken, serializedPublicKey) {
@@ -159,29 +157,33 @@ add_test(function test_non_https_remote_
   }, "Firefox Accounts server must use HTTPS");
 
   Services.prefs.clearUserPref("identity.fxaccounts.remote.signup.uri");
 
   run_next_test();
 });
 
 add_task(function test_get_signed_in_user_initially_unset() {
-  // This test, unlike the rest, uses an un-mocked FxAccounts instance.
-  // However, we still need to pass an object to the constructor to
-  // force it to expose "internal", so we can test the disk storage.
-  let account = new FxAccounts({onlySetInternal: true})
+  // This test, unlike many of the the rest, uses a (largely) un-mocked
+  // FxAccounts instance.
+  // We do mock the storage to keep the test fast on b2g.
+  let account = new FxAccounts({
+    signedInUserStorage: new MockStorage(),
+  });
   let credentials = {
     email: "foo@example.com",
     uid: "1234@lcip.org",
     assertion: "foobar",
     sessionToken: "dead",
     kA: "beef",
     kB: "cafe",
     verified: true
   };
+  // and a sad hack to ensure the mocked storage is used for the initial reads.
+  account.internal.currentAccountState.signedInUserStorage = account.internal.signedInUserStorage;
 
   let result = yield account.getSignedInUser();
   do_check_eq(result, null);
 
   yield account.setSignedInUser(credentials);
 
   result = yield account.getSignedInUser();
   do_check_eq(result.email, credentials.email);
@@ -200,52 +202,55 @@ add_task(function test_get_signed_in_use
   let localOnly = true;
   yield account.signOut(localOnly);
 
   // user should be undefined after sign out
   result = yield account.getSignedInUser();
   do_check_eq(result, null);
 });
 
-add_task(function test_getCertificate() {
+add_task(function* test_getCertificate() {
   _("getCertificate()");
-  // This test, unlike the rest, uses an un-mocked FxAccounts instance.
-  // However, we still need to pass an object to the constructor to
-  // force it to expose "internal".
-  let fxa = new FxAccounts({onlySetInternal: true});
+  // This test, unlike many of the the rest, uses a (largely) un-mocked
+  // FxAccounts instance.
+  // We do mock the storage to keep the test fast on b2g.
+  let fxa = new FxAccounts({
+    signedInUserStorage: new MockStorage(),
+  });
   let credentials = {
     email: "foo@example.com",
     uid: "1234@lcip.org",
     assertion: "foobar",
     sessionToken: "dead",
     kA: "beef",
     kB: "cafe",
     verified: true
   };
+  // and a sad hack to ensure the mocked storage is used for the initial reads.
+  fxa.internal.currentAccountState.signedInUserStorage = fxa.internal.signedInUserStorage;
   yield fxa.setSignedInUser(credentials);
 
   // Test that an expired cert throws if we're offline.
   fxa.internal.currentAccountState.cert = {
     validUntil: Date.parse("Mon, 13 Jan 2000 21:45:06 GMT")
   };
   let offline = Services.io.offline;
   Services.io.offline = true;
   // This call would break from missing parameters ...
-  fxa.internal.currentAccountState.getCertificate().then(
+  yield fxa.internal.currentAccountState.getCertificate().then(
     result => {
       Services.io.offline = offline;
       do_throw("Unexpected success");
     },
     err => {
       Services.io.offline = offline;
       // ... so we have to check the error string.
       do_check_eq(err, "Error: OFFLINE");
     }
   );
-  _("----- DONE ----\n");
 });
 
 
 // Sanity-check that our mocked client is working correctly
 add_test(function test_client_mock() {
   do_test_pending();
 
   let fxa = new MockFxAccounts();