Merge f-t to m-c, a=merge
authorPhil Ringnalda <philringnalda@gmail.com>
Sat, 28 Mar 2015 21:33:32 -0700
changeset 266703 385840329d91d1b469111b1e0471d41e36e257be
parent 266696 02f2f4c75007651c63bbc0791d9a58dea88f545f (current diff)
parent 266702 b34ca540cf734796091643dad3c26978814bdef6 (diff)
child 266704 df57b44c69248e829611511e19838b35c42ee7aa
child 266707 671d6f9f0d85294fd1211f218c88e786a3cd9c7d
child 266731 880c836bd395fd012ef8831ce99ce636a77d1bea
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmerge
milestone39.0a1
first release with
nightly linux32
385840329d91 / 39.0a1 / 20150329030238 / files
nightly linux64
385840329d91 / 39.0a1 / 20150329030238 / files
nightly mac
385840329d91 / 39.0a1 / 20150329030238 / files
nightly win32
385840329d91 / 39.0a1 / 20150329030238 / files
nightly win64
385840329d91 / 39.0a1 / 20150329030238 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Merge f-t to m-c, a=merge
--- a/browser/components/readinglist/Scheduler.jsm
+++ b/browser/components/readinglist/Scheduler.jsm
@@ -65,17 +65,17 @@ let intervals = {
     // All pref values are seconds, but we return ms.
     return prefs.get(prefName, def) * 1000;
   },
 
   // How long after startup do we do an initial sync?
   get initial() this._fixupIntervalPref("initial", 10), // 10 seconds.
   // Every interval after the first.
   get schedule() this._fixupIntervalPref("schedule", 2 * 60 * 60), // 2 hours
-  // After an error
+  // Initial retry after an error (exponentially backed-off to .schedule)
   get retry() this._fixupIntervalPref("retry", 2 * 60), // 2 mins
 };
 
 // This is the implementation, but it's not exposed directly.
 function InternalScheduler(readingList = null) {
   // oh, I don't know what logs yet - let's guess!
   let logs = [
     "browserwindow.syncui",
@@ -106,16 +106,19 @@ InternalScheduler.prototype = {
   _backoffUntil: 0,
   // Our current timer.
   _timer: null,
   // Our timer fires a promise - _timerRunning is true until it resolves or
   // rejects.
   _timerRunning: false,
   // Our sync engine - XXX - maybe just a callback?
   _engine: Sync,
+  // Our current "error backoff" timeout. zero if no error backoff is in
+  // progress and incremented after successive errors until a max is reached.
+  _currentErrorBackoff: 0,
 
   // Our state variable and constants.
   state: null,
   STATE_OK: "ok",
   STATE_ERROR_AUTHENTICATION: "authentication error",
   STATE_ERROR_OTHER: "other error",
 
   init() {
@@ -287,39 +290,44 @@ InternalScheduler.prototype = {
     this._engine.start().then(() => {
       this.log.info("Sync completed successfully");
       // Write a pref in the same format used to services/sync to indicate
       // the last success.
       prefs.set("lastSync", new Date().toString());
       this.state = this.STATE_OK;
       this._logManager.resetFileLog(this._logManager.REASON_SUCCESS);
       Services.obs.notifyObservers(null, "readinglist:sync:finish", null);
+      this._currentErrorBackoff = 0; // error retry interval is reset on success.
       return intervals.schedule;
     }).catch(err => {
       // This isn't ideal - we really should have _canSync() check this - but
       // that requires a refactor to turn _canSync() into a promise-based
       // function.
       if (err.message == fxAccountsCommon.ERROR_NO_ACCOUNT ||
           err.message == fxAccountsCommon.ERROR_UNVERIFIED_ACCOUNT) {
         // make everything look like success.
+        this._currentErrorBackoff = 0; // error retry interval is reset on success.
         this.log.info("Can't sync due to FxA account state " + err.message);
         this.state = this.STATE_OK;
         this._logManager.resetFileLog(this._logManager.REASON_SUCCESS);
         Services.obs.notifyObservers(null, "readinglist:sync:finish", null);
         // it's unfortunate that we are probably going to hit this every
         // 2 hours, but it should be invisible to the user.
         return intervals.schedule;
       }
       this.state = err.message == fxAccountsCommon.ERROR_AUTH_ERROR ?
                    this.STATE_ERROR_AUTHENTICATION : this.STATE_ERROR_OTHER;
       this.log.error("Sync failed, now in state '${state}': ${err}",
                      {state: this.state, err});
       this._logManager.resetFileLog(this._logManager.REASON_ERROR);
       Services.obs.notifyObservers(null, "readinglist:sync:error", null);
-      return intervals.retry;
+      // We back-off on error retries until it hits our normally scheduled interval.
+      this._currentErrorBackoff = this._currentErrorBackoff == 0 ? intervals.retry :
+                                  Math.min(intervals.schedule, this._currentErrorBackoff * 2);
+      return this._currentErrorBackoff;
     }).then(nextDelay => {
       this._timerRunning = false;
       // ensure a new timer is setup for the appropriate next time.
       this._maybeReschedule(nextDelay);
       this._setupTimer();
       this._onAutoReschedule(); // just for tests...
     }).catch(err => {
       // We should never get here, but better safe than sorry...
--- a/browser/components/readinglist/test/xpcshell/test_scheduler.js
+++ b/browser/components/readinglist/test/xpcshell/test_scheduler.js
@@ -169,11 +169,86 @@ add_task(function* testAuthError() {
 add_task(function* testBackoff() {
   let scheduler = createScheduler({expectedDelay: 1000});
   Services.obs.notifyObservers(null, "readinglist:backoff-requested", 1000);
   // XXX - this needs a little love as nothing checks createScheduler actually
   // made the checks we think it does.
   scheduler.finalize();
 });
 
+add_task(function testErrorBackoff() {
+  // This test can't sanely use the "test scheduler" above, so make one more
+  // suited.
+  let rlMock = new ReadingListMock();
+  let scheduler = createTestableScheduler(rlMock);
+  scheduler._setTimeout = function(delay) {
+    // create a timer that fires immediately
+    return setTimeout(() => scheduler._doSync(), 0);
+  }
+
+  // This does all the work...
+  function checkBackoffs(expectedSequences) {
+    let orig_maybeReschedule = scheduler._maybeReschedule;
+    return new Promise(resolve => {
+      let isSuccess = true; // ie, first run will put us in "fail" mode.
+      let expected;
+      function nextSequence() {
+        if (expectedSequences.length == 0) {
+          resolve();
+          return true; // we are done.
+        }
+        // setup the current set of expected results.
+        expected = expectedSequences.shift()
+        // and toggle the success status of the engine.
+        isSuccess = !isSuccess;
+        if (isSuccess) {
+          scheduler._engine.start = Promise.resolve;
+        } else {
+          scheduler._engine.start = () => {
+            return Promise.reject(new Error("oh no"))
+          }
+        }
+        return false; // not done.
+      };
+      // get the first sequence;
+      nextSequence();
+      // and setup the scheduler to check the sequences.
+      scheduler._maybeReschedule = function(nextDelay) {
+        let thisExpected = expected.shift();
+        equal(thisExpected * 1000, nextDelay);
+        if (expected.length == 0) {
+          if (nextSequence()) {
+            // we are done, so do nothing.
+            return;
+          }
+        }
+        // call the original impl to get the next schedule.
+        return orig_maybeReschedule.call(scheduler, nextDelay);
+      }
+    });
+  }
+
+  prefs.set("schedule", 100);
+  prefs.set("retry", 5);
+  // The sequences of timeouts we expect as the Sync error state changes.
+  let backoffsChecked = checkBackoffs([
+    // first sequence is in failure mode - expect the timeout to double until 'schedule'
+    [5, 10, 20, 40, 80, 100, 100],
+    // Sync just started working - more 'schedule'
+    [100, 100],
+    // Just stopped again - error backoff process restarts.
+    [5, 10],
+    // Another success and we are back to 'schedule'
+    [100, 100],
+  ]);
+
+  // fire things off.
+  scheduler.init();
+
+  // and wait for completion.
+  yield backoffsChecked;
+
+  scheduler.finalize();
+});
+
 function run_test() {
   run_next_test();
 }
--- a/toolkit/components/passwordmgr/LoginManagerContent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ -455,23 +455,28 @@ var LoginManagerContent = {
           usernameField = element;
           break;
         }
       }
     }
 
     if (!usernameField)
       log("(form -- no username field found)");
-
+    else
+      log("Username field id/name/value is: ", usernameField.id, " / ",
+          usernameField.name, " / ", usernameField.value);
 
     // If we're not submitting a form (it's a page load), there are no
     // password field values for us to use for identifying fields. So,
     // just assume the first password field is the one to be filled in.
-    if (!isSubmission || pwFields.length == 1)
-      return [usernameField, pwFields[0].element, null];
+    if (!isSubmission || pwFields.length == 1) {
+      var passwordField = pwFields[0].element;
+      log("Password field id/name is: ", passwordField.id, " / ", passwordField.name);
+      return [usernameField, passwordField, null];
+    }
 
 
     // Try to figure out WTF is in the form based on the password values.
     var oldPasswordField, newPasswordField;
     var pw1 = pwFields[0].element.value;
     var pw2 = pwFields[1].element.value;
     var pw3 = (pwFields[2] ? pwFields[2].element.value : null);
 
@@ -504,16 +509,18 @@ var LoginManagerContent = {
         oldPasswordField = null;
       } else {
         // Just assume that the 2nd password is the new password
         oldPasswordField = pwFields[0].element;
         newPasswordField = pwFields[1].element;
       }
     }
 
+    log("Password field (new) id/name is: ", newPasswordField.id, " / ", newPasswordField.name);
+    log("Password field (old) id/name is: ", oldPasswordField.id, " / ", oldPasswordField.name);
     return [usernameField, newPasswordField, oldPasswordField];
   },
 
 
   /*
    * _isAutoCompleteDisabled
    *
    * Returns true if the page requests autocomplete be disabled for the
--- a/toolkit/components/passwordmgr/LoginRecipes.jsm
+++ b/toolkit/components/passwordmgr/LoginRecipes.jsm
@@ -231,11 +231,16 @@ const DEFAULT_RECIPES = {
       "description": "okta uses a hidden password field to disable filling",
       "hosts": ["mozilla.okta.com"],
       "passwordSelector": "#pass-signin"
     },
     {
       "description": "anthem uses a hidden password and username field to disable filling",
       "hosts": ["www.anthem.com"],
       "passwordSelector": "#LoginContent_txtLoginPass"
+    },
+    {
+      "description": "An ephemeral password-shim field is incorrectly selected as the username field.",
+      "hosts": ["www.discover.com"],
+      "usernameSelector": "#login-account"
     }
   ]
 };