Bug 1432437 - Remove synchronous changeBookmarkURI. r=standard8

MozReview-Commit-ID: Jy76eKCN7wp

--HG--
extra : rebase_source : 2d453281eb9c77c3f23f15a5a390a17e7b880c34

services/sync/tests/unit/test_bookmark_tracker.js

@@ -599,26 +599,27 @@ add_task(async function test_onItemChanged_changeBookmarkURI() {
     await stopTracking();
 
     _("Insert a bookmark");
-    let fx_id = PlacesUtils.bookmarks.insertBookmark(
-      PlacesUtils.bookmarks.bookmarksMenuFolder,
-      CommonUtils.makeURI("http://getfirefox.com"),
-      PlacesUtils.bookmarks.DEFAULT_INDEX,
-      "Get Firefox!");
-    let fx_guid = await engine._store.GUIDForId(fx_id);
-    _(`Firefox GUID: ${fx_guid}`);
+    let bm = await PlacesUtils.bookmarks.insert({
+      parentGuid: PlacesUtils.bookmarks.menuGuid,
+      url: "http://getfirefox.com",
+      title: "Get Firefox!"
+    });
+    _(`Firefox GUID: ${bm.guid}`);
 
     _("Set a tracked annotation to make sure we only notify once");
+    let id = await PlacesUtils.promiseItemId(bm.guid);
     PlacesUtils.annotations.setItemAnnotation(
-      fx_id, PlacesSyncUtils.bookmarks.DESCRIPTION_ANNO, "A test description", 0,
+      id, PlacesSyncUtils.bookmarks.DESCRIPTION_ANNO, "A test description", 0,
       PlacesUtils.annotations.EXPIRE_NEVER);
 
     await startTracking();
 
     _("Change the bookmark's URI");
-    PlacesUtils.bookmarks.changeBookmarkURI(fx_id,
-      CommonUtils.makeURI("https://www.mozilla.org/firefox"));
-    await verifyTrackedItems([fx_guid]);
-    Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE);
+    bm.url = "https://www.mozilla.org/firefox";
+    bm = await PlacesUtils.bookmarks.update(bm);
+
+    await verifyTrackedItems([bm.guid]);
+    Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE * 2);
   } finally {
     _("Clean up.");
     await cleanup();

toolkit/components/places/nsINavBookmarksService.idl

@@ -605,15 +605,6 @@ interface nsINavBookmarksService : nsISupports
                     in long aNewIndex,
                     [optional] in unsigned short aSource);
 
-  /**
-   * Change the bookmarked URI for a bookmark.
-   * This changes which "place" the bookmark points at,
-   * which means all annotations, etc are carried along.
-   */
-  void changeBookmarkURI(in long long aItemId,
-                         in nsIURI aNewURI,
-                         [optional] in unsigned short aSource);
-
   /**
    * Get the parent folder's id for an item.
    */

toolkit/components/places/nsNavBookmarks.cpp

@@ -2348,96 +2348,6 @@ nsNavBookmarks::FetchFolderInfo(int64_t aFolderId,
 }
 
 
-NS_IMETHODIMP
-nsNavBookmarks::ChangeBookmarkURI(int64_t aBookmarkId, nsIURI* aNewURI,
-                                  uint16_t aSource)
-{
-  NS_ENSURE_ARG_MIN(aBookmarkId, 1);
-  NS_ENSURE_ARG(aNewURI);
-
-  BookmarkData bookmark;
-  nsresult rv = FetchItemInfo(aBookmarkId, bookmark);
-  NS_ENSURE_SUCCESS(rv, rv);
-  NS_ENSURE_ARG(bookmark.type == TYPE_BOOKMARK);
-
-  mozStorageTransaction transaction(mDB->MainConn(), false);
-
-  int64_t tagsRootId = TagsRootId();
-  bool isTagging = bookmark.grandParentId == tagsRootId;
-  int64_t syncChangeDelta = DetermineSyncChangeDelta(aSource);
-
-  nsNavHistory* history = nsNavHistory::GetHistoryService();
-  NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
-  int64_t newPlaceId;
-  nsAutoCString newPlaceGuid;
-  rv = history->GetOrCreateIdForPage(aNewURI, &newPlaceId, newPlaceGuid);
-  NS_ENSURE_SUCCESS(rv, rv);
-  if (!newPlaceId)
-    return NS_ERROR_INVALID_ARG;
-
-  nsCOMPtr<mozIStorageStatement> statement = mDB->GetStatement(
-    "UPDATE moz_bookmarks SET "
-     "fk = :page_id, lastModified = :date, "
-     "syncChangeCounter = syncChangeCounter + :delta "
-    "WHERE id = :item_id "
-  );
-  NS_ENSURE_STATE(statement);
-  mozStorageStatementScoper scoper(statement);
-
-  rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), newPlaceId);
-  NS_ENSURE_SUCCESS(rv, rv);
-  bookmark.lastModified = RoundedPRNow();
-  rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("date"),
-                                  bookmark.lastModified);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("item_id"), bookmark.id);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("delta"), syncChangeDelta);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = statement->Execute();
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (isTagging) {
-    // For consistency with the tagging service behavior, changing a tag entry's
-    // URL bumps the change counter for bookmarks with the old and new URIs.
-    rv = AddSyncChangesForBookmarksWithURL(bookmark.url, syncChangeDelta);
-    NS_ENSURE_SUCCESS(rv, rv);
-    rv = AddSyncChangesForBookmarksWithURI(aNewURI, syncChangeDelta);
-    NS_ENSURE_SUCCESS(rv, rv);
-  }
-
-  rv = transaction.Commit();
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  rv = history->UpdateFrecency(newPlaceId);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  // Upon changing the URI for a bookmark, update the frecency for the old
-  // place as well.
-  rv = history->UpdateFrecency(bookmark.placeId);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  nsAutoCString spec;
-  rv = aNewURI->GetSpec(spec);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
-                   nsINavBookmarkObserver,
-                   OnItemChanged(bookmark.id,
-                                 NS_LITERAL_CSTRING("uri"),
-                                 false,
-                                 spec,
-                                 bookmark.lastModified,
-                                 bookmark.type,
-                                 bookmark.parentId,
-                                 bookmark.guid,
-                                 bookmark.parentGuid,
-                                 bookmark.url,
-                                 aSource));
-  return NS_OK;
-}
-
-
 NS_IMETHODIMP
 nsNavBookmarks::GetFolderIdForItem(int64_t aItemId, int64_t* _parentId)
 {

toolkit/components/places/tests/legacy/test_bookmarks.js

@@ -425,27 +425,11 @@ add_task(async function test_bookmarks() {
   dateAdded -= 1000;
   bs.setItemDateAdded(newId10, dateAdded);
 
-  bs.changeBookmarkURI(newId10, uri("http://foo11.com/"));
-
-  // check that lastModified is set after we change the bookmark uri
-  lastModified2 = bs.getItemLastModified(newId10);
-  info("test changeBookmarkURI");
-  info("dateAdded = " + dateAdded);
-  info("lastModified = " + lastModified);
-  info("lastModified2 = " + lastModified2);
-  Assert.ok(is_time_ordered(lastModified, lastModified2));
-  Assert.ok(is_time_ordered(dateAdded, lastModified2));
-
-  Assert.equal(bookmarksObserver._itemChangedId, newId10);
-  Assert.equal(bookmarksObserver._itemChangedProperty, "uri");
-  Assert.equal(bookmarksObserver._itemChangedValue, "http://foo11.com/");
-  Assert.equal(bookmarksObserver._itemChangedOldValue, "http://foo10.com/");
-
   // test getBookmarkURI
-  let newId11 = bs.insertBookmark(testRoot, uri("http://foo11.com/"),
+  let newId11 = bs.insertBookmark(testRoot, uri("http://foo10.com/"),
                                   bs.DEFAULT_INDEX, "");
   let bmURI = bs.getBookmarkURI(newId11);
-  Assert.equal("http://foo11.com/", bmURI.spec);
+  Assert.equal("http://foo10.com/", bmURI.spec);
 
   // test getBookmarkURI with non-bookmark items
   try {
@@ -454,7 +438,7 @@ add_task(async function test_bookmarks() {
   } catch (ex) {}
 
   // test getItemIndex
-  let newId12 = bs.insertBookmark(testRoot, uri("http://foo11.com/"), 1, "");
+  let newId12 = bs.insertBookmark(testRoot, uri("http://foo10.com/"), 1, "");
   let bmIndex = bs.getItemIndex(newId12);
   Assert.equal(1, bmIndex);
 

toolkit/components/places/tests/legacy/test_changeBookmarkURI.js

@@ -1,62 +0,0 @@
-/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
-/* vim:set ts=2 sw=2 sts=2 et: */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-// Get bookmark service
-var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
-            getService(Ci.nsINavBookmarksService);
-
-/**
- * Ensures that the Places APIs recognize that aBookmarkedUri is bookmarked
- * via aBookmarkId and that aUnbookmarkedUri is not bookmarked at all.
- *
- * @param aBookmarkId
- *        an item ID whose corresponding URI is aBookmarkedUri
- * @param aBookmarkedUri
- *        a bookmarked URI that has a corresponding item ID aBookmarkId
- * @param aUnbookmarkedUri
- *        a URI that is not currently bookmarked at all
- */
-async function checkUris(aBookmarkId, aBookmarkedUri, aUnbookmarkedUri) {
-  // Ensure that aBookmarkedUri equals some URI that is bookmarked
-  let bm = await PlacesUtils.bookmarks.fetch({url: aBookmarkedUri});
-
-  Assert.notEqual(bm, null);
-  Assert.ok(uri(bm.url).equals(aBookmarkedUri));
-
-  // Ensure that the URI corresponding to aBookmarkId equals aBookmarkedUri
-  Assert.ok(bmsvc.getBookmarkURI(aBookmarkId).equals(aBookmarkedUri));
-
-  // Ensure that aUnbookmarkedUri does not equal any URI that is bookmarked
-  bm = await PlacesUtils.bookmarks.fetch({url: aUnbookmarkedUri});
-  Assert.equal(bm, null);
-}
-
-// main
-add_task(async function() {
-  // Create a folder
-  var folderId = bmsvc.createFolder(bmsvc.toolbarFolder,
-                                    "test",
-                                    bmsvc.DEFAULT_INDEX);
-
-  // Create 2 URIs
-  var uri1 = uri("http://www.dogs.com/");
-  var uri2 = uri("http://www.cats.com/");
-
-  // Bookmark the first one
-  var bookmarkId = bmsvc.insertBookmark(folderId,
-                                        uri1,
-                                        bmsvc.DEFAULT_INDEX,
-                                        "Dogs");
-
-  // uri1 is bookmarked via bookmarkId, uri2 is not
-  await checkUris(bookmarkId, uri1, uri2);
-
-  // Change the URI of the bookmark to uri2
-  bmsvc.changeBookmarkURI(bookmarkId, uri2);
-
-  // uri2 is now bookmarked via bookmarkId, uri1 is not
-  await checkUris(bookmarkId, uri2, uri1);
-});

toolkit/components/places/tests/legacy/xpcshell.ini

@@ -8,6 +8,5 @@ firefox-appdir = browser
 [test_418643_removeFolderChildren.js]
 [test_bookmarks.js]
 [test_bookmarks_setNullTitle.js]
-[test_changeBookmarkURI.js]
 [test_protectRoots.js]
 [test_removeItem.js]

toolkit/components/places/tests/unit/test_sync_utils.js

@@ -703,19 +703,6 @@ add_task(async function test_pullChanges_tags() {
     await setChangesSynced(changes);
   }
 
-  info("Change tag entry URI using Bookmarks.changeBookmarkURI");
-  {
-    let tagId = PlacesUtils.bookmarks.getIdForItemAt(tagFolderId, 0);
-    PlacesUtils.bookmarks.changeBookmarkURI(tagId, uri("https://bugzilla.org"));
-    let changes = await PlacesSyncUtils.bookmarks.pullChanges();
-    deepEqual(Object.keys(changes).sort(),
-      [firstItem.recordId, secondItem.recordId, untaggedItem.recordId].sort(),
-      "Should include tagged bookmarks after changing tag entry URI");
-    assertTagForURLs("tricky", ["https://bugzilla.org/", "https://mozilla.org/"],
-      "Should remove tag entry for old URI");
-    await setChangesSynced(changes);
-  }
-
   info("Change tag entry URL using Bookmarks.update");
   {
     let tagGuid = await PlacesUtils.promiseItemGuid(