Bug 1434607 - Remove synchronous Bookmarks::setKeywordForBookmark. draft
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 01 Feb 2018 11:02:41 +0100
changeset 750094 7fdb0733eee96523cc3d846c9429c39de4117ed0
parent 750093 2ee3ae443e4ef944be45b034c11ab2ca5bfa4829
push id97545
push usermak77@bonardo.net
push dateThu, 01 Feb 2018 14:57:12 +0000
bugs1434607
milestone60.0a1
Bug 1434607 - Remove synchronous Bookmarks::setKeywordForBookmark. MozReview-Commit-ID: 3thDN9FIDgm
services/sync/tests/unit/test_bookmark_tracker.js
toolkit/components/places/PlacesUtils.jsm
toolkit/components/places/nsINavBookmarksService.idl
toolkit/components/places/nsNavBookmarks.cpp
toolkit/components/places/tests/bookmarks/test_keywords.js
toolkit/components/places/tests/bookmarks/test_sync_fields.js
toolkit/components/places/tests/legacy/test_bookmarks.js
toolkit/components/places/tests/unit/test_keywords.js
--- a/services/sync/tests/unit/test_bookmark_tracker.js
+++ b/services/sync/tests/unit/test_bookmark_tracker.js
@@ -757,48 +757,16 @@ add_task(async function test_async_onIte
     await verifyTrackedItems([fxBmk1.guid, fxBmk2.guid]);
     Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE * 4);
   } finally {
     _("Clean up.");
     await cleanup();
   }
 });
 
-add_task(async function test_onItemKeywordChanged() {
-  _("Keyword changes via the synchronous API should be tracked");
-
-  try {
-    await stopTracking();
-    let folder = PlacesUtils.bookmarks.createFolder(
-      PlacesUtils.bookmarks.bookmarksMenuFolder, "Parent",
-      PlacesUtils.bookmarks.DEFAULT_INDEX);
-    _("Track changes to keywords");
-    let uri = CommonUtils.makeURI("http://getfirefox.com");
-    let b = PlacesUtils.bookmarks.insertBookmark(
-      folder, uri,
-      PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
-    let bGUID = await PlacesUtils.promiseItemGuid(b);
-    _("New item is " + b);
-    _("GUID: " + bGUID);
-
-    await startTracking();
-
-    _("Give the item a keyword");
-    PlacesUtils.bookmarks.setKeywordForBookmark(b, "the_keyword");
-
-    // bookmark should be tracked, folder should not be.
-    await verifyTrackedItems([bGUID]);
-    Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE);
-
-  } finally {
-    _("Clean up.");
-    await cleanup();
-  }
-});
-
 add_task(async function test_async_onItemKeywordChanged() {
   _("Keyword changes via the asynchronous API should be tracked");
 
   try {
     await stopTracking();
 
     _("Insert two bookmarks with the same URL");
     let fxBmk1 = await PlacesUtils.bookmarks.insert({
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -27,22 +27,20 @@ XPCOMUtils.defineLazyModuleGetters(this,
 // we really just want "\n". On other platforms, the transferable system
 // converts "\r\n" to "\n".
 const NEWLINE = AppConstants.platform == "macosx" ? "\n" : "\r\n";
 
 // Timers resolution is not always good, it can have a 16ms precision on Win.
 const TIMERS_RESOLUTION_SKEW_MS = 16;
 
 function QI_node(aNode, aIID) {
-  var result = null;
   try {
-    result = aNode.QueryInterface(aIID);
-  } catch (e) {
-  }
-  return result;
+    return aNode.QueryInterface(aIID);
+  } catch (ex) {}
+  return null;
 }
 function asContainer(aNode) {
   return QI_node(aNode, Ci.nsINavHistoryContainerResultNode);
 }
 function asQuery(aNode) {
   return QI_node(aNode, Ci.nsINavHistoryQueryResultNode);
 }
 
@@ -71,34 +69,33 @@ function notify(observers, notification,
  *        the url to notify about.
  * @param keyword
  *        The keyword to notify, or empty string if a keyword was removed.
  */
 async function notifyKeywordChange(url, keyword, source) {
   // Notify bookmarks about the removal.
   let bookmarks = [];
   await PlacesUtils.bookmarks.fetch({ url }, b => bookmarks.push(b));
-  // We don't want to yield in the gIgnoreKeywordNotifications section.
   for (let bookmark of bookmarks) {
-    bookmark.id = await PlacesUtils.promiseItemId(bookmark.guid);
-    bookmark.parentId = await PlacesUtils.promiseItemId(bookmark.parentGuid);
+    let ids = await PlacesUtils.promiseManyItemIds([bookmark.guid,
+                                                    bookmark.parentGuid]);
+    bookmark.id = ids.get(bookmark.guid);
+    bookmark.parentId = ids.get(bookmark.parentGuid);
   }
   let observers = PlacesUtils.bookmarks.getObservers();
-  gIgnoreKeywordNotifications = true;
   for (let bookmark of bookmarks) {
     notify(observers, "onItemChanged", [ bookmark.id, "keyword", false,
                                          keyword,
                                          bookmark.lastModified * 1000,
                                          bookmark.type,
                                          bookmark.parentId,
                                          bookmark.guid, bookmark.parentGuid,
                                          "", source
                                        ]);
   }
-  gIgnoreKeywordNotifications = false;
 }
 
 /**
  * Serializes the given node in JSON format.
  *
  * @param aNode
  *        An nsINavHistoryResultNode
  * @param aIsLivemark
@@ -179,16 +176,32 @@ function serializeNode(aNode, aIsLivemar
 }
 
 // Imposed to limit database size.
 const DB_URL_LENGTH_MAX = 65536;
 const DB_TITLE_LENGTH_MAX = 4096;
 const DB_DESCRIPTION_LENGTH_MAX = 256;
 
 /**
+ * Executes a boolean validate function, throwing if it returns false.
+ *
+ * @param boolValidateFn
+ *        A boolean validate function.
+ * @return the input value.
+ * @throws if input doesn't pass the validate function.
+ */
+function simpleValidateFunc(boolValidateFn) {
+  return (v, input) => {
+    if (!boolValidateFn(v, input))
+      throw new Error("Invalid value");
+    return v;
+  };
+}
+
+/**
  * List of bookmark object validators, one per each known property.
  * Validators must throw if the property value is invalid and return a fixed up
  * version of the value, if needed.
  */
 const BOOKMARK_VALIDATORS = Object.freeze({
   guid: simpleValidateFunc(v => PlacesUtils.isValidGuid(v)),
   parentGuid: simpleValidateFunc(v => typeof(v) == "string" &&
                                       /^[a-zA-Z0-9\-_]{12}$/.test(v)),
@@ -617,19 +630,16 @@ this.PlacesUtils = {
         Services.obs.removeObserver(this, this.TOPIC_SHUTDOWN);
         while (this._shutdownFunctions.length > 0) {
           this._shutdownFunctions.shift().apply(this);
         }
         break;
     }
   },
 
-  onPageAnnotationSet() {},
-  onPageAnnotationRemoved() {},
-
   /**
    * Determines whether or not a ResultNode is a host container.
    * @param   aNode
    *          A result node
    * @returns true if the node is a host container, false otherwise
    */
   nodeIsHost: function PU_nodeIsHost(aNode) {
     return aNode.type == Ci.nsINavHistoryResultNode.RESULT_TYPE_QUERY &&
@@ -895,17 +905,17 @@ this.PlacesUtils = {
           let titleString = "";
           if (parts.length > i + 1)
             titleString = parts[i + 1];
           else {
             // for drag and drop of files, try to use the leafName as title
             try {
               titleString = this._uri(uriString).QueryInterface(Ci.nsIURL)
                                 .fileName;
-            } catch (e) {}
+            } catch (ex) {}
           }
           // note:  this._uri() will throw if uriString is not a valid URI
           if (this._uri(uriString)) {
             nodes.push({ uri: uriString,
                          title: titleString ? titleString : uriString,
                          type: this.TYPE_X_MOZ_URL });
           }
         }
@@ -1358,75 +1368,66 @@ this.PlacesUtils = {
    * Sets the character-set for a URI.
    *
    * @param {nsIURI} aURI
    * @param {String} aCharset character-set value.
    * @return {Promise}
    */
   setCharsetForURI: function PU_setCharsetForURI(aURI, aCharset) {
     return new Promise(resolve => {
-
       // Delaying to catch issues with asynchronous behavior while waiting
       // to implement asynchronous annotations in bug 699844.
       Services.tm.dispatchToMainThread(function() {
         if (aCharset && aCharset.length > 0) {
           PlacesUtils.annotations.setPageAnnotation(
             aURI, PlacesUtils.CHARSET_ANNO, aCharset, 0,
             Ci.nsIAnnotationService.EXPIRE_NEVER);
         } else {
           PlacesUtils.annotations.removePageAnnotation(
             aURI, PlacesUtils.CHARSET_ANNO);
         }
         resolve();
       });
-
     });
   },
 
   /**
    * Gets the last saved character-set for a URI.
    *
    * @param aURI nsIURI
    * @return {Promise}
    * @resolve a character-set or null.
    */
   getCharsetForURI: function PU_getCharsetForURI(aURI) {
     return new Promise(resolve => {
-
       Services.tm.dispatchToMainThread(function() {
         let charset = null;
-
         try {
           charset = PlacesUtils.annotations.getPageAnnotation(aURI,
                                                               PlacesUtils.CHARSET_ANNO);
         } catch (ex) { }
-
         resolve(charset);
       });
-
     });
   },
 
   /**
    * Gets favicon data for a given page url.
    *
    * @param aPageUrl url of the page to look favicon for.
    * @resolves to an object representing a favicon entry, having the following
    *           properties: { uri, dataLen, data, mimeType }
    * @rejects JavaScript exception if the given url has no associated favicon.
    */
   promiseFaviconData(aPageUrl) {
     return new Promise((resolve, reject) => {
       PlacesUtils.favicons.getFaviconDataForPage(NetUtil.newURI(aPageUrl),
-        function(aURI, aDataLen, aData, aMimeType) {
-          if (aURI) {
-            resolve({ uri: aURI,
-                               dataLen: aDataLen,
-                               data: aData,
-                               mimeType: aMimeType });
+        function(uri, dataLen, data, mimeType) {
+          if (uri) {
+            resolve({ uri, dataLen, data, mimeType });
           } else {
             reject();
           }
         });
     });
   },
 
   /**
@@ -1616,18 +1617,18 @@ this.PlacesUtils = {
       item.typeCode = type;
       if (type == Ci.nsINavBookmarksService.TYPE_BOOKMARK)
         copyProps("charset", "tags", "iconuri");
 
       // Add annotations.
       if (aRow.getResultByName("has_annos")) {
         try {
           item.annos = PlacesUtils.getAnnotationsForItem(itemId);
-        } catch (e) {
-          Cu.reportError("Unexpected error while reading annotations " + e);
+        } catch (ex) {
+          Cu.reportError("Unexpected error while reading annotations " + ex);
         }
       }
 
       switch (type) {
         case Ci.nsINavBookmarksService.TYPE_BOOKMARK:
           item.type = PlacesUtils.TYPE_X_MOZ_PLACE;
           // If this throws due to an invalid url, the item will be skipped.
           item.uri = NetUtil.newURI(aRow.getResultByName("url")).spec;
@@ -1695,17 +1696,16 @@ this.PlacesUtils = {
                JOIN moz_anno_attributes n ON a.anno_attribute_id = n.id
                WHERE place_id = h.id AND n.name = :charset_anno
               ) AS charset
        FROM descendants d
        LEFT JOIN moz_bookmarks b3 ON b3.id = d.parent
        LEFT JOIN moz_places h ON h.id = d.fk
        ORDER BY d.level, d.parent, d.position`;
 
-
     if (!aItemGuid)
       aItemGuid = this.bookmarks.rootGuid;
 
     let hasExcludeItemsCallback =
       aOptions.hasOwnProperty("excludeItemsCallback");
     let excludedParents = new Set();
     let shouldExcludeItem = (aItem, aParentGuid) => {
       let exclude = excludedParents.has(aParentGuid) ||
@@ -1948,18 +1948,23 @@ var Keywords = {
     let hasKeyword = "keyword" in keywordOrEntry;
     let hasUrl = "url" in keywordOrEntry;
 
     if (!hasKeyword && !hasUrl)
       throw new Error("At least keyword or url must be provided");
     if (onResult && typeof onResult != "function")
       throw new Error("onResult callback must be a valid function");
 
-    if (hasUrl)
-      keywordOrEntry.url = new URL(keywordOrEntry.url);
+    if (hasUrl) {
+      try {
+        keywordOrEntry.url = BOOKMARK_VALIDATORS.url(keywordOrEntry.url);
+      } catch (ex) {
+        throw new Error(keywordOrEntry.url + " is not a valid URL");
+      }
+    }
     if (hasKeyword)
       keywordOrEntry.keyword = keywordOrEntry.keyword.trim().toLowerCase();
 
     let safeOnResult = entry => {
       if (onResult) {
         try {
           onResult(entry);
         } catch (ex) {
@@ -1994,17 +1999,17 @@ 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,
+   *          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) {
@@ -2022,19 +2027,23 @@ var Keywords = {
 
     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);
+    try {
+      url = BOOKMARK_VALIDATORS.url(url);
+    } catch (ex) {
+      throw new Error(url + " is not a valid URL");
+    }
 
-    return PlacesUtils.withConnectionWrapper("Keywords.insert", async function(db) {
+    return PlacesUtils.withConnectionWrapper("PlacesUtils.keywords.insert", async db => {
         let cache = await gKeywordsCachePromise;
 
         // Trying to set the same keyword is a no-op.
         let oldEntry = cache.get(keyword);
         if (oldEntry && oldEntry.url.href == url.href &&
                         oldEntry.postData == keywordEntry.postData) {
           return;
         }
@@ -2100,17 +2109,17 @@ var Keywords = {
 
     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();
-    return PlacesUtils.withConnectionWrapper("Keywords.remove", async function(db) {
+    return PlacesUtils.withConnectionWrapper("PlacesUtils.keywords.remove", async db => {
       let cache = await gKeywordsCachePromise;
       if (!cache.has(keyword))
         return;
       let { url } = cache.get(keyword);
       cache.delete(keyword);
 
       await db.execute(`DELETE FROM moz_keywords WHERE keyword = :keyword`,
                        { keyword });
@@ -2119,25 +2128,20 @@ var Keywords = {
         db, url, PlacesSyncUtils.bookmarks.determineSyncChangeDelta(source));
 
       // Notify bookmarks about the removal.
       await notifyKeywordChange(url.href, "", source);
     });
   }
 };
 
-// Set by the keywords API to distinguish notifications fired by the old API.
-// Once the old API will be gone, we can remove this and stop observing.
-var gIgnoreKeywordNotifications = false;
-
 XPCOMUtils.defineLazyGetter(this, "gKeywordsCachePromise", () =>
   PlacesUtils.withConnectionWrapper("PlacesUtils: gKeywordsCachePromise",
     async function(db) {
       let cache = new Map();
-
       // Start observing changes to bookmarks. For now we are going to keep that
       // relation for backwards compatibility reasons, but mostly because we are
       // lacking a UI to manage keywords directly.
       let observer = {
         QueryInterface: XPCOMUtils.generateQI(Ci.nsINavBookmarkObserver),
         onBeginUpdateBatch() {},
         onEndUpdateBatch() {},
         onItemAdded() {},
@@ -2162,62 +2166,37 @@ XPCOMUtils.defineLazyGetter(this, "gKeyw
                 await PlacesUtils.keywords.remove({ keyword, source });
               }
             }
           })().catch(Cu.reportError);
         },
 
         onItemChanged(id, prop, isAnno, val, lastMod, itemType, parentId, guid,
                       parentGuid, oldVal, source) {
-          if (gIgnoreKeywordNotifications) {
-            return;
-          }
-
-          if (prop == "keyword") {
-            this._onKeywordChanged(guid, val, oldVal);
-          } else if (prop == "uri") {
-            this._onUrlChanged(guid, val, oldVal, source).catch(Cu.reportError);
-          }
-        },
-
-        _onKeywordChanged(guid, keyword, href) {
-          if (keyword.length == 0) {
-            // We are removing a keyword.
-            let keywords = keywordsForHref(href);
-            for (let kw of keywords) {
-              cache.delete(kw);
-            }
-          } else {
-            // We are adding a new keyword.
-            cache.set(keyword, { keyword, url: new URL(href) });
+          if (prop == "uri") {
+            // Check if the old url is associated with keywords.
+            (async function() {
+              let entries = [];
+              await PlacesUtils.keywords.fetch({ url: oldVal }, e => entries.push(e));
+              // Move eventually existing keywords to the new url.
+              for (let entry of entries) {
+                await PlacesUtils.keywords.remove({
+                  keyword: entry.keyword,
+                  source,
+                });
+                await PlacesUtils.keywords.insert({
+                  keyword: entry.keyword,
+                  url: val,
+                  postData: entry.postData,
+                  source,
+                });
+              }
+            })().catch(Cu.reportError);
           }
-        },
-
-        async _onUrlChanged(guid, url, oldUrl, source) {
-          // Check if the old url is associated with keywords.
-          let entries = [];
-          await PlacesUtils.keywords.fetch({ url: oldUrl }, e => entries.push(e));
-          if (entries.length == 0) {
-            return;
-          }
-
-          // Move the keywords to the new url.
-          for (let entry of entries) {
-            await PlacesUtils.keywords.remove({
-              keyword: entry.keyword,
-              source,
-            });
-            await PlacesUtils.keywords.insert({
-              keyword: entry.keyword,
-              url,
-              postData: entry.postData,
-              source,
-            });
-          }
-        },
+        }
       };
 
       PlacesUtils.bookmarks.addObserver(observer);
       PlacesUtils.registerShutdownFunction(() => {
         PlacesUtils.bookmarks.removeObserver(observer);
       });
 
       let rows = await db.execute(
@@ -2397,24 +2376,8 @@ var GuidHelper = {
       };
       PlacesUtils.bookmarks.addObserver(this.observer);
       PlacesUtils.registerShutdownFunction(() => {
         PlacesUtils.bookmarks.removeObserver(this.observer);
       });
     }
   }
 };
-
-/**
- * Executes a boolean validate function, throwing if it returns false.
- *
- * @param boolValidateFn
- *        A boolean validate function.
- * @return the input value.
- * @throws if input doesn't pass the validate function.
- */
-function simpleValidateFunc(boolValidateFn) {
-  return (v, input) => {
-    if (!boolValidateFn(v, input))
-      throw new Error("Invalid value");
-    return v;
-  };
-}
--- a/toolkit/components/places/nsINavBookmarksService.idl
+++ b/toolkit/components/places/nsINavBookmarksService.idl
@@ -606,34 +606,23 @@ interface nsINavBookmarksService : nsISu
                     [optional] in unsigned short aSource);
 
   /**
    * Get the parent folder's id for an item.
    */
   long long getFolderIdForItem(in long long aItemId);
 
   /**
-   * Associates the given keyword with the given bookmark.
-   *
-   * Use an empty keyword to clear the keyword associated with the URI.
-   * In both of these cases, succeeds but does nothing if the URL/keyword is not found.
-   *
-   * @deprecated Use PlacesUtils.keywords.insert() API instead.
-   */
-  void setKeywordForBookmark(in long long aItemId,
-                             in AString aKeyword,
-                             [optional] in unsigned short aSource);
-
-  /**
    * Retrieves the keyword for the given bookmark. Will be void string
    * (null in JS) if no such keyword is found.
    *
    * @deprecated Use PlacesUtils.keywords.fetch() API instead.
    */
   AString getKeywordForBookmark(in long long aItemId);
+
   /**
    * Adds a bookmark observer. If ownsWeak is false, the bookmark service will
    * keep an owning reference to the observer.  If ownsWeak is true, then
    * aObserver must implement nsISupportsWeakReference, and the bookmark
    * service will keep a weak reference to the observer.
    */
   void addObserver(in nsINavBookmarkObserver observer,
                    [optional] in boolean ownsWeak);
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -2528,263 +2528,16 @@ nsNavBookmarks::SetItemIndex(int64_t aIt
                                bookmark.parentGuid,
                                aSource));
 
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
-nsNavBookmarks::SetKeywordForBookmark(int64_t aBookmarkId,
-                                      const nsAString& aUserCasedKeyword,
-                                      uint16_t aSource)
-{
-  NS_ENSURE_ARG_MIN(aBookmarkId, 1);
-
-  // This also ensures the bookmark is valid.
-  BookmarkData bookmark;
-  nsresult rv = FetchItemInfo(aBookmarkId, bookmark);
-  NS_ENSURE_SUCCESS(rv, rv);
-  nsCOMPtr<nsIURI> uri;
-  rv = NS_NewURI(getter_AddRefs(uri), bookmark.url);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  // Shortcuts are always lowercased internally.
-  nsAutoString keyword(aUserCasedKeyword);
-  ToLowerCase(keyword);
-
-  // The same URI can be associated to more than one keyword, provided the post
-  // data differs.  Check if there are already keywords associated to this uri.
-  nsTArray<nsString> oldKeywords;
-  {
-    nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
-      "SELECT keyword FROM moz_keywords WHERE place_id = :place_id"
-    );
-    NS_ENSURE_STATE(stmt);
-    mozStorageStatementScoper scoper(stmt);
-    rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("place_id"), bookmark.placeId);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    bool hasMore;
-    while (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
-      nsString oldKeyword;
-      rv = stmt->GetString(0, oldKeyword);
-      NS_ENSURE_SUCCESS(rv, rv);
-      oldKeywords.AppendElement(oldKeyword);
-    }
-  }
-
-  // Trying to remove a non-existent keyword is a no-op.
-  if (keyword.IsEmpty() && oldKeywords.Length() == 0) {
-    return NS_OK;
-  }
-
-  int64_t syncChangeDelta = DetermineSyncChangeDelta(aSource);
-
-  if (keyword.IsEmpty()) {
-    mozStorageTransaction removeTxn(mDB->MainConn(), false);
-
-    // We are removing the existing keywords.
-    for (uint32_t i = 0; i < oldKeywords.Length(); ++i) {
-      nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
-        "DELETE FROM moz_keywords WHERE keyword = :old_keyword"
-      );
-      NS_ENSURE_STATE(stmt);
-      mozStorageStatementScoper scoper(stmt);
-      rv = stmt->BindStringByName(NS_LITERAL_CSTRING("old_keyword"),
-                                  oldKeywords[i]);
-      NS_ENSURE_SUCCESS(rv, rv);
-      rv = stmt->Execute();
-      NS_ENSURE_SUCCESS(rv, rv);
-    }
-
-    nsTArray<BookmarkData> bookmarks;
-    rv = GetBookmarksForURI(uri, bookmarks);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    if (syncChangeDelta && !bookmarks.IsEmpty()) {
-      // Build a query to update all bookmarks in a single statement.
-      nsAutoCString changedIds;
-      changedIds.AppendInt(bookmarks[0].id);
-      for (uint32_t i = 1; i < bookmarks.Length(); ++i) {
-        changedIds.Append(',');
-        changedIds.AppendInt(bookmarks[i].id);
-      }
-      // Update the sync change counter for all bookmarks with the removed
-      // keyword.
-      nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
-        NS_LITERAL_CSTRING(
-        "UPDATE moz_bookmarks SET "
-         "syncChangeCounter = syncChangeCounter + :delta "
-        "WHERE id IN (") + changedIds + NS_LITERAL_CSTRING(")")
-      );
-      NS_ENSURE_STATE(stmt);
-      mozStorageStatementScoper scoper(stmt);
-
-      rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("delta"),
-                                 syncChangeDelta);
-      NS_ENSURE_SUCCESS(rv, rv);
-
-      rv = stmt->Execute();
-      NS_ENSURE_SUCCESS(rv, rv);
-    }
-
-    rv = removeTxn.Commit();
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    for (uint32_t i = 0; i < bookmarks.Length(); ++i) {
-      NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
-                       nsINavBookmarkObserver,
-                       OnItemChanged(bookmarks[i].id,
-                                     NS_LITERAL_CSTRING("keyword"),
-                                     false,
-                                     EmptyCString(),
-                                     bookmarks[i].lastModified,
-                                     TYPE_BOOKMARK,
-                                     bookmarks[i].parentId,
-                                     bookmarks[i].guid,
-                                     bookmarks[i].parentGuid,
-                                     // Abusing oldVal to pass out the url.
-                                     bookmark.url,
-                                     aSource));
-    }
-
-    return NS_OK;
-  }
-
-  // A keyword can only be associated to a single URI.  Check if the requested
-  // keyword was already associated, in such a case we will need to notify about
-  // the change.
-  nsAutoCString oldSpec;
-  nsCOMPtr<nsIURI> oldUri;
-  {
-    nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
-      "SELECT url "
-      "FROM moz_keywords "
-      "JOIN moz_places h ON h.id = place_id "
-      "WHERE keyword = :keyword"
-    );
-    NS_ENSURE_STATE(stmt);
-    mozStorageStatementScoper scoper(stmt);
-    rv = stmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), keyword);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    bool hasMore;
-    if (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
-      rv = stmt->GetUTF8String(0, oldSpec);
-      NS_ENSURE_SUCCESS(rv, rv);
-      rv = NS_NewURI(getter_AddRefs(oldUri), oldSpec);
-      NS_ENSURE_SUCCESS(rv, rv);
-    }
-  }
-
-  // If another uri is using the new keyword, we must update the keyword entry.
-  // Note we cannot use INSERT OR REPLACE cause it wouldn't invoke the delete
-  // trigger.
-  mozStorageTransaction updateTxn(mDB->MainConn(), false);
-
-  nsCOMPtr<mozIStorageStatement> stmt;
-  if (oldUri) {
-    // In both cases, notify about the change.
-    nsTArray<BookmarkData> bookmarks;
-    rv = GetBookmarksForURI(oldUri, bookmarks);
-    NS_ENSURE_SUCCESS(rv, rv);
-    for (uint32_t i = 0; i < bookmarks.Length(); ++i) {
-      NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
-                       nsINavBookmarkObserver,
-                       OnItemChanged(bookmarks[i].id,
-                                     NS_LITERAL_CSTRING("keyword"),
-                                     false,
-                                     EmptyCString(),
-                                     bookmarks[i].lastModified,
-                                     TYPE_BOOKMARK,
-                                     bookmarks[i].parentId,
-                                     bookmarks[i].guid,
-                                     bookmarks[i].parentGuid,
-                                     // Abusing oldVal to pass out the url.
-                                     oldSpec,
-                                     aSource));
-    }
-
-    stmt = mDB->GetStatement(
-      "UPDATE moz_keywords SET place_id = :place_id WHERE keyword = :keyword"
-    );
-    NS_ENSURE_STATE(stmt);
-  }
-  else {
-    stmt = mDB->GetStatement(
-      "INSERT INTO moz_keywords (keyword, place_id) "
-      "VALUES (:keyword, :place_id)"
-    );
-  }
-  NS_ENSURE_STATE(stmt);
-  mozStorageStatementScoper scoper(stmt);
-
-  rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("place_id"), bookmark.placeId);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = stmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), keyword);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = stmt->Execute();
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  nsTArray<BookmarkData> bookmarks;
-  rv = GetBookmarksForURI(uri, bookmarks);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (syncChangeDelta && !bookmarks.IsEmpty()) {
-    // Build a query to update all bookmarks in a single statement.
-    nsAutoCString changedIds;
-    changedIds.AppendInt(bookmarks[0].id);
-    for (uint32_t i = 1; i < bookmarks.Length(); ++i) {
-      changedIds.Append(',');
-      changedIds.AppendInt(bookmarks[i].id);
-    }
-    // Update the sync change counter for all bookmarks with the new keyword.
-    nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
-      NS_LITERAL_CSTRING(
-      "UPDATE moz_bookmarks SET "
-       "syncChangeCounter = syncChangeCounter + :delta "
-      "WHERE id IN (") + changedIds + NS_LITERAL_CSTRING(")")
-    );
-    NS_ENSURE_STATE(stmt);
-    mozStorageStatementScoper scoper(stmt);
-
-    rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("delta"), syncChangeDelta);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    rv = stmt->Execute();
-    NS_ENSURE_SUCCESS(rv, rv);
-  }
-
-  rv = updateTxn.Commit();
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  // In both cases, notify about the change.
-  for (uint32_t i = 0; i < bookmarks.Length(); ++i) {
-    NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
-                     nsINavBookmarkObserver,
-                     OnItemChanged(bookmarks[i].id,
-                                   NS_LITERAL_CSTRING("keyword"),
-                                   false,
-                                   NS_ConvertUTF16toUTF8(keyword),
-                                   bookmarks[i].lastModified,
-                                   TYPE_BOOKMARK,
-                                   bookmarks[i].parentId,
-                                   bookmarks[i].guid,
-                                   bookmarks[i].parentGuid,
-                                   // Abusing oldVal to pass out the url.
-                                   bookmark.url,
-                                   aSource));
-  }
-
-  return NS_OK;
-}
-
-
-NS_IMETHODIMP
 nsNavBookmarks::GetKeywordForBookmark(int64_t aBookmarkId, nsAString& aKeyword)
 {
   NS_ENSURE_ARG_MIN(aBookmarkId, 1);
   aKeyword.Truncate(0);
 
   // We can have multiple keywords for the same uri, here we'll just return the
   // last created one.
   nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(NS_LITERAL_CSTRING(
--- a/toolkit/components/places/tests/bookmarks/test_keywords.js
+++ b/toolkit/components/places/tests/bookmarks/test_keywords.js
@@ -1,23 +1,26 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
 const URI1 = NetUtil.newURI("http://test1.mozilla.org/");
 const URI2 = NetUtil.newURI("http://test2.mozilla.org/");
 const URI3 = NetUtil.newURI("http://test3.mozilla.org/");
 
 async function check_keyword(aURI, aKeyword) {
   if (aKeyword)
     aKeyword = aKeyword.toLowerCase();
 
   let bms = [];
   await PlacesUtils.bookmarks.fetch({ url: aURI }, bm => bms.push(bm));
   for (let bm of bms) {
     let itemId = await PlacesUtils.promiseItemId(bm.guid);
     let keyword = PlacesUtils.bookmarks.getKeywordForBookmark(itemId);
     if (keyword && !aKeyword) {
-      throw (`${aURI.spec} should not have a keyword`);
+      throw (`${aURI.spec} should not have a keyword, found keyword "${keyword}"`);
     } else if (aKeyword && keyword == aKeyword) {
       Assert.equal(keyword, aKeyword);
     }
   }
 
   if (aKeyword) {
     let uri = await PlacesUtils.keywords.fetch(aKeyword);
     Assert.equal(uri.url, aURI.spec);
@@ -68,247 +71,210 @@ function expectNotifications() {
   return observer;
 }
 
 add_task(function test_invalid_input() {
   Assert.throws(() => PlacesUtils.bookmarks.getKeywordForBookmark(null),
                 /NS_ERROR_ILLEGAL_VALUE/);
   Assert.throws(() => PlacesUtils.bookmarks.getKeywordForBookmark(0),
                 /NS_ERROR_ILLEGAL_VALUE/);
-  Assert.throws(() => PlacesUtils.bookmarks.setKeywordForBookmark(null, "k"),
-                /NS_ERROR_ILLEGAL_VALUE/);
-  Assert.throws(() => PlacesUtils.bookmarks.setKeywordForBookmark(0, "k"),
-                /NS_ERROR_ILLEGAL_VALUE/);
 });
 
 add_task(async function test_addBookmarkAndKeyword() {
   await check_keyword(URI1, null);
   let fc = await foreign_count(URI1);
   let observer = expectNotifications();
 
-  let itemId =
-    PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
-                                         URI1,
-                                         PlacesUtils.bookmarks.DEFAULT_INDEX,
-                                         "test");
-
-  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword");
-  let bookmark = await PlacesUtils.bookmarks.fetch({ url: URI1 });
+  let bookmark = await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+    url: URI1,
+    title: "test"
+  });
+  await PlacesUtils.keywords.insert({url: URI1, keyword: "keyword"});
+  let itemId = await PlacesUtils.promiseItemId(bookmark.guid);
   observer.check([ { name: "onItemChanged",
                      arguments: [ itemId, "keyword", false, "keyword",
                                   bookmark.lastModified * 1000, bookmark.type,
                                   (await PlacesUtils.promiseItemId(bookmark.parentGuid)),
-                                  bookmark.guid, bookmark.parentGuid,
-                                  bookmark.url.href,
+                                  bookmark.guid, bookmark.parentGuid, "",
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
                  ]);
-  await PlacesTestUtils.promiseAsyncUpdates();
-
   await check_keyword(URI1, "keyword");
   Assert.equal((await foreign_count(URI1)), fc + 2); // + 1 bookmark + 1 keyword
-
-  await PlacesTestUtils.promiseAsyncUpdates();
   await check_orphans();
 });
 
 add_task(async function test_addBookmarkToURIHavingKeyword() {
   // The uri has already a keyword.
   await check_keyword(URI1, "keyword");
   let fc = await foreign_count(URI1);
 
-  let itemId =
-    PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
-                                         URI1,
-                                         PlacesUtils.bookmarks.DEFAULT_INDEX,
-                                         "test");
+  let bookmark = await PlacesUtils.bookmarks.insert({
+      parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+      url: URI1,
+      title: "test"
+    });
   await check_keyword(URI1, "keyword");
   Assert.equal((await foreign_count(URI1)), fc + 1); // + 1 bookmark
-
-  PlacesUtils.bookmarks.removeItem(itemId);
-  await PlacesTestUtils.promiseAsyncUpdates();
+  await PlacesUtils.bookmarks.remove(bookmark);
   await check_orphans();
 });
 
 add_task(async function test_sameKeywordDifferentURI() {
   let fc1 = await foreign_count(URI1);
   let fc2 = await foreign_count(URI2);
   let observer = expectNotifications();
 
-  let itemId =
-    PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
-                                         URI2,
-                                         PlacesUtils.bookmarks.DEFAULT_INDEX,
-                                         "test2");
+  let bookmark2 = await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+    url: URI2,
+    title: "test2"
+  });
   await check_keyword(URI1, "keyword");
   await check_keyword(URI2, null);
 
-  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "kEyWoRd");
+  await PlacesUtils.keywords.insert({ url: URI2, keyword: "kEyWoRd" });
 
   let bookmark1 = await PlacesUtils.bookmarks.fetch({ url: URI1 });
-  let bookmark2 = await PlacesUtils.bookmarks.fetch({ url: URI2 });
   observer.check([ { name: "onItemChanged",
                      arguments: [ (await PlacesUtils.promiseItemId(bookmark1.guid)),
                                   "keyword", false, "",
                                   bookmark1.lastModified * 1000, bookmark1.type,
                                   (await PlacesUtils.promiseItemId(bookmark1.parentGuid)),
-                                  bookmark1.guid, bookmark1.parentGuid,
-                                  bookmark1.url.href,
+                                  bookmark1.guid, bookmark1.parentGuid, "",
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] },
                     { name: "onItemChanged",
-                     arguments: [ itemId, "keyword", false, "keyword",
+                     arguments: [ (await PlacesUtils.promiseItemId(bookmark2.guid)),
+                                  "keyword", false, "keyword",
                                   bookmark2.lastModified * 1000, bookmark2.type,
                                   (await PlacesUtils.promiseItemId(bookmark2.parentGuid)),
-                                  bookmark2.guid, bookmark2.parentGuid,
-                                  bookmark2.url.href,
+                                  bookmark2.guid, bookmark2.parentGuid, "",
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
                  ]);
-  await PlacesTestUtils.promiseAsyncUpdates();
 
   // The keyword should have been "moved" to the new URI.
   await check_keyword(URI1, null);
   Assert.equal((await foreign_count(URI1)), fc1 - 1); // - 1 keyword
   await check_keyword(URI2, "keyword");
   Assert.equal((await foreign_count(URI2)), fc2 + 2); // + 1 bookmark + 1 keyword
-
-  await PlacesTestUtils.promiseAsyncUpdates();
   await check_orphans();
 });
 
 add_task(async function test_sameURIDifferentKeyword() {
   let fc = await foreign_count(URI2);
   let observer = expectNotifications();
 
-  let itemId =
-    PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
-                                         URI2,
-                                         PlacesUtils.bookmarks.DEFAULT_INDEX,
-                                         "test2");
+  await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+    url: URI2,
+    title: "test2"
+  });
   await check_keyword(URI2, "keyword");
 
-  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword2");
-
+  await PlacesUtils.keywords.insert({ url: URI2, keyword: "keyword2" });
   let bookmarks = [];
-  await PlacesUtils.bookmarks.fetch({ url: URI2 }, bookmark => bookmarks.push(bookmark));
+  await PlacesUtils.bookmarks.fetch({ url: URI2 }, bm => bookmarks.push(bm));
   observer.check([ { name: "onItemChanged",
                      arguments: [ (await PlacesUtils.promiseItemId(bookmarks[0].guid)),
                                   "keyword", false, "keyword2",
                                   bookmarks[0].lastModified * 1000, bookmarks[0].type,
                                   (await PlacesUtils.promiseItemId(bookmarks[0].parentGuid)),
-                                  bookmarks[0].guid, bookmarks[0].parentGuid,
-                                  bookmarks[0].url.href,
+                                  bookmarks[0].guid, bookmarks[0].parentGuid, "",
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] },
                     { name: "onItemChanged",
                      arguments: [ (await PlacesUtils.promiseItemId(bookmarks[1].guid)),
                                   "keyword", false, "keyword2",
                                   bookmarks[1].lastModified * 1000, bookmarks[1].type,
                                   (await PlacesUtils.promiseItemId(bookmarks[1].parentGuid)),
-                                  bookmarks[1].guid, bookmarks[1].parentGuid,
-                                  bookmarks[0].url.href,
+                                  bookmarks[1].guid, bookmarks[1].parentGuid, "",
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
                  ]);
-  await PlacesTestUtils.promiseAsyncUpdates();
-
   await check_keyword(URI2, "keyword2");
   Assert.equal((await foreign_count(URI2)), fc + 2); // + 1 bookmark + 1 keyword
-
-  await PlacesTestUtils.promiseAsyncUpdates();
   await check_orphans();
 });
 
 add_task(async function test_removeBookmarkWithKeyword() {
   let fc = await foreign_count(URI2);
-  let itemId =
-    PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
-                                         URI2,
-                                         PlacesUtils.bookmarks.DEFAULT_INDEX,
-                                         "test");
+  let bookmark = await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+    url: URI2,
+    title: "test"
+  });
 
    // The keyword should not be removed, since there are other bookmarks yet.
-   PlacesUtils.bookmarks.removeItem(itemId);
+   await PlacesUtils.bookmarks.remove(bookmark);
 
   await check_keyword(URI2, "keyword2");
   Assert.equal((await foreign_count(URI2)), fc); // + 1 bookmark - 1 bookmark
-
-  await PlacesTestUtils.promiseAsyncUpdates();
   await check_orphans();
 });
 
 add_task(async function test_unsetKeyword() {
   let fc = await foreign_count(URI2);
   let observer = expectNotifications();
 
-  let itemId =
-    PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
-                                         URI2,
-                                         PlacesUtils.bookmarks.DEFAULT_INDEX,
-                                         "test");
+  await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+    url: URI2,
+    title: "test"
+  });
 
   // The keyword should be removed from any bookmark.
-  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, null);
+  await PlacesUtils.keywords.remove("keyword2");
 
   let bookmarks = [];
   await PlacesUtils.bookmarks.fetch({ url: URI2 }, bookmark => bookmarks.push(bookmark));
-  info(bookmarks.length);
+  Assert.equal(bookmarks.length, 3, "Check number of bookmarks");
   observer.check([ { name: "onItemChanged",
                      arguments: [ (await PlacesUtils.promiseItemId(bookmarks[0].guid)),
                                   "keyword", false, "",
                                   bookmarks[0].lastModified * 1000, bookmarks[0].type,
                                   (await PlacesUtils.promiseItemId(bookmarks[0].parentGuid)),
-                                  bookmarks[0].guid, bookmarks[0].parentGuid,
-                                  bookmarks[0].url.href,
+                                  bookmarks[0].guid, bookmarks[0].parentGuid, "",
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] },
                     { name: "onItemChanged",
                      arguments: [ (await PlacesUtils.promiseItemId(bookmarks[1].guid)),
                                   "keyword", false, "",
                                   bookmarks[1].lastModified * 1000, bookmarks[1].type,
                                   (await PlacesUtils.promiseItemId(bookmarks[1].parentGuid)),
-                                  bookmarks[1].guid, bookmarks[1].parentGuid,
-                                  bookmarks[1].url.href,
+                                  bookmarks[1].guid, bookmarks[1].parentGuid, "",
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] },
                     { name: "onItemChanged",
                      arguments: [ (await PlacesUtils.promiseItemId(bookmarks[2].guid)),
                                   "keyword", false, "",
                                   bookmarks[2].lastModified * 1000, bookmarks[2].type,
                                   (await PlacesUtils.promiseItemId(bookmarks[2].parentGuid)),
-                                  bookmarks[2].guid, bookmarks[2].parentGuid,
-                                  bookmarks[2].url.href,
+                                  bookmarks[2].guid, bookmarks[2].parentGuid, "",
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
                  ]);
 
   await check_keyword(URI1, null);
   await check_keyword(URI2, null);
   Assert.equal((await foreign_count(URI2)), fc - 1); // + 1 bookmark - 2 keyword
-
-  await PlacesTestUtils.promiseAsyncUpdates();
   await check_orphans();
 });
 
 add_task(async function test_addRemoveBookmark() {
+  let fc = await foreign_count(URI3);
   let observer = expectNotifications();
 
-  let itemId =
-    PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
-                                         URI3,
-                                         PlacesUtils.bookmarks.DEFAULT_INDEX,
-                                         "test3");
-
-  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword");
-  let bookmark = await PlacesUtils.bookmarks.fetch({ url: URI3 });
-  let parentId = await PlacesUtils.promiseItemId(bookmark.parentGuid);
-  PlacesUtils.bookmarks.removeItem(itemId);
+  let bookmark = await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+    url: URI3,
+    title: "test3"
+  });
+  await PlacesUtils.keywords.insert({ url: URI3, keyword: "keyword" });
+  await PlacesUtils.bookmarks.remove(bookmark);
 
   observer.check([ { name: "onItemChanged",
-                     arguments: [ itemId,
+                     arguments: [ (await PlacesUtils.promiseItemId(bookmark.guid)),
                                   "keyword", false, "keyword",
                                   bookmark.lastModified * 1000, bookmark.type,
-                                  parentId,
-                                  bookmark.guid, bookmark.parentGuid,
-                                  bookmark.url.href,
+                                  (await PlacesUtils.promiseItemId(bookmark.parentGuid)),
+                                  bookmark.guid, bookmark.parentGuid, "",
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
                  ]);
 
   await check_keyword(URI3, null);
-  // Don't check the foreign count since the process is async.
-  // The new test_keywords.js in unit is checking this though.
-
-  await PlacesTestUtils.promiseAsyncUpdates();
+  Assert.equal((await foreign_count(URI3)), fc); // +- 1 bookmark +- 1 keyword
   await check_orphans();
 });
--- a/toolkit/components/places/tests/bookmarks/test_sync_fields.js
+++ b/toolkit/components/places/tests/bookmarks/test_sync_fields.js
@@ -112,23 +112,26 @@ class TestCases {
     info(`Set anno on ${guid}`);
     await checkSyncFields(guid, { syncChangeCounter: 3 });
 
     // Tagging a bookmark should update its change counter.
     await this.tagURI(testUri, ["test-tag"]);
     info(`Tagged bookmark ${guid}`);
     await checkSyncFields(guid, { syncChangeCounter: 4 });
 
-    await this.setKeyword(guid, "keyword");
-    info(`Set keyword for bookmark ${guid}`);
-    await checkSyncFields(guid, { syncChangeCounter: 5 });
-
-    await this.removeKeyword(guid, "keyword");
-    info(`Removed keyword from bookmark ${guid}`);
-    await checkSyncFields(guid, { syncChangeCounter: 6 });
+    if ("setKeyword" in this) {
+      await this.setKeyword(guid, "keyword");
+      info(`Set keyword for bookmark ${guid}`);
+      await checkSyncFields(guid, { syncChangeCounter: 5 });
+    }
+    if ("removeKeyword" in this) {
+      await this.removeKeyword(guid, "keyword");
+      info(`Removed keyword from bookmark ${guid}`);
+      await checkSyncFields(guid, { syncChangeCounter: 6 });
+    }
   }
 
   async testSeparators() {
     let insertSyncedBookmark = uri => {
       return this.insertBookmark(PlacesUtils.bookmarks.unfiledGuid,
                                  NetUtil.newURI(uri),
                                  PlacesUtils.bookmarks.DEFAULT_INDEX,
                                  "A bookmark name");
@@ -279,29 +282,16 @@ class SyncTestCases extends TestCases {
     PlacesUtils.bookmarks.removeItem(id);
   }
 
   async setTitle(guid, title) {
     let id = await PlacesUtils.promiseItemId(guid);
     PlacesUtils.bookmarks.setItemTitle(id, title);
   }
 
-  async setKeyword(guid, keyword) {
-    let id = await PlacesUtils.promiseItemId(guid);
-    PlacesUtils.bookmarks.setKeywordForBookmark(id, keyword);
-  }
-
-  async removeKeyword(guid, keyword) {
-    let id = await PlacesUtils.promiseItemId(guid);
-    if (PlacesUtils.bookmarks.getKeywordForBookmark(id) != keyword) {
-      throw new Error(`Keyword ${keyword} not set for bookmark ${guid}`);
-    }
-    PlacesUtils.bookmarks.setKeywordForBookmark(id, "");
-  }
-
   async tagURI(uri, tags) {
     PlacesUtils.tagging.tagURI(uri, tags);
   }
 
   async reorder(parentGuid, childGuids) {
     let parentId = await PlacesUtils.promiseItemId(parentGuid);
     for (let index = 0; index < childGuids.length; ++index) {
       let id = await PlacesUtils.promiseItemId(childGuids[index]);
--- a/toolkit/components/places/tests/legacy/test_bookmarks.js
+++ b/toolkit/components/places/tests/legacy/test_bookmarks.js
@@ -304,28 +304,16 @@ add_task(async function test_bookmarks()
   Assert.equal(bookmarksObserver._itemMovedOldIndex, 0);
   Assert.equal(bookmarksObserver._itemMovedNewParent, testRoot);
   Assert.equal(bookmarksObserver._itemMovedNewIndex, 3);
 
   // test get folder's index
   let tmpFolder = bs.createFolder(testRoot, "tmp", 2);
   Assert.equal(bs.getItemIndex(tmpFolder), 2);
 
-  // test setKeywordForBookmark
-  let kwTestItemId = bs.insertBookmark(testRoot, uri("http://keywordtest.com"),
-                                       bs.DEFAULT_INDEX, "");
-  bs.setKeywordForBookmark(kwTestItemId, "bar");
-
-  // test getKeywordForBookmark
-  let k = bs.getKeywordForBookmark(kwTestItemId);
-  Assert.equal("bar", k);
-
-  // test PlacesUtils.keywords.fetch()
-  let u = await PlacesUtils.keywords.fetch("bar");
-  Assert.equal("http://keywordtest.com/", u.url);
   // test removeFolderChildren
   // 1) add/remove each child type (bookmark, separator, folder)
   tmpFolder = bs.createFolder(testRoot, "removeFolderChildren",
                               bs.DEFAULT_INDEX);
   bs.insertBookmark(tmpFolder, uri("http://foo9.com/"), bs.DEFAULT_INDEX, "");
   bs.createFolder(tmpFolder, "subfolder", bs.DEFAULT_INDEX);
   bs.insertSeparator(tmpFolder, bs.DEFAULT_INDEX);
   // 2) confirm that folder has 3 children
@@ -445,17 +433,17 @@ add_task(async function test_bookmarks()
   // insert a bookmark with title ZZZXXXYYY and then search for it.
   // this test confirms that we can find bookmarks that we haven't visited
   // (which are "hidden") and that we can find by title.
   // see bug #369887 for more details
   let newId13 = bs.insertBookmark(testRoot, uri("http://foobarcheese.com/"),
                                   bs.DEFAULT_INDEX, "");
   Assert.equal(bookmarksObserver._itemAddedId, newId13);
   Assert.equal(bookmarksObserver._itemAddedParent, testRoot);
-  Assert.equal(bookmarksObserver._itemAddedIndex, 11);
+  Assert.equal(bookmarksObserver._itemAddedIndex, 10);
 
   // set bookmark title
   bs.setItemTitle(newId13, "ZZZXXXYYY");
   Assert.equal(bookmarksObserver._itemChangedId, newId13);
   Assert.equal(bookmarksObserver._itemChangedProperty, "title");
   Assert.equal(bookmarksObserver._itemChangedValue, "ZZZXXXYYY");
 
   // check if setting an item annotation triggers onItemChanged
--- a/toolkit/components/places/tests/unit/test_keywords.js
+++ b/toolkit/components/places/tests/unit/test_keywords.js
@@ -479,40 +479,16 @@ add_task(async function test_multipleKey
                        /constraint failed/);
   await check_keyword(false, "http://example.com/", "keyword2", "postData1");
 
   await PlacesUtils.keywords.remove("keyword");
 
   await check_no_orphans();
 });
 
-add_task(async function test_oldKeywordsAPI() {
-  let bookmark = await PlacesUtils.bookmarks.insert({ url: "http://example.com/",
-                                                    parentGuid: PlacesUtils.bookmarks.unfiledGuid });
-  await check_keyword(false, "http://example.com/", "keyword");
-  let itemId = await PlacesUtils.promiseItemId(bookmark.guid);
-
-  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword");
-  await promiseKeyword("keyword", "http://example.com/");
-
-  // Remove the keyword.
-  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "");
-  await promiseKeyword("keyword", null);
-
-  await PlacesUtils.keywords.insert({ keyword: "keyword", url: "http://example.com" });
-  Assert.equal(PlacesUtils.bookmarks.getKeywordForBookmark(itemId), "keyword");
-
-  let entry = await PlacesUtils.keywords.fetch("keyword");
-  Assert.equal(entry.url, "http://example.com/");
-
-  await PlacesUtils.bookmarks.remove(bookmark);
-
-  await check_no_orphans();
-});
-
 add_task(async function test_bookmarkURLChange() {
   let fc1 = await foreign_count("http://example1.com/");
   let fc2 = await foreign_count("http://example2.com/");
   let bookmark = await PlacesUtils.bookmarks.insert({ url: "http://example1.com/",
                                                       parentGuid: PlacesUtils.bookmarks.unfiledGuid });
   await PlacesUtils.keywords.insert({ keyword: "keyword",
                                       url: "http://example1.com/" });