Bug 1376232 - When updating date added the async Bookmarks.update API should also change the last modified date. r?mak draft
authorMark Banner <standard8@mozilla.com>
Sun, 25 Jun 2017 20:17:12 +0100
changeset 600939 a8e9a1c112c81ca01e416646302b4cc56d7cf4b6
parent 600616 f4e52135d9bdc6ce98bb37b450021445aed894ce
child 635129 0252b3f7c03252d2722b10e6093e9ac126edded3
push id65914
push userbmo:standard8@mozilla.com
push dateTue, 27 Jun 2017 22:14:56 +0000
reviewersmak
bugs1376232
milestone56.0a1
Bug 1376232 - When updating date added the async Bookmarks.update API should also change the last modified date. r?mak MozReview-Commit-ID: BGiT1w8BVlq
toolkit/components/places/Bookmarks.jsm
toolkit/components/places/tests/unit/test_lastModified.js
--- a/toolkit/components/places/Bookmarks.jsm
+++ b/toolkit/components/places/Bookmarks.jsm
@@ -533,21 +533,28 @@ var Bookmarks = Object.freeze({
       // Remove any property that will stay the same.
       removeSameValueProperties(updateInfo, item);
       // Check if anything should still be updated.
       if (Object.keys(updateInfo).length < 3) {
         // Remove non-enumerable properties.
         return Object.assign({}, item);
       }
       const now = new Date();
+      let lastModifiedDefault = now;
+      // In the case where `dateAdded` is specified, but `lastModified` is not,
+      // we only update `lastModified` if it is older than the new `dateAdded`.
+      if (!("lastModified" in updateInfo) &&
+          "dateAdded" in updateInfo) {
+        lastModifiedDefault = new Date(Math.max(item.lastModified, updateInfo.dateAdded));
+      }
       updateInfo = validateBookmarkObject(updateInfo,
         { url: { validIf: () => item.type == this.TYPE_BOOKMARK },
           title: { validIf: () => [ this.TYPE_BOOKMARK,
                                     this.TYPE_FOLDER ].includes(item.type) },
-          lastModified: { defaultValue: now,
+          lastModified: { defaultValue: lastModifiedDefault,
                           validIf: b => b.lastModified >= now ||
                                         b.lastModified >= (b.dateAdded || item.dateAdded) },
           dateAdded: { defaultValue: item.dateAdded }
         });
 
       return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: update",
         async db => {
         let parent;
--- a/toolkit/components/places/tests/unit/test_lastModified.js
+++ b/toolkit/components/places/tests/unit/test_lastModified.js
@@ -1,34 +1,76 @@
 /* -*- 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/. */
 
+function assert_date_eq(a, b) {
+  if (typeof(a) != "number") {
+    a = PlacesUtils.toPRTime(a);
+  }
+  if (typeof(b) != "number") {
+    b = PlacesUtils.toPRTime(b);
+  }
+  Assert.equal(a, b, "The dates should match");
+}
+
  /**
   * Test that inserting a new bookmark will set lastModified to the same
   * values as dateAdded.
   */
-// main
-function run_test() {
-  var bs = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
-           getService(Ci.nsINavBookmarksService);
-  var itemId = bs.insertBookmark(bs.bookmarksMenuFolder,
-                                 uri("http://www.mozilla.org/"),
-                                 bs.DEFAULT_INDEX,
-                                 "itemTitle");
-  var dateAdded = bs.getItemDateAdded(itemId);
-  do_check_eq(dateAdded, bs.getItemLastModified(itemId));
+add_task(async function test_bookmarkLastModified() {
+  let bookmark = await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.menuGuid,
+    url: "http://www.mozilla.org/",
+    title: "itemTitle"
+  });
+
+  let guid = bookmark.guid;
+
+  // Check the bookmark from the database.
+  bookmark = await PlacesUtils.bookmarks.fetch(guid);
+
+  let dateAdded = PlacesUtils.toPRTime(bookmark.dateAdded);
+  assert_date_eq(dateAdded, bookmark.lastModified);
 
   // Change lastModified, then change dateAdded.  LastModified should be set
   // to the new dateAdded.
   // This could randomly fail on virtual machines due to timing issues, so
   // we manually increase the time value.  See bug 500640 for details.
-  bs.setItemLastModified(itemId, dateAdded + 1000);
-  do_check_true(bs.getItemLastModified(itemId) === dateAdded + 1000);
-  do_check_true(bs.getItemDateAdded(itemId) < bs.getItemLastModified(itemId));
-  bs.setItemDateAdded(itemId, dateAdded + 2000);
-  do_check_true(bs.getItemDateAdded(itemId) === dateAdded + 2000);
-  do_check_eq(bs.getItemDateAdded(itemId), bs.getItemLastModified(itemId));
+  await PlacesUtils.bookmarks.update({
+    guid,
+    lastModified: PlacesUtils.toDate(dateAdded + 1000)
+  });
+
+  bookmark = await PlacesUtils.bookmarks.fetch(guid);
+
+  assert_date_eq(bookmark.lastModified, dateAdded + 1000);
+  Assert.ok(bookmark.dateAdded < bookmark.lastModified,
+    "Date added should be earlier than last modified.");
+
+  await PlacesUtils.bookmarks.update({
+    guid,
+    dateAdded: PlacesUtils.toDate(dateAdded + 2000)
+  });
+
+  bookmark = await PlacesUtils.bookmarks.fetch(guid);
 
-  bs.removeItem(itemId);
-}
+  assert_date_eq(bookmark.dateAdded, dateAdded + 2000);
+  assert_date_eq(bookmark.dateAdded, bookmark.lastModified);
+
+  // If dateAdded is set to older than lastModified, then we shouldn't
+  // update lastModified to keep sync happy.
+  let origLastModified = bookmark.lastModified;
+
+  await PlacesUtils.bookmarks.update({
+    guid,
+    dateAdded: PlacesUtils.toDate(dateAdded - 10000)
+  });
+
+  bookmark = await PlacesUtils.bookmarks.fetch(guid);
+
+  assert_date_eq(bookmark.dateAdded, dateAdded - 10000);
+  assert_date_eq(bookmark.lastModified, origLastModified);
+
+  await PlacesUtils.bookmarks.remove(guid);
+});