Bug 1366829 - Fix various 'undefined property' errors raised in places tests. r=mak
authorMark Banner <standard8@mozilla.com>
Mon, 22 May 2017 18:28:39 +0100
changeset 360174 7fbfa19e0d73b7ee86363efd052870ae3c81170d
parent 360173 952d8db302c26b9783ab9af1d0164d88c7443f25
child 360175 74d4005d165ff18bb7fa8948b740450d7c3b414f
push id31871
push userryanvm@gmail.com
push dateTue, 23 May 2017 22:02:07 +0000
treeherdermozilla-central@545ffce30eac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1366829
milestone55.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 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;