Bug 636312 - Forms and passwords engines: yield back to the main thread during batches of synchronous I/O. r=rnewman a=blocking-fennec
authorPhilipp von Weitershausen <philipp@weitershausen.de>
Mon, 07 Mar 2011 13:07:59 -0800
changeset 63345 eff51222337e295a29e087f43064718313570ecc
parent 63344 cf2463e452bac530deb85e22075d02f30ceb560a
child 63346 e32a8deafe71878f4e7502a87e091da19a9aa4bf
push id1
push userroot
push dateTue, 10 Dec 2013 15:46:25 +0000
reviewersrnewman, blocking-fennec
bugs636312
Bug 636312 - Forms and passwords engines: yield back to the main thread during batches of synchronous I/O. r=rnewman a=blocking-fennec
services/sync/modules/engines.js
services/sync/modules/engines/forms.js
services/sync/modules/engines/passwords.js
services/sync/tests/unit/test_syncengine.js
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -187,30 +187,41 @@ Tracker.prototype = {
 
 function Store(name) {
   name = name || "Unnamed";
   this.name = name.toLowerCase();
 
   this._log = Log4Moz.repository.getLogger("Store." + name);
   let level = Svc.Prefs.get("log.logger.engine." + this.name, "Debug");
   this._log.level = Log4Moz.Level[level];
+
+  Utils.lazy2(this, "_timer", function() {
+    return Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
+  });
 }
 Store.prototype = {
 
+  _sleep: function _sleep(delay) {
+    let cb = Utils.makeSyncCallback();
+    this._timer.initWithCallback({notify: cb}, delay,
+                                 Ci.nsITimer.TYPE_ONE_SHOT);
+    Utils.waitForSyncCallback(cb);
+  },
+
   applyIncomingBatch: function applyIncomingBatch(records) {
     let failed = [];
-    records.forEach(function (record) {
+    for each (let record in records) {
       try {
         this.applyIncoming(record);
       } catch (ex) {
         this._log.warn("Failed to apply incoming record " + record.id);
         this._log.warn("Encountered exception: " + Utils.exceptionStr(ex));
         failed.push(record.id);
       }
-    }, this);
+    };
     return failed;
   },
 
   applyIncoming: function Store_applyIncoming(record) {
     if (record.deleted)
       this.remove(record);
     else if (!this.itemExists(record.id))
       this.create(record);
@@ -399,20 +410,16 @@ Engine.prototype = {
   wipeClient: function Engine_wipeClient() {
     this._notify("wipe-client", this.name, this._wipeClient)();
   }
 };
 
 function SyncEngine(name) {
   Engine.call(this, name || "SyncEngine");
   this.loadToFetch();
-
-  Utils.lazy2(this, "_timer", function() {
-    return Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
-  });
 }
 SyncEngine.prototype = {
   __proto__: Engine.prototype,
   _recordObj: CryptoWrapper,
   version: 1,
   downloadLimit: null,
   applyIncomingBatchSize: DEFAULT_STORE_BATCH_SIZE,
 
@@ -494,23 +501,16 @@ SyncEngine.prototype = {
   // Create a new record using the store and add in crypto fields
   _createRecord: function SyncEngine__createRecord(id) {
     let record = this._store.createRecord(id, this.name);
     record.id = id;
     record.collection = this.name;
     return record;
   },
 
-  _sleep: function _sleep(delay) {
-    let cb = Utils.makeSyncCallback();
-    this._timer.initWithCallback({notify: cb}, delay,
-                                 Ci.nsITimer.TYPE_ONE_SHOT);
-    Utils.waitForSyncCallback(cb);
-  },
-
   // Any setup that needs to happen at the beginning of each sync.
   _syncStartup: function SyncEngine__syncStartup() {
 
     // Determine if we need to wipe on outdated versions
     let metaGlobal = Records.get(this.metaURL);
     let engines = metaGlobal.payload.engines || {};
     let engineData = engines[this.name] || {};
 
@@ -670,17 +670,17 @@ SyncEngine.prototype = {
       } else {
         count.reconciled++;
         self._log.trace("Skipping reconciled incoming item " + item.id);
       }
 
       if (applyBatch.length == self.applyIncomingBatchSize) {
         doApplyBatch.call(self);
       }
-      self._sleep(0);
+      self._store._sleep(0);
     };
 
     // Only bother getting data from the server if there's new things
     if (this.lastModified == null || this.lastModified > this.lastSync) {
       let resp = newitems.get();
       doApplyBatchAndPersistFailed.call(this);
       if (!resp.success) {
         resp.failureCode = ENGINE_DOWNLOAD_FAIL;
@@ -904,17 +904,17 @@ SyncEngine.prototype = {
         catch(ex) {
           this._log.warn("Error creating record: " + Utils.exceptionStr(ex));
         }
 
         // Partial upload
         if ((++count % MAX_UPLOAD_RECORDS) == 0)
           doUpload((count - MAX_UPLOAD_RECORDS) + " - " + count + " out");
 
-        this._sleep(0);
+        this._store._sleep(0);
       }
 
       // Final upload
       if (count % MAX_UPLOAD_RECORDS > 0)
         doUpload(count >= MAX_UPLOAD_RECORDS ? "last batch" : "all");
     }
   },
 
--- a/services/sync/modules/engines/forms.js
+++ b/services/sync/modules/engines/forms.js
@@ -177,16 +177,21 @@ FormStore.prototype = {
   __proto__: Store.prototype,
 
   applyIncomingBatch: function applyIncomingBatch(records) {
     return Utils.runInTransaction(Svc.Form.DBConnection, function() {
       return Store.prototype.applyIncomingBatch.call(this, records);
     }, this);
   },
 
+  applyIncoming: function applyIncoming(record) {
+    Store.prototype.applyIncoming.call(this, record);
+    this._sleep(0); // Yield back to main thread after synchronous operation.
+  },
+
   getAllIDs: function FormStore_getAllIDs() {
     let guids = {};
     for each (let {name, value} in FormWrapper.getAllEntries())
       guids[FormWrapper.getGUID(name, value)] = true;
     return guids;
   },
 
   changeItemID: function FormStore_changeItemID(oldID, newID) {
@@ -197,17 +202,17 @@ FormStore.prototype = {
     return FormWrapper.hasGUID(id);
   },
 
   createRecord: function createRecord(id, collection) {
     let record = new FormRec(collection, id);
     let entry = FormWrapper.getEntry(id);
     if (entry != null) {
       record.name = entry.name;
-      record.value = entry.value
+      record.value = entry.value;
     }
     else
       record.deleted = true;
     return record;
   },
 
   create: function FormStore_create(record) {
     this._log.trace("Adding form record for " + record.name);
--- a/services/sync/modules/engines/passwords.js
+++ b/services/sync/modules/engines/passwords.js
@@ -93,17 +93,18 @@ PasswordEngine.prototype = {
   },
 
   _findDupe: function _findDupe(item) {
     let login = this._store._nsLoginInfoFromRecord(item);
     if (!login)
       return;
 
     let logins = Svc.Login.findLogins({}, login.hostname, login.formSubmitURL,
-      login.httpRealm);
+                                      login.httpRealm);
+    this._store._sleep(0); // Yield back to main thread after synchronous operation.
 
     // Look for existing logins that match the hostname but ignore the password
     for each (let local in logins)
       if (login.matches(local, true) && local instanceof Ci.nsILoginMetaInfo)
         return local.guid;
   }
 };
 
@@ -150,35 +151,41 @@ PasswordStore.prototype = {
   },
 
   _getLoginFromGUID: function PasswordStore__getLoginFromGUID(id) {
     let prop = Cc["@mozilla.org/hash-property-bag;1"].
       createInstance(Ci.nsIWritablePropertyBag2);
     prop.setPropertyAsAUTF8String("guid", id);
 
     let logins = Svc.Login.searchLogins({}, prop);
+    this._sleep(0); // Yield back to main thread after synchronous operation.
     if (logins.length > 0) {
       this._log.trace(logins.length + " items matching " + id + " found.");
       return logins[0];
     } else {
       this._log.trace("No items matching " + id + " found. Ignoring");
     }
-    return false;
+    return null;
   },
 
   applyIncomingBatch: function applyIncomingBatch(records) {
     if (!this.DBConnection) {
       return Store.prototype.applyIncomingBatch.call(this, records);
     }
 
     return Utils.runInTransaction(this.DBConnection, function() {
       return Store.prototype.applyIncomingBatch.call(this, records);
     }, this);
   },
 
+  applyIncoming: function applyIncoming(record) {
+    Store.prototype.applyIncoming.call(this, record);
+    this._sleep(0); // Yield back to main thread after synchronous operation.
+  },
+
   getAllIDs: function PasswordStore__getAllIDs() {
     let items = {};
     let logins = Svc.Login.getAllLogins({});
 
     for (let i = 0; i < logins.length; i++) {
       // Skip over Weave password/passphrase entries
       let metaInfo = logins[i].QueryInterface(Ci.nsILoginMetaInfo);
       if (metaInfo.hostname == PWDMGR_HOST)
--- a/services/sync/tests/unit/test_syncengine.js
+++ b/services/sync/tests/unit/test_syncengine.js
@@ -77,17 +77,17 @@ function test_toFetch() {
     // Ensure pristine environment
     do_check_eq(engine.toFetch.length, 0);
 
     // Write file to disk
     let toFetch = [Utils.makeGUID(), Utils.makeGUID(), Utils.makeGUID()];
     engine.toFetch = toFetch;
     do_check_eq(engine.toFetch, toFetch);
     // toFetch is written asynchronously
-    engine._sleep(0);
+    engine._store._sleep(0);
     let fakefile = syncTesting.fakeFilesystem.fakeContents[filename];
     do_check_eq(fakefile, JSON.stringify(toFetch));
 
     // Read file from disk
     toFetch = [Utils.makeGUID(), Utils.makeGUID()];
     syncTesting.fakeFilesystem.fakeContents[filename] = JSON.stringify(toFetch);
     engine.loadToFetch();
     do_check_eq(engine.toFetch.length, 2);