Bug 1364488 - Allow fetch to use a concurrent connection. r=standard8

MozReview-Commit-ID: 4L4PFtXsjsy

--HG--
extra : rebase_source : f12cbc4f9f6bb5cf9fc791c1c44978c98e3ca325

browser/base/content/browser-places.js

@@ -1674,7 +1674,7 @@ var BookmarkingUI = {
     // returning any result.
     let pendingUpdate = this._pendingUpdate = {};
 
-    PlacesUtils.bookmarks.fetch({url: this._uri}, b => guids.add(b.guid))
+    PlacesUtils.bookmarks.fetch({url: this._uri}, b => guids.add(b.guid), { concurrent: true })
       .catch(Components.utils.reportError)
       .then(() => {
          if (pendingUpdate != this._pendingUpdate) {

toolkit/components/places/Bookmarks.jsm

@@ -761,6 +761,11 @@ var Bookmarks = Object.freeze({
    *        object representing it, as defined above.
    * @param onResult [optional]
    *        Callback invoked for each found bookmark.
+   * @param options [optional]
+   *        an optional object whose properties describe options for the fetch:
+   *         - concurrent: fetches concurrently to any writes, returning results
+   *                       faster. On the negative side, it may return stale
+   *                       information missing the currently ongoing write.
    *
    * @return {Promise} resolved when the fetch is complete.
    * @resolves to an object representing the found item, as described above, or
@@ -772,7 +777,7 @@ var Bookmarks = Object.freeze({
    * @note Any unknown property in the info object is ignored.  Known properties
    *       may be overwritten.
    */
-  fetch(guidOrInfo, onResult = null) {
+  fetch(guidOrInfo, onResult = null, options = {}) {
     if (onResult && typeof onResult != "function")
       throw new Error("onResult callback must be a valid function");
     let info = guidOrInfo;
@@ -812,11 +817,11 @@ var Bookmarks = Object.freeze({
     return (async function() {
       let results;
       if (fetchInfo.hasOwnProperty("url"))
-        results = await fetchBookmarksByURL(fetchInfo);
+        results = await fetchBookmarksByURL(fetchInfo, options && options.concurrent);
       else if (fetchInfo.hasOwnProperty("guid"))
-        results = await fetchBookmark(fetchInfo);
+        results = await fetchBookmark(fetchInfo, options && options.concurrent);
       else if (fetchInfo.hasOwnProperty("parentGuid") && fetchInfo.hasOwnProperty("index"))
-        results = await fetchBookmarkByPosition(fetchInfo);
+        results = await fetchBookmarkByPosition(fetchInfo, options && options.concurrent);
 
       if (!results)
         return null;
@@ -1386,10 +1391,8 @@ async function queryBookmarks(info) {
 
 // Fetch implementation.
 
-function fetchBookmark(info) {
-  return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: fetchBookmark",
-    async function(db) {
-
+async function fetchBookmark(info, concurrent) {
+  let query = async function(db) {
     let rows = await db.executeCached(
       `SELECT b.guid, IFNULL(p.guid, "") AS parentGuid, b.position AS 'index',
               b.dateAdded, b.lastModified, b.type, b.title, h.url AS url,
@@ -1403,14 +1406,18 @@ function fetchBookmark(info) {
       `, { guid: info.guid });
 
     return rows.length ? rowsToItemsArray(rows)[0] : null;
-  });
+  };
+  if (concurrent) {
+    let db = await PlacesUtils.promiseDBConnection();
+    return query(db);
+  }
+  return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: fetchBookmark",
+                                           query);
 }
 
-function fetchBookmarkByPosition(info) {
-  return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: fetchBookmarkByPosition",
-    async function(db) {
+async function fetchBookmarkByPosition(info, concurrent) {
+  let query = async function(db) {
     let index = info.index == Bookmarks.DEFAULT_INDEX ? null : info.index;
-
     let rows = await db.executeCached(
       `SELECT b.guid, IFNULL(p.guid, "") AS parentGuid, b.position AS 'index',
               b.dateAdded, b.lastModified, b.type, b.title, h.url AS url,
@@ -1427,31 +1434,42 @@ function fetchBookmarkByPosition(info) {
       `, { parentGuid: info.parentGuid, index });
 
     return rows.length ? rowsToItemsArray(rows)[0] : null;
-  });
+  };
+  if (concurrent) {
+    let db = await PlacesUtils.promiseDBConnection();
+    return query(db);
+  }
+  return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: fetchBookmarkByPosition",
+                                           query);
 }
 
-function fetchBookmarksByURL(info) {
-  return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: fetchBookmarksByURL",
-    async function(db) {
-      let tagsFolderId = await promiseTagsFolderId();
-      let rows = await db.executeCached(
-        `/* do not warn (bug no): not worth to add an index */
-        SELECT b.guid, IFNULL(p.guid, "") AS parentGuid, b.position AS 'index',
-                b.dateAdded, b.lastModified, b.type, b.title, h.url AS url,
-                b.id AS _id, b.parent AS _parentId,
-                NULL AS _childCount, /* Unused for now */
-                p.parent AS _grandParentId, b.syncStatus AS _syncStatus
-        FROM moz_bookmarks b
-        JOIN moz_bookmarks p ON p.id = b.parent
-        JOIN moz_places h ON h.id = b.fk
-        WHERE h.url_hash = hash(:url) AND h.url = :url
-        AND _grandParentId <> :tagsFolderId
-        ORDER BY b.lastModified DESC
-        `, { url: info.url.href, tagsFolderId });
+async function fetchBookmarksByURL(info, concurrent) {
+  let query = async function(db) {
+    let tagsFolderId = await promiseTagsFolderId();
+    let rows = await db.executeCached(
+      `/* do not warn (bug no): not worth to add an index */
+      SELECT b.guid, IFNULL(p.guid, "") AS parentGuid, b.position AS 'index',
+              b.dateAdded, b.lastModified, b.type, b.title, h.url AS url,
+              b.id AS _id, b.parent AS _parentId,
+              NULL AS _childCount, /* Unused for now */
+              p.parent AS _grandParentId, b.syncStatus AS _syncStatus
+      FROM moz_bookmarks b
+      JOIN moz_bookmarks p ON p.id = b.parent
+      JOIN moz_places h ON h.id = b.fk
+      WHERE h.url_hash = hash(:url) AND h.url = :url
+      AND _grandParentId <> :tagsFolderId
+      ORDER BY b.lastModified DESC
+      `, { url: info.url.href, tagsFolderId });
 
-      return rows.length ? rowsToItemsArray(rows) : null;
-    }
-  );
+    return rows.length ? rowsToItemsArray(rows) : null;
+  };
+
+  if (concurrent) {
+    let db = await PlacesUtils.promiseDBConnection();
+    return query(db);
+  }
+  return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: fetchBookmarksByURL",
+                                           query);
 }
 
 function fetchRecentBookmarks(numberOfItems) {

toolkit/components/places/tests/bookmarks/test_bookmarks_fetch.js

@@ -307,6 +307,30 @@ add_task(async function fetch_byurl() {
   PlacesUtils.tagging.untagURI(uri(bm1.url.href), ["Test Tag"]);
 });
 
-function run_test() {
-  run_next_test();
-}
+add_task(async function fetch_concurrent() {
+  let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+                                                 url: "http://concurrent.url.com/" });
+  checkBookmarkObject(bm1);
+
+  let bm2 = await PlacesUtils.bookmarks.fetch({ url: bm1.url },
+                                              gAccumulator.callback,
+                                              { concurrent: true });
+  checkBookmarkObject(bm2);
+  Assert.equal(gAccumulator.results.length, 1);
+  Assert.deepEqual(gAccumulator.results[0], bm1);
+  Assert.deepEqual(bm1, bm2);
+  let bm3 = await PlacesUtils.bookmarks.fetch({ url: bm1.url },
+                                              gAccumulator.callback,
+                                              { concurrent: false });
+  checkBookmarkObject(bm3);
+  Assert.equal(gAccumulator.results.length, 1);
+  Assert.deepEqual(gAccumulator.results[0], bm1);
+  Assert.deepEqual(bm1, bm3);
+  let bm4 = await PlacesUtils.bookmarks.fetch({ url: bm1.url },
+                                              gAccumulator.callback,
+                                              {});
+  checkBookmarkObject(bm4);
+  Assert.equal(gAccumulator.results.length, 1);
+  Assert.deepEqual(gAccumulator.results[0], bm1);
+  Assert.deepEqual(bm1, bm4);
+});

toolkit/modules/Sqlite.jsm

@@ -90,17 +90,16 @@ function logScriptError(message) {
 }
 
 /**
- * Gets connection identifier from its database file path.
+ * Gets connection identifier from its database file name.
  *
- * @param path
- *        A file string path pointing to a database file.
+ * @param fileName
+ *        A database file string name.
  * @return the connection identifier.
  */
-function getIdentifierByPath(path) {
-  let basename = OS.Path.basename(path);
-  let number = connectionCounters.get(basename) || 0;
-  connectionCounters.set(basename, number + 1);
-  return basename + "#" + number;
+function getIdentifierByFileName(fileName) {
+  let number = connectionCounters.get(fileName) || 0;
+  connectionCounters.set(fileName, number + 1);
+  return fileName + "#" + number;
 }
 
 /**
@@ -215,7 +214,7 @@ function ConnectionData(connection, identifier, options = {}) {
   this._dbConn = connection;
 
   // This is a unique identifier for the connection, generated through
-  // getIdentifierByPath.  It may be used for logging or as a key in Maps.
+  // getIdentifierByFileName.  It may be used for logging or as a key in Maps.
   this._identifier = identifier;
 
   this._open = true;
@@ -932,7 +931,7 @@ function openConnection(options) {
   }
 
   let file = FileUtils.File(path);
-  let identifier = getIdentifierByPath(path);
+  let identifier = getIdentifierByFileName(OS.Path.basename(path));
 
   log.info("Opening database: " + path + " (" + identifier + ")");
 
@@ -1028,7 +1027,7 @@ function cloneStorageConnection(options) {
   }
 
   let path = source.databaseFile.path;
-  let identifier = getIdentifierByPath(path);
+  let identifier = getIdentifierByFileName(OS.Path.basename(path));
 
   log.info("Cloning database: " + path + " (" + identifier + ")");
 
@@ -1081,10 +1080,9 @@ function wrapStorageConnection(options) {
     throw new Error("Sqlite.jsm has been shutdown. Cannot wrap connection to: " + connection.database.path);
   }
 
-  let path = connection.databaseFile.path;
-  let identifier = getIdentifierByPath(path);
+  let identifier = getIdentifierByFileName(connection.databaseFile.leafName);
 
-  log.info("Wrapping database: " + path + " (" + identifier + ")");
+  log.info("Wrapping database: " + identifier);
   return new Promise(resolve => {
     try {
       let conn = connection.QueryInterface(Ci.mozIStorageAsyncConnection);