Bug 1432434 - Remove Bookmarks::getItemDateAdded and setItemDateAdded. r=kitcambridge
MozReview-Commit-ID: XVoil1CK7c
--- a/services/sync/tests/unit/test_bookmark_tracker.js
+++ b/services/sync/tests/unit/test_bookmark_tracker.js
@@ -554,38 +554,53 @@ add_task(async function test_async_onIte
add_task(async function test_onItemChanged_itemDates() {
_("Changes to item dates should be tracked");
try {
await tracker.stop();
_("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 PlacesUtils.promiseItemGuid(fx_id);
- _(`Firefox GUID: ${fx_guid}`);
+ let fx_bm = await PlacesUtils.bookmarks.insert({
+ parentGuid: PlacesUtils.bookmarks.menuGuid,
+ url: "http://getfirefox.com",
+ title: "Get Firefox!"
+ });
+ _(`Firefox GUID: ${fx_bm.guid}`);
await startTracking();
- _("Reset the bookmark's added date");
- // Convert to microseconds for PRTime.
- let dateAdded = (Date.now() - DAY_IN_MS) * 1000;
- PlacesUtils.bookmarks.setItemDateAdded(fx_id, dateAdded);
- await verifyTrackedItems([fx_guid]);
+ _("Reset the bookmark's added date, should not be tracked");
+ let dateAdded = new Date(Date.now() - DAY_IN_MS);
+ await PlacesUtils.bookmarks.update({
+ guid: fx_bm.guid,
+ dateAdded
+ });
+ await verifyTrackedCount(0);
Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE);
+
+ await resetTracker();
+
+ _("Reset the bookmark's added date and another property, should be tracked");
+ dateAdded = new Date();
+ await PlacesUtils.bookmarks.update({
+ guid: fx_bm.guid,
+ dateAdded,
+ title: "test"
+ });
+ await verifyTrackedItems([fx_bm.guid]);
+ Assert.equal(tracker.score, 2 * SCORE_INCREMENT_XLARGE);
+
await resetTracker();
_("Set the bookmark's last modified date");
+ let fx_id = await PlacesUtils.promiseItemId(fx_bm.guid);
let dateModified = Date.now() * 1000;
PlacesUtils.bookmarks.setItemLastModified(fx_id, dateModified);
- await verifyTrackedItems([fx_guid]);
+ await verifyTrackedItems([fx_bm.guid]);
Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE);
} finally {
_("Clean up.");
await cleanup();
}
});
add_task(async function test_onItemTagged() {
--- a/toolkit/components/places/nsINavBookmarksService.idl
+++ b/toolkit/components/places/nsINavBookmarksService.idl
@@ -499,42 +499,16 @@ interface nsINavBookmarksService : nsISu
*
* @param aItemId
* The id of the item whose title should be retrieved
* @return The title of the item.
*/
AUTF8String getItemTitle(in long long aItemId);
/**
- * Set the date added time for an item.
- *
- * @param aItemId
- * the id of the item whose date added time should be updated.
- * @param aDateAdded
- * the new date added value in microseconds. Note that it is rounded
- * down to milliseconds precision.
- * @param [optional] aSource
- * The change source, forwarded to all bookmark observers. Defaults
- * to SOURCE_DEFAULT.
- */
- void setItemDateAdded(in long long aItemId,
- in PRTime aDateAdded,
- [optional] in unsigned short aSource);
-
- /**
- * Get the date added time for an item.
- *
- * @param aItemId
- * the id of the item whose date added time should be retrieved.
- *
- * @return the date added value in microseconds.
- */
- PRTime getItemDateAdded(in long long aItemId);
-
- /**
* Set the last modified time for an item.
*
* @param aItemId
* the id of the item whose last modified time should be updated.
* @param aLastModified
* the new last modified value in microseconds. Note that it is
* rounded down to milliseconds precision.
* @param [optional] aSource
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -1500,34 +1500,21 @@ nsNavBookmarks::FetchItemInfo(int64_t aI
nsresult
nsNavBookmarks::SetItemDateInternal(enum BookmarkDate aDateType,
int64_t aSyncChangeDelta,
int64_t aItemId,
PRTime aValue)
{
aValue = RoundToMilliseconds(aValue);
- nsCOMPtr<mozIStorageStatement> stmt;
- if (aDateType == DATE_ADDED) {
- // lastModified is set to the same value as dateAdded. We do this for
- // performance reasons, since it will allow us to use an index to sort items
- // by date.
- stmt = mDB->GetStatement(
- "UPDATE moz_bookmarks SET dateAdded = :date, lastModified = :date, "
- "syncChangeCounter = syncChangeCounter + :delta "
- "WHERE id = :item_id"
- );
- }
- else {
- stmt = mDB->GetStatement(
- "UPDATE moz_bookmarks SET lastModified = :date, "
- "syncChangeCounter = syncChangeCounter + :delta "
- "WHERE id = :item_id"
- );
- }
+ nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
+ "UPDATE moz_bookmarks SET lastModified = :date, "
+ "syncChangeCounter = syncChangeCounter + :delta "
+ "WHERE id = :item_id"
+ );
NS_ENSURE_STATE(stmt);
mozStorageStatementScoper scoper(stmt);
nsresult rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("date"), aValue);
NS_ENSURE_SUCCESS(rv, rv);
rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("item_id"), aItemId);
NS_ENSURE_SUCCESS(rv, rv);
rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("delta"), aSyncChangeDelta);
@@ -1537,86 +1524,16 @@ nsNavBookmarks::SetItemDateInternal(enum
NS_ENSURE_SUCCESS(rv, rv);
// note, we are not notifying the observers
// that the item has changed.
return NS_OK;
}
-
-NS_IMETHODIMP
-nsNavBookmarks::SetItemDateAdded(int64_t aItemId, PRTime aDateAdded,
- uint16_t aSource)
-{
- NS_ENSURE_ARG_MIN(aItemId, 1);
-
- BookmarkData bookmark;
- nsresult rv = FetchItemInfo(aItemId, bookmark);
- NS_ENSURE_SUCCESS(rv, rv);
- int64_t tagsRootId = TagsRootId();
- bool isTagging = bookmark.grandParentId == tagsRootId;
- int64_t syncChangeDelta = DetermineSyncChangeDelta(aSource);
-
- // Round here so that we notify with the right value.
- bookmark.dateAdded = RoundToMilliseconds(aDateAdded);
-
- if (isTagging) {
- // If we're changing a tag, bump the change counter for all tagged
- // bookmarks. We use a separate code path to avoid a transaction for
- // non-tags.
- mozStorageTransaction transaction(mDB->MainConn(), false);
-
- rv = SetItemDateInternal(DATE_ADDED, syncChangeDelta, bookmark.id,
- bookmark.dateAdded);
- NS_ENSURE_SUCCESS(rv, rv);
-
- rv = AddSyncChangesForBookmarksWithURL(bookmark.url, syncChangeDelta);
- NS_ENSURE_SUCCESS(rv, rv);
-
- rv = transaction.Commit();
- NS_ENSURE_SUCCESS(rv, rv);
- } else {
- rv = SetItemDateInternal(DATE_ADDED, syncChangeDelta, bookmark.id,
- bookmark.dateAdded);
- NS_ENSURE_SUCCESS(rv, rv);
- }
-
- // Note: mDBSetItemDateAdded also sets lastModified to aDateAdded.
- NOTIFY_OBSERVERS(mCanNotify, mObservers, nsINavBookmarkObserver,
- OnItemChanged(bookmark.id,
- NS_LITERAL_CSTRING("dateAdded"),
- false,
- nsPrintfCString("%" PRId64, bookmark.dateAdded),
- bookmark.dateAdded,
- bookmark.type,
- bookmark.parentId,
- bookmark.guid,
- bookmark.parentGuid,
- EmptyCString(),
- aSource));
- return NS_OK;
-}
-
-
-NS_IMETHODIMP
-nsNavBookmarks::GetItemDateAdded(int64_t aItemId, PRTime* _dateAdded)
-{
- NS_ENSURE_ARG_MIN(aItemId, 1);
- NS_ENSURE_ARG_POINTER(_dateAdded);
-
- BookmarkData bookmark;
- nsresult rv = FetchItemInfo(aItemId, bookmark);
- NS_ENSURE_SUCCESS(rv, rv);
-
- *_dateAdded = bookmark.dateAdded;
- return NS_OK;
-}
-
-
NS_IMETHODIMP
nsNavBookmarks::SetItemLastModified(int64_t aItemId, PRTime aLastModified,
uint16_t aSource)
{
NS_ENSURE_ARG_MIN(aItemId, 1);
BookmarkData bookmark;
nsresult rv = FetchItemInfo(aItemId, bookmark);
--- a/toolkit/components/places/nsNavBookmarks.h
+++ b/toolkit/components/places/nsNavBookmarks.h
@@ -64,18 +64,17 @@ namespace places {
nsCString guid;
PRTime dateRemoved;
};
typedef void (nsNavBookmarks::*ItemVisitMethod)(const ItemVisitData&);
typedef void (nsNavBookmarks::*ItemChangeMethod)(const ItemChangeData&);
enum BookmarkDate {
- DATE_ADDED = 0
- , LAST_MODIFIED
+ LAST_MODIFIED
};
} // namespace places
} // namespace mozilla
class nsNavBookmarks final : public nsINavBookmarksService
, public nsINavHistoryObserver
, public nsIObserver
--- a/toolkit/components/places/tests/legacy/test_bookmarks.js
+++ b/toolkit/components/places/tests/legacy/test_bookmarks.js
@@ -124,56 +124,43 @@ add_task(async function test_bookmarks()
let newId = bs.insertBookmark(testRoot, uri("http://google.com/"),
bs.DEFAULT_INDEX, "");
Assert.equal(bookmarksObserver._itemAddedId, newId);
Assert.equal(bookmarksObserver._itemAddedParent, testRoot);
Assert.equal(bookmarksObserver._itemAddedIndex, testStartIndex);
Assert.ok(bookmarksObserver._itemAddedURI.equals(uri("http://google.com/")));
Assert.equal(bs.getBookmarkURI(newId).spec, "http://google.com/");
- let dateAdded = bs.getItemDateAdded(newId);
- // dateAdded can equal beforeInsert
- Assert.ok(is_time_ordered(beforeInsert, dateAdded));
-
// after just inserting, modified should not be set
let lastModified = PlacesUtils.toPRTime((await PlacesUtils.bookmarks.fetch(
await PlacesUtils.promiseItemGuid(newId))).lastModified);
- Assert.equal(lastModified, dateAdded);
// The time before we set the title, in microseconds.
let beforeSetTitle = Date.now() * 1000;
Assert.ok(beforeSetTitle >= beforeInsert);
// Workaround possible VM timers issues moving lastModified and dateAdded
// to the past.
lastModified -= 1000;
bs.setItemLastModified(newId, lastModified);
- dateAdded -= 1000;
- bs.setItemDateAdded(newId, dateAdded);
// set bookmark title
bs.setItemTitle(newId, "Google");
Assert.equal(bookmarksObserver._itemChangedId, newId);
Assert.equal(bookmarksObserver._itemChangedProperty, "title");
Assert.equal(bookmarksObserver._itemChangedValue, "Google");
- // check that dateAdded hasn't changed
- let dateAdded2 = bs.getItemDateAdded(newId);
- Assert.equal(dateAdded2, dateAdded);
-
// check lastModified after we set the title
let lastModified2 = PlacesUtils.toPRTime((await PlacesUtils.bookmarks.fetch(
await PlacesUtils.promiseItemGuid(newId))).lastModified);
info("test setItemTitle");
- info("dateAdded = " + dateAdded);
info("beforeSetTitle = " + beforeSetTitle);
info("lastModified = " + lastModified);
info("lastModified2 = " + lastModified2);
Assert.ok(is_time_ordered(lastModified, lastModified2));
- Assert.ok(is_time_ordered(dateAdded, lastModified2));
// get item title
let title = bs.getItemTitle(newId);
Assert.equal(title, "Google");
// get item title bad input
try {
bs.getItemTitle(-3);
@@ -392,28 +379,21 @@ add_task(async function test_bookmarks()
rootNode.containerOpen = false;
} catch (ex) {
do_throw("bookmarks query: " + ex);
}
// test change bookmark uri
let newId10 = bs.insertBookmark(testRoot, uri("http://foo10.com/"),
bs.DEFAULT_INDEX, "");
- dateAdded = bs.getItemDateAdded(newId10);
- // after just inserting, modified should not be set
- lastModified = PlacesUtils.toPRTime((await PlacesUtils.bookmarks.fetch(
- await PlacesUtils.promiseItemGuid(newId10))).lastModified);
- Assert.equal(lastModified, dateAdded);
// Workaround possible VM timers issues moving lastModified and dateAdded
// to the past.
lastModified -= 1000;
bs.setItemLastModified(newId10, lastModified);
- dateAdded -= 1000;
- bs.setItemDateAdded(newId10, dateAdded);
// test getBookmarkURI
let newId11 = bs.insertBookmark(testRoot, uri("http://foo10.com/"),
bs.DEFAULT_INDEX, "");
let bmURI = bs.getBookmarkURI(newId11);
Assert.equal("http://foo10.com/", bmURI.spec);
// test getBookmarkURI with non-bookmark items
@@ -520,30 +500,23 @@ add_task(async function test_bookmarks()
break;
}
}
rootNode.containerOpen = false;
} catch (ex) {
do_throw("bookmarks query: " + ex);
}
- // check setItemLastModified() and setItemDateAdded()
+ // check setItemLastModified()
let newId14 = bs.insertBookmark(testRoot, uri("http://bar.tld/"),
bs.DEFAULT_INDEX, "");
- dateAdded = bs.getItemDateAdded(newId14);
- lastModified = PlacesUtils.toPRTime((await PlacesUtils.bookmarks.fetch(
- await PlacesUtils.promiseItemGuid(newId14))).lastModified);
- Assert.equal(lastModified, dateAdded);
bs.setItemLastModified(newId14, 1234000000000000);
let fakeLastModified = PlacesUtils.toPRTime((await PlacesUtils.bookmarks.fetch(
await PlacesUtils.promiseItemGuid(newId14))).lastModified);
Assert.equal(fakeLastModified, 1234000000000000);
- bs.setItemDateAdded(newId14, 4321000000000000);
- let fakeDateAdded = bs.getItemDateAdded(newId14);
- Assert.equal(fakeDateAdded, 4321000000000000);
// ensure that removing an item removes its annotations
Assert.ok(anno.itemHasAnnotation(newId3, "test-annotation"));
bs.removeItem(newId3);
Assert.ok(!anno.itemHasAnnotation(newId3, "test-annotation"));
// bug 378820
let uri1 = uri("http://foo.tld/a");
@@ -574,36 +547,24 @@ function testSimpleFolderResult() {
// the time before we create a folder, in microseconds
// Workaround possible VM timers issues subtracting 1us.
let beforeCreate = Date.now() * 1000 - 1;
Assert.ok(beforeCreate > 0);
// create a folder
let parent = bs.createFolder(root, "test", bs.DEFAULT_INDEX);
- let dateCreated = bs.getItemDateAdded(parent);
- info("check that the folder was created with a valid dateAdded");
- info("beforeCreate = " + beforeCreate);
- info("dateCreated = " + dateCreated);
- Assert.ok(is_time_ordered(beforeCreate, dateCreated));
-
// the time before we insert, in microseconds
// Workaround possible VM timers issues subtracting 1ms.
let beforeInsert = Date.now() * 1000 - 1;
Assert.ok(beforeInsert > 0);
// insert a separator
let sep = bs.insertSeparator(parent, bs.DEFAULT_INDEX);
- let dateAdded = bs.getItemDateAdded(sep);
- info("check that the separator was created with a valid dateAdded");
- info("beforeInsert = " + beforeInsert);
- info("dateAdded = " + dateAdded);
- Assert.ok(is_time_ordered(beforeInsert, dateAdded));
-
// re-set item title separately so can test nodes' last modified
let item = bs.insertBookmark(parent, uri("about:blank"),
bs.DEFAULT_INDEX, "");
bs.setItemTitle(item, "test bookmark");
// see above
let folder = bs.createFolder(parent, "test folder", bs.DEFAULT_INDEX);
bs.setItemTitle(folder, "test folder");