Bug 980549 - Fix various races in settings and contacts, r=gwagner.
☠☠ backed out by ad6cc6b98a9d ☠ ☠
authorBen Turner <bent.mozilla@gmail.com>
Thu, 17 Apr 2014 19:26:42 -0700
changeset 197730 a24917a6882124565822a3e1a81d6a39e4504661
parent 197692 456954b7051c611f8c0c1cf2df386c79e8e5fd73
child 197731 da6ddd1630b6bb9bee239f6051d0a0862dd9ae8c
push id3624
push userasasaki@mozilla.com
push dateMon, 09 Jun 2014 21:49:01 +0000
treeherdermozilla-beta@b1a5da15899a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgwagner
bugs980549
milestone31.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 980549 - Fix various races in settings and contacts, r=gwagner.
dom/base/IndexedDBHelper.jsm
dom/contacts/fallback/ContactDB.jsm
dom/settings/SettingsManager.js
--- a/dom/base/IndexedDBHelper.jsm
+++ b/dom/base/IndexedDBHelper.jsm
@@ -14,19 +14,21 @@ if (DEBUG) {
 
 const Cu = Components.utils;
 const Cc = Components.classes;
 const Ci = Components.interfaces;
 
 this.EXPORTED_SYMBOLS = ["IndexedDBHelper"];
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
-Cu.import("resource://gre/modules/Services.jsm");
 Cu.importGlobalProperties(["indexedDB"]);
 
+XPCOMUtils.defineLazyModuleGetter(this, 'Services',
+  'resource://gre/modules/Services.jsm');
+
 this.IndexedDBHelper = function IndexedDBHelper() {}
 
 IndexedDBHelper.prototype = {
 
   // Cache the database
   _db: null,
 
   // Close the database
@@ -83,17 +85,20 @@ IndexedDBHelper.prototype = {
    * @param successCb
    *        Success callback to call.
    * @param failureCb
    *        Error callback to call when an error is encountered.
    */
   ensureDB: function ensureDB(aSuccessCb, aFailureCb) {
     if (this._db) {
       if (DEBUG) debug("ensureDB: already have a database, returning early.");
-      aSuccessCb && aSuccessCb();
+      if (aSuccessCb) {
+        Services.tm.currentThread.dispatch(aSuccessCb,
+                                           Ci.nsIThread.DISPATCH_NORMAL);
+      }
       return;
     }
     this.open(aSuccessCb, aFailureCb);
   },
 
   /**
    * Start a new transaction.
    *
--- a/dom/contacts/fallback/ContactDB.jsm
+++ b/dom/contacts/fallback/ContactDB.jsm
@@ -966,17 +966,17 @@ ContactDB.prototype = {
   },
 
   // Invalidate the entire cache. It will be incrementally regenerated on demand
   // See getCacheForQuery
   invalidateCache: function CDB_invalidateCache(aErrorCb) {
     if (DEBUG) debug("invalidate cache");
     this.newTxn("readwrite", SAVED_GETALL_STORE_NAME, function (txn, store) {
       store.clear();
-    }, aErrorCb);
+    }, null, aErrorCb);
   },
 
   incrementRevision: function CDB_incrementRevision(txn) {
     let revStore = txn.objectStore(REVISION_STORE);
     revStore.get(REVISION_KEY).onsuccess = function(e) {
       revStore.put(parseInt(e.target.result, 10) + 1, REVISION_KEY);
     };
   },
@@ -1037,19 +1037,21 @@ ContactDB.prototype = {
   createCacheForQuery: function CDB_createCacheForQuery(aQuery, aSuccessCb, aFailureCb) {
     this.find(function (aContacts) {
       if (aContacts) {
         let contactsArray = [];
         for (let i in aContacts) {
           contactsArray.push(aContacts[i]);
         }
 
+        let contactIdsArray = contactsArray.map(function(el) el.id);
+
         // save contact ids in cache
         this.newTxn("readwrite", SAVED_GETALL_STORE_NAME, function(txn, store) {
-          store.put(contactsArray.map(function(el) el.id), aQuery);
+          store.put(contactIdsArray, aQuery);
         }, null, aFailureCb);
 
         // send full contacts
         aSuccessCb(contactsArray, true);
       } else {
         aSuccessCb([], true);
       }
     }.bind(this),
--- a/dom/settings/SettingsManager.js
+++ b/dom/settings/SettingsManager.js
@@ -27,16 +27,23 @@ XPCOMUtils.defineLazyServiceGetter(this,
                                    "nsIMemoryReporterManager");
 
 function SettingsLock(aSettingsManager) {
   this._open = true;
   this._isBusy = false;
   this._requests = new Queue();
   this._settingsManager = aSettingsManager;
   this._transaction = null;
+
+  let closeHelper = function() {
+    if (DEBUG) debug("closing lock");
+    this._open = false;
+  }.bind(this);
+
+  Services.tm.currentThread.dispatch(closeHelper, Ci.nsIThread.DISPATCH_NORMAL);
 }
 
 SettingsLock.prototype = {
   get closed() {
     return !this._open;
   },
 
   _wrap: function _wrap(obj) {
@@ -60,21 +67,23 @@ SettingsLock.prototype = {
             this._open = false;
           }.bind(lock);
           clearReq.onerror = function() {
             Services.DOMRequest.fireError(request, 0)
           };
           break;
         case "set":
           let keys = Object.getOwnPropertyNames(info.settings);
+          if (keys.length) {
+            lock._isBusy = true;
+          }
           for (let i = 0; i < keys.length; i++) {
             let key = keys[i];
             let last = i === keys.length - 1;
             if (DEBUG) debug("key: " + key + ", val: " + JSON.stringify(info.settings[key]) + ", type: " + typeof(info.settings[key]));
-            lock._isBusy = true;
             let checkKeyRequest = store.get(key);
 
             checkKeyRequest.onsuccess = function (event) {
               let defaultValue;
               let userValue = info.settings[key];
               if (event.target.result) {
                 defaultValue = event.target.result.defaultValue;
               } else {
@@ -82,41 +91,46 @@ SettingsLock.prototype = {
                 if (DEBUG) debug("MOZSETTINGS-SET-WARNING: " + key + " is not in the database.\n");
               }
 
               let obj = {settingName: key, defaultValue: defaultValue, userValue: userValue};
               if (DEBUG) debug("store1: " + JSON.stringify(obj));
               let setReq = store.put(obj);
 
               setReq.onsuccess = function() {
-                lock._isBusy = false;
                 cpmm.sendAsyncMessage("Settings:Changed", { key: key, value: userValue });
                 if (last && !request.error) {
                   lock._open = true;
                   Services.DOMRequest.fireSuccess(request, 0);
                   lock._open = false;
-                  if (!lock._requests.isEmpty()) {
-                    lock.process();
-                  }
                 }
               };
 
               setReq.onerror = function() {
                 if (!request.error) {
                   Services.DOMRequest.fireError(request, setReq.error.name)
                 }
               };
+
+              if (last) {
+                lock._isBusy = false;
+                if (!lock._requests.isEmpty()) {
+                  lock.process();
+                }
+              }
             };
             checkKeyRequest.onerror = function(event) {
               if (!request.error) {
                 Services.DOMRequest.fireError(request, checkKeyRequest.error.name)
               }
             };
           }
-          break;
+          // Don't break here, instead return. Once the previous requests have
+          // finished this loop will start again.
+          return;
         case "get":
           let getReq = (info.name === "*") ? store.mozGetAll()
                                            : store.mozGetAll(info.name);
 
           getReq.onsuccess = function(event) {
             if (DEBUG) debug("Request for '" + info.name + "' successful. " +
                              "Record count: " + event.target.result.length);
 
@@ -143,45 +157,43 @@ SettingsLock.prototype = {
             Services.DOMRequest.fireError(request, 0)
           };
           break;
       }
     }
   },
 
   createTransactionAndProcess: function() {
-    if (this._settingsManager._settingsDB._db) {
-      var lock;
-      while (lock = this._settingsManager._locks.dequeue()) {
-        if (!lock._transaction) {
-          let transactionType = this._settingsManager.hasWritePrivileges ? "readwrite" : "readonly";
-          lock._transaction = lock._settingsManager._settingsDB._db.transaction(SETTINGSSTORE_NAME, transactionType);
-        }
-        if (!lock._isBusy) {
-          lock.process();
-        } else {
-          this._settingsManager._locks.enqueue(lock);
-        }
-      }
-      if (!this._requests.isEmpty() && !this._isBusy) {
-        this.process();
-      }
+    if (DEBUG) debug("database opened, creating transaction");
+
+    let manager = this._settingsManager;
+    let transactionType = manager.hasWritePrivileges ? "readwrite" : "readonly";
+
+    this._transaction =
+      manager._settingsDB._db.transaction(SETTINGSSTORE_NAME, transactionType);
+
+    this.process();
+  },
+
+  maybeProcess: function() {
+    if (this._transaction && !this._isBusy) {
+      this.process();
     }
   },
 
   get: function get(aName) {
     if (!this._open) {
       dump("Settings lock not open!\n");
       throw Components.results.NS_ERROR_ABORT;
     }
 
     if (this._settingsManager.hasReadPrivileges) {
       let req = Services.DOMRequest.createRequest(this._settingsManager._window);
       this._requests.enqueue({ request: req, intent:"get", name: aName });
-      this.createTransactionAndProcess();
+      this.maybeProcess();
       return req;
     } else {
       if (DEBUG) debug("get not allowed");
       throw Components.results.NS_ERROR_NOT_IMPLEMENTED;
     }
   },
 
   _serializePreservingBinaries: function _serializePreservingBinaries(aObject) {
@@ -216,82 +228,72 @@ SettingsLock.prototype = {
       throw "Settings lock not open";
     }
 
     if (this._settingsManager.hasWritePrivileges) {
       let req = Services.DOMRequest.createRequest(this._settingsManager._window);
       if (DEBUG) debug("send: " + JSON.stringify(aSettings));
       let settings = this._serializePreservingBinaries(aSettings);
       this._requests.enqueue({request: req, intent: "set", settings: settings});
-      this.createTransactionAndProcess();
+      this.maybeProcess();
       return req;
     } else {
       if (DEBUG) debug("set not allowed");
       throw "No permission to call set";
     }
   },
 
   clear: function clear() {
     if (!this._open) {
       throw "Settings lock not open";
     }
 
     if (this._settingsManager.hasWritePrivileges) {
       let req = Services.DOMRequest.createRequest(this._settingsManager._window);
       this._requests.enqueue({ request: req, intent: "clear"});
-      this.createTransactionAndProcess();
+      this.maybeProcess();
       return req;
     } else {
       if (DEBUG) debug("clear not allowed");
       throw "No permission to call clear";
     }
   },
 
   classID: Components.ID("{60c9357c-3ae0-4222-8f55-da01428470d5}"),
   contractID: "@mozilla.org/settingsLock;1",
   QueryInterface: XPCOMUtils.generateQI([Ci.nsISupports]),
 };
 
 function SettingsManager() {
-  this._locks = new Queue();
   this._settingsDB = new SettingsDB();
   this._settingsDB.init();
 }
 
 SettingsManager.prototype = {
   _callbacks: null,
 
   _wrap: function _wrap(obj) {
     return Cu.cloneInto(obj, this._window);
   },
 
-  nextTick: function nextTick(aCallback, thisObj) {
-    if (thisObj)
-      aCallback = aCallback.bind(thisObj);
-
-    Services.tm.currentThread.dispatch(aCallback, Ci.nsIThread.DISPATCH_NORMAL);
-  },
-
   set onsettingchange(aHandler) {
     this.__DOM_IMPL__.setEventHandler("onsettingchange", aHandler);
   },
 
   get onsettingchange() {
     return this.__DOM_IMPL__.getEventHandler("onsettingchange");
   },
 
   createLock: function() {
     if (DEBUG) debug("get lock!");
     var lock = new SettingsLock(this);
-    this._locks.enqueue(lock);
     this._settingsDB.ensureDB(
       function() { lock.createTransactionAndProcess(); },
       function() { dump("Cannot open Settings DB. Trying to open an old version?\n"); }
     );
-    this.nextTick(function() { this._open = false; }, lock);
     return lock;
   },
 
   receiveMessage: function(aMessage) {
     if (DEBUG) debug("Settings::receiveMessage: " + aMessage.name);
     let msg = aMessage.json;
 
     switch (aMessage.name) {