Bug 1433305 - Remove synchronous Bookmarks::GetItemLastModified. r=standard8 draft
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 07 Feb 2018 18:23:06 +0100
changeset 752456 c673e78f9656aeecc065867c441b9aaf116c5f06
parent 752454 8af9e2af669a076e83844c309245b43a2a71e983
push id98275
push usermak77@bonardo.net
push dateThu, 08 Feb 2018 09:35:35 +0000
reviewersstandard8
bugs1433305
milestone60.0a1
Bug 1433305 - Remove synchronous Bookmarks::GetItemLastModified. r=standard8 MozReview-Commit-ID: 5bQfv3SqaR3
toolkit/components/places/nsINavBookmarksService.idl
toolkit/components/places/nsNavBookmarks.cpp
toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js
toolkit/components/places/tests/legacy/test_bookmarks.js
--- a/toolkit/components/places/nsINavBookmarksService.idl
+++ b/toolkit/components/places/nsINavBookmarksService.idl
@@ -557,29 +557,16 @@ interface nsINavBookmarksService : nsISu
    *       any other method that changes an item property, but we will send
    *       the corresponding itemChanged notification instead.
    */
   void setItemLastModified(in long long aItemId,
                            in PRTime aLastModified,
                            [optional] in unsigned short aSource);
 
   /**
-   * Get the last modified time for an item.
-   *
-   * @param aItemId
-   *        the id of the item whose last modified time should be retrieved.
-   *
-   * @return the date added value in microseconds.
-   *
-   * @note When an item is added lastModified is set to the same value as
-   *       dateAdded.
-   */
-  PRTime getItemLastModified(in long long aItemId);
-
-  /**
    * Get the URI for a bookmark item.
    */
   nsIURI getBookmarkURI(in long long aItemId);
 
   /**
    * Get the index for an item.
    */
   long getItemIndex(in long long aItemId);
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -1738,31 +1738,16 @@ nsNavBookmarks::SetItemLastModified(int6
                                  bookmark.guid,
                                  bookmark.parentGuid,
                                  EmptyCString(),
                                  aSource));
   return NS_OK;
 }
 
 
-NS_IMETHODIMP
-nsNavBookmarks::GetItemLastModified(int64_t aItemId, PRTime* _lastModified)
-{
-  NS_ENSURE_ARG_MIN(aItemId, 1);
-  NS_ENSURE_ARG_POINTER(_lastModified);
-
-  BookmarkData bookmark;
-  nsresult rv = FetchItemInfo(aItemId, bookmark);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  *_lastModified = bookmark.lastModified;
-  return NS_OK;
-}
-
-
 nsresult
 nsNavBookmarks::AddSyncChangesForBookmarksWithURL(const nsACString& aURL,
                                                   int64_t aSyncChangeDelta)
 {
   if (!aSyncChangeDelta) {
     return NS_OK;
   }
   nsCOMPtr<nsIURI> uri;
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js
@@ -4,79 +4,79 @@
 add_task(async function insert_separator_notification() {
   let observer = expectNotifications();
   let bm = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_SEPARATOR,
                                                 parentGuid: PlacesUtils.bookmarks.unfiledGuid});
   let itemId = await PlacesUtils.promiseItemId(bm.guid);
   let parentId = await PlacesUtils.promiseItemId(bm.parentGuid);
   observer.check([ { name: "onItemAdded",
                      arguments: [ itemId, parentId, bm.index, bm.type,
-                                  null, "", bm.dateAdded * 1000,
+                                  null, "", PlacesUtils.toPRTime(bm.dateAdded),
                                   bm.guid, bm.parentGuid,
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
                  ]);
 });
 
 add_task(async function insert_folder_notification() {
   let observer = expectNotifications();
   let bm = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_FOLDER,
                                                 parentGuid: PlacesUtils.bookmarks.unfiledGuid,
                                                 title: "a folder" });
   let itemId = await PlacesUtils.promiseItemId(bm.guid);
   let parentId = await PlacesUtils.promiseItemId(bm.parentGuid);
   observer.check([ { name: "onItemAdded",
                      arguments: [ itemId, parentId, bm.index, bm.type,
-                                  null, bm.title, bm.dateAdded * 1000,
+                                  null, bm.title, PlacesUtils.toPRTime(bm.dateAdded),
                                   bm.guid, bm.parentGuid,
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
                  ]);
 });
 
 add_task(async function insert_folder_notitle_notification() {
   let observer = expectNotifications();
   let bm = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_FOLDER,
                                                 parentGuid: PlacesUtils.bookmarks.unfiledGuid });
   strictEqual(bm.title, "", "Should return empty string for untitled folder");
   let itemId = await PlacesUtils.promiseItemId(bm.guid);
   let parentId = await PlacesUtils.promiseItemId(bm.parentGuid);
   observer.check([ { name: "onItemAdded",
                      arguments: [ itemId, parentId, bm.index, bm.type,
-                                  null, "", bm.dateAdded * 1000,
+                                  null, "", PlacesUtils.toPRTime(bm.dateAdded),
                                   bm.guid, bm.parentGuid,
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
                  ]);
 });
 
 add_task(async function insert_bookmark_notification() {
   let observer = expectNotifications();
   let bm = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
                                                 parentGuid: PlacesUtils.bookmarks.unfiledGuid,
                                                 url: new URL("http://example.com/"),
                                                 title: "a bookmark" });
   let itemId = await PlacesUtils.promiseItemId(bm.guid);
   let parentId = await PlacesUtils.promiseItemId(bm.parentGuid);
   observer.check([ { name: "onItemAdded",
                      arguments: [ itemId, parentId, bm.index, bm.type,
-                                  bm.url, bm.title, bm.dateAdded * 1000,
+                                  bm.url, bm.title, PlacesUtils.toPRTime(bm.dateAdded),
                                   bm.guid, bm.parentGuid,
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
                  ]);
 });
 
 add_task(async function insert_bookmark_notitle_notification() {
   let observer = expectNotifications();
   let bm = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
                                                 parentGuid: PlacesUtils.bookmarks.unfiledGuid,
                                                 url: new URL("http://example.com/") });
   strictEqual(bm.title, "", "Should return empty string for untitled bookmark");
   let itemId = await PlacesUtils.promiseItemId(bm.guid);
   let parentId = await PlacesUtils.promiseItemId(bm.parentGuid);
   observer.check([ { name: "onItemAdded",
                      arguments: [ itemId, parentId, bm.index, bm.type,
-                                  bm.url, "", bm.dateAdded * 1000,
+                                  bm.url, "", PlacesUtils.toPRTime(bm.dateAdded),
                                   bm.guid, bm.parentGuid,
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
                  ]);
 });
 
 add_task(async function insert_bookmark_tag_notification() {
   let bm = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
                                                 parentGuid: PlacesUtils.bookmarks.unfiledGuid,
@@ -91,40 +91,41 @@ add_task(async function insert_bookmark_
   let tag = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
                                                  parentGuid: tagFolder.guid,
                                                  url: new URL("http://tag.example.com/") });
   let tagId = await PlacesUtils.promiseItemId(tag.guid);
   let tagParentId = await PlacesUtils.promiseItemId(tag.parentGuid);
 
   observer.check([ { name: "onItemAdded",
                      arguments: [ tagId, tagParentId, tag.index, tag.type,
-                                  tag.url, "", tag.dateAdded * 1000,
+                                  tag.url, "", PlacesUtils.toPRTime(tag.dateAdded),
                                   tag.guid, tag.parentGuid,
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] },
                    { name: "onItemChanged",
                      arguments: [ itemId, "tags", false, "",
-                                  bm.lastModified * 1000, bm.type, parentId,
-                                  bm.guid, bm.parentGuid, "",
+                                  PlacesUtils.toPRTime(bm.lastModified),
+                                  bm.type, parentId, bm.guid, bm.parentGuid, "",
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
                  ]);
 });
 
 add_task(async function update_bookmark_lastModified() {
   let bm = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
                                                 parentGuid: PlacesUtils.bookmarks.unfiledGuid,
                                                 url: new URL("http://lastmod.example.com/") });
   let observer = expectNotifications();
   bm = await PlacesUtils.bookmarks.update({ guid: bm.guid,
                                             lastModified: new Date() });
   let itemId = await PlacesUtils.promiseItemId(bm.guid);
   let parentId = await PlacesUtils.promiseItemId(bm.parentGuid);
 
   observer.check([ { name: "onItemChanged",
                      arguments: [ itemId, "lastModified", false,
-                                  `${bm.lastModified * 1000}`, bm.lastModified * 1000,
+                                  `${PlacesUtils.toPRTime(bm.lastModified)}`,
+                                  PlacesUtils.toPRTime(bm.lastModified),
                                   bm.type, parentId, bm.guid, bm.parentGuid, "",
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
                  ]);
 });
 
 add_task(async function update_bookmark_title() {
   let bm = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
                                                 parentGuid: PlacesUtils.bookmarks.unfiledGuid,
@@ -132,34 +133,36 @@ add_task(async function update_bookmark_
   let observer = expectNotifications();
   bm = await PlacesUtils.bookmarks.update({ guid: bm.guid,
                                             title: "new title" });
   let itemId = await PlacesUtils.promiseItemId(bm.guid);
   let parentId = await PlacesUtils.promiseItemId(bm.parentGuid);
 
   observer.check([ { name: "onItemChanged",
                      arguments: [ itemId, "title", false, bm.title,
-                                  bm.lastModified * 1000, bm.type, parentId, bm.guid,
+                                  PlacesUtils.toPRTime(bm.lastModified),
+                                  bm.type, parentId, bm.guid,
                                   bm.parentGuid, "", Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
                  ]);
 });
 
 add_task(async function update_bookmark_uri() {
   let bm = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
                                                 parentGuid: PlacesUtils.bookmarks.unfiledGuid,
                                                 url: new URL("http://url.example.com/") });
   let observer = expectNotifications();
   bm = await PlacesUtils.bookmarks.update({ guid: bm.guid,
                                             url: "http://mozilla.org/" });
   let itemId = await PlacesUtils.promiseItemId(bm.guid);
   let parentId = await PlacesUtils.promiseItemId(bm.parentGuid);
 
   observer.check([ { name: "onItemChanged",
                      arguments: [ itemId, "uri", false, bm.url.href,
-                                  bm.lastModified * 1000, bm.type, parentId, bm.guid,
+                                  PlacesUtils.toPRTime(bm.lastModified),
+                                  bm.type, parentId, bm.guid,
                                   bm.parentGuid, "http://url.example.com/",
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
                  ]);
 });
 
 add_task(async function update_move_same_folder() {
   // Ensure there are at least two items in place (others test do so for us,
   // but we don't have to depend on that).
@@ -298,18 +301,18 @@ add_task(async function remove_bookmark_
   await PlacesUtils.bookmarks.remove(tag.guid);
 
   observer.check([ { name: "onItemRemoved",
                      arguments: [ tagId, tagParentId, tag.index, tag.type,
                                   tag.url, tag.guid, tag.parentGuid,
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] },
                    { name: "onItemChanged",
                      arguments: [ itemId, "tags", false, "",
-                                  bm.lastModified * 1000, bm.type, parentId,
-                                  bm.guid, bm.parentGuid, "",
+                                  PlacesUtils.toPRTime(bm.lastModified),
+                                  bm.type, parentId, bm.guid, bm.parentGuid, "",
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
                  ]);
 });
 
 add_task(async function remove_folder_notification() {
   let folder1 = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_FOLDER,
                                                      parentGuid: PlacesUtils.bookmarks.unfiledGuid });
   let folder1Id = await PlacesUtils.promiseItemId(folder1.guid);
@@ -538,29 +541,29 @@ add_task(async function update_notitle_n
 
   let updatedMenuBm = await PlacesUtils.bookmarks.update({
     guid: menuFolder.guid,
     title: null,
   });
   strictEqual(updatedMenuBm.title, "",
     "Async API should return empty string for untitled bookmark");
 
-  let toolbarBmModified =
-    PlacesUtils.toDate(PlacesUtils.bookmarks.getItemLastModified(toolbarBmId));
+  let toolbarBmModified = await PlacesUtils.bookmarks.fetch(toolbarBmGuid);
   observer.check([{
     name: "onItemChanged",
-    arguments: [toolbarBmId, "title", false, "", toolbarBmModified * 1000,
+    arguments: [toolbarBmId, "title", false, "",
+                PlacesUtils.toPRTime(toolbarBmModified.lastModified),
                 PlacesUtils.bookmarks.TYPE_BOOKMARK,
                 PlacesUtils.toolbarFolderId, toolbarBmGuid,
                 PlacesUtils.bookmarks.toolbarGuid,
                 "", PlacesUtils.bookmarks.SOURCES.DEFAULT],
   }, {
     name: "onItemChanged",
     arguments: [menuFolderId, "title", false, "",
-                updatedMenuBm.lastModified * 1000,
+                PlacesUtils.toPRTime(updatedMenuBm.lastModified),
                 PlacesUtils.bookmarks.TYPE_FOLDER,
                 PlacesUtils.bookmarksMenuFolderId, menuFolder.guid,
                 PlacesUtils.bookmarks.menuGuid,
                 "", PlacesUtils.bookmarks.SOURCES.DEFAULT],
   }]);
 });
 
 function expectNotifications() {
--- a/toolkit/components/places/tests/legacy/test_bookmarks.js
+++ b/toolkit/components/places/tests/legacy/test_bookmarks.js
@@ -129,17 +129,18 @@ 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 = bs.getItemLastModified(newId);
+  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.
@@ -154,17 +155,18 @@ 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 = bs.getItemLastModified(newId);
+  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));
 
@@ -410,17 +412,18 @@ add_task(async function test_bookmarks()
     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 = bs.getItemLastModified(newId10);
+  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);
@@ -539,20 +542,22 @@ add_task(async function test_bookmarks()
   } catch (ex) {
     do_throw("bookmarks query: " + ex);
   }
 
   // check setItemLastModified() and setItemDateAdded()
   let newId14 = bs.insertBookmark(testRoot, uri("http://bar.tld/"),
                                   bs.DEFAULT_INDEX, "");
   dateAdded = bs.getItemDateAdded(newId14);
-  lastModified = bs.getItemLastModified(newId14);
+  lastModified = PlacesUtils.toPRTime((await PlacesUtils.bookmarks.fetch(
+    await PlacesUtils.promiseItemGuid(newId14))).lastModified);
   Assert.equal(lastModified, dateAdded);
   bs.setItemLastModified(newId14, 1234000000000000);
-  let fakeLastModified = bs.getItemLastModified(newId14);
+  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);