locking fixes (still commented out); better error checking; fail if the remote format version is higher than we can read; refactor generator code, bring back asyncRun()

services/sync/nsBookmarksSyncService.js

@@ -272,6 +272,7 @@ BookmarksSyncService.prototype = {
     items[guid] = item;
   },
 
+  // FIXME: this won't work for deep objects, or objects with optional properties
   _getEdits: function BSS__getEdits(a, b) {
     // check the type separately, just in case
     if (a.type != b.type)
@@ -447,13 +448,12 @@ BookmarksSyncService.prototype = {
   // [[[dcA1, dcB1], [dcA2. dcB2], ...],
   //  [2dcA1, 2dcA2, ...], [2dcB1, 2dcB2, ...]]
 
-  _reconcile: function BSS__reconcile(onComplete, commandLists) {
+  _reconcile: function BSS__reconcile(onComplete, listA, listB) {
     let generator = yield;
     this._timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
-    let handlers = this._handlersForGenerator(generator);
+    let handlers = generatorHandlers(generator);
     let listener = new EventListener(handlers['complete']);
 
-    let [listA, listB] = commandLists;
     let propagations = [[], []];
     let conflicts = [[], []];
 
@@ -514,7 +514,7 @@ BookmarksSyncService.prototype = {
 
     this._timer = null;
     let ret = {propagations: propagations, conflicts: conflicts};
-    this._generatorDone(onComplete, ret);
+    generatorDone(this, onComplete, ret);
 
     // shutdown protocol
     let cookie = yield;
@@ -724,24 +724,43 @@ BookmarksSyncService.prototype = {
 
   _doSync: function BSS__doSync() {
     var generator = yield;
-    var handlers = this._handlersForGenerator(generator);
+    var handlers = generatorHandlers(generator);
 
     this._os.notifyObservers(null, "bookmarks-sync:start", "");
     this.notice("Beginning sync");
 
     try {
-      //this._dav.lock(handlers);
+//      if (!asyncRun(this._dav, this._dav.lock, handlers.complete))
-      //var data = yield;
+//        return;
-      var data;
+//      let locked = yield;
+//
+//      if (locked)
+//        this.notice("Lock acquired");
+//      else {
+//        this.notice("Could not acquire lock, aborting");
+// FIXME
+//        this.notice("Could not acquire lock, attempting to steal it");
+//
+//        if (!asyncRun(this._dav, this._dav.stealLock, handlers.complete))
+//          return;
+//        let stolen = yield;
+//
+//        if (stolen)
+//          this.notice("Lock stolen");
+//        else {
+//          this.notice("Could not steal lock, aborting");
+//          return;
+//        }
+//      }
 
       var localJson = this._getBookmarks();
       //this.notice("local json:\n" + this._mungeNodes(localJson));
 
       // 1) Fetch server deltas
-      let gsd_gen = this._getServerData(handlers['complete'], localJson);
+      if (asyncRun(this, this._getServerData, handlers.complete, localJson))
-      gsd_gen.next(); // must initialize before sending
-      gsd_gen.send(gsd_gen);
         let server = yield;
+      else
+        return
 
       this.notice("Local snapshot version: " + this._snapshotVersion);
       this.notice("Server status: " + server.status);
@@ -757,12 +776,6 @@ BookmarksSyncService.prototype = {
       this.notice("Server snapVersion: " + server.snapVersion);
       this.notice("Server updates: " + this._mungeCommands(server.updates));
 
-      // if (server['status'] == 2) {
-      //   this._os.notifyObservers(null, "bookmarks-sync:end", "");
-      //   this.notice("Sync complete");
-      //   return;
-      // } else 
-
       // 2) Generate local deltas from snapshot -> current client status
 
       let localUpdates = this._detectUpdates(this._snapshot, localJson);
@@ -778,13 +791,13 @@ BookmarksSyncService.prototype = {
       // 3) Reconcile client/server deltas and generate new deltas for them.
 
       this.notice("Reconciling client/server updates");
-      let rec_gen = this._reconcile(handlers.load,
+      if (asyncRun(this, this._reconcile, handlers.complete,
-                                    [localUpdates, server.updates]);
+                   localUpdates, server.updates))
-      rec_gen.next(); // must initialize before sending
-      rec_gen.send(rec_gen);
         let ret = yield;
+      else
+        return
       // FIXME: Need to come up with a closing protocol for generators
-      rec_gen.close();
+      //rec_gen.close();
 
       let clientChanges = [];
       let serverChanges = [];
@@ -873,8 +886,14 @@ BookmarksSyncService.prototype = {
       this._os.notifyObservers(null, "bookmarks-sync:end", "");
       this.notice("Sync complete");
     } finally {
-      //this._dav.unlock(handlers);
+// FIXME
-      //data = yield;
+//      if (!asyncRun(this._dav, this._dav.unlock, handlers.complete))
+//        return;
+//      let unlocked = yield;
+//      if (unlocked)
+//        this.notice("Lock released");
+//      else
+//        this.notice("Error: could not unlock DAV collection");
       this._os.notifyObservers(null, "bookmarks-sync:end", "");
     }
   },
@@ -899,11 +918,9 @@ BookmarksSyncService.prototype = {
    *     the relevant deltas (from our snapshot version to current),
    *     combined into a single set.
    */
-  // FIXME: this function needs to get check the remote format version and bail out earlier if needed
-  // FIXME: deal with errors!
   _getServerData: function BSS__getServerData(onComplete, localJson) {
     let generator = yield;
-    let handlers = this._handlersForGenerator(generator);
+    let handlers = generatorHandlers(generator);
 
     let ret = {status: -1,
                formatVersion: null, maxVersion: null, snapVersion: null,
@@ -920,6 +937,14 @@ BookmarksSyncService.prototype = {
       let status = eval(statusResp.target.responseText);
       let snap, deltas, allDeltas;
 
+      // Bail out if the server has a newer format version than we can parse
+      if (status.formatVersion > STORAGE_FORMAT_VERSION) {
+        this.notice("Error: server uses storage format v" + status.formatVersion +
+                    ", this client understands up to v" + STORAGE_FORMAT_VERSION);
+        generatorDone(this, onComplete, ret)
+        return;
+      }
+
       if (status.guid != this._snapshotGuid) {
         this.notice("Remote/local sync guids do not match.  " +
                     "Forcing initial sync.");
@@ -935,19 +960,22 @@ BookmarksSyncService.prototype = {
         this.notice("Downloading server snapshot");
         this._dav.GET("bookmarks-snapshot.json", handlers);
         let snapResp = yield;
-        if (snapResp.target.status == 200)
+        if (snapResp.target.status < 200 || snapResp.target.status >= 300) {
-          snap = eval(snapResp.target.responseText);
-        else
           this.notice("Error: could not download server snapshot");
+          generatorDone(this, onComplete, ret)
+          return;
+        }
+        snap = eval(snapResp.target.responseText);
 
         this.notice("Downloading server deltas");
         this._dav.GET("bookmarks-deltas.json", handlers);
         let deltasResp = yield;
-        if (deltasResp.target.status == 200)
+        if (deltasResp.target.status < 200 || deltasResp.target.status >= 300) {
-          allDeltas = eval(deltasResp.target.responseText);
-        else
           this.notice("Error: could not download server deltas");
-
+          generatorDone(this, onComplete, ret)
+          return;
+        }
+        allDeltas = eval(deltasResp.target.responseText);
         deltas = eval(uneval(allDeltas));
 
       } else if (this._snapshotVersion >= status.snapVersion &&
@@ -957,13 +985,13 @@ BookmarksSyncService.prototype = {
         this.notice("Downloading server deltas");
         this._dav.GET("bookmarks-deltas.json", handlers);
         let deltasResp = yield;
-        if (deltasResp.target.status == 200)
+        if (deltasResp.target.status < 200 || deltasResp.target.status >= 300) {
-          allDeltas = eval(deltasResp.target.responseText);
-        else
           this.notice("Error: could not download server deltas");
-
+          generatorDone(this, onComplete, ret)
-        let start = this._snapshotVersion - status.snapVersion;
+          return;
-        deltas = allDeltas.slice(start);
+        }
+        allDeltas = eval(deltasResp.target.responseText);
+        deltas = allDeltas.slice(this._snapshotVersion - status.snapVersion);
 
       } else if (this._snapshotVersion == status.maxVersion) {
         snap = eval(uneval(this._snapshot));
@@ -972,16 +1000,18 @@ BookmarksSyncService.prototype = {
         this.notice("Downloading server deltas");
         this._dav.GET("bookmarks-deltas.json", handlers);
         let deltasResp = yield;
-        if (deltasResp.target.status == 200)
+        if (deltasResp.target.status < 200 || deltasResp.target.status >= 300) {
-          allDeltas = eval(deltasResp.target.responseText);
-        else
           this.notice("Error: could not download server deltas");
-
+          generatorDone(this, onComplete, ret)
+          return;
+        }
+        allDeltas = eval(deltasResp.target.responseText);
         deltas = [];
 
       } else { // this._snapshotVersion > status.maxVersion
         this.notice("Error: server snapshot is older than local snapshot");
-        // FIXME: eep?
+        generatorDone(this, onComplete, ret)
+        return;
       }
 
       for (var i = 0; i < deltas.length; i++) {
@@ -1004,14 +1034,22 @@ BookmarksSyncService.prototype = {
       this._snapshotVersion = 0;
       this._snapshotGuid = null; // in case there are other snapshots out there
 
-      // FIXME: hmm check status for each and bail out earlier on error?
-
       this._dav.PUT("bookmarks-snapshot.json",
                     uneval(this._snapshot), handlers);
       let snapPut = yield;
+      if (snapPut.target.status < 200 || snapPut.target.status >= 300) {
+        this.notice("Error: could not upload snapshot to server");
+        generatorDone(this, onComplete, ret)
+        return;
+      }
 
       this._dav.PUT("bookmarks-deltas.json", uneval([]), handlers);
       let deltasPut = yield;
+      if (deltasPut.target.status < 200 || deltasPut.target.status >= 300) {
+        this.notice("Error: could not upload deltas to server");
+        generatorDone(this, onComplete, ret)
+        return;
+      }
 
       this._dav.PUT("bookmarks-status.json",
                     uneval({guid: this._snapshotGuid,
@@ -1019,16 +1057,14 @@ BookmarksSyncService.prototype = {
                             snapVersion: this._snapshotVersion,
                             maxVersion: this._snapshotVersion}), handlers);
       let statusPut = yield;
+      if (statusPut.target.status < 200 || statusPut.target.status >= 300) {
+        this.notice("Error: could not upload status file to server");
+        generatorDone(this, onComplete, ret)
+        return;
+      }
 
-      if (snapPut.target.status >= 200 && snapPut.target.status < 300 &&
-          deltasPut.target.status >= 200 && deltasPut.target.status < 300 &&
-          statusPut.target.status >= 200 && statusPut.target.status < 300) {
       this.notice("Initial upload to server successful");
       this._saveSnapshot();
-      } else {
-        // FIXME: eep?
-        this.notice("Error: could not upload files to server");
-      }
 
       ret.status = 0;
       ret.formatVersion = STORAGE_FORMAT_VERSION;
@@ -1044,35 +1080,7 @@ BookmarksSyncService.prototype = {
                   statusResp.target.status);
       break;
     }
-    this._generatorDone(onComplete, ret)
+    generatorDone(this, onComplete, ret)
-
-  },
-
-  _handlersForGenerator: function BSS__handlersForGenerator(generator) {
-    var h = {load: bind2(this, function(data) { continueGenerator(generator, data); }),
-             error: bind2(this, function(data) { this.notice("Request failed: " + uneval(data)); })};
-    h['complete'] = h['load'];
-    return h;
-  },
-
-  // generators called using asyncRun can't simply call the callback
-  // with the return value, since that would cause the calling
-  // function to end up running (after the yield) from inside the
-  // generator.  Instead, generators can call this method which sets
-  // up a timer to call the callback from a timer (and cleans up the
-  // timer to avoid leaks).
-  _generatorDone: function BSS__generatorDone(callback, retval) {
-    if (this._timer)
-      throw "Called generatorDone when there is a timer already set."
-
-    let cb = bind2(this, function(event) {
-      this._timer = null;
-      callback(retval);
-    });
-
-    this._timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
-    this._timer.initWithCallback(new EventListener(cb),
-                                 0, this._timer.TYPE_ONE_SHOT);
   },
 
   _onLogin: function BSS__onLogin(event) {
@@ -1098,9 +1106,7 @@ BookmarksSyncService.prototype = {
   // nsIBookmarksSyncService
 
   sync: function BSS_sync() {
-    let sync_gen = this._doSync();
+    asyncRun(this, this._doSync);
-    sync_gen.next(); // must initialize before sending
-    sync_gen.send(sync_gen);
   },
 
   login: function BSS_login() {
@@ -1144,10 +1150,26 @@ function bind2(object, method) {
   return function innerBind() { return method.apply(object, arguments); }
 }
 
+function asyncRun(object, method, onComplete, extra) {
+  let args = Array.prototype.slice.call(arguments, 2); // onComplete + extra + ...
+  let ret = false;
+  try {
+    let gen = method.apply(object, args);
+    gen.next(); // must initialize before sending
+    gen.send(gen);
+    ret = true;
+  } catch (e) {
+    if (e instanceof StopIteration)
+      dump("Warning: generator stopped unexpectedly");
+    else
+      throw e;
+  }
+  return ret;
+}
+
 function continueGenerator(generator, data) {
   try { generator.send(data); }
   catch (e) {
-    //notice("continueGenerator exception! - " + e);
     if (e instanceof StopIteration)
       generator = null;
     else
@@ -1155,6 +1177,33 @@ function continueGenerator(generator, data) {
   }
 }
 
+function generatorHandlers(generator) {
+  let cb = function(data) {
+    continueGenerator(generator, data);
+  };
+  return {load: cb, error: cb, complete: cb};
+}
+
+// generators called using asyncRun can't simply call the callback
+// with the return value, since that would cause the calling
+// function to end up running (after the yield) from inside the
+// generator.  Instead, generators can call this method which sets
+// up a timer to call the callback from a timer (and cleans up the
+// timer to avoid leaks).
+function generatorDone(object, callback, retval) {
+  if (object._timer)
+    throw "Called generatorDone when there is a timer already set."
+
+  let cb = bind2(object, function(event) {
+    object._timer = null;
+    callback(retval);
+  });
+
+  object._timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
+  object._timer.initWithCallback(new EventListener(cb),
+                                 0, object._timer.TYPE_ONE_SHOT);
+}
+
 function EventListener(handler) {
   this._handler = handler;
 }
@@ -1179,15 +1228,26 @@ EventListener.prototype = {
   }
 };
 
+function xpath(xmlDoc, xpathString) {
+  let root = xmlDoc.ownerDocument == null ?
+    xmlDoc.documentElement : xmlDoc.ownerDocument.documentElement
+  let nsResolver = xmlDoc.createNSResolver(root);
+
+  return xmlDoc.evaluate(xpathString, xmlDoc, nsResolver,
+                         Ci.nsIDOMXPathResult.ANY_TYPE, null);
+}
+
 function DAVCollection(baseURL) {
   this._baseURL = baseURL;
   this._authProvider = new DummyAuthProvider();
 }
 DAVCollection.prototype = {
-  _loggedIn: false,
+  __dp: null,
-
+  get _dp() {
-  get baseURL() {
+    if (!this.__dp)
-    return this._baseURL;
+      this.__dp = Cc["@mozilla.org/xmlextras/domparser;1"].
+        createInstance(Ci.nsIDOMParser);
+    return this.__dp;
   },
 
   __base64: {},
@@ -1202,6 +1262,11 @@ DAVCollection.prototype = {
     return this.__base64;
   },
 
+  get baseURL() {
+    return this._baseURL;
+  },
+
+  _loggedIn: false,
   _authString: null,
   _currentUserPath: "nobody",
 
@@ -1242,6 +1307,8 @@ DAVCollection.prototype = {
       headers = {'Content-type': 'text/plain'};
     if (this._authString)
       headers['Authorization'] = this._authString;
+    if (this._token)
+      headers['If'] = "(<" + this._token + ">)";
 
     var request = this._makeRequest("GET", path, handlers, headers);
     request.send(null);
@@ -1252,6 +1319,8 @@ DAVCollection.prototype = {
       headers = {'Content-type': 'text/plain'};
     if (this._authString)
       headers['Authorization'] = this._authString;
+    if (this._token)
+      headers['If'] = "(<" + this._token + ">)";
 
     var request = this._makeRequest("PUT", path, handlers, headers);
     request.send(data);
@@ -1352,52 +1421,113 @@ DAVCollection.prototype = {
 
   // Locking
 
-  // FIXME: make this function not reentrant
+  _getActiveLock: function DC__getActiveLock(onComplete) {
-  lock: function DC_lock(handlers) {
+    let generator = yield;
-    this._lockHandlers = handlers;
+    let handlers = generatorHandlers(generator);
-    internalHandlers = {load: bind2(this, this._onLock),
+
-                        error: bind2(this, this._onLockError)};
+    let headers = {'Content-Type': 'text/xml; charset="utf-8"',
+                   'Depth': '0'};
+    if (this._authString)
+      headers['Authorization'] = this._authString;
+
+    let request = this._makeRequest("PROPFIND", "", handlers, headers);
+    request.send("<?xml version=\"1.0\" encoding=\"utf-8\" ?>" +
+                 "<D:propfind xmlns:D='DAV:'>" +
+                 "<D:prop><D:lockdiscovery/></D:prop>" +
+                 "</D:propfind>");
+    let event = yield;
+    let tokens = xpath(event.target.responseXML, '//D:locktoken');
+    let token = tokens.iterateNext();
+    if (token)
+      generatorDone(this, onComplete, token.textContent);
+    else
+      generatorDone(this, onComplete, null);
+  },
+
+  lock: function DC_lock(onComplete) {
+    let generator = yield;
+    let handlers = generatorHandlers(generator);
+
     headers = {'Content-Type': 'text/xml; charset="utf-8"'};
-    var request = this._makeRequest("LOCK", "", internalHandlers, headers);
+    if (this._authString)
+      headers['Authorization'] = this._authString;
+
+    let request = this._makeRequest("LOCK", "", handlers, headers);
     request.send("<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n" +
                  "<D:lockinfo xmlns:D=\"DAV:\">\n" +
                  "  <D:locktype><D:write/></D:locktype>\n" +
                  "  <D:lockscope><D:exclusive/></D:lockscope>\n" +
                  "</D:lockinfo>");
+    let event = yield;
+
+    if (event.target.status < 200 || event.target.status >= 300) {
+      generatorDone(this, onComplete, false);
+      return;
+    }
+
+    let tokens = xpath(event.target.responseXML, '//D:locktoken');
+    let token = tokens.iterateNext();
+    if (token) {
+      this._token = token.textContent;
+      this._token = this._token.replace("\n", "").replace("\n", "");
+      generatorDone(this, onComplete, true);
+    }
+
+    generatorDone(this, onComplete, false);
   },
 
-  // FIXME: make this function not reentrant
+  unlock: function DC_unlock(onComplete) {
-  unlock: function DC_unlock(handlers) {
+    let generator = yield;
-    this._lockHandlers = handlers;
+    let handlers = generatorHandlers(generator);
-    internalHandlers = {load: bind2(this, this._onUnlock),
+
-                        error: bind2(this, this._onUnlockError)};
+    if (!this._token) {
+      generatorDone(this, onComplete, false);
+      return;
+    }
+
     headers = {'Lock-Token': "<" + this._token + ">"};
-    var request = this._makeRequest("UNLOCK", "", internalHandlers, headers);
+    if (this._authString)
+      headers['Authorization'] = this._authString;
+
+    let request = this._makeRequest("UNLOCK", "", handlers, headers);
     request.send(null);
-  },
+    let event = yield;
 
-  _onLock: function DC__onLock(event) {
+    if (event.target.status < 200 || event.target.status >= 300) {
-    //notice("acquired lock (" + event.target.status + "):\n" + event.target.responseText + "\n");
+      generatorDone(this, onComplete, false);
-    this._token = "woo";
+      return;
-    if (this._lockHandlers && this._lockHandlers.load)
+    }
-      this._lockHandlers.load(event);
-  },
-  _onLockError: function DC__onLockError(event) {
-    //notice("lock failed (" + event.target.status + "):\n" + event.target.responseText + "\n");
-    if (this._lockHandlers && this._lockHandlers.error)
-      this._lockHandlers.error(event);
-  },
 
-  _onUnlock: function DC__onUnlock(event) {
-    //notice("removed lock (" + event.target.status + "):\n" + event.target.responseText + "\n");
     this._token = null;
-    if (this._lockHandlers && this._lockHandlers.load)
+
-      this._lockHandlers.load(event);
+    generatorDone(this, onComplete, true);
   },
-  _onUnlockError: function DC__onUnlockError(event) {
+
-    //notice("unlock failed (" + event.target.status + "):\n" + event.target.responseText + "\n");
+  stealLock: function DC_stealLock(onComplete) {
-    if (this._lockHandlers && this._lockHandlers.error)
+    let generator = yield;
-      this._lockHandlers.error(event);
+    let handlers = generatorHandlers(generator);
+    let stolen = false;
+
+    try {
+      if (!asyncRun(this, this._getActiveLock, handlers.complete))
+        return;
+      this._token = yield;
+      this._token = this._token.replace("\n", "").replace("\n", "");
+  
+      if (!asyncRun(this, this.unlock, handlers.complete))
+        return;
+      let unlocked = yield;
+
+      if (!unlocked)
+        return;
+  
+      if (!asyncRun(this, this.lock, handlers.complete))
+        return;
+      stolen = yield;
+
+    } finally {
+      generatorDone(this, onComplete, stolen);
+    }
   }
 };
 
@@ -1413,6 +1543,7 @@ DummyAuthProvider.prototype = {
                Ci.nsIAuthPromptProvider,
                Ci.nsIAuthPrompt,
                Ci.nsIPrompt,
+               Ci.nsIProgressEventSink,
                Ci.nsIInterfaceRequestor,
                Ci.nsISupports],
 
@@ -1534,6 +1665,16 @@ DummyAuthProvider.prototype = {
         throw Cr.NS_ERROR_NOT_IMPLEMENTED;
       }
     };
+  },
+
+  // nsIProgressEventSink
+
+  onProgress: function DAP_onProgress(aRequest, aContext,
+                                      aProgress, aProgressMax) {
+  },
+
+  onStatus: function DAP_onStatus(aRequest, aContext,
+                                  aStatus, aStatusArg) {
   }
 };