Bug 980549 - Simplify SettingsManager transaction management. r=gwagner, a=1.4+
authorBen Turner <bent.mozilla@gmail.com>
Tue, 06 May 2014 14:14:23 -0700
changeset 187376 94888eb83c1db441ab875e151449fcd0014c3558
parent 187375 81cd7fac8df9ad21f16161dd1a11ead9e7b998fd
child 187377 60145dd8edb75edaff8f6d6510d985c26231c633
push id62
push userryanvm@gmail.com
push dateWed, 07 May 2014 16:46:36 +0000
treeherdermozilla-b2g30_v1_4@60145dd8edb7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgwagner, 1
bugs980549
milestone30.0
Bug 980549 - Simplify SettingsManager transaction management. r=gwagner, a=1.4+
dom/base/IndexedDBHelper.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/settings/SettingsManager.js
+++ b/dom/settings/SettingsManager.js
@@ -24,16 +24,23 @@ XPCOMUtils.defineLazyServiceGetter(this,
                                    "nsIMessageSender");
 
 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) {
@@ -57,21 +64,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 {
@@ -79,41 +88,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);
 
@@ -140,45 +154,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) {
@@ -213,82 +225,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) {