Bug 1489409 - Remove annotation options for PlacesTransactions.NewBookmark/NewFolder. r=mak
authorKajal Kumari Sah <kajalksah07@gmail.com>
Fri, 23 Nov 2018 10:35:31 +0000
changeset 504237 bd2ec1be4ead26c0e0a7620d7f963b7934e37319
parent 504236 c5c9652c6ef3507b0fc844a7b7416e21f9faed20
child 504238 f67b6fdd714f01fab6ae4d3984a6c298b52c0f71
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1489409
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1489409 - Remove annotation options for PlacesTransactions.NewBookmark/NewFolder. r=mak Differential Revision: https://phabricator.services.mozilla.com/D6093
toolkit/components/places/PlacesTransactions.jsm
toolkit/components/places/tests/unit/test_async_transactions.js
--- a/toolkit/components/places/PlacesTransactions.jsm
+++ b/toolkit/components/places/PlacesTransactions.jsm
@@ -718,33 +718,16 @@ function isPrimitive(v) {
 
 function checkProperty(obj, prop, required, checkFn) {
   if (prop in obj)
     return checkFn(obj[prop]);
 
   return !required;
 }
 
-DefineTransaction.annotationObjectValidate = function(obj) {
-  if (obj &&
-      checkProperty(obj, "name", true, v => typeof(v) == "string" && v.length > 0) &&
-      checkProperty(obj, "expires", false, Number.isInteger) &&
-      checkProperty(obj, "flags", false, Number.isInteger) &&
-      checkProperty(obj, "value", false, isPrimitive) ) {
-    // Nothing else should be set
-    let validKeys = ["name", "value", "flags", "expires"];
-    if (Object.keys(obj).every(k => validKeys.includes(k))) {
-      // Annotations objects are passed through to the backend, to avoid memory
-      // leaks, we must clone the object.
-      return {...obj};
-    }
-  }
-  throw new Error("Invalid annotation object");
-};
-
 DefineTransaction.childObjectValidate = function(obj) {
   if (obj &&
       checkProperty(obj, "title", false, v => typeof(v) == "string") &&
       !("type" in obj && obj.type != PlacesUtils.bookmarks.TYPE_BOOKMARK)) {
     obj.url = DefineTransaction.urlValidate(obj.url);
     let validKeys = ["title", "url"];
     if (Object.keys(obj).every(k => validKeys.includes(k))) {
       return obj;
@@ -903,24 +886,21 @@ DefineTransaction.defineInputProps(["gui
 DefineTransaction.defineInputProps(["title", "postData"],
                                    DefineTransaction.strOrNullValidate, null);
 DefineTransaction.defineInputProps(["keyword", "oldKeyword", "oldTag", "tag",
                                     "excludingAnnotation"],
                                    DefineTransaction.strValidate, "");
 DefineTransaction.defineInputProps(["index", "newIndex"],
                                    DefineTransaction.indexValidate,
                                    PlacesUtils.bookmarks.DEFAULT_INDEX);
-DefineTransaction.defineInputProps(["annotation"],
-                                   DefineTransaction.annotationObjectValidate);
 DefineTransaction.defineInputProps(["child"],
                                    DefineTransaction.childObjectValidate);
 DefineTransaction.defineArrayInputProp("guids", "guid");
 DefineTransaction.defineArrayInputProp("urls", "url");
 DefineTransaction.defineArrayInputProp("tags", "tag");
-DefineTransaction.defineArrayInputProp("annotations", "annotation");
 DefineTransaction.defineArrayInputProp("children", "child");
 DefineTransaction.defineArrayInputProp("excludingAnnotations",
                                        "excludingAnnotation");
 
 /**
  * Creates items (all types) from a bookmarks tree representation, as defined
  * in PlacesUtils.promiseBookmarksTree.
  *
@@ -1029,34 +1009,29 @@ var PT = PlacesTransactions;
  * Transaction for creating a bookmark.
  *
  * Required Input Properties: url, parentGuid.
  * Optional Input Properties: index, title, keyword, annotations, tags.
  *
  * When this transaction is executed, it's resolved to the new bookmark's GUID.
  */
 PT.NewBookmark = DefineTransaction(["parentGuid", "url"],
-                                   ["index", "title", "annotations", "tags"]);
+                                   ["index", "title", "tags"]);
 PT.NewBookmark.prototype = Object.seal({
-  async execute({ parentGuid, url, index, title, annotations, tags }) {
+  async execute({ parentGuid, url, index, title, tags }) {
     let info = { parentGuid, index, url, title };
     // Filter tags to exclude already existing ones.
     if (tags.length > 0) {
       let currentTags = PlacesUtils.tagging
                                    .getTagsForURI(Services.io.newURI(url.href));
       tags = tags.filter(t => !currentTags.includes(t));
     }
 
     async function createItem() {
       info = await PlacesUtils.bookmarks.insert(info);
-      if (annotations.length > 0) {
-        let itemId = await PlacesUtils.promiseItemId(info.guid);
-        PlacesUtils.setAnnotationsForItem(itemId, annotations,
-          Ci.nsINavBookmarksService.SOURCE_DEFAULT, true);
-      }
       if (tags.length > 0) {
         PlacesUtils.tagging.tagURI(Services.io.newURI(url.href), tags);
       }
     }
 
     await createItem();
 
     this.undo = async function() {
@@ -1078,19 +1053,19 @@ PT.NewBookmark.prototype = Object.seal({
  * Transaction for creating a folder.
  *
  * Required Input Properties: title, parentGuid.
  * Optional Input Properties: index, annotations, children
  *
  * When this transaction is executed, it's resolved to the new folder's GUID.
  */
 PT.NewFolder = DefineTransaction(["parentGuid", "title"],
-                                 ["index", "annotations", "children"]);
+                                 ["index", "children"]);
 PT.NewFolder.prototype = Object.seal({
-  async execute({ parentGuid, title, index, annotations, children }) {
+  async execute({ parentGuid, title, index, children }) {
     let folderGuid;
     let info = {
       children: [{
         // Ensure to specify a guid to be restored on redo.
         guid: PlacesUtils.history.makeGuid(),
         title,
         type: PlacesUtils.bookmarks.TYPE_FOLDER,
       }],
@@ -1117,22 +1092,16 @@ PT.NewFolder.prototype = Object.seal({
       folderGuid = bmInfo[0].guid;
 
       // Bug 1388097: insertTree doesn't handle inserting at a specific index for the folder,
       // therefore we update the bookmark manually afterwards.
       if (index != PlacesUtils.bookmarks.DEFAULT_INDEX) {
         bmInfo[0].index = index;
         bmInfo = await PlacesUtils.bookmarks.update(bmInfo[0]);
       }
-
-      if (annotations.length > 0) {
-        let itemId = await PlacesUtils.promiseItemId(folderGuid);
-        PlacesUtils.setAnnotationsForItem(itemId, annotations,
-          Ci.nsINavBookmarksService.SOURCE_DEFAULT, true);
-      }
     }
     await createItem();
 
     this.undo = async function() {
       await PlacesUtils.bookmarks.remove(folderGuid);
     };
     this.redo = async function() {
       await createItem();
--- a/toolkit/components/places/tests/unit/test_async_transactions.js
+++ b/toolkit/components/places/tests/unit/test_async_transactions.js
@@ -417,56 +417,16 @@ add_task(async function test_recycled_tr
   ensureTransactThrowsFor(txn_a);
   ensureTransactThrowsFor(txn_b);
 
   await PT.clearTransactionsHistory();
   ensureUndoState();
   observer.reset();
 });
 
-add_task(async function test_new_folder_with_annotation() {
-  const ANNO = { name: "TestAnno", value: "TestValue" };
-  let folder_info = createTestFolderInfo();
-  folder_info.index = bmStartIndex;
-  folder_info.annotations = [ANNO];
-  ensureUndoState();
-  let txn = PT.NewFolder(folder_info);
-  folder_info.guid = await txn.transact();
-  let originalInfo = await PlacesUtils.promiseBookmarksTree(folder_info.guid);
-  let ensureDo = async function(aRedo = false) {
-    ensureUndoState([[txn]], 0);
-    await ensureItemsAdded(folder_info);
-    ensureAnnotationsSet(folder_info.guid, [ANNO]);
-    if (aRedo) {
-      // Ignore lastModified in the comparison, for performance reasons.
-      originalInfo.lastModified = null;
-      await ensureBookmarksTreeRestoredCorrectlyExceptDates(originalInfo);
-    }
-    observer.reset();
-  };
-
-  let ensureUndo = () => {
-    ensureUndoState([[txn]], 1);
-    ensureItemsRemoved({ guid:       folder_info.guid,
-                         parentGuid: folder_info.parentGuid,
-                         index:      bmStartIndex });
-    observer.reset();
-  };
-
-  await ensureDo();
-  await PT.undo();
-  await ensureUndo();
-  await PT.redo();
-  await ensureDo(true);
-  await PT.undo();
-  ensureUndo();
-  await PT.clearTransactionsHistory();
-  ensureUndoState();
-});
-
 add_task(async function test_new_folder_with_children() {
   let folder_info = createTestFolderInfo("Test folder", PlacesUtils.bookmarks.menuGuid, [{
     url: "http://test_create_item.com",
     title: "Test creating an item",
   }]);
   ensureUndoState();
   let txn = PT.NewFolder(folder_info);
   folder_info.guid = await txn.transact();
@@ -917,17 +877,16 @@ add_task(async function test_remove_fold
   await PT.clearTransactionsHistory();
   ensureUndoState();
 });
 
 add_task(async function test_add_and_remove_bookmarks_with_additional_info() {
   const testURI = "http://add.remove.tag";
   const TAG_1 = "TestTag1";
   const TAG_2 = "TestTag2";
-  const ANNO = { name: "TestAnno", value: "TestAnnoValue" };
 
   let folder_info = createTestFolderInfo();
   folder_info.guid = await PT.NewFolder(folder_info).transact();
   let ensureTags = ensureTagsForURI.bind(null, testURI);
 
   // Check that the NewBookmark transaction preserves tags.
   observer.reset();
   let b1_info = { parentGuid: folder_info.guid, url: testURI, tags: [TAG_1] };
@@ -960,39 +919,28 @@ add_task(async function test_add_and_rem
   await ensureBookmarksTreeRestoredCorrectly(b1_originalInfo);
   ensureTags([TAG_1]);
 
   // * Check that no-op tagging (the uri is already tagged with TAG_1) is
   //   also a no-op on undo.
   observer.reset();
   let b2_info = { parentGuid:  folder_info.guid,
                   url:         testURI,
-                  tags:        [TAG_1, TAG_2],
-                  annotations: [ANNO] };
+                  tags:        [TAG_1, TAG_2] };
   b2_info.guid = await PT.NewBookmark(b2_info).transact();
-  let b2_post_creation_changes = [
-   { guid: b2_info.guid,
-     isAnnoProperty: true,
-     property: ANNO.name,
-     newValue: ANNO.value } ];
-  ensureItemsChanged(...b2_post_creation_changes);
   ensureTags([TAG_1, TAG_2]);
 
   observer.reset();
   await PT.undo();
   await ensureItemsRemoved(b2_info);
   ensureTags([TAG_1]);
 
   // Check if Remove correctly restores tags and annotations.
   observer.reset();
   await PT.redo();
-  ensureItemsChanged({ guid: b2_info.guid,
-                       isAnnoProperty: true,
-                       property: ANNO.name,
-                       newValue: ANNO.value });
   ensureTags([TAG_1, TAG_2]);
 
   // Test Remove for multiple items.
   observer.reset();
   await PT.Remove(b1_info.guid).transact();
   await PT.Remove(b2_info.guid).transact();
   await PT.Remove(folder_info.guid).transact();
   await ensureItemsRemoved(b1_info, b2_info, folder_info);
@@ -1000,17 +948,16 @@ add_task(async function test_add_and_rem
 
   observer.reset();
   await PT.undo();
   await ensureItemsAdded(folder_info);
   ensureTags([]);
 
   observer.reset();
   await PT.undo();
-  ensureItemsChanged(...b2_post_creation_changes);
   ensureTags([TAG_1, TAG_2]);
 
   observer.reset();
   await PT.undo();
   await ensureItemsAdded(b1_info);
   ensureTags([TAG_1, TAG_2]);
 
   // The redo calls below cleanup everything we did.
@@ -1651,50 +1598,16 @@ add_task(async function test_array_input
 
   await PT.undo();
   Assert.equal((await PlacesUtils.promiseBookmarksTree(folderGuid)), null);
 
   // Cleanup
   await PT.clearTransactionsHistory();
 });
 
-add_task(async function test_copy_excluding_annotations() {
-  let folderInfo = createTestFolderInfo();
-  let anno = n => { return { name: n, value: 1 }; };
-  folderInfo.annotations = [anno("a"), anno("b"), anno("c")];
-  let folderGuid = await PT.NewFolder(folderInfo).transact();
-
-  let ensureAnnosSet = async function(guid, ...expectedAnnoNames) {
-    let tree = await PlacesUtils.promiseBookmarksTree(guid);
-    let annoNames = "annos" in tree ?
-                      tree.annos.map(a => a.name).sort() : [];
-    Assert.deepEqual(annoNames, expectedAnnoNames);
-  };
-
-  await ensureAnnosSet(folderGuid, "a", "b", "c");
-
-  let excluding_a_dupeGuid =
-    await PT.Copy({ guid: folderGuid,
-                    newParentGuid: PlacesUtils.bookmarks.unfiledGuid,
-                    excludingAnnotation: "a" }).transact();
-  await ensureAnnosSet(excluding_a_dupeGuid, "b", "c");
-
-  let excluding_ac_dupeGuid =
-    await PT.Copy({ guid: folderGuid,
-                    newParentGuid: PlacesUtils.bookmarks.unfiledGuid,
-                    excludingAnnotations: ["a", "c"] }).transact();
-  await ensureAnnosSet(excluding_ac_dupeGuid, "b");
-
-  // Cleanup
-  await PT.undo();
-  await PT.undo();
-  await PT.undo();
-  await PT.clearTransactionsHistory();
-});
-
 add_task(async function test_invalid_uri_spec_throws() {
   Assert.throws(() =>
     PT.NewBookmark({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
                      url:        "invalid uri spec",
                      title:      "test bookmark"}),
                     /invalid uri spec is not a valid URL/);
   Assert.throws(() =>
     PT.Tag({ tag: "TheTag",