Bug 1403486 - Convert some xpcshell-tests in toolkit/components/places/tests/bookmarks to Bookmarks.jsm API. r?mak draft
authorMark Banner <standard8@mozilla.com>
Tue, 26 Sep 2017 21:09:30 +0100
changeset 671842 2fb9e2d8521e96d660606466e272df15860b41f8
parent 671801 76a26ef7c493311c170ae83eb0c1d6592a21396d
child 733643 d38979274d21f307ce26cac90ca96901fcce1650
push id82073
push userbmo:standard8@mozilla.com
push dateThu, 28 Sep 2017 13:14:06 +0000
reviewersmak
bugs1403486
milestone58.0a1
Bug 1403486 - Convert some xpcshell-tests in toolkit/components/places/tests/bookmarks to Bookmarks.jsm API. r?mak MozReview-Commit-ID: IjnRFqF95CP
toolkit/components/places/tests/bookmarks/test_1017502-bookmarks_foreign_count.js
toolkit/components/places/tests/bookmarks/test_bmindex.js
toolkit/components/places/tests/bookmarks/test_changeBookmarkURI.js
toolkit/components/places/tests/bookmarks/test_nsINavBookmarkObserver.js
toolkit/components/places/tests/bookmarks/test_removeItem.js
toolkit/components/places/tests/bookmarks/xpcshell.ini
toolkit/components/places/tests/legacy/test_changeBookmarkURI.js
toolkit/components/places/tests/legacy/test_removeItem.js
toolkit/components/places/tests/legacy/xpcshell.ini
--- a/toolkit/components/places/tests/bookmarks/test_1017502-bookmarks_foreign_count.js
+++ b/toolkit/components/places/tests/bookmarks/test_1017502-bookmarks_foreign_count.js
@@ -4,17 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* Bug 1017502 - Add a foreign_count column to moz_places
 This tests, tests the triggers that adjust the foreign_count when a bookmark is
 added or removed and also the maintenance task to fix wrong counts.
 */
 
-const T_URI = NetUtil.newURI("https://www.mozilla.org/firefox/nightly/firstrun/");
+const T_URI = Services.io.newURI("https://www.mozilla.org/firefox/nightly/firstrun/");
 
 async function getForeignCountForURL(conn, url) {
   await PlacesTestUtils.promiseAsyncUpdates();
   url = url instanceof Ci.nsIURI ? url.spec : url;
   let rows = await conn.executeCached(
     `SELECT foreign_count FROM moz_places WHERE url_hash = hash(:t_url)
                                             AND url = :t_url`, { t_url: url });
   return rows[0].getResultByName("foreign_count");
@@ -23,40 +23,46 @@ async function getForeignCountForURL(con
 add_task(async function add_remove_change_bookmark_test() {
   let conn = await PlacesUtils.promiseDBConnection();
 
   // Simulate a visit to the url
   await PlacesTestUtils.addVisits(T_URI);
   Assert.equal((await getForeignCountForURL(conn, T_URI)), 0);
 
   // Add 1st bookmark which should increment foreign_count by 1
-  let id1 = PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
-                    T_URI, PlacesUtils.bookmarks.DEFAULT_INDEX, "First Run");
+  let bm1 = await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+    title: "First Run",
+    url: T_URI,
+  });
   Assert.equal((await getForeignCountForURL(conn, T_URI)), 1);
 
   // Add 2nd bookmark
-  let id2 = PlacesUtils.bookmarks.insertBookmark(PlacesUtils.bookmarksMenuFolderId,
-                      T_URI, PlacesUtils.bookmarks.DEFAULT_INDEX, "First Run");
+  let bm2 = await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.menuGuid,
+    title: "First Run",
+    url: T_URI,
+  });
   Assert.equal((await getForeignCountForURL(conn, T_URI)), 2);
 
   // Remove 2nd bookmark which should decrement foreign_count by 1
-  PlacesUtils.bookmarks.removeItem(id2);
+  await PlacesUtils.bookmarks.remove(bm2);
   Assert.equal((await getForeignCountForURL(conn, T_URI)), 1);
 
   // Change first bookmark's URI
-  const URI2 = NetUtil.newURI("http://www.mozilla.org");
-  PlacesUtils.bookmarks.changeBookmarkURI(id1, URI2);
+  const URI2 = Services.io.newURI("http://www.mozilla.org")
+  bm1.url = URI2;
+  bm1 = await PlacesUtils.bookmarks.update(bm1);
   // Check foreign count for original URI
   Assert.equal((await getForeignCountForURL(conn, T_URI)), 0);
   // Check foreign count for new URI
   Assert.equal((await getForeignCountForURL(conn, URI2)), 1);
 
   // Cleanup - Remove changed bookmark
-  let id = PlacesUtils.bookmarks.getBookmarkIdsForURI(URI2);
-  PlacesUtils.bookmarks.removeItem(id);
+  await PlacesUtils.bookmarks.remove(bm1);
   Assert.equal((await getForeignCountForURL(conn, URI2)), 0);
 
 });
 
 add_task(async function maintenance_foreign_count_test() {
   let conn = await PlacesUtils.promiseDBConnection();
 
   // Simulate a visit to the url
--- a/toolkit/components/places/tests/bookmarks/test_bmindex.js
+++ b/toolkit/components/places/tests/bookmarks/test_bmindex.js
@@ -6,115 +6,112 @@
 
 const NUM_BOOKMARKS = 20;
 const NUM_SEPARATORS = 5;
 const NUM_FOLDERS = 10;
 const NUM_ITEMS = NUM_BOOKMARKS + NUM_SEPARATORS + NUM_FOLDERS;
 const MIN_RAND = -5;
 const MAX_RAND = 40;
 
-var bs = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
-         getService(Ci.nsINavBookmarksService);
-
-function check_contiguous_indexes(aBookmarks) {
+async function check_contiguous_indexes(bookmarks) {
   var indexes = [];
-  aBookmarks.forEach(function(aBookmarkId) {
-    let bmIndex = bs.getItemIndex(aBookmarkId);
-    dump("Index: " + bmIndex + "\n");
-    dump("Checking duplicates\n");
-    do_check_eq(indexes.indexOf(bmIndex), -1);
-    dump("Checking out of range, found " + aBookmarks.length + " items\n");
-    do_check_true(bmIndex >= 0 && bmIndex < aBookmarks.length);
+  for (let bm of bookmarks) {
+    let bmIndex = (await PlacesUtils.bookmarks.fetch(bm.guid)).index;
+    do_print(`Index: ${bmIndex}\n`);
+    do_print("Checking duplicates\n");
+    do_check_false(indexes.includes(bmIndex));
+    do_print(`Checking out of range, found ${bookmarks.length} items\n`);
+    do_check_true(bmIndex >= 0 && bmIndex < bookmarks.length);
     indexes.push(bmIndex);
-  });
-  dump("Checking all valid indexes have been used\n");
-  do_check_eq(indexes.length, aBookmarks.length);
+  }
+  do_print("Checking all valid indexes have been used\n");
+  do_check_eq(indexes.length, bookmarks.length);
 }
 
-// main
-function run_test() {
-  var bookmarks = [];
+add_task(async function test_bookmarks_indexing() {
+  let bookmarks = [];
   // Insert bookmarks with random indexes.
   for (let i = 0; bookmarks.length < NUM_BOOKMARKS; i++) {
     let randIndex = Math.round(MIN_RAND + (Math.random() * (MAX_RAND - MIN_RAND)));
     try {
-      let id = bs.insertBookmark(bs.unfiledBookmarksFolder,
-                                 uri("http://" + i + ".mozilla.org/"),
-                                 randIndex, "Test bookmark " + i);
+      let bm = await PlacesUtils.bookmarks.insert({
+        index: randIndex,
+        parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+        title: `Test bookmark ${i}`,
+        url: `http://${i}.mozilla.org/`,
+      });
       if (randIndex < -1)
         do_throw("Creating a bookmark at an invalid index should throw");
-      bookmarks.push(id);
+      bookmarks.push(bm);
     } catch (ex) {
       if (randIndex >= -1)
         do_throw("Creating a bookmark at a valid index should not throw");
     }
   }
-  check_contiguous_indexes(bookmarks);
+  await check_contiguous_indexes(bookmarks);
 
   // Insert separators with random indexes.
   for (let i = 0; bookmarks.length < NUM_BOOKMARKS + NUM_SEPARATORS; i++) {
     let randIndex = Math.round(MIN_RAND + (Math.random() * (MAX_RAND - MIN_RAND)));
     try {
-      let id = bs.insertSeparator(bs.unfiledBookmarksFolder, randIndex);
+      let bm = await PlacesUtils.bookmarks.insert({
+        index: randIndex,
+        parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+        type: PlacesUtils.bookmarks.TYPE_SEPARATOR,
+      });
       if (randIndex < -1)
         do_throw("Creating a separator at an invalid index should throw");
-      bookmarks.push(id);
+      bookmarks.push(bm);
     } catch (ex) {
       if (randIndex >= -1)
         do_throw("Creating a separator at a valid index should not throw");
     }
   }
-  check_contiguous_indexes(bookmarks);
+  await check_contiguous_indexes(bookmarks);
 
   // Insert folders with random indexes.
   for (let i = 0; bookmarks.length < NUM_ITEMS; i++) {
     let randIndex = Math.round(MIN_RAND + (Math.random() * (MAX_RAND - MIN_RAND)));
     try {
-      let id = bs.createFolder(bs.unfiledBookmarksFolder,
-                               "Test folder " + i, randIndex);
+      let bm = await PlacesUtils.bookmarks.insert({
+        index: randIndex,
+        parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+        title: `Test folder ${i}`,
+        type: PlacesUtils.bookmarks.TYPE_FOLDER,
+      });
       if (randIndex < -1)
         do_throw("Creating a folder at an invalid index should throw");
-      bookmarks.push(id);
+      bookmarks.push(bm);
     } catch (ex) {
       if (randIndex >= -1)
         do_throw("Creating a folder at a valid index should not throw");
     }
   }
-  check_contiguous_indexes(bookmarks);
+  await check_contiguous_indexes(bookmarks);
 
   // Execute some random bookmark delete.
   for (let i = 0; i < Math.ceil(NUM_ITEMS / 4); i++) {
-    let id = bookmarks.splice(Math.floor(Math.random() * bookmarks.length), 1);
-    dump("Removing item with id " + id + "\n");
-    bs.removeItem(id);
+    let bm = bookmarks.splice(Math.floor(Math.random() * bookmarks.length), 1)[0];
+    do_print(`Removing item with guid ${bm.guid}\n`);
+    await PlacesUtils.bookmarks.remove(bm);
   }
-  check_contiguous_indexes(bookmarks);
+  await check_contiguous_indexes(bookmarks);
 
   // Execute some random bookmark move.  This will also try to move it to
   // invalid index values.
   for (let i = 0; i < Math.ceil(NUM_ITEMS / 4); i++) {
     let randIndex = Math.floor(Math.random() * bookmarks.length);
-    let id = bookmarks[randIndex];
+    let bm = bookmarks[randIndex];
     let newIndex = Math.round(MIN_RAND + (Math.random() * (MAX_RAND - MIN_RAND)));
-    dump("Moving item with id " + id + " to index " + newIndex + "\n");
+    do_print(`Moving item with guid ${bm.guid} to index ${newIndex}\n`);
     try {
-      bs.moveItem(id, bs.unfiledBookmarksFolder, newIndex);
+      bm.index = newIndex;
+      await PlacesUtils.bookmarks.update(bm);
       if (newIndex < -1)
         do_throw("Moving an item to a negative index should throw\n");
     } catch (ex) {
       if (newIndex >= -1)
         do_throw("Moving an item to a valid index should not throw\n");
     }
 
   }
-  check_contiguous_indexes(bookmarks);
-
-  // Ensure setItemIndex throws if we pass it a negative index.
-  try {
-    bs.setItemIndex(bookmarks[0], -1);
-    do_throw("setItemIndex should throw for a negative index");
-  } catch (ex) {}
-  // Ensure setItemIndex throws if we pass it a bad itemId.
-  try {
-    bs.setItemIndex(0, 5);
-    do_throw("setItemIndex should throw for a bad itemId");
-  } catch (ex) {}
-}
+  await check_contiguous_indexes(bookmarks);
+});
--- a/toolkit/components/places/tests/bookmarks/test_nsINavBookmarkObserver.js
+++ b/toolkit/components/places/tests/bookmarks/test_nsINavBookmarkObserver.js
@@ -227,17 +227,19 @@ add_task(async function onItemChanged_ti
         ] },
   ])]);
   PlacesUtils.bookmarks.setItemTitle(id, TITLE);
   await promise;
 });
 
 add_task(async function onItemChanged_tags_bookmark() {
   let id = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.unfiledBookmarksFolderId, 0);
-  let uri = PlacesUtils.bookmarks.getBookmarkURI(id);
+  let guid = await PlacesUtils.promiseItemGuid(id);
+  let url = (await PlacesUtils.bookmarks.fetch(guid)).url;
+  let uri = Services.io.newURI(url);
   const TAG = "tag";
   let promise = Promise.all([
     gBookmarkSkipObserver.setup([
       "onItemChanged", "onItemChanged"
     ]),
     gBookmarksObserver.setup([
       { name: "onItemAdded", // This is the tag folder.
         args: [
@@ -381,17 +383,19 @@ add_task(async function onItemMoved_book
         ] },
   ])]);
   PlacesTestUtils.addVisits({ uri, transition: TRANSITION_TYPED });
   await promise;
 });
 
 add_task(async function onItemRemoved_bookmark() {
   let id = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.unfiledBookmarksFolderId, 0);
-  let uri = PlacesUtils.bookmarks.getBookmarkURI(id);
+  let guid = await PlacesUtils.promiseItemGuid(id);
+  let url = (await PlacesUtils.bookmarks.fetch(guid)).url;
+  let uri = Services.io.newURI(url);
   let promise = Promise.all([
     gBookmarkSkipObserver.setup([
       "onItemChanged", "onItemRemoved"
     ]),
     gBookmarksObserver.setup([
       { name: "onItemChanged", // This is an unfortunate effect of bug 653910.
         args: [
           { name: "itemId", check: v => typeof(v) == "number" && v > 0 },
--- a/toolkit/components/places/tests/bookmarks/xpcshell.ini
+++ b/toolkit/components/places/tests/bookmarks/xpcshell.ini
@@ -32,17 +32,15 @@ skip-if = toolkit == 'android'
 [test_bookmarks_getRecent.js]
 [test_bookmarks_insert.js]
 [test_bookmarks_insertTree.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_keywords.js]
 [test_nsINavBookmarkObserver.js]
 [test_protectRoots.js]
 [test_removeFolderTransaction_reinsert.js]
-[test_removeItem.js]
 [test_savedsearches.js]
 [test_sync_fields.js]
 [test_untitled.js]
rename from toolkit/components/places/tests/bookmarks/test_changeBookmarkURI.js
rename to toolkit/components/places/tests/legacy/test_changeBookmarkURI.js
rename from toolkit/components/places/tests/bookmarks/test_removeItem.js
rename to toolkit/components/places/tests/legacy/test_removeItem.js
--- a/toolkit/components/places/tests/legacy/xpcshell.ini
+++ b/toolkit/components/places/tests/legacy/xpcshell.ini
@@ -2,9 +2,11 @@
 # until we remove the APIs themselves.
 
 [DEFAULT]
 head = head_legacy.js
 firefox-appdir = browser
 
 [test_418643_removeFolderChildren.js]
 [test_bookmarks_setNullTitle.js]
+[test_changeBookmarkURI.js]
 [test_placesTxn.js]
+[test_removeItem.js]