Bug 1366829 - Fix various 'undefined property' errors raised in places tests. r?mak draft
authorMark Banner <standard8@mozilla.com>
Mon, 22 May 2017 18:28:39 +0100
changeset 583019 e2608e5ef8135068b86110719a9ec35997c0d120
parent 582730 5bc1c758ab57c1885dceab4e7837e58af27b998c
child 629940 f265d58bf6ca1649720a53a56cb06ed082cbd276
push id60268
push userbmo:standard8@mozilla.com
push dateTue, 23 May 2017 15:23:28 +0000
reviewersmak
bugs1366829
milestone55.0a1
Bug 1366829 - Fix various 'undefined property' errors raised in places tests. r?mak MozReview-Commit-ID: FaTSwf5QMnr
toolkit/components/places/Bookmarks.jsm
toolkit/components/places/PlacesSyncUtils.jsm
toolkit/components/places/PlacesUtils.jsm
toolkit/components/places/tests/unit/test_async_transactions.js
toolkit/components/places/tests/unit/test_hosts_triggers.js
toolkit/components/places/tests/unit/test_mozIAsyncLivemarks.js
toolkit/components/places/tests/unit/test_promiseBookmarksTree.js
--- a/toolkit/components/places/Bookmarks.jsm
+++ b/toolkit/components/places/Bookmarks.jsm
@@ -635,45 +635,48 @@ var Bookmarks = Object.freeze({
       info = { guid: guidOrInfo };
 
     // Disallow removing the root folders.
     if ([this.rootGuid, this.menuGuid, this.toolbarGuid, this.unfiledGuid,
          this.tagsGuid, this.mobileGuid].includes(info.guid)) {
       throw new Error("It's not possible to remove Places root folders.");
     }
 
+    if (!("source" in options)) {
+      options.source = Bookmarks.SOURCES.DEFAULT;
+    }
+
     // Even if we ignore any other unneeded property, we still validate any
     // known property to reduce likelihood of hidden bugs.
     let removeInfo = validateBookmarkObject(info);
 
     return (async function() {
       let item = await fetchBookmark(removeInfo);
       if (!item)
         throw new Error("No bookmarks found for the provided GUID.");
 
       item = await removeBookmark(item, options);
 
       // Notify onItemRemoved to listeners.
-      let { source = Bookmarks.SOURCES.DEFAULT } = options;
       let observers = PlacesUtils.bookmarks.getObservers();
       let uri = item.hasOwnProperty("url") ? PlacesUtils.toURI(item.url) : null;
       let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
       notify(observers, "onItemRemoved", [ item._id, item._parentId, item.index,
                                            item.type, uri, item.guid,
                                            item.parentGuid,
-                                           source ],
+                                           options.source ],
                                          { isTagging: isUntagging });
 
       if (isUntagging) {
         for (let entry of (await fetchBookmarksByURL(item))) {
           notify(observers, "onItemChanged", [ entry._id, "tags", false, "",
                                                PlacesUtils.toPRTime(entry.lastModified),
                                                entry.type, entry._parentId,
                                                entry.guid, entry.parentGuid,
-                                               "", source ]);
+                                               "", options.source ]);
         }
       }
 
       // Remove non-enumerable properties.
       return Object.assign({}, item);
     })();
   },
 
@@ -686,16 +689,20 @@ var Bookmarks = Object.freeze({
    *        Additional options. Currently supports the following properties:
    *         - source: The change source, forwarded to all bookmark observers.
    *           Defaults to nsINavBookmarksService::SOURCE_DEFAULT.
    *
    * @return {Promise} resolved when the removal is complete.
    * @resolves once the removal is complete.
    */
   eraseEverything(options = {}) {
+    if (!options.source) {
+      options.source = Bookmarks.SOURCES.DEFAULT;
+    }
+
     const folderGuids = [this.toolbarGuid, this.menuGuid, this.unfiledGuid,
                           this.mobileGuid];
     return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: eraseEverything",
       db => db.executeTransaction(async function() {
         await removeFoldersContents(db, folderGuids, options);
         const time = PlacesUtils.toPRTime(new Date());
         const syncChangeDelta =
           PlacesSyncUtils.bookmarks.determineSyncChangeDelta(options.source);
@@ -773,16 +780,19 @@ var Bookmarks = Object.freeze({
    *           promise is resolved to null.
    * @rejects if an error happens while fetching.
    * @throws if the arguments are invalid.
    *
    * @note Any unknown property in the info object is ignored.  Known properties
    *       may be overwritten.
    */
   fetch(guidOrInfo, onResult = null, options = {}) {
+    if (!("concurrent" in options)) {
+      options.concurrent = false;
+    }
     if (onResult && typeof onResult != "function")
       throw new Error("onResult callback must be a valid function");
     let info = guidOrInfo;
     if (!info)
       throw new Error("Input should be a valid object");
     if (typeof(info) != "object") {
       info = { guid: guidOrInfo };
     } else if (Object.keys(info).length == 1) {
@@ -942,16 +952,20 @@ var Bookmarks = Object.freeze({
     if (!Array.isArray(orderedChildrenGuids) || !orderedChildrenGuids.length)
       throw new Error("Must provide a sorted array of children GUIDs.");
     try {
       orderedChildrenGuids.forEach(PlacesUtils.BOOKMARK_VALIDATORS.guid);
     } catch (ex) {
       throw new Error("Invalid GUID found in the sorted children array.");
     }
 
+    if (!("source" in options)) {
+      options.source = Bookmarks.SOURCES.DEFAULT;
+    }
+
     return (async () => {
       let parent = await fetchBookmark(info);
       if (!parent || parent.type != this.TYPE_FOLDER)
         throw new Error("No folder found for the provided GUID.");
 
       let sortedChildren = await reorderChildren(parent, orderedChildrenGuids,
                                                  options);
 
@@ -2153,9 +2167,8 @@ function adjustSeparatorsSyncCounter(db,
     `,
     {
       delta: syncChangeDelta,
       parent: parentId,
       start_index: startIndex,
       item_type: Bookmarks.TYPE_SEPARATOR
     });
 }
-
--- a/toolkit/components/places/PlacesSyncUtils.jsm
+++ b/toolkit/components/places/PlacesSyncUtils.jsm
@@ -909,17 +909,17 @@ var annotateOrphan = async function(item
   let itemId = await PlacesUtils.promiseItemId(guid);
   PlacesUtils.annotations.setItemAnnotation(itemId,
     BookmarkSyncUtils.SYNC_PARENT_ANNO, requestedParentSyncId, 0,
     PlacesUtils.annotations.EXPIRE_NEVER,
     SOURCE_SYNC);
 };
 
 var reparentOrphans = async function(item) {
-  if (item.kind != BookmarkSyncUtils.KINDS.FOLDER) {
+  if (!item.kind || item.kind != BookmarkSyncUtils.KINDS.FOLDER) {
     return;
   }
   let orphanGuids = await fetchGuidsWithAnno(BookmarkSyncUtils.SYNC_PARENT_ANNO,
                                              item.syncId);
   let folderGuid = BookmarkSyncUtils.syncIdToGuid(item.syncId);
   BookmarkSyncLog.debug(`reparentOrphans: Reparenting ${
     JSON.stringify(orphanGuids)} to ${item.syncId}`);
   for (let i = 0; i < orphanGuids.length; ++i) {
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -2271,34 +2271,39 @@ var Keywords = {
    * Adds a new keyword and postData for the given URL.
    *
    * @param keywordEntry
    *        An object describing the keyword to insert, in the form:
    *        {
    *          keyword: non-empty string,
    *          URL: URL or href to associate to the keyword,
    *          postData: optional POST data to associate to the keyword
+   *          source: The change source, forwarded to all bookmark observers.
+   *            Defaults to nsINavBookmarksService::SOURCE_DEFAULT.
    *        }
    * @note Do not define a postData property if there isn't any POST data.
    * @resolves when the addition is complete.
    */
   insert(keywordEntry) {
     if (!keywordEntry || typeof keywordEntry != "object")
       throw new Error("Input should be a valid object");
 
     if (!("keyword" in keywordEntry) || !keywordEntry.keyword ||
         typeof(keywordEntry.keyword) != "string")
       throw new Error("Invalid keyword");
     if (("postData" in keywordEntry) && keywordEntry.postData &&
                                         typeof(keywordEntry.postData) != "string")
       throw new Error("Invalid POST data");
     if (!("url" in keywordEntry))
       throw new Error("undefined is not a valid URL");
-    let { keyword, url,
-          source = Ci.nsINavBookmarksService.SOURCE_DEFAULT } = keywordEntry;
+
+    if (!("source" in keywordEntry)) {
+      keywordEntry.source = PlacesUtils.bookmarks.SOURCES.DEFAULT;
+    }
+    let { keyword, url, source } = keywordEntry;
     keyword = keyword.trim().toLowerCase();
     let postData = keywordEntry.postData || null;
     // This also checks href for validity
     url = new URL(url);
 
     return PlacesUtils.withConnectionWrapper("Keywords.insert", async function(db) {
         let cache = await gKeywordsCachePromise;
 
@@ -2353,18 +2358,22 @@ var Keywords = {
    * Removes a keyword.
    *
    * @param keyword
    *        The keyword to remove.
    * @return {Promise}
    * @resolves when the removal is complete.
    */
   remove(keywordOrEntry) {
-    if (typeof(keywordOrEntry) == "string")
-      keywordOrEntry = { keyword: keywordOrEntry };
+    if (typeof(keywordOrEntry) == "string") {
+      keywordOrEntry = {
+        keyword: keywordOrEntry,
+        source: Ci.nsINavBookmarksService.SOURCE_DEFAULT
+      };
+    }
 
     if (keywordOrEntry === null || typeof(keywordOrEntry) != "object" ||
         !keywordOrEntry.keyword || typeof keywordOrEntry.keyword != "string")
       throw new Error("Invalid keyword");
 
     let { keyword,
           source = Ci.nsINavBookmarksService.SOURCE_DEFAULT } = keywordOrEntry;
     keyword = keywordOrEntry.keyword.trim().toLowerCase();
--- a/toolkit/components/places/tests/unit/test_async_transactions.js
+++ b/toolkit/components/places/tests/unit/test_async_transactions.js
@@ -192,17 +192,21 @@ function ensureItemsRemoved(...items) {
 }
 
 function ensureItemsChanged(...items) {
   for (let item of items) {
     do_check_true(observer.itemsChanged.has(item.guid));
     let changes = observer.itemsChanged.get(item.guid);
     do_check_true(changes.has(item.property));
     let info = changes.get(item.property);
-    do_check_eq(info.isAnnoProperty, Boolean(item.isAnnoProperty));
+    if (!("isAnnoProperty" in item)) {
+      do_check_false(info.isAnnoProperty);
+    } else {
+      do_check_eq(info.isAnnoProperty, Boolean(item.isAnnoProperty));
+    }
     do_check_eq(info.newValue, item.newValue);
     if ("url" in item)
       do_check_true(item.url.equals(info.url));
   }
 }
 
 function ensureAnnotationsSet(aGuid, aAnnos) {
   do_check_true(observer.itemsChanged.has(aGuid));
--- a/toolkit/components/places/tests/unit/test_hosts_triggers.js
+++ b/toolkit/components/places/tests/unit/test_hosts_triggers.js
@@ -79,17 +79,17 @@ var urls = [{uri: NetUtil.newURI("http:/
            ];
 
 const NEW_URL = "http://different.mozilla.org/";
 
 add_task(async function test_moz_hosts_update() {
   let places = [];
   urls.forEach(function(url) {
     let place = { uri: url.uri,
-                  title: "test for " + url.url,
+                  title: "test for " + url.uri.spec,
                   transition: url.typed ? TRANSITION_TYPED : undefined };
     places.push(place);
   });
 
   await PlacesTestUtils.addVisits(places);
 
   checkHostInMozHosts(urls[0].uri, urls[0].typed, urls[0].prefix);
   checkHostInMozHosts(urls[1].uri, urls[1].typed, urls[1].prefix);
--- a/toolkit/components/places/tests/unit/test_mozIAsyncLivemarks.js
+++ b/toolkit/components/places/tests/unit/test_mozIAsyncLivemarks.js
@@ -138,17 +138,17 @@ add_task(async function test_addLivemark
   } catch (ex) {
     do_check_eq(ex.result, Cr.NS_ERROR_INVALID_ARG);
   }
 });
 
 add_task(async function test_addLivemark_badGuid_throws() {
   try {
     await PlacesUtils.livemarks.addLivemark(
-      { parentGuid: PlacesUtils.bookmarks.unfileGuid
+      { parentGuid: PlacesUtils.bookmarks.unfiledGuid
       , feedURI: FEED_URI
       , guid: "123456" });
     do_throw("Invoking addLivemark with a bad guid should throw");
   } catch (ex) {
     do_check_eq(ex.result, Cr.NS_ERROR_INVALID_ARG);
   }
 });
 
--- a/toolkit/components/places/tests/unit/test_promiseBookmarksTree.js
+++ b/toolkit/components/places/tests/unit/test_promiseBookmarksTree.js
@@ -165,16 +165,20 @@ async function new_folder(aInfo) {
 // Walks a result nodes tree and test promiseBookmarksTree for each node.
 // DO NOT COPY THIS LOGIC:  It is done here to accomplish a more comprehensive
 // test of the API (the entire hierarchy data is available in the very test).
 async function test_promiseBookmarksTreeForEachNode(aNode, aOptions, aExcludedGuids) {
   Assert.ok(aNode.bookmarkGuid && aNode.bookmarkGuid.length > 0);
   let item = await PlacesUtils.promiseBookmarksTree(aNode.bookmarkGuid, aOptions);
   await compareToNode(item, aNode, true, aExcludedGuids);
 
+  if (!PlacesUtils.nodeIsContainer(aNode)) {
+    return item;
+  }
+
   for (let i = 0; i < aNode.childCount; i++) {
     let child = aNode.getChild(i);
     if (child.itemId != PlacesUtils.tagsFolderId)
       await test_promiseBookmarksTreeForEachNode(child,
                                                  { includeItemIds: true },
                                                  aExcludedGuids);
   }
   return item;