Bug 1148273 - have the readinglist scheduler handle FxA error states correctly. r=rnewman
authorMark Hammond <mhammond@skippinet.com.au>
Fri, 27 Mar 2015 16:41:48 +1100
changeset 266264 bf843cab375a04c7823cf8cac76b3f132555ca01
parent 266263 024efc08edd84682c4f56573de18718157f76782
child 266265 a752e16a5251cd9b047f41dcef33f1c8d8187517
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)
reviewersrnewman
bugs1148273
milestone39.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1148273 - have the readinglist scheduler handle FxA error states correctly. r=rnewman
browser/components/readinglist/Scheduler.jsm
--- a/browser/components/readinglist/Scheduler.jsm
+++ b/browser/components/readinglist/Scheduler.jsm
@@ -28,16 +28,22 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 // The main readinglist module.
 XPCOMUtils.defineLazyModuleGetter(this, 'ReadingList',
   'resource:///modules/readinglist/ReadingList.jsm');
 
 // The "engine"
 XPCOMUtils.defineLazyModuleGetter(this, 'Sync',
   'resource:///modules/readinglist/Sync.jsm');
 
+// FxAccountsCommon.js doesn't use a "namespace", so create one here.
+XPCOMUtils.defineLazyGetter(this, "fxAccountsCommon", function() {
+  let namespace = {};
+  Cu.import("resource://gre/modules/FxAccountsCommon.js", namespace);
+  return namespace;
+});
 
 this.EXPORTED_SYMBOLS = ["ReadingListScheduler"];
 
 // A list of "external" observer topics that may cause us to change when we
 // sync.
 const OBSERVERS = [
   // We don't sync when offline and restart when online.
   "network:offline-status-changed",
@@ -70,16 +76,17 @@ let intervals = {
 
 // 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",
     "FirefoxAccounts",
     "readinglist.api",
+    "readinglist.scheduler",
     "readinglist.serverclient",
     "readinglist.sync",
   ];
 
   this._logManager = new LogManager("readinglist.", logs, "readinglist");
   this.log = Log.repository.getLogger("readinglist.scheduler");
   this.log.info("readinglist scheduler created.")
   this.state = this.STATE_OK;
@@ -170,16 +177,18 @@ InternalScheduler.prototype = {
       case "readinglist:user-sync":
         this._syncNow();
         break;
       case "fxaccounts:onverified":
         // If we were in an authentication error state, reset that now.
         if (this.state == this.STATE_ERROR_AUTHENTICATION) {
           this.state = this.STATE_OK;
         }
+        // and sync now.
+        this._syncNow();
         break;
 
       // The rest just indicate that now is probably a good time to check if
       // we can sync as normal using whatever schedule was previously set.
       default:
         break;
     }
     // When observers fire we ignore the current sync error state as the
@@ -280,20 +289,34 @@ InternalScheduler.prototype = {
       // 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);
       return intervals.schedule;
     }).catch(err => {
-      this.log.error("Sync failed", err);
-      // XXX - how to detect an auth error?
-      this.state = err == this._engine.ERROR_AUTHENTICATION ?
+      // 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.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;
     }).then(nextDelay => {
       this._timerRunning = false;
       // ensure a new timer is setup for the appropriate next time.
       this._maybeReschedule(nextDelay);
       this._setupTimer();