Bug 623788 - Merge fx-sync to mozilla-central. a=blockers

services/sync/modules/constants.js

@@ -81,6 +81,9 @@ PARTIAL_DATA_SYNC:                     60 * 1000, // 1 minute
 // and the mobile sync interval.
 HMAC_EVENT_INTERVAL:                   600000,
 
+// How long to wait between sync attempts if the Master Password is locked.
+MASTER_PASSWORD_LOCKED_RETRY_INTERVAL: 15 * 60 * 1000,   // 15 minutes
+
 // 50 is hardcoded here because of URL length restrictions.
 // (GUIDs can be up to 64 chars long)
 MOBILE_BATCH_SIZE:                     50,
@@ -116,6 +119,7 @@ LOGIN_FAILED:                          "error.login.failed",
 SYNC_FAILED_PARTIAL:                   "error.sync.failed_partial",
 CLIENT_NOT_CONFIGURED:                 "service.client_not_configured",
 STATUS_DISABLED:                       "service.disabled",
+MASTER_PASSWORD_LOCKED:                "service.master_password_locked",
 
 // success states
 LOGIN_SUCCEEDED:                       "success.login",
@@ -161,6 +165,7 @@ JPAKE_ERROR_KEYMISMATCH:               "jpake.error.keymismatch",
 JPAKE_ERROR_WRONGMESSAGE:              "jpake.error.wrongmessage",
 
 // Ways that a sync can be disabled (messages only to be printed in debug log)
+kSyncMasterPasswordLocked:             "User elected to leave Master Password locked",
 kSyncWeaveDisabled:                    "Weave is disabled",
 kSyncNotLoggedIn:                      "User is not logged in",
 kSyncNetworkOffline:                   "Network is offline",

services/sync/modules/engines/history.js

@@ -41,6 +41,7 @@ const EXPORTED_SYMBOLS = ['HistoryEngine'];
 const Cc = Components.classes;
 const Ci = Components.interfaces;
 const Cu = Components.utils;
+const Cr = Components.results;
 
 const GUID_ANNO = "sync/guid";
 
@@ -428,7 +429,13 @@ HistoryStore.prototype = {
         Svc.History.addVisit(uri, date, null, type, type == 5 || type == 6, 0);
 
     if (record.title) {
-      this._hsvc.setPageTitle(uri, record.title);
+      try {
+        this._hsvc.setPageTitle(uri, record.title);
+      } catch (ex if ex.result == Cr.NS_ERROR_NOT_AVAILABLE) {
+        // There's no entry for the given URI, either because it's a
+        // URI that Places ignores (e.g. javascript:) or there were no
+        // visits.  We can just ignore those cases.
+      }
     }
   },
 

services/sync/modules/resource.js

@@ -465,7 +465,7 @@ ChannelListener.prototype = {
     } catch (ex) {
       this._log.warn("Got exception calling onProgress handler during fetch of "
                      + req.URI.spec);
-      this._log.debug(Utils.stackTrace(ex));
+      this._log.debug(Utils.exceptionStr(ex));
       this._log.trace("Rethrowing; expect a failure code from the HTTP channel.");
       throw ex;
     }

services/sync/modules/service.js

@@ -83,7 +83,6 @@ function WeaveSvc() {
 WeaveSvc.prototype = {
 
   _lock: Utils.lock,
-  _catch: Utils.catch,
   _locked: false,
   _loggedIn: false,
 
@@ -220,6 +219,20 @@ WeaveSvc.prototype = {
     this._locked = false;
   },
 
+  // A specialized variant of Utils.catch.
+  // This provides a more informative error message when we're already syncing:
+  // see Bug 616568.
+  _catch: function _catch(func) {
+    function lockExceptions(ex) {
+      if (Utils.isLockException(ex)) {
+        // This only happens if we're syncing already.
+        this._log.info("Cannot start sync: already syncing?");
+      }
+    }
+      
+    return Utils.catch.call(this, func, lockExceptions);
+  },
+
   _updateCachedURLs: function _updateCachedURLs() {
     // Nothing to cache yet if we don't have the building blocks
     if (this.clusterURL == "" || this.username == "")
@@ -825,9 +838,24 @@ WeaveSvc.prototype = {
         return false;
       }
 
+      // Unlock master password, or return.
       try {
         // Fetch collection info on every startup.
+        // Attaching auth credentials to a request requires access to
+        // passwords, which means that Resource.get can throw MP-related
+        // exceptions!
+        // Try to fetch the passphrase first, while we still have control.
+        try {
+          this.passphrase;
+        } catch (ex) {
+          this._log.debug("Fetching passphrase threw " + ex +
+                          "; assuming master password locked.");
+          Status.login = MASTER_PASSWORD_LOCKED;
+          return false;
+        }
+        
         let test = new Resource(this.infoURL).get();
+        
         switch (test.status) {
           case 200:
             // The user is authenticated.
@@ -1065,6 +1093,11 @@ WeaveSvc.prototype = {
       this._log.info("Logging in user " + this.username);
 
       if (!this.verifyLogin()) {
+        if (Status.login == MASTER_PASSWORD_LOCKED) {
+          // Drat.
+          this._log.debug("Login failed: " + Status.login);
+          return false;
+        }
         // verifyLogin sets the failure states here.
         throw "Login failed: " + Status.login;
       }
@@ -1361,6 +1394,9 @@ WeaveSvc.prototype = {
       reason = kSyncNetworkOffline;
     else if (Status.minimumNextSync > Date.now())
       reason = kSyncBackoffNotMet;
+    else if ((Status.login == MASTER_PASSWORD_LOCKED) &&
+             Utils.mpLocked())
+      reason = kSyncMasterPasswordLocked;
     else if (!this._loggedIn)
       reason = kSyncNotLoggedIn;
     else if (Svc.Prefs.get("firstSync") == "notReady")
@@ -1376,6 +1412,8 @@ WeaveSvc.prototype = {
    * Remove any timers/observers that might trigger a sync
    */
   _clearSyncTriggers: function _clearSyncTriggers() {
+    this._log.debug("Clearing sync triggers.");
+    
     // Clear out any scheduled syncs
     if (this._syncTimer)
       this._syncTimer.clear();
@@ -1400,8 +1438,10 @@ WeaveSvc.prototype = {
     
     // We're ready to sync even if we're not logged in... so long as the 
     // master password isn't locked.
-    if (Utils.mpLocked())
+    if (Utils.mpLocked()) {
       ignore.push(kSyncNotLoggedIn);
+      ignore.push(kSyncMasterPasswordLocked);
+    }
     
     let skip = this._checkSync(ignore);
     this._log.trace("_checkSync returned \"" + skip + "\".");
@@ -1425,9 +1465,19 @@ WeaveSvc.prototype = {
    * delay is optional
    */
   syncOnIdle: function WeaveSvc_syncOnIdle(delay) {
-    // No need to add a duplicate idle observer
+    // No need to add a duplicate idle observer, and no point if we got kicked
+    // out by the master password dialog.
+    if (Status.login == MASTER_PASSWORD_LOCKED &&
+        Utils.mpLocked()) {
+      this._log.debug("Not syncing on idle: Login status is " + Status.login);
+      
+      // If we're not syncing now, we need to schedule the next one.
+      this._scheduleAtInterval(MASTER_PASSWORD_LOCKED_RETRY_INTERVAL);
+      return false;
+    }
+
     if (this._idleTime)
-      return;
+      return false;
 
     this._idleTime = delay || IDLE_TIME;
     this._log.debug("Idle timer created for sync, will sync after " +
@@ -1451,8 +1501,8 @@ WeaveSvc.prototype = {
 
     // Start the sync right away if we're already late
     if (interval <= 0) {
-      this._log.debug("Syncing as soon as we're idle.");
-      this.syncOnIdle();
+      if (this.syncOnIdle())
+        this._log.debug("Syncing as soon as we're idle.");
       return;
     }
 
@@ -1536,6 +1586,22 @@ WeaveSvc.prototype = {
     Utils.delay(function() this._doHeartbeat(), interval, this, "_heartbeatTimer");
   },
 
+  /**
+   * Incorporates the backoff/retry logic used in error handling and elective
+   * non-syncing.
+   */
+  _scheduleAtInterval: function _scheduleAtInterval(minimumInterval) {
+    const MINIMUM_BACKOFF_INTERVAL = 15 * 60 * 1000;     // 15 minutes
+    let interval = this._calculateBackoff(this._syncErrors, MINIMUM_BACKOFF_INTERVAL);
+    if (minimumInterval)
+      interval = Math.max(minimumInterval, interval);
+
+    let d = new Date(Date.now() + interval);
+    this._log.config("Starting backoff, next sync at:" + d.toString());
+
+    this._scheduleNextSync(interval);
+  },
+  
   _syncErrors: 0,
   /**
    * Deal with sync errors appropriately
@@ -1551,37 +1617,38 @@ WeaveSvc.prototype = {
       }
       Status.enforceBackoff = true;
     }
-
-    const MINIMUM_BACKOFF_INTERVAL = 15 * 60 * 1000;     // 15 minutes
-    let interval = this._calculateBackoff(this._syncErrors, MINIMUM_BACKOFF_INTERVAL);
-
-    this._scheduleNextSync(interval);
-
-    let d = new Date(Date.now() + interval);
-    this._log.config("Starting backoff, next sync at:" + d.toString());
+    
+    this._scheduleAtInterval();
   },
 
-  // Call _lockedSync with a specialized variant of Utils.catch.
-  // This provides a more informative error message when we're already syncing:
-  // see Bug 616568.
+  _skipScheduledRetry: function _skipScheduledRetry() {
+    return [LOGIN_FAILED_INVALID_PASSPHRASE,
+            LOGIN_FAILED_LOGIN_REJECTED].indexOf(Status.login) == -1;
+  },
+  
   sync: function sync() {
-    try {
+    this._log.debug("In wrapping sync().");
+    this._catch(function () {
       // Make sure we're logged in.
       if (this._shouldLogin()) {
-        this._log.trace("In sync: should login.");
-        this.login();
+        this._log.debug("In sync: should login.");
+        if (!this.login()) {
+          this._log.debug("Not syncing: login returned false.");
+          this._clearSyncTriggers();    // No more pending syncs, please.
+          
+          // Try again later, just as if we threw an error... only without the
+          // error count.
+          if (!this._skipScheduledRetry())
+            this._scheduleAtInterval(MASTER_PASSWORD_LOCKED_RETRY_INTERVAL);
+    
+          return;
+        }
       }
       else {
         this._log.trace("In sync: no need to login.");
       }
       return this._lockedSync.apply(this, arguments);
-    } catch (ex) {
-      this._log.debug("Exception: " + Utils.exceptionStr(ex));
-      if (Utils.isLockException(ex)) {
-        // This only happens if we're syncing already.
-        this._log.info("Cannot start sync: already syncing?");
-      }
-    }
+    })();
   },
   
   /**

services/sync/modules/util.js

@@ -89,8 +89,11 @@ let Utils = {
    *
    * @usage MyObj._catch = Utils.catch;
    *        MyObj.foo = function() { this._catch(func)(); }
+   *        
+   * Optionally pass a function which will be called if an
+   * exception occurs.
    */
-  catch: function Utils_catch(func) {
+  catch: function Utils_catch(func, exceptionCallback) {
     let thisArg = this;
     return function WrappedCatch() {
       try {
@@ -98,6 +101,10 @@ let Utils = {
       }
       catch(ex) {
         thisArg._log.debug("Exception: " + Utils.exceptionStr(ex));
+        if (exceptionCallback) {
+          return exceptionCallback.call(thisArg, ex);
+        }
+        return null;
       }
     };
   },
@@ -125,7 +132,7 @@ let Utils = {
   },
   
   isLockException: function isLockException(ex) {
-    return ex && (ex.indexOf("Could not acquire lock.") == 0);
+    return ex && ex.indexOf && ex.indexOf("Could not acquire lock.") == 0;
   },
 
   /**

services/sync/tests/unit/test_history_store.js

@@ -156,6 +156,19 @@ function run_test() {
     let result = Utils.queryAsync(stmt);    
     do_check_eq([id for (id in store.getAllIDs())].length, 4);
 
+    _("Make sure we handle records with javascript: URLs gracefully.");
+    store.create({id: Utils.makeGUID(),
+                  histUri: "javascript:''",
+                  title: "javascript:''",
+                  visits: [{date: TIMESTAMP3,
+                            type: Ci.nsINavHistoryService.TRANSITION_EMBED}]});
+
+    _("Make sure we handle records without any visits gracefully.");
+    store.create({id: Utils.makeGUID(),
+                  histUri: "http://getfirebug.com",
+                  title: "Get Firebug!",
+                  visits: []});
+
     _("Remove a record from the store.");
     store.remove({id: fxguid});
     do_check_false(store.itemExists(fxguid));

services/sync/tests/unit/test_service_login.js

@@ -174,6 +174,43 @@ function run_test() {
     
     // TODO: need better tests around master password prompting. See Bug 620583.
 
+    mpLocked = true;
+    
+    // Testing exception handling if master password dialog is canceled.
+    // Do this by stubbing out Service.passphrase.
+    let oldPP = Service.__lookupGetter__("passphrase");
+    _("Old passphrase function is " + oldPP);
+    Service.__defineGetter__("passphrase",
+                           function() {
+                             throw "User canceled Master Password entry";
+                           });
+    
+    let oldClearSyncTriggers = Service._clearSyncTriggers;
+    let oldLockedSync = Service._lockedSync;
+    
+    let cSTCalled = false;
+    let lockedSyncCalled = false;
+    
+    Service._clearSyncTriggers = function() { cSTCalled = true; };
+    Service._lockedSync = function() { lockedSyncCalled = true; };
+    
+    _("If master password is canceled, login fails and we report lockage.");
+    do_check_false(!!Service.login());
+    do_check_eq(Status.login, MASTER_PASSWORD_LOCKED);
+    do_check_eq(Status.service, LOGIN_FAILED);
+    _("Locked? " + Utils.mpLocked());
+    _("checkSync reports the correct term.");
+    do_check_eq(Service._checkSync(), kSyncMasterPasswordLocked);
+    
+    _("Sync doesn't proceed and clears triggers if MP is still locked.");
+    Service.sync();
+    
+    do_check_true(cSTCalled);
+    do_check_false(lockedSyncCalled);
+
+    // N.B., a bunch of methods are stubbed at this point. Be careful putting
+    // new tests after this point!
+    
   } finally {
     Svc.Prefs.resetBranch("");
     server.stop(do_test_finished);

services/sync/tests/unit/test_utils_catch.js

@@ -1,13 +1,17 @@
-_("Make sure catch when copied to an object will correctly catch stuff");
 Cu.import("resource://services-sync/util.js");
+Cu.import("resource://services-sync/service.js");
 
 function run_test() {
-  let ret, rightThis, didCall, didThrow;
+  _("Make sure catch when copied to an object will correctly catch stuff");
+  let ret, rightThis, didCall, didThrow, wasTen, wasLocked;
   let obj = {
     catch: Utils.catch,
     _log: {
       debug: function(str) {
         didThrow = str.search(/^Exception: /) == 0;
+      },
+      info: function(str) {
+        wasLocked = str.indexOf("Cannot start sync: already syncing?") == 0;
       }
     },
 
@@ -21,22 +25,62 @@ function run_test() {
       rightThis = this == obj;
       didCall = true;
       throw 10;
+    })(),
+    
+    callbacky: function() this.catch(function() {
+      rightThis = this == obj;
+      didCall = true;
+      throw 10;
+    }, function(ex) {
+      wasTen = (ex == 10)
+    })(),
+    
+    lockedy: function() this.catch(function() {
+      rightThis = this == obj;
+      didCall = true;
+      throw("Could not acquire lock.");
     })()
   };
 
   _("Make sure a normal call will call and return");
-  rightThis = didCall = didThrow = false;
+  rightThis = didCall = didThrow = wasLocked = false;
   ret = obj.func();
   do_check_eq(ret, 5);
   do_check_true(rightThis);
   do_check_true(didCall);
   do_check_false(didThrow);
+  do_check_eq(wasTen, undefined);
+  do_check_false(wasLocked);
 
   _("Make sure catch/throw results in debug call and caller doesn't need to handle exception");
-  rightThis = didCall = didThrow = false;
+  rightThis = didCall = didThrow = wasLocked = false;
   ret = obj.throwy();
   do_check_eq(ret, undefined);
   do_check_true(rightThis);
   do_check_true(didCall);
   do_check_true(didThrow);
+  do_check_eq(wasTen, undefined);
+  do_check_false(wasLocked);
+  
+  _("Test callback for exception testing.");
+  rightThis = didCall = didThrow = wasLocked = false;
+  ret = obj.callbacky();
+  do_check_eq(ret, undefined);
+  do_check_true(rightThis);
+  do_check_true(didCall);
+  do_check_true(didThrow);
+  do_check_true(wasTen);
+  do_check_false(wasLocked);
+  
+  _("Test the lock-aware catch that Service uses.");
+  obj.catch = Service._catch;
+  rightThis = didCall = didThrow = wasLocked = false;
+  wasTen = undefined;
+  ret = obj.lockedy();
+  do_check_eq(ret, undefined);
+  do_check_true(rightThis);
+  do_check_true(didCall);
+  do_check_true(didThrow);
+  do_check_eq(wasTen, undefined);
+  do_check_true(wasLocked);
 }