Bug 1432434 - Remove Bookmarks::getItemDateAdded and setItemDateAdded. r=kitcambridge

MozReview-Commit-ID: XVoil1CK7c

--HG--
extra : rebase_source : 18213447f00177686dbc56d263674fa4e630663e

services/sync/tests/unit/test_bookmark_tracker.js

@@ -559,28 +559,43 @@ add_task(async function test_onItemChanged_itemDates() {
     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.");

toolkit/components/places/nsINavBookmarksService.idl

@@ -503,32 +503,6 @@ interface nsINavBookmarksService : nsISupports
    */
   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.
    *

toolkit/components/places/nsNavBookmarks.cpp

@@ -1505,24 +1505,11 @@ nsNavBookmarks::SetItemDateInternal(enum BookmarkDate aDateType,
 {
   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);
 
@@ -1542,76 +1529,6 @@ nsNavBookmarks::SetItemDateInternal(enum BookmarkDate aDateType,
   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)

toolkit/components/places/nsNavBookmarks.h

@@ -69,8 +69,7 @@ namespace places {
   typedef void (nsNavBookmarks::*ItemChangeMethod)(const ItemChangeData&);
 
   enum BookmarkDate {
-    DATE_ADDED = 0
-  , LAST_MODIFIED
+    LAST_MODIFIED
   };
 
 } // namespace places

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

@@ -129,14 +129,9 @@ add_task(async function test_bookmarks() {
   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;
@@ -146,8 +141,6 @@ add_task(async function test_bookmarks() {
   // to the past.
   lastModified -= 1000;
   bs.setItemLastModified(newId, lastModified);
-  dateAdded -= 1000;
-  bs.setItemDateAdded(newId, dateAdded);
 
   // set bookmark title
   bs.setItemTitle(newId, "Google");
@@ -155,20 +148,14 @@ add_task(async function test_bookmarks() {
   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);
@@ -397,18 +384,11 @@ add_task(async function test_bookmarks() {
   // 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/"),
@@ -525,20 +505,13 @@ add_task(async function test_bookmarks() {
     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"));
@@ -579,12 +552,6 @@ function testSimpleFolderResult() {
   // 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;
@@ -593,12 +560,6 @@ function testSimpleFolderResult() {
   // 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, "");