Bug 844274 - Move contacts API getAll cache to child process. r=sicking
authorReuben Morais <reuben.morais@gmail.com>
Sun, 24 Feb 2013 19:53:55 -0800
changeset 122841 5f7a0300ac1f671806c25407fc7ad6ee3a3ae8c4
parent 122840 e6130d508d7db3c6bcb215c5da44822ae8025491
child 122842 990ce017ec914e443c89ec85a21e4987461d7fb3
push id24361
push userryanvm@gmail.com
push dateMon, 25 Feb 2013 19:16:30 +0000
treeherdermozilla-central@06935f2db267 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking
bugs844274
milestone22.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 844274 - Move contacts API getAll cache to child process. r=sicking
dom/contacts/ContactManager.js
dom/contacts/fallback/ContactDB.jsm
dom/contacts/fallback/ContactService.jsm
dom/contacts/tests/test_contacts_getall.html
--- a/dom/contacts/ContactManager.js
+++ b/dom/contacts/ContactManager.js
@@ -385,16 +385,24 @@ ContactManager.prototype = {
   _convertContacts: function(aContacts) {
     let contacts = [];
     for (let i in aContacts) {
       contacts.push(this._convertContact(aContacts[i]));
     }
     return contacts;
   },
 
+  _fireSuccessOrDone: function(aCursor, aResult) {
+    if (aResult == null) {
+      Services.DOMRequest.fireDone(aCursor);
+    } else {
+      Services.DOMRequest.fireSuccess(aCursor, aResult);
+    }
+  },
+
   receiveMessage: function(aMessage) {
     if (DEBUG) debug("receiveMessage: " + aMessage.name);
     let msg = aMessage.json;
     let contacts = msg.contacts;
 
     let req;
     switch (aMessage.name) {
       case "Contacts:Find:Return:OK":
@@ -402,22 +410,25 @@ ContactManager.prototype = {
         if (req) {
           let result = this._convertContacts(contacts);
           Services.DOMRequest.fireSuccess(req.request, result);
         } else {
           if (DEBUG) debug("no request stored!" + msg.requestID);
         }
         break;
       case "Contacts:GetAll:Next":
-        let cursor = this._cursorData[msg.cursorId];
+        let data = this._cursorData[msg.cursorId];
         let contact = msg.contact ? this._convertContact(msg.contact) : null;
-        if (contact == null) {
-          Services.DOMRequest.fireDone(cursor);
+        if (data.waitingForNext) {
+          if (DEBUG) debug("cursor waiting for contact, sending");
+          data.waitingForNext = false;
+          this._fireSuccessOrDone(data.cursor, contact);
         } else {
-          Services.DOMRequest.fireSuccess(cursor, contact);
+          if (DEBUG) debug("cursor not waiting, saving");
+          data.cachedContacts.push(contact);
         }
         break;
       case "Contacts:GetSimContacts:Return:OK":
         req = this.getRequest(msg.requestID);
         if (req) {
           let result = contacts.map(function(c) {
             let contact = new Contact();
             let prop = {name: [c.alphaId], tel: [ { value: c.number } ]};
@@ -586,40 +597,49 @@ ContactManager.prototype = {
       cpmm.sendAsyncMessage("Contacts:Find", {requestID: this.getRequestId({request: request, reason: "find"}), options: options});
     }.bind(this)
     this.askPermission("find", request, allowCallback);
     return request;
   },
 
   createCursor: function CM_createCursor(aRequest) {
     let id = this._getRandomId();
-    let cursor = Services.DOMRequest.createCursor(this._window, function() {
-      this.handleContinue(id);
-    }.bind(this));
+    let data = {
+      cursor: Services.DOMRequest.createCursor(this._window, function() {
+        this.handleContinue(id);
+      }.bind(this)),
+      cachedContacts: [],
+      waitingForNext: true,
+    };
     if (DEBUG) debug("saved cursor id: " + id);
-    this._cursorData[id] = cursor;
-    return [id, cursor];
+    this._cursorData[id] = data;
+    return [id, data.cursor];
   },
 
   getAll: function CM_getAll(aOptions) {
     if (DEBUG) debug("getAll: " + JSON.stringify(aOptions));
     let [cursorId, cursor] = this.createCursor();
     let allowCallback = function() {
       cpmm.sendAsyncMessage("Contacts:GetAll", {
         cursorId: cursorId, findOptions: aOptions});
     }.bind(this);
     this.askPermission("find", cursor, allowCallback);
     return cursor;
   },
 
   handleContinue: function CM_handleContinue(aCursorId) {
     if (DEBUG) debug("handleContinue: " + aCursorId);
-    cpmm.sendAsyncMessage("Contacts:GetAll:Continue", {
-      cursorId: aCursorId
-    });
+    let data = this._cursorData[aCursorId];
+    if (data.cachedContacts.length > 0) {
+      if (DEBUG) debug("contact in cache");
+      this._fireSuccessOrDone(data.cursor, data.cachedContacts.shift());
+    } else {
+      if (DEBUG) debug("waiting for contact");
+      data.waitingForNext = true;
+    }
   },
 
   remove: function removeContact(aRecord) {
     let request;
     request = this.createRequest();
     let options = { id: aRecord.id };
     let allowCallback = function() {
       cpmm.sendAsyncMessage("Contact:Remove", {requestID: this.getRequestId({request: request, reason: "remove"}), options: options});
--- a/dom/contacts/fallback/ContactDB.jsm
+++ b/dom/contacts/fallback/ContactDB.jsm
@@ -25,18 +25,16 @@ const SAVED_GETALL_STORE_NAME = "getallc
 this.ContactDB = function ContactDB(aGlobal) {
   if (DEBUG) debug("Constructor");
   this._global = aGlobal;
 }
 
 ContactDB.prototype = {
   __proto__: IndexedDBHelper.prototype,
 
-  cursorData: {},
-
   upgradeSchema: function upgradeSchema(aTransaction, aDb, aOldVersion, aNewVersion) {
     if (DEBUG) debug("upgrade schema from: " + aOldVersion + " to " + aNewVersion + " called!");
     let db = aDb;
     let objectStore;
     for (let currVersion = aOldVersion; currVersion < aNewVersion; currVersion++) {
       if (currVersion == 0) {
         /**
          * Create the initial database schema.
@@ -467,127 +465,98 @@ ContactDB.prototype = {
 
   clear: function clear(aSuccessCb, aErrorCb) {
     this.newTxn("readwrite", STORE_NAME, function (txn, store) {
       if (DEBUG) debug("Going to clear all!");
       store.clear();
     }, aSuccessCb, aErrorCb);
   },
 
-  getObjectById: function CDB_getObjectById(aStore, aObjectId, aCallback) {
-    if (DEBUG) debug("getObjectById: " + aStore + ":" + aObjectId);
-    this.newTxn("readonly", aStore, function (txn, store) {
-      let req = store.get(aObjectId);
-      req.onsuccess = function (event) {
-        aCallback(event.target.result);
-      };
-      req.onerror = function (event) {
-        aCallback(null);
-      };
-    });
-  },
-
-  getCacheForQuery: function CDB_getCacheForQuery(aQuery, aCursorId, aSuccessCb) {
-    if (DEBUG) debug("getCacheForQuery");
-    // Here we try to get the cached results for query `aQuery'. If they don't
-    // exist, it means the cache was invalidated and needs to be recreated, so
-    // we do that. Otherwise, we just return the existing cache.
-    this.getObjectById(SAVED_GETALL_STORE_NAME, aQuery, function (aCache) {
-      if (!aCache) {
-        if (DEBUG) debug("creating cache for query " + aQuery);
-        this.createCacheForQuery(aQuery, aCursorId, aSuccessCb);
-      } else {
-        if (DEBUG) debug("cache exists");
-        if (!this.cursorData[aCursorId]) {
-          this.cursorData[aCursorId] = aCache;
-        }
-        aSuccessCb(aCache);
-      }
-    }.bind(this));
-  },
-
-  setCacheForQuery: function CDB_setCacheForQuery(aQuery, aCache, aCallback) {
-    this.newTxn("readwrite", SAVED_GETALL_STORE_NAME, function (txn, store) {
-      let req = store.put(aCache, aQuery);
-      if (!aCallback) {
-        return;
-      }
-      req.onsuccess = function () {
-        aCallback(true);
-      };
-      req.onerror = function () {
-        aCallback(false);
-      };
-    });
-  },
-
-  createCacheForQuery: function CDB_createCacheForQuery(aQuery, aCursorId, aSuccessCb, aFailureCb) {
+  createCacheForQuery: function CDB_createCacheForQuery(aQuery, aSuccessCb, aFailureCb) {
     this.find(function (aContacts) {
       if (aContacts) {
         let contactsArray = [];
         for (let i in aContacts) {
-          contactsArray.push(aContacts[i].id);
+          contactsArray.push(aContacts[i]);
         }
 
-        this.setCacheForQuery(aQuery, contactsArray);
-        this.cursorData[aCursorId] = contactsArray;
-        aSuccessCb(contactsArray);
+        // save contact ids in cache
+        this.newTxn("readwrite", SAVED_GETALL_STORE_NAME, function(txn, store) {
+          store.put(contactsArray.map(function(el) el.id), aQuery);
+        });
+
+        // send full contacts
+        aSuccessCb(contactsArray, true);
       } else {
-        aSuccessCb(null);
+        aSuccessCb([], true);
       }
     }.bind(this),
     function (aErrorMsg) { aFailureCb(aErrorMsg); },
     JSON.parse(aQuery));
   },
 
-  getAll: function CDB_getAll(aSuccessCb, aFailureCb, aOptions, aCursorId) {
-    // Recreate the cache for this query if needed
+  getCacheForQuery: function CDB_getCacheForQuery(aQuery, aSuccessCb) {
+    if (DEBUG) debug("getCacheForQuery");
+    // Here we try to get the cached results for query `aQuery'. If they don't
+    // exist, it means the cache was invalidated and needs to be recreated, so
+    // we do that. Otherwise, we just return the existing cache.
+    this.newTxn("readonly", SAVED_GETALL_STORE_NAME, function(txn, store) {
+      let req = store.get(aQuery);
+      req.onsuccess = function(e) {
+        if (e.target.result) {
+          if (DEBUG) debug("cache exists");
+          debug("sending: " + JSON.stringify(e.target.result));
+          aSuccessCb(e.target.result, false);
+        } else {
+          if (DEBUG) debug("creating cache for query " + aQuery);
+          this.createCacheForQuery(aQuery, aSuccessCb);
+        }
+      }.bind(this);
+      req.onerror = function() {
+
+      };
+    }.bind(this));
+  },
+
+  getAll: function CDB_getAll(aSuccessCb, aFailureCb, aOptions) {
     let optionStr = JSON.stringify(aOptions);
-    this.getCacheForQuery(optionStr, aCursorId, function (aCachedResults) {
+    this.getCacheForQuery(optionStr, function(aCachedResults, aFullContacts) {
+      // aFullContacts is true if the cache didn't exist and had to be created.
+      // In that case, we receive the full contacts since we already have them
+      // in memory to create the cache anyway. This allows us to avoid accessing
+      // the main object store again.
       if (aCachedResults && aCachedResults.length > 0) {
         if (DEBUG) debug("query returned at least one contact");
-        this.getObjectById(STORE_NAME, aCachedResults[0], function (aContact) {
-          this.cursorData[aCursorId].shift();
-          aSuccessCb(aContact);
-        }.bind(this));
+        if (aFullContacts) {
+          if (DEBUG) debug("full contacts: " + aCachedResults.length);
+          for (let i = 0; i < aCachedResults.length; ++i) {
+            aSuccessCb(aCachedResults[i]);
+          }
+          aSuccessCb(null);
+        } else {
+          let count = 0;
+          this.newTxn("readonly", STORE_NAME, function(txn, store) {
+            for (let i = 0; i < aCachedResults.length; ++i) {
+              store.get(aCachedResults[i]).onsuccess = function(e) {
+                aSuccessCb(e.target.result);
+                count++;
+                if (count == aCachedResults.length) {
+                  aSuccessCb(null);
+                }
+              };
+            }
+          });
+        }
       } else { // no contacts
         if (DEBUG) debug("query returned no contacts");
         aSuccessCb(null);
       }
     }.bind(this));
   },
 
-  getNext: function CDB_getNext(aSuccessCb, aFailureCb, aCursorId) {
-    if (DEBUG) debug("ContactDB:getNext: " + aCursorId);
-    let aCachedResults = this.cursorData[aCursorId];
-    if (DEBUG) debug("got transient cache");
-    if (aCachedResults.length > 0) {
-      this.getObjectById(STORE_NAME, aCachedResults[0], function(aContact) {
-        this.cursorData[aCursorId].shift();
-        if (aContact) {
-          aSuccessCb(aContact);
-        } else {
-          // If the contact ID in cache is invalid, it was removed recently and
-          // the cache hasn't been updated to reflect the change, so we skip it.
-          if (DEBUG) debug("invalid contact in cache: " + aCachedResults[0]);
-          return this.getNext(aSuccessCb, aFailureCb, aCursorId);
-        }
-      }.bind(this));
-    } else { // last contact
-      delete this.cursorData[aCursorId];
-      aSuccessCb(null);
-    }
-  },
-
-  releaseCursors: function CDB_releaseCursors(aCursors) {
-    for (let i of aCursors) {
-      delete this.cursorData[i];
-    }
-  },
-
   /*
    * Sorting the contacts by sortBy field. aSortBy can either be familyName or givenName.
    * If 2 entries have the same sortyBy field or no sortBy field is present, we continue
    * sorting with the other sortyBy field.
    */
   sortResults: function CDB_sortResults(aResults, aFindOptions) {
     if (!aFindOptions)
       return;
--- a/dom/contacts/fallback/ContactService.jsm
+++ b/dom/contacts/fallback/ContactService.jsm
@@ -32,22 +32,19 @@ XPCOMUtils.defineLazyGetter(this, "mRIL"
     };
   }
   return telephony.getService(Ci.nsIRadioInterfaceLayer);
 });
 
 let myGlobal = this;
 
 this.DOMContactManager = {
-  // maps children to their live cursors so we can cleanup on shutdown/crash
-  _liveCursors: {},
-
   init: function() {
     if (DEBUG) debug("Init");
-    this._messages = ["Contacts:Find", "Contacts:GetAll", "Contacts:GetAll:Continue", "Contacts:Clear", "Contact:Save",
+    this._messages = ["Contacts:Find", "Contacts:GetAll", "Contacts:Clear", "Contact:Save",
                       "Contact:Remove", "Contacts:GetSimContacts",
                       "Contacts:RegisterForMessages", "child-process-shutdown"];
     this._children = [];
     this._messages.forEach(function(msgName) {
       ppmm.addMessageListener(msgName, this);
     }.bind(this));
 
     var idbManager = Components.classes["@mozilla.org/dom/indexeddb/manager;1"].getService(Ci.nsIIndexedDatabaseManager);
@@ -114,33 +111,16 @@ this.DOMContactManager = {
           return null;
         }
         this._db.getAll(
           function(aContact) {
             mm.sendAsyncMessage("Contacts:GetAll:Next", {cursorId: msg.cursorId, contact: aContact});
           },
           function(aErrorMsg) { mm.sendAsyncMessage("Contacts:Find:Return:KO", { errorMsg: aErrorMsg }); },
           msg.findOptions, msg.cursorId);
-        if (Array.isArray(this._liveCursors[mm])) {
-          this._liveCursors[mm].push(msg.cursorId);
-        } else {
-          this._liveCursors[mm] = [msg.cursorId];
-        }
-        break;
-      case "Contacts:GetAll:Continue":
-        this._db.getNext(
-          function(aContact) {
-            if (aContact == null) { // last contact, release the cursor
-              let cursors = this._liveCursors[mm];
-              cursors.splice(cursors.indexOf(msg.cursorId), 1);
-            }
-            mm.sendAsyncMessage("Contacts:GetAll:Next", {cursorId: msg.cursorId, contact: aContact});
-          }.bind(this),
-          function(aErrorMsg) { mm.sendAsyncMessage("Contacts:Find:Return:KO", { errorMsg: aErrorMsg }); },
-          msg.cursorId);
         break;
       case "Contact:Save":
         if (msg.options.reason === "create") {
           if (!this.assertPermission(aMessage, "contacts-create")) {
             return null;
           }
         } else {
           if (!this.assertPermission(aMessage, "contacts-write")) {
@@ -205,20 +185,16 @@ this.DOMContactManager = {
         }
         if (DEBUG) debug("Register!");
         if (this._children.indexOf(mm) == -1) {
           this._children.push(mm);
         }
         break;
       case "child-process-shutdown":
         if (DEBUG) debug("Unregister");
-        if (this._liveCursors[mm]) {
-          this._db.releaseCursors(this._liveCursors[mm]);
-          delete this._liveCursors[mm];
-        }
         let index = this._children.indexOf(mm);
         if (index != -1) {
           if (DEBUG) debug("Unregister index: " + index);
           this._children.splice(index, 1);
         }
         break;
       default:
         if (DEBUG) debug("WRONG MESSAGE NAME: " + aMessage.name);
--- a/dom/contacts/tests/test_contacts_getall.html
+++ b/dom/contacts/tests/test_contacts_getall.html
@@ -261,17 +261,17 @@ let steps = [
     };
     req.onerror = onFailure;
   },
 
   clearDatabase,
   add20Contacts,
 
   function() {
-    ok(true, "Test cache invalidation between getAll and getNext");
+    ok(true, "Test cache consistency when deleting contact during getAll");
     req = mozContacts.find({});
     req.onsuccess = function(e) {
       let lastContact = e.target.result[e.target.result.length-1];
       req = mozContacts.getAll({});
       let count = 0;
       let firstResult = true;
       req.onsuccess = function(event) {
         ok(true, "on success");
@@ -285,30 +285,29 @@ let steps = [
             req.continue();
           };
         } else {
           if (req.result) {
             ok(true, "result is valid");
             count++;
             req.continue();
           } else {
-            is(count, 19, "19 contacts returned");
-            ok(true, "last contact");
+            is(count, 20, "last contact - 20 contacts returned");
             next();
           }
         }
       };
     };
   },
 
   clearDatabase,
   add20Contacts,
 
   function() {
-    ok(true, "Delete the currect contact while iterating");
+    ok(true, "Delete the current contact while iterating");
     req = mozContacts.getAll({});
     let count = 0;
     let previousId = null;
     req.onsuccess = function() {
       if (req.result) {
         ok(true, "on success");
         if (previousId) {
           isnot(previousId, req.result.id, "different contacts returned");