Bug 1251269 - Implement browser.bookmarks.getRecent(), r=kmag r=mak
authorbsilverberg <bsilverberg@mozilla.com>
Thu, 03 Mar 2016 08:07:16 -0500
changeset 323145 1cfb0f3e893ebe7fe0979b3f74b3dac4b7bb7752
parent 323144 af9a9799032d6cdff853c70b9524ecc89096f590
child 323146 a67f0b208af6351f1f804aeac976fa1814c00c75
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag, mak
bugs1251269
milestone47.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 1251269 - Implement browser.bookmarks.getRecent(), r=kmag r=mak MozReview-Commit-ID: 7nYCplcQZuk
browser/components/extensions/ext-bookmarks.js
browser/components/extensions/schemas/bookmarks.json
toolkit/components/extensions/test/mochitest/test_ext_bookmarks.html
toolkit/components/places/Bookmarks.jsm
toolkit/components/places/tests/bookmarks/test_bookmarks_getRecent.js
toolkit/components/places/tests/bookmarks/xpcshell.ini
--- a/browser/components/extensions/ext-bookmarks.js
+++ b/browser/components/extensions/ext-bookmarks.js
@@ -110,16 +110,20 @@ extensions.registerSchemaAPI("bookmarks"
       getSubTree: function(id) {
         return getTree(id, false);
       },
 
       search: function(query) {
         return Bookmarks.search(query).then(result => result.map(convert));
       },
 
+      getRecent: function(numberOfItems) {
+        return Bookmarks.getRecent(numberOfItems).then(result => result.map(convert));
+      },
+
       create: function(bookmark) {
         let info = {
           title: bookmark.title || "",
         };
 
         // If url is NULL or missing, it will be a folder.
         if (bookmark.url !== null) {
           info.type = Bookmarks.TYPE_BOOKMARK;
--- a/browser/components/extensions/schemas/bookmarks.json
+++ b/browser/components/extensions/schemas/bookmarks.json
@@ -160,17 +160,16 @@
                 "items": { "$ref": "BookmarkTreeNode"}
               }
             ]
           }
         ]
       },
       {
         "name": "getRecent",
-        "unsupported": true,
         "type": "function",
         "description": "Retrieves the recently added bookmarks.",
         "async": "callback",
         "parameters": [
           {
             "type": "integer",
             "minimum": 1,
             "name": "numberOfItems",
--- a/toolkit/components/extensions/test/mochitest/test_ext_bookmarks.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_bookmarks.html
@@ -302,17 +302,44 @@ function backgroundScript() {
       "Expected number of results returned for non-matching title and query fields"
     );
 
     // returns an empty array on item not found
     return browser.bookmarks.search("microsoft");
   }).then(results => {
     browser.test.assertEq(0, results.length, "Expected number of results returned for non-matching search");
 
-    browser.test.notifyPass("bookmarks");
+    return Promise.resolve().then(() => {
+      return browser.bookmarks.getRecent("");
+    }).then(expectedError, error => {
+      browser.test.assertTrue(
+        error.message.includes("Incorrect argument types for bookmarks.getRecent"),
+        "Expected error thrown when calling getRecent with an empty string"
+      );
+    });
+  }).then(() => {
+    return Promise.resolve().then(() => {
+      return browser.bookmarks.getRecent(1.234);
+    }).then(expectedError, error => {
+      browser.test.assertTrue(
+        error.message.includes("Incorrect argument types for bookmarks.getRecent"),
+        "Expected error thrown when calling getRecent with a decimal number"
+      );
+    });
+  }).then(() => {
+    return browser.bookmarks.getRecent(5);
+  }).then(results => {
+    browser.test.assertEq(5, results.length, "Expected number of results returned by getRecent");
+    browser.test.assertEq("Firefox", results[0].title, "Bookmark has the expected title");
+    browser.test.assertEq("Mozilla Corporation", results[1].title, "Bookmark has the expected title");
+    browser.test.assertEq("Mozilla", results[2].title, "Bookmark has the expected title");
+    browser.test.assertEq("Toolbar Item", results[3].title, "Bookmark has the expected title");
+    browser.test.assertEq("Menu Item", results[4].title, "Bookmark has the expected title");
+
+    return browser.test.notifyPass("bookmarks");
   }).catch(error => {
     browser.test.fail(`Error: ${String(error)} :: ${error.stack}`);
   });
 }
 
 let extensionData = {
   background: `(${backgroundScript})()`,
   manifest: {
--- a/toolkit/components/places/Bookmarks.jsm
+++ b/toolkit/components/places/Bookmarks.jsm
@@ -496,16 +496,42 @@ var Bookmarks = Object.freeze({
     return Task.spawn(function* () {
       let results = yield queryBookmarks(query);
 
       return results;
     });
   },
 
   /**
+   * Returns a list of recently bookmarked items.
+   *
+   * @param {integer} numberOfItems
+   *        The maximum number of bookmark items to return.
+   *
+   * @return {Promise} resolved when the listing is complete.
+   * @resolves to an array of recent bookmark-items.
+   * @rejects if an error happens while querying.
+   */
+  getRecent(numberOfItems) {
+    if (numberOfItems === undefined) {
+      throw new Error("numberOfItems argument is required");
+    }
+    if (!typeof numberOfItems === 'number' || (numberOfItems % 1) !== 0) {
+      throw new Error("numberOfItems argument must be an integer");
+    }
+    if (numberOfItems <= 0) {
+      throw new Error("numberOfItems argument must be greater than zero");
+    }
+
+    return Task.spawn(function* () {
+      return yield fetchRecentBookmarks(numberOfItems);
+    });
+  },
+
+  /**
    * Fetches information about a bookmark-item.
    *
    * REMARK: any successful call to this method resolves to a single
    *         bookmark-item (or null), even when multiple bookmarks may exist
    *         (e.g. fetching by url).  If you wish to retrieve all of the
    *         bookmarks for a given match, use the callback instead.
    *
    * Input can be either a guid or an object with one, and only one, of these
@@ -993,16 +1019,36 @@ function fetchBookmarksByURL(info) {
        ORDER BY b.lastModified DESC
       `, { url: info.url.href,
            tags_folder: PlacesUtils.tagsFolderId });
 
     return rows.length ? rowsToItemsArray(rows) : null;
   }));
 }
 
+function fetchRecentBookmarks(numberOfItems) {
+  return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: fetchRecentBookmarks",
+    Task.async(function*(db) {
+
+    let rows = yield 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,
+              NULL AS _id, NULL AS _parentId, NULL AS _childCount, NULL AS _grandParentId
+       FROM moz_bookmarks b
+       LEFT JOIN moz_bookmarks p ON p.id = b.parent
+       LEFT JOIN moz_places h ON h.id = b.fk
+       WHERE p.parent <> :tags_folder
+       ORDER BY b.dateAdded DESC, b.ROWID DESC
+       LIMIT :numberOfItems
+      `, { tags_folder: PlacesUtils.tagsFolderId, numberOfItems });
+
+    return rows.length ? rowsToItemsArray(rows) : [];
+  }));
+}
+
 function fetchBookmarksByParent(info) {
   return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: fetchBookmarksByParent",
     Task.async(function*(db) {
 
     let rows = yield 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,
               b.id AS _id, b.parent AS _parentId,
copy from toolkit/components/places/tests/bookmarks/test_bookmarks_search.js
copy to toolkit/components/places/tests/bookmarks/test_bookmarks_getRecent.js
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_search.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_getRecent.js
@@ -1,223 +1,44 @@
 add_task(function* invalid_input_throws() {
-  Assert.throws(() => PlacesUtils.bookmarks.search(),
-                /Query object is required/);
-  Assert.throws(() => PlacesUtils.bookmarks.search(null),
-                /Query object is required/);
-  Assert.throws(() => PlacesUtils.bookmarks.search({title: 50}),
-                /Title option must be a string/);
-  Assert.throws(() => PlacesUtils.bookmarks.search({url: {url: "wombat"}}),
-                /Url option must be a string or a URL object/);
-  Assert.throws(() => PlacesUtils.bookmarks.search(50),
-                /Query must be an object or a string/);
-  Assert.throws(() => PlacesUtils.bookmarks.search(true),
-                /Query must be an object or a string/);
-});
-
-add_task(function* search_bookmark() {
-  yield PlacesUtils.bookmarks.eraseEverything();
-
-  let bm1 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
-                                                 url: "http://example.com/",
-                                                 title: "a bookmark" });
-  let bm2 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
-                                                 url: "http://example.org/",
-                                                 title: "another bookmark" });
-  let bm3 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.menuGuid,
-                                                 url: "http://menu.org/",
-                                                 title: "a menu bookmark" });
-  let bm4 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.toolbarGuid,
-                                                 url: "http://toolbar.org/",
-                                                 title: "a toolbar bookmark" });
-  checkBookmarkObject(bm1);
-  checkBookmarkObject(bm2);
-  checkBookmarkObject(bm3);
-  checkBookmarkObject(bm4);
-
-  // finds a result by query
-  let results = yield PlacesUtils.bookmarks.search("example.com");
-  Assert.equal(results.length, 1);
-  checkBookmarkObject(results[0]);
-  Assert.deepEqual(bm1, results[0]);
-
-  // finds multiple results
-  results = yield PlacesUtils.bookmarks.search("example");
-  Assert.equal(results.length, 2);
-  checkBookmarkObject(results[0]);
-  checkBookmarkObject(results[1]);
-
-  // finds menu bookmarks
-  results = yield PlacesUtils.bookmarks.search("a menu bookmark");
-  Assert.equal(results.length, 1);
-  checkBookmarkObject(results[0]);
-  Assert.deepEqual(bm3, results[0]);
-
-  // finds toolbar bookmarks
-  results = yield PlacesUtils.bookmarks.search("a toolbar bookmark");
-  Assert.equal(results.length, 1);
-  checkBookmarkObject(results[0]);
-  Assert.deepEqual(bm4, results[0]);
-
-  yield PlacesUtils.bookmarks.eraseEverything();
+  Assert.throws(() => PlacesUtils.bookmarks.getRecent(),
+                /numberOfItems argument is required/);
+  Assert.throws(() => PlacesUtils.bookmarks.getRecent("abc"),
+                /numberOfItems argument must be an integer/);
+  Assert.throws(() => PlacesUtils.bookmarks.getRecent(1.2),
+                /numberOfItems argument must be an integer/);
+  Assert.throws(() => PlacesUtils.bookmarks.getRecent(0),
+                /numberOfItems argument must be greater than zero/);
+  Assert.throws(() => PlacesUtils.bookmarks.getRecent(-1),
+                /numberOfItems argument must be greater than zero/);
 });
 
-add_task(function* search_bookmark_by_query_object() {
-  let bm1 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
-                                                 url: "http://example.com/",
-                                                 title: "a bookmark" });
-  let bm2 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
-                                                 url: "http://example.org/",
-                                                 title: "another bookmark" });
-  checkBookmarkObject(bm1);
-  checkBookmarkObject(bm2);
-
-  let results = yield PlacesUtils.bookmarks.search({query: "example.com"});
-  Assert.equal(results.length, 1);
-  checkBookmarkObject(results[0]);
-
-  Assert.deepEqual(bm1, results[0]);
-
-  results = yield PlacesUtils.bookmarks.search({query: "example"});
-  Assert.equal(results.length, 2);
-  checkBookmarkObject(results[0]);
-  checkBookmarkObject(results[1]);
-
+add_task(function* getRecent_returns_recent_bookmarks() {
   yield PlacesUtils.bookmarks.eraseEverything();
-});
 
-add_task(function* search_bookmark_by_url() {
-  let bm1 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
-                                                 url: "http://example.com/",
-                                                 title: "a bookmark" });
-  let bm2 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
-                                                 url: "http://example.org/path",
-                                                 title: "another bookmark" });
-  let bm3 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
-                                                 url: "http://example.org/path",
-                                                 title: "third bookmark" });
-  checkBookmarkObject(bm1);
-  checkBookmarkObject(bm2);
-  checkBookmarkObject(bm3);
-
-  // finds the correct result by url
-  let results = yield PlacesUtils.bookmarks.search({url: "http://example.com/"});
-  Assert.equal(results.length, 1);
-  checkBookmarkObject(results[0]);
-  Assert.deepEqual(bm1, results[0]);
-
-  // normalizes the url
-  results = yield PlacesUtils.bookmarks.search({url: "http:/example.com"});
-  Assert.equal(results.length, 1);
-  checkBookmarkObject(results[0]);
-  Assert.deepEqual(bm1, results[0]);
-
-  // returns multiple matches
-  results = yield PlacesUtils.bookmarks.search({url: "http://example.org/path"});
-  Assert.equal(results.length, 2);
-
-  // requires exact match
-  results = yield PlacesUtils.bookmarks.search({url: "http://example.org/"});
-  Assert.equal(results.length, 0);
-
-  yield PlacesUtils.bookmarks.eraseEverything();
-});
-
-add_task(function* search_bookmark_by_title() {
   let bm1 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
                                                  url: "http://example.com/",
                                                  title: "a bookmark" });
   let bm2 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
                                                  url: "http://example.org/path",
                                                  title: "another bookmark" });
   let bm3 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
                                                  url: "http://example.net/",
                                                  title: "another bookmark" });
-  checkBookmarkObject(bm1);
-  checkBookmarkObject(bm2);
-  checkBookmarkObject(bm3);
-
-  // finds the correct result by title
-  let results = yield PlacesUtils.bookmarks.search({title: "a bookmark"});
-  Assert.equal(results.length, 1);
-  checkBookmarkObject(results[0]);
-  Assert.deepEqual(bm1, results[0]);
-
-  // returns multiple matches
-  results = yield PlacesUtils.bookmarks.search({title: "another bookmark"});
-  Assert.equal(results.length, 2);
-
-  // requires exact match
-  results = yield PlacesUtils.bookmarks.search({title: "bookmark"});
-  Assert.equal(results.length, 0);
-
-  yield PlacesUtils.bookmarks.eraseEverything();
-});
-
-add_task(function* search_bookmark_combinations() {
-  let bm1 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
-                                                 url: "http://example.com/",
-                                                 title: "a bookmark" });
-  let bm2 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
-                                                 url: "http://example.org/path",
-                                                 title: "another bookmark" });
-  let bm3 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
-                                                 url: "http://example.net/",
-                                                 title: "third bookmark" });
+  let bm4 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+                                                 url: "http://example.net/path",
+                                                 title: "yet another bookmark" });
   checkBookmarkObject(bm1);
   checkBookmarkObject(bm2);
   checkBookmarkObject(bm3);
-
-  // finds the correct result if title and url match
-  let results = yield PlacesUtils.bookmarks.search({url: "http://example.com/", title: "a bookmark"});
-  Assert.equal(results.length, 1);
-  checkBookmarkObject(results[0]);
-  Assert.deepEqual(bm1, results[0]);
-
-  // does not match if query is not matching but url and title match
-  results = yield PlacesUtils.bookmarks.search({url: "http://example.com/", title: "a bookmark", query: "nonexistent"});
-  Assert.equal(results.length, 0);
+  checkBookmarkObject(bm4);
 
-  // does not match if one parameter is not matching
-  results = yield PlacesUtils.bookmarks.search({url: "http://what.ever", title: "a bookmark"});
-  Assert.equal(results.length, 0);
-
-  // query only matches if other fields match as well
-  results = yield PlacesUtils.bookmarks.search({query: "bookmark", url: "http://example.net/"});
-  Assert.equal(results.length, 1);
+  let results = yield PlacesUtils.bookmarks.getRecent(3);
+  Assert.equal(results.length, 3);
   checkBookmarkObject(results[0]);
-  Assert.deepEqual(bm3, results[0]);
-
-  // non-matching query will also return no results
-  results = yield PlacesUtils.bookmarks.search({query: "nonexistent", url: "http://example.net/"});
-  Assert.equal(results.length, 0);
+  Assert.deepEqual(bm4, results[0]);
+  checkBookmarkObject(results[1]);
+  Assert.deepEqual(bm3, results[1]);
+  checkBookmarkObject(results[2]);
+  Assert.deepEqual(bm2, results[2]);
 
   yield PlacesUtils.bookmarks.eraseEverything();
 });
-
-add_task(function* search_folder() {
-  let folder = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.menuGuid,
-                                                 type: PlacesUtils.bookmarks.TYPE_FOLDER,
-                                                 title: "a test folder" });
-  let bm = yield PlacesUtils.bookmarks.insert({ parentGuid: folder.guid,
-                                                 url: "http://example.com/",
-                                                 title: "a bookmark" });
-  checkBookmarkObject(folder);
-  checkBookmarkObject(bm);
-
-  // also finds folders
-  let results = yield PlacesUtils.bookmarks.search("a test folder");
-  Assert.equal(results.length, 1);
-  checkBookmarkObject(results[0]);
-  Assert.equal(folder.title, results[0].title);
-  Assert.equal(folder.type, results[0].type);
-  Assert.equal(folder.parentGuid, results[0].parentGuid);
-
-  // finds elements in folders
-  results = yield PlacesUtils.bookmarks.search("example.com");
-  Assert.equal(results.length, 1);
-  checkBookmarkObject(results[0]);
-  Assert.deepEqual(bm, results[0]);
-  Assert.equal(folder.guid, results[0].parentGuid);
-
-  yield PlacesUtils.bookmarks.eraseEverything();
-});
-
--- a/toolkit/components/places/tests/bookmarks/xpcshell.ini
+++ b/toolkit/components/places/tests/bookmarks/xpcshell.ini
@@ -28,16 +28,17 @@ skip-if = toolkit == 'android' || toolki
 [test_997030-bookmarks-html-encode.js]
 [test_1129529.js]
 [test_async_observers.js]
 [test_bmindex.js]
 [test_bookmarkstree_cache.js]
 [test_bookmarks.js]
 [test_bookmarks_eraseEverything.js]
 [test_bookmarks_fetch.js]
+[test_bookmarks_getRecent.js]
 [test_bookmarks_insert.js]
 [test_bookmarks_notifications.js]
 [test_bookmarks_remove.js]
 [test_bookmarks_reorder.js]
 [test_bookmarks_search.js]
 [test_bookmarks_update.js]
 [test_changeBookmarkURI.js]
 [test_getBookmarkedURIFor.js]