Bug 1376929 - Fix places-related mochitests in chrome/ for async-transactions. r=standard8 draft
authorMarco Bonardo <mbonardo@mozilla.com>
Mon, 03 Jul 2017 15:46:56 +0200
changeset 603349 de00f7f734711e2d2c8521626ad8857ab858bb97
parent 603265 283debe8155accfec5812c66e33384ce217d02c2
child 603779 6f9b121439f6c63ead4477b6049f01585ffcafad
push id66767
push usermak77@bonardo.net
push dateMon, 03 Jul 2017 20:40:55 +0000
reviewersstandard8
bugs1376929
milestone56.0a1
Bug 1376929 - Fix places-related mochitests in chrome/ for async-transactions. r=standard8 MozReview-Commit-ID: ILrvOGzu1zo
addon-sdk/source/lib/sdk/places/host/host-tags.js
browser/components/places/content/editBookmarkOverlay.js
browser/components/places/tests/browser/browser_bookmarkProperties_editTagContainer.js
browser/components/places/tests/chrome/test_bug485100-change-case-loses-tag.xul
browser/components/places/tests/chrome/test_bug631374_tags_selector_scroll.xul
toolkit/components/places/Bookmarks.jsm
toolkit/components/places/PlacesSyncUtils.jsm
toolkit/components/places/PlacesTransactions.jsm
toolkit/components/places/nsNavBookmarks.cpp
toolkit/components/places/nsTaggingService.js
toolkit/components/places/tests/PlacesTestUtils.jsm
--- a/addon-sdk/source/lib/sdk/places/host/host-tags.js
+++ b/addon-sdk/source/lib/sdk/places/host/host-tags.js
@@ -32,28 +32,30 @@ const EVENT_MAP = {
 
 function tag (message) {
   let data = message.data;
   let resData = {
     id: message.id,
     event: message.event
   };
 
-  resData.data = taggingService.tagURI(newURI(data.url), data.tags);
+  if (data.tags && data.tags.length > 0)
+    resData.data = taggingService.tagURI(newURI(data.url), data.tags);
   respond(resData);
 }
 
 function untag (message) {
   let data = message.data;
   let resData = {
     id: message.id,
     event: message.event
   };
 
-  resData.data = taggingService.untagURI(newURI(data.url), data.tags);
+  if (!data.tags || data.tags.length > 0)
+    resData.data = taggingService.untagURI(newURI(data.url), data.tags);
   respond(resData);
 }
 
 function getURLsByTag (message) {
   let data = message.data;
   let resData = {
     id: message.id,
     event: message.event
--- a/browser/components/places/content/editBookmarkOverlay.js
+++ b/browser/components/places/content/editBookmarkOverlay.js
@@ -506,17 +506,20 @@ var gEditItemOverlay = {
     // Optimize the trivial cases (which are actually the most common).
     if (inputTags.length == 0 && aCurrentTags.length == 0)
       return { newTags: [], removedTags: [] };
     if (inputTags.length == 0)
       return { newTags: [], removedTags: aCurrentTags };
     if (aCurrentTags.length == 0)
       return { newTags: inputTags, removedTags: [] };
 
-    let removedTags = aCurrentTags.filter(t => !inputTags.includes(t));
+    // Do not remove tags that may be reinserted with a different
+    // case, since the tagging service may handle those more efficiently.
+    let lcInputTags = inputTags.map(t => t.toLowerCase());
+    let removedTags = aCurrentTags.filter(t => !lcInputTags.includes(t.toLowerCase()));
     let newTags = inputTags.filter(t => !aCurrentTags.includes(t));
     return { removedTags, newTags };
   },
 
   // Adds and removes tags for one or more uris.
   async _setTagsFromTagsInputField(aCurrentTags, aURIs) {
     let { removedTags, newTags } = this._getTagsChanges(aCurrentTags);
     if (removedTags.length + newTags.length == 0)
@@ -532,24 +535,24 @@ var gEditItemOverlay = {
       }
 
       PlacesUtils.transactionManager.doTransaction(
         new PlacesAggregatedTransaction("Update tags", txns));
       return true;
     }
 
     let setTags = async function() {
+      if (removedTags.length > 0) {
+        await PlacesTransactions.Untag({ urls: aURIs, tags: removedTags })
+                                .transact();
+      }
       if (newTags.length > 0) {
         await PlacesTransactions.Tag({ urls: aURIs, tags: newTags })
                                 .transact();
       }
-      if (removedTags.length > 0) {
-        await PlacesTransactions.Untag({ urls: aURIs, tags: removedTags })
-                          .transact();
-      }
     };
 
     // Only in the library info-pane it's safe (and necessary) to batch these.
     // TODO bug 1093030: cleanup this mess when the bookmarksProperties dialog
     // and star UI code don't "run a batch in the background".
     if (window.document.documentElement.id == "places")
       PlacesTransactions.batch(setTags).catch(Components.utils.reportError);
     else
--- a/browser/components/places/tests/browser/browser_bookmarkProperties_editTagContainer.js
+++ b/browser/components/places/tests/browser/browser_bookmarkProperties_editTagContainer.js
@@ -26,34 +26,34 @@ add_task(async function() {
   let fooTag = tagsContainer.getChild(0);
   let tagNode = fooTag;
   tree.selectNode(fooTag);
   Assert.equal(tagNode.title, "tag1", "tagNode title is correct");
 
   Assert.ok(tree.controller.isCommandEnabled("placesCmd_show:info"),
             "'placesCmd_show:info' on current selected node is enabled");
 
-  let promiseTitleResetNotification = promiseBookmarksNotification(
+  let promiseTitleResetNotification = PlacesTestUtils.waitForNotification(
       "onItemChanged", (itemId, prop, isAnno, val) => prop == "title" && val == "tag1");
 
   await withBookmarksDialog(
     true,
     function openDialog() {
       tree.controller.doCommand("placesCmd_show:info");
     },
     async function test(dialogWin) {
       // Check that the dialog is not read-only.
       Assert.ok(!dialogWin.gEditItemOverlay.readOnly, "Dialog should not be read-only");
 
       // Check that name picker is not read only
       let namepicker = dialogWin.document.getElementById("editBMPanel_namePicker");
       Assert.ok(!namepicker.readOnly, "Name field should not be read-only");
       Assert.equal(namepicker.value, "tag1", "Node title is correct");
 
-      let promiseTitleChangeNotification = promiseBookmarksNotification(
+      let promiseTitleChangeNotification = PlacesTestUtils.waitForNotification(
           "onItemChanged", (itemId, prop, isAnno, val) => prop == "title" && val == "tag2");
 
       fillBookmarkTextField("editBMPanel_namePicker", "tag2", dialogWin);
 
       await promiseTitleChangeNotification;
 
       Assert.equal(namepicker.value, "tag2", "Node title has been properly edited");
 
--- a/browser/components/places/tests/chrome/test_bug485100-change-case-loses-tag.xul
+++ b/browser/components/places/tests/chrome/test_bug485100-change-case-loses-tag.xul
@@ -23,27 +23,26 @@
 <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
         title="485100: Exchanging a letter of a tag name with its big/small equivalent removes tag from bookmark"
         onload="runTest();">
 
   <script type="application/javascript"
           src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js" />
   <script type="application/javascript"
           src="chrome://browser/content/places/editBookmarkOverlay.js"/>
+  <script type="application/javascript" src="head.js" />
 
   <body xmlns="http://www.w3.org/1999/xhtml" />
 
   <vbox id="editBookmarkPanelContent"/>
 
   <script type="application/javascript">
   <![CDATA[
-
     function runTest() {
       SimpleTest.waitForExplicitFinish();
-
       (async function() {
         let testTag = "foo";
         let testTagUpper = "Foo";
         let testURI = Services.io.newURI("http://www.example.com/");
 
         // Add a bookmark.
         let bm = await PlacesUtils.bookmarks.insert({
           parentGuid: PlacesUtils.bookmarks.toolbarGuid,
@@ -55,24 +54,33 @@
 
         // Init panel
         ok(gEditItemOverlay, "gEditItemOverlay is in context");
         let node = await PlacesUIUtils.promiseNodeLikeFromFetchInfo(bm);
         gEditItemOverlay.initPanel({ node });
 
         // add a tag
         document.getElementById("editBMPanel_tagsField").value = testTag;
+        let promiseNotification = PlacesTestUtils.waitForNotification(
+          "onItemChanged", (id, property) => property == "tags");
         gEditItemOverlay.onTagsFieldChange();
+        await promiseNotification;
 
         // test that the tag has been added in the backend
         is(PlacesUtils.tagging.getTagsForURI(testURI)[0], testTag, "tags match");
 
         // change the tag
         document.getElementById("editBMPanel_tagsField").value = testTagUpper;
+        // The old sync API doesn't notify a tags change, and fixing it would be
+        // quite complex, so we just wait for a title change until tags are
+        // refactored.
+        promiseNotification = PlacesTestUtils.waitForNotification(
+          "onItemChanged", (id, property) => property == "title");
         gEditItemOverlay.onTagsFieldChange();
+        await promiseNotification;
 
         // test that the tag has been added in the backend
         is(PlacesUtils.tagging.getTagsForURI(testURI)[0], testTagUpper, "tags match");
 
         // Cleanup.
         PlacesUtils.tagging.untagURI(testURI, [testTag]);
         await PlacesUtils.bookmarks.remove(bm.guid);
       })().then(() => SimpleTest.finish());
--- a/browser/components/places/tests/chrome/test_bug631374_tags_selector_scroll.xul
+++ b/browser/components/places/tests/chrome/test_bug631374_tags_selector_scroll.xul
@@ -22,55 +22,49 @@
 <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
         title="Bug 631374 - Editing tags in the selector scrolls up the listbox"
         onload="runTest();">
 
   <script type="application/javascript"
           src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js" />
   <script type="application/javascript"
           src="chrome://browser/content/places/editBookmarkOverlay.js"/>
+  <script type="application/javascript" src="head.js" />
 
   <body xmlns="http://www.w3.org/1999/xhtml" />
 
   <vbox id="editBookmarkPanelContent"/>
 
   <script type="application/javascript">
   <![CDATA[
-
      /**
       * This test checks that editing tags doesn't scroll the tags selector
       * listbox to wrong positions.
       */
 
     function runTest() {
       SimpleTest.waitForExplicitFinish();
-
       (async function() {
-        let bs = PlacesUtils.bookmarks;
-
+        await PlacesUtils.bookmarks.eraseEverything();
         let tags = ["a", "b", "c", "d", "e", "f", "g",
                     "h", "i", "l", "m", "n", "o", "p"];
 
         // Add a bookmark and tag it.
         let uri1 = Services.io.newURI("http://www1.mozilla.org/");
-        let bm1 = await bs.insert({
-          parentGuid: bs.toolbarGuid,
-          index: bs.DEFAULT_INDEX,
-          type: bs.TYPE_BOOKMARK,
+        let bm1 = await PlacesUtils.bookmarks.insert({
+          parentGuid: PlacesUtils.bookmarks.toolbarGuid,
           title: "mozilla",
           url: uri1.spec
         });
         PlacesUtils.tagging.tagURI(uri1, tags);
 
         // Add a second bookmark so that tags won't disappear when unchecked.
         let uri2 = Services.io.newURI("http://www2.mozilla.org/");
-        let bm2 = await bs.insert({
-          parentGuid: bs.toolbarGuid,
-          index: bs.DEFAULT_INDEX,
-          type: bs.TYPE_BOOKMARK,
+        let bm2 = await PlacesUtils.bookmarks.insert({
+          parentGuid: PlacesUtils.bookmarks.toolbarGuid,
           title: "mozilla",
           url: uri2.spec
         });
         PlacesUtils.tagging.tagURI(uri2, tags);
 
         // Init panel.
         ok(gEditItemOverlay, "gEditItemOverlay is in context");
         let node1 = await PlacesUIUtils.promiseNodeLikeFromFetchInfo(bm1);
@@ -88,49 +82,58 @@
 
           tagsSelector.ensureElementIsVisible(listItem);
           let visibleIndex = tagsSelector.getIndexOfFirstVisibleRow();
 
           ok(listItem.checked, "Item is checked " + i);
           let selectedTag = listItem.label;
 
           // Uncheck the tag.
+          let promiseNotification = PlacesTestUtils.waitForNotification(
+            "onItemChanged", (id, property) => property == "tags");
           listItem.checked = false;
+          await promiseNotification;
           is(visibleIndex, tagsSelector.getIndexOfFirstVisibleRow(),
              "Scroll position did not change");
 
           // The listbox is rebuilt, so we have to get the new element.
           let newItem = tagsSelector.selectedItem;
           isnot(newItem, null, "Valid new listItem found");
           ok(!newItem.checked, "New listItem is unchecked " + i);
           is(newItem.label, selectedTag, "Correct tag is still selected");
 
           // Check the tag.
+          promiseNotification = PlacesTestUtils.waitForNotification(
+            "onItemChanged", (id, property) => property == "tags");
           newItem.checked = true;
+          await promiseNotification;
           is(visibleIndex, tagsSelector.getIndexOfFirstVisibleRow(),
              "Scroll position did not change");
         }
 
         // Remove the second bookmark, then nuke some of the tags.
-        await bs.remove(bm2.guid);
+        await PlacesUtils.bookmarks.remove(bm2);
 
         // Doing this backwords tests more interesting paths.
         for (let i = tags.length - 1; i >= 0 ; i -= 2) {
           tagsSelector.selectedIndex = i;
           let listItem = tagsSelector.selectedItem;
           isnot(listItem, null, "Valid listItem found");
 
           tagsSelector.ensureElementIsVisible(listItem);
           let firstVisibleTag = tags[tagsSelector.getIndexOfFirstVisibleRow()];
 
           ok(listItem.checked, "Item is checked " + i);
           let selectedTag = listItem.label;
 
           // Uncheck the tag.
+          let promiseNotification = PlacesTestUtils.waitForNotification(
+            "onItemChanged", (id, property) => property == "tags");
           listItem.checked = false;
+          await promiseNotification;
 
           // Ensure the first visible tag is still visible in the list.
           let firstVisibleIndex = tagsSelector.getIndexOfFirstVisibleRow();
           let lastVisibleIndex = firstVisibleIndex + tagsSelector.getNumberOfVisibleRows() -1;
           let expectedTagIndex = tags.indexOf(firstVisibleTag);
           ok(expectedTagIndex >= firstVisibleIndex &&
              expectedTagIndex <= lastVisibleIndex,
              "Scroll position is correct");
@@ -140,31 +143,29 @@
           isnot(newItem, null, "Valid new listItem found");
           ok(newItem.checked, "New listItem is checked " + i);
           is(tagsSelector.selectedItem.label,
              tags[Math.min(i + 1, tags.length - 2)],
              "The next tag is now selected");
         }
 
         // Cleanup.
-        await bs.remove(bm1.guid);
-      })().then(SimpleTest.finish).catch(alert);
+        await PlacesUtils.bookmarks.remove(bm1);
+      })().catch(ex => ok(false, "test failed: " + ex)).then(SimpleTest.finish);
     }
 
     function openTagSelector() {
       // Wait for the tags selector to be open.
       let promise = new Promise(resolve => {
         let row = document.getElementById("editBMPanel_tagsSelectorRow");
         row.addEventListener("DOMAttrModified", function onAttrModified() {
           row.removeEventListener("DOMAttrModified", onAttrModified);
           resolve();
         });
       });
-
       // Open the tags selector.
       document.getElementById("editBMPanel_tagsSelectorExpander").doCommand();
-
       return promise;
     }
   ]]>
   </script>
 
 </window>
--- a/toolkit/components/places/Bookmarks.jsm
+++ b/toolkit/components/places/Bookmarks.jsm
@@ -226,17 +226,17 @@ var Bookmarks = Object.freeze({
       notify(observers, "onItemAdded", [ itemId, parent._id, item.index,
                                          item.type, uri, item.title || null,
                                          PlacesUtils.toPRTime(item.dateAdded), item.guid,
                                          item.parentGuid, item.source ],
                                        { isTagging: isTagging || isTagsFolder });
 
       // If it's a tag, notify OnItemChanged to all bookmarks for this URL.
       if (isTagging) {
-        for (let entry of (await fetchBookmarksByURL(item))) {
+        for (let entry of (await fetchBookmarksByURL(item, true))) {
           notify(observers, "onItemChanged", [ entry._id, "tags", false, "",
                                                PlacesUtils.toPRTime(entry.lastModified),
                                                entry.type, entry._parentId,
                                                entry.guid, entry.parentGuid,
                                                "", item.source ]);
         }
       }
 
@@ -634,24 +634,40 @@ var Bookmarks = Object.freeze({
                                                updatedItem.type,
                                                updatedItem._parentId,
                                                updatedItem.guid,
                                                updatedItem.parentGuid,
                                                "",
                                                updatedItem.source ]);
         }
         if (updateInfo.hasOwnProperty("title")) {
+          let isTagging = updatedItem.parentGuid == Bookmarks.tagsGuid;
           notify(observers, "onItemChanged", [ updatedItem._id, "title",
                                                false, updatedItem.title,
                                                PlacesUtils.toPRTime(updatedItem.lastModified),
                                                updatedItem.type,
                                                updatedItem._parentId,
                                                updatedItem.guid,
                                                updatedItem.parentGuid, "",
-                                               updatedItem.source ]);
+                                               updatedItem.source ],
+                                               { isTagging });
+          // If we're updating a tag, we must notify all the tagged bookmarks
+          // about the change.
+          if (isTagging) {
+            let URIs = PlacesUtils.tagging.getURIsForTag(updatedItem.title);
+            for (let uri of URIs) {
+              for (let entry of (await fetchBookmarksByURL({ url: new URL(uri.spec) }, true))) {
+                notify(observers, "onItemChanged", [ entry._id, "tags", false, "",
+                                                     PlacesUtils.toPRTime(entry.lastModified),
+                                                     entry.type, entry._parentId,
+                                                     entry.guid, entry.parentGuid,
+                                                     "", updatedItem.source ]);
+              }
+            }
+          }
         }
         if (updateInfo.hasOwnProperty("url")) {
           notify(observers, "onItemChanged", [ updatedItem._id, "uri",
                                                false, updatedItem.url.href,
                                                PlacesUtils.toPRTime(updatedItem.lastModified),
                                                updatedItem.type,
                                                updatedItem._parentId,
                                                updatedItem.guid,
@@ -730,17 +746,17 @@ var Bookmarks = Object.freeze({
       let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
       notify(observers, "onItemRemoved", [ item._id, item._parentId, item.index,
                                            item.type, uri, item.guid,
                                            item.parentGuid,
                                            options.source ],
                                          { isTagging: isUntagging });
 
       if (isUntagging) {
-        for (let entry of (await fetchBookmarksByURL(item))) {
+        for (let entry of (await fetchBookmarksByURL(item, true))) {
           notify(observers, "onItemChanged", [ entry._id, "tags", false, "",
                                                PlacesUtils.toPRTime(entry.lastModified),
                                                entry.type, entry._parentId,
                                                entry.guid, entry.parentGuid,
                                                "", options.source ]);
         }
       }
 
@@ -2217,17 +2233,17 @@ async function(db, folderGuids, options)
                                          item.guid, item.parentGuid,
                                          source ],
                                        // Notify observers that this item is being
                                        // removed as a descendent.
                                        { isDescendantRemoval: true });
 
     let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
     if (isUntagging) {
-      for (let entry of (await fetchBookmarksByURL(item))) {
+      for (let entry of (await fetchBookmarksByURL(item, true))) {
         notify(observers, "onItemChanged", [ entry._id, "tags", false, "",
                                              PlacesUtils.toPRTime(entry.lastModified),
                                              entry.type, entry._parentId,
                                              entry.guid, entry.parentGuid,
                                              "", source ]);
       }
     }
   }
--- a/toolkit/components/places/PlacesSyncUtils.jsm
+++ b/toolkit/components/places/PlacesSyncUtils.jsm
@@ -1415,19 +1415,21 @@ function tagItem(item, tags) {
 
   // Remove leading and trailing whitespace, then filter out empty tags.
   let newTags = tags ? tags.map(tag => tag.trim()).filter(Boolean) : [];
 
   // Removing the last tagged item will also remove the tag. To preserve
   // tag IDs, we temporarily tag a dummy URI, ensuring the tags exist.
   let dummyURI = PlacesUtils.toURI("about:weave#BStore_tagURI");
   let bookmarkURI = PlacesUtils.toURI(item.url.href);
-  PlacesUtils.tagging.tagURI(dummyURI, newTags, SOURCE_SYNC);
+  if (newTags && newTags.length > 0)
+    PlacesUtils.tagging.tagURI(dummyURI, newTags, SOURCE_SYNC);
   PlacesUtils.tagging.untagURI(bookmarkURI, null, SOURCE_SYNC);
-  PlacesUtils.tagging.tagURI(bookmarkURI, newTags, SOURCE_SYNC);
+  if (newTags && newTags.length > 0)
+    PlacesUtils.tagging.tagURI(bookmarkURI, newTags, SOURCE_SYNC);
   PlacesUtils.tagging.untagURI(dummyURI, null, SOURCE_SYNC);
 
   return newTags;
 }
 
 // `PlacesUtils.bookmarks.update` checks if we've supplied enough properties,
 // but doesn't know about additional livemark properties. We check this to avoid
 // having it throw in case we only pass properties like `{ guid, feedURI }`.
--- a/toolkit/components/places/PlacesTransactions.jsm
+++ b/toolkit/components/places/PlacesTransactions.jsm
@@ -1262,17 +1262,17 @@ PT.EditUrl.prototype = Object.seal({
       updatedInfo = await PlacesUtils.bookmarks.update(updatedInfo);
       // Move tags from the original URI to the new URI.
       if (originalTags.length > 0) {
         // Untag the original URI only if this was the only bookmark.
         if (!(await PlacesUtils.bookmarks.fetch({ url: originalInfo.url })))
           PlacesUtils.tagging.untagURI(originalURI, originalTags);
         let currentNewURITags = PlacesUtils.tagging.getTagsForURI(uri);
         newURIAdditionalTags = originalTags.filter(t => !currentNewURITags.includes(t));
-        if (newURIAdditionalTags)
+        if (newURIAdditionalTags && newURIAdditionalTags.length > 0)
           PlacesUtils.tagging.tagURI(uri, newURIAdditionalTags);
       }
     }
     await updateItem();
 
     this.undo = async function() {
       await PlacesUtils.bookmarks.update(originalInfo);
       // Move tags from new URI to original URI.
@@ -1551,22 +1551,28 @@ PT.Untag.prototype = {
       let uri = Services.io.newURI(url.href);
       let tagsToRemove;
       let tagsSet = PlacesUtils.tagging.getTagsForURI(uri);
       if (tags.length > 0) {
         tagsToRemove = tags.filter(t => tagsSet.includes(t));
       } else {
         tagsToRemove = tagsSet;
       }
-      PlacesUtils.tagging.untagURI(uri, tagsToRemove);
+      if (tagsToRemove.length > 0) {
+        PlacesUtils.tagging.untagURI(uri, tagsToRemove);
+      }
       onUndo.unshift(() => {
-        PlacesUtils.tagging.tagURI(uri, tagsToRemove);
+        if (tagsToRemove.length > 0) {
+          PlacesUtils.tagging.tagURI(uri, tagsToRemove);
+        }
       });
       onRedo.push(() => {
-        PlacesUtils.tagging.untagURI(uri, tagsToRemove);
+        if (tagsToRemove.length > 0) {
+          PlacesUtils.tagging.untagURI(uri, tagsToRemove);
+        }
       });
     }
     this.undo = async function() {
       for (let f of onUndo) {
         await f();
       }
     };
     this.redo = async function() {
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -1989,29 +1989,29 @@ nsNavBookmarks::SetItemTitle(int64_t aIt
 
     rv = transaction.Commit();
     NS_ENSURE_SUCCESS(rv, rv);
   } else {
     rv = SetItemTitleInternal(bookmark, title, syncChangeDelta);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
-                   nsINavBookmarkObserver,
-                   OnItemChanged(bookmark.id,
-                                 NS_LITERAL_CSTRING("title"),
-                                 false,
-                                 title,
-                                 bookmark.lastModified,
-                                 bookmark.type,
-                                 bookmark.parentId,
-                                 bookmark.guid,
-                                 bookmark.parentGuid,
-                                 EmptyCString(),
-                                 aSource));
+  NOTIFY_BOOKMARKS_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
+                             SKIP_TAGS(isChangingTagFolder),
+                             OnItemChanged(bookmark.id,
+                                           NS_LITERAL_CSTRING("title"),
+                                           false,
+                                           title,
+                                           bookmark.lastModified,
+                                           bookmark.type,
+                                           bookmark.parentId,
+                                           bookmark.guid,
+                                           bookmark.parentGuid,
+                                           EmptyCString(),
+                                           aSource));
   return NS_OK;
 }
 
 
 nsresult
 nsNavBookmarks::SetItemTitleInternal(BookmarkData& aBookmark,
                                      const nsACString& aTitle,
                                      int64_t aSyncChangeDelta)
--- a/toolkit/components/places/nsTaggingService.js
+++ b/toolkit/components/places/nsTaggingService.js
@@ -123,26 +123,26 @@ TaggingService.prototype = {
       } else if (typeof(idOrName) == "string" && idOrName.length > 0 &&
                idOrName.length <= Ci.nsITaggingService.MAX_TAG_LENGTH) {
         // This is a tag name.
         tag.name = trim ? idOrName.trim() : idOrName;
         // We can't know the id at this point, since a previous tag could
         // have created it.
         tag.__defineGetter__("id", () => this._getItemIdForTag(tag.name));
       } else {
-        throw Cr.NS_ERROR_INVALID_ARG;
+        throw Components.Exception("Invalid tag value", Cr.NS_ERROR_INVALID_ARG);
       }
       return tag;
     });
   },
 
   // nsITaggingService
   tagURI: function TS_tagURI(aURI, aTags, aSource) {
-    if (!aURI || !aTags || !Array.isArray(aTags)) {
-      throw Cr.NS_ERROR_INVALID_ARG;
+    if (!aURI || !aTags || !Array.isArray(aTags) || aTags.length == 0) {
+      throw Components.Exception("Invalid value for tags", Cr.NS_ERROR_INVALID_ARG);
     }
 
     // This also does some input validation.
     let tags = this._convertInputMixedTagsArray(aTags, true);
 
     let taggingFunction = () => {
       for (let tag of tags) {
         if (tag.id == -1) {
@@ -210,18 +210,18 @@ TaggingService.prototype = {
 
     if (count == 0) {
       PlacesUtils.bookmarks.removeItem(aTagId, aSource);
     }
   },
 
   // nsITaggingService
   untagURI: function TS_untagURI(aURI, aTags, aSource) {
-    if (!aURI || (aTags && !Array.isArray(aTags))) {
-      throw Cr.NS_ERROR_INVALID_ARG;
+    if (!aURI || (aTags && (!Array.isArray(aTags) || aTags.length == 0))) {
+      throw Components.Exception("Invalid value for tags", Cr.NS_ERROR_INVALID_ARG);
     }
 
     if (!aTags) {
       // Passing null should clear all tags for aURI, see the IDL.
       // XXXmano: write a perf-sensitive version of this code path...
       aTags = this.getTagsForURI(aURI);
     }
 
@@ -252,18 +252,19 @@ TaggingService.prototype = {
       untaggingFunction();
     } else {
       PlacesUtils.bookmarks.runInBatchMode(untaggingFunction, null);
     }
   },
 
   // nsITaggingService
   getURIsForTag: function TS_getURIsForTag(aTagName) {
-    if (!aTagName || aTagName.length == 0)
-      throw Cr.NS_ERROR_INVALID_ARG;
+    if (!aTagName || aTagName.length == 0) {
+      throw Components.Exception("Invalid tag name", Cr.NS_ERROR_INVALID_ARG);
+    }
 
     if (/^\s|\s$/.test(aTagName)) {
       Deprecated.warning("Tag passed to getURIsForTag was not trimmed",
                          "https://bugzilla.mozilla.org/show_bug.cgi?id=967196");
     }
 
     let uris = [];
     let tagId = this._getItemIdForTag(aTagName);
@@ -288,18 +289,19 @@ TaggingService.prototype = {
       stmt.finalize();
     }
 
     return uris;
   },
 
   // nsITaggingService
   getTagsForURI: function TS_getTagsForURI(aURI, aCount) {
-    if (!aURI)
-      throw Cr.NS_ERROR_INVALID_ARG;
+    if (!aURI) {
+      throw Components.Exception("Invalid uri", Cr.NS_ERROR_INVALID_ARG);
+    }
 
     let tags = [];
     let db = PlacesUtils.history.DBConnection;
     let stmt = db.createStatement(
       `SELECT t.id AS folderId
        FROM moz_bookmarks b
        JOIN moz_bookmarks t on t.id = b.parent
        WHERE b.fk = (SELECT id FROM moz_places WHERE url_hash = hash(:url) AND url = :url) AND
--- a/toolkit/components/places/tests/PlacesTestUtils.jsm
+++ b/toolkit/components/places/tests/PlacesTestUtils.jsm
@@ -320,9 +320,34 @@ this.PlacesTestUtils = Object.freeze({
       SELECT guid, dateRemoved
       FROM moz_bookmarks_deleted
       ORDER BY guid`);
     return rows.map(row => ({
       guid: row.getResultByName("guid"),
       dateRemoved: PlacesUtils.toDate(row.getResultByName("dateRemoved")),
     }));
   },
+
+  waitForNotification(notification, conditionFn = () => true, type = "bookmarks") {
+    let iface = type == "bookmarks" ? Ci.nsINavBookmarkObserver
+                                    : Ci.nsINavHistoryObserver;
+    return new Promise(resolve => {
+      let proxifiedObserver = new Proxy({}, {
+        get: (target, name) => {
+          if (name == "QueryInterface")
+            return XPCOMUtils.generateQI([iface]);
+          if (name == notification)
+            return (...args) => {
+              if (conditionFn.apply(this, args)) {
+                PlacesUtils[type].removeObserver(proxifiedObserver);
+                resolve();
+              }
+            }
+          if (name == "skipTags" || name == "skipDescendantsOnItemRemoval") {
+            return false;
+          }
+          return () => false;
+        }
+      });
+      PlacesUtils[type].addObserver(proxifiedObserver);
+    });
+  },
 });