Bug 539517 - remove synchronous Bookmarks::SetItemIndex API. r=standard8
MozReview-Commit-ID: 20kwLPazqPC
--- a/services/sync/tests/unit/test_bookmark_tracker.js
+++ b/services/sync/tests/unit/test_bookmark_tracker.js
@@ -1213,18 +1213,18 @@ add_task(async function test_async_onIte
_(`Mozilla GUID: ${mozBmk.guid}`);
await startTracking();
_("Reorder bookmarks");
await PlacesUtils.bookmarks.reorder(PlacesUtils.bookmarks.menuGuid,
[mozBmk.guid, fxBmk.guid, tbBmk.guid]);
- // As with setItemIndex, we should only track the folder if we reorder
- // its children, but we should bump the score for every changed item.
+ // We only track the folder if we reorder its children, but we should
+ // bump the score for every changed item.
await verifyTrackedItems(["menu"]);
Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE * 3);
} finally {
_("Clean up.");
await cleanup();
}
});
--- a/toolkit/components/places/nsINavBookmarksService.idl
+++ b/toolkit/components/places/nsINavBookmarksService.idl
@@ -556,37 +556,16 @@ interface nsINavBookmarksService : nsISu
nsIURI getBookmarkURI(in long long aItemId);
/**
* Get the index for an item.
*/
long getItemIndex(in long long aItemId);
/**
- * Changes the index for a item. This method does not change the indices of
- * any other items in the same folder, so ensure that the new index does not
- * already exist, or change the index of other items accordingly, otherwise
- * the indices will become corrupted.
- *
- * WARNING: This is API is intended for scenarios such as folder sorting,
- * where the caller manages the indices of *all* items in the folder.
- * You must always ensure each index is unique after a reordering.
- *
- * @param aItemId The id of the item to modify
- * @param aNewIndex The new index
- * @param aSource The optional change source, forwarded to all bookmark
- * observers. Defaults to SOURCE_DEFAULT.
- *
- * @throws If aNewIndex is out of bounds.
- */
- void setItemIndex(in long long aItemId,
- in long aNewIndex,
- [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.
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -2352,107 +2352,16 @@ nsNavBookmarks::GetItemIndex(int64_t aIt
*_index = -1;
return NS_OK;
}
*_index = bookmark.position;
return NS_OK;
}
-NS_IMETHODIMP
-nsNavBookmarks::SetItemIndex(int64_t aItemId,
- int32_t aNewIndex,
- uint16_t aSource)
-{
- NS_ENSURE_ARG_MIN(aItemId, 1);
- NS_ENSURE_ARG_MIN(aNewIndex, 0);
-
- BookmarkData bookmark;
- nsresult rv = FetchItemInfo(aItemId, bookmark);
- NS_ENSURE_SUCCESS(rv, rv);
-
- // Ensure we are not going out of range.
- int32_t folderCount;
- int64_t grandParentId;
- nsAutoCString folderGuid;
- rv = FetchFolderInfo(bookmark.parentId, &folderCount, folderGuid, &grandParentId);
- NS_ENSURE_SUCCESS(rv, rv);
- NS_ENSURE_TRUE(aNewIndex < folderCount, NS_ERROR_INVALID_ARG);
- // Check the parent's guid is the expected one.
- MOZ_ASSERT(bookmark.parentGuid == folderGuid);
-
- mozStorageTransaction transaction(mDB->MainConn(), false);
-
- {
- nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
- "UPDATE moz_bookmarks SET "
- "position = :item_index "
- "WHERE id = :item_id"
- );
- NS_ENSURE_STATE(stmt);
- mozStorageStatementScoper scoper(stmt);
-
- rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("item_id"), aItemId);
- NS_ENSURE_SUCCESS(rv, rv);
- rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("item_index"), aNewIndex);
- NS_ENSURE_SUCCESS(rv, rv);
-
- rv = stmt->Execute();
- NS_ENSURE_SUCCESS(rv, rv);
- }
-
- int64_t syncChangeDelta = DetermineSyncChangeDelta(aSource);
-
- {
- // Sync stores child indices in the parent's record, so we only need to
- // bump the parent's change counter.
- nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
- "UPDATE moz_bookmarks SET "
- "syncChangeCounter = syncChangeCounter + :delta "
- "WHERE id = :parent_id"
- );
- NS_ENSURE_STATE(stmt);
- mozStorageStatementScoper scoper(stmt);
-
- rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("parent_id"),
- bookmark.parentId);
- NS_ENSURE_SUCCESS(rv, rv);
- rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("delta"),
- syncChangeDelta);
- NS_ENSURE_SUCCESS(rv, rv);
-
- rv = stmt->Execute();
- NS_ENSURE_SUCCESS(rv, rv);
- }
-
- rv = AdjustSeparatorsSyncCounter(bookmark.parentId, aNewIndex, syncChangeDelta);
- NS_ENSURE_SUCCESS(rv, rv);
-
- rv = PreventSyncReparenting(bookmark);
- NS_ENSURE_SUCCESS(rv, rv);
-
- rv = transaction.Commit();
- NS_ENSURE_SUCCESS(rv, rv);
-
- NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
- nsINavBookmarkObserver,
- OnItemMoved(bookmark.id,
- bookmark.parentId,
- bookmark.position,
- bookmark.parentId,
- aNewIndex,
- bookmark.type,
- bookmark.guid,
- bookmark.parentGuid,
- 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);
--- a/toolkit/components/places/tests/unit/test_sync_utils.js
+++ b/toolkit/components/places/tests/unit/test_sync_utils.js
@@ -1781,31 +1781,16 @@ add_task(async function test_set_orphan_
info("Verify synced orphan annos match");
{
let orphanGuids = await PlacesSyncUtils.bookmarks.fetchGuidsWithAnno(
SYNC_PARENT_ANNO, nonexistentRecordId);
deepEqual(orphanGuids.sort(), [fxBmk.recordId, tbBmk.recordId].sort(),
"Orphaned bookmarks should match before changing indices");
}
- info("Set synced orphan indices");
- {
- let fxId = await recordIdToId(fxBmk.recordId);
- let tbId = await recordIdToId(tbBmk.recordId);
- PlacesUtils.bookmarks.runInBatchMode(_ => {
- PlacesUtils.bookmarks.setItemIndex(fxId, 1);
- PlacesUtils.bookmarks.setItemIndex(tbId, 0);
- }, null);
- await PlacesTestUtils.promiseAsyncUpdates();
- let orphanGuids = await PlacesSyncUtils.bookmarks.fetchGuidsWithAnno(
- SYNC_PARENT_ANNO, nonexistentRecordId);
- deepEqual(orphanGuids, [],
- "Should remove orphan annos after updating indices");
- }
-
await PlacesUtils.bookmarks.eraseEverything();
await PlacesSyncUtils.bookmarks.reset();
});
add_task(async function test_unsynced_orphans() {
let nonexistentRecordId = makeGuid();
let newBmk = await PlacesSyncUtils.bookmarks.insert({
kind: "bookmark",