Bug 1047819 - Add an async version of PlacesUtils.getAnnotationsForItem. r=mak
authorMark Banner <standard8@mozilla.com>
Thu, 29 Mar 2018 09:57:43 +0100
changeset 410634 6bc3bc48fc87fa38855de66807720d900a6613ac
parent 410633 cb8cbe3c000f80ea886c6406cdad87e3c6236bfc
child 410635 729748e603e5614b5d1bf674aa4baeedbc0ff792
push id61924
push usermbanner@mozilla.com
push dateThu, 29 Mar 2018 16:35:51 +0000
treeherderautoland@729748e603e5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1047819
milestone61.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 1047819 - Add an async version of PlacesUtils.getAnnotationsForItem. r=mak MozReview-Commit-ID: yGPd0mhuPt
toolkit/components/places/PlacesTransactions.jsm
toolkit/components/places/PlacesUtils.jsm
toolkit/components/places/tests/unit/test_PlacesUtils_annotations.js
toolkit/components/places/tests/unit/test_async_transactions.js
toolkit/components/places/tests/unit/test_promiseBookmarksTree.js
--- a/toolkit/components/places/PlacesTransactions.jsm
+++ b/toolkit/components/places/PlacesTransactions.jsm
@@ -1351,17 +1351,17 @@ PT.EditUrl.prototype = Object.seal({
  * Required Input Properties: guid, annotationObject
  */
 PT.Annotate = DefineTransaction(["guids", "annotations"]);
 PT.Annotate.prototype = {
   async execute({ guids, annotations }) {
     let undoAnnosForItemId = new Map();
     for (let guid of guids) {
       let itemId = await PlacesUtils.promiseItemId(guid);
-      let currentAnnos = PlacesUtils.getAnnotationsForItem(itemId);
+      let currentAnnos = await PlacesUtils.promiseAnnotationsForItem(itemId);
 
       let undoAnnos = [];
       for (let newAnno of annotations) {
         let currentAnno = currentAnnos.find(a => a.name == newAnno.name);
         if (currentAnno) {
           undoAnnos.push(currentAnno);
         } else {
           // An unset value removes the annotation.
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -91,16 +91,44 @@ async function notifyKeywordChange(url, 
                                          bookmark.parentId,
                                          bookmark.guid, bookmark.parentGuid,
                                          "", source
                                        ]);
   }
 }
 
 /**
+ * Synchonously fetches all annotations for an item, including all properties of
+ * each annotation which would be required to recreate it.
+ * @note The async version (PlacesUtils.promiseAnnotationsForItem) should be
+ *       used, unless there's absolutely no way to make the caller async.
+ * @param aItemId
+ *        The identifier of the itme for which annotations are to be
+ *        retrieved.
+ * @return Array of objects, each containing the following properties:
+ *         name, flags, expires, mimeType, type, value
+ */
+function getAnnotationsForItem(aItemId) {
+  var annos = [];
+  var annoNames = PlacesUtils.annotations.getItemAnnotationNames(aItemId);
+  for (let name of annoNames) {
+    let value = {}, flags = {}, exp = {}, storageType = {};
+    PlacesUtils.annotations.getItemAnnotationInfo(aItemId, name, value,
+                                                  flags, exp, storageType);
+    annos.push({
+      name,
+      flags: flags.value,
+      expires: exp.value,
+      value: value.value
+    });
+  }
+  return annos;
+}
+
+/**
  * Serializes the given node in JSON format.
  *
  * @param aNode
  *        An nsINavHistoryResultNode
  * @param aIsLivemark
  *        Whether the node represents a livemark.
  */
 function serializeNode(aNode, aIsLivemark) {
@@ -132,17 +160,17 @@ function serializeNode(aNode, aIsLivemar
     let grandParent = aNode.parent && aNode.parent.parent;
     if (grandParent) {
       grandParentId = grandParent.itemId;
     }
 
     data.dateAdded = aNode.dateAdded;
     data.lastModified = aNode.lastModified;
 
-    let annos = PlacesUtils.getAnnotationsForItem(data.id);
+    let annos = getAnnotationsForItem(data.id);
     if (annos.length > 0)
       data.annos = annos;
   }
 
   if (PlacesUtils.nodeIsURI(aNode)) {
     // Check for url validity.
     NetUtil.newURI(aNode.uri);
 
@@ -1173,35 +1201,43 @@ var PlacesUtils = {
     var result = this.history.executeQuery(query, options);
     result.root.containerOpen = true;
     return result;
   },
 
   /**
    * Fetch all annotations for an item, including all properties of each
    * annotation which would be required to recreate it.
-   * @param aItemId
+   * @param itemId
    *        The identifier of the itme for which annotations are to be
    *        retrieved.
    * @return Array of objects, each containing the following properties:
    *         name, flags, expires, mimeType, type, value
    */
-  getAnnotationsForItem: function PU_getAnnotationsForItem(aItemId) {
-    var annosvc = this.annotations;
-    var annos = [];
-    var annoNames = annosvc.getItemAnnotationNames(aItemId);
-    for (var i = 0; i < annoNames.length; i++) {
-      let value = {}, flags = {}, exp = {}, storageType = {};
-      annosvc.getItemAnnotationInfo(aItemId, annoNames[i], value, flags, exp, storageType);
-      annos.push({name: annoNames[i],
-                  flags: flags.value,
-                  expires: exp.value,
-                  value: value.value});
+  async promiseAnnotationsForItem(itemId) {
+    let db =  await PlacesUtils.promiseDBConnection();
+    let rows = await db.executeCached(
+      `SELECT n.name, a.content, a.expiration, a.flags
+       FROM moz_items_annos a
+       JOIN moz_anno_attributes n ON a.anno_attribute_id = n.id
+       WHERE a.item_id = :itemId
+      `, { itemId });
+
+    let result = [];
+    for (let row of rows) {
+      let anno = {
+        name: row.getResultByName("name"),
+        value: row.getResultByName("content"),
+        expires: row.getResultByName("expiration"),
+        flags: row.getResultByName("flags"),
+      };
+      result.push(anno);
     }
-    return annos;
+
+    return result;
   },
 
   /**
    * Annotate an item with a batch of annotations.
    * @param aItemId
    *        The identifier of the item for which annotations are to be set
    * @param aAnnotations
    *        Array of objects, each containing the following properties:
@@ -1705,17 +1741,17 @@ var PlacesUtils = {
       let type = aRow.getResultByName("type");
       item.typeCode = type;
       if (type == Ci.nsINavBookmarksService.TYPE_BOOKMARK)
         copyProps("charset", "tags", "iconuri");
 
       // Add annotations.
       if (aRow.getResultByName("has_annos")) {
         try {
-          item.annos = PlacesUtils.getAnnotationsForItem(itemId);
+          item.annos = await PlacesUtils.promiseAnnotationsForItem(itemId);
         } catch (ex) {
           Cu.reportError("Unexpected error while reading annotations " + ex);
         }
       }
 
       switch (type) {
         case Ci.nsINavBookmarksService.TYPE_BOOKMARK:
           item.type = PlacesUtils.TYPE_X_MOZ_PLACE;
--- a/toolkit/components/places/tests/unit/test_PlacesUtils_annotations.js
+++ b/toolkit/components/places/tests/unit/test_PlacesUtils_annotations.js
@@ -40,12 +40,12 @@ add_task(async function test_getAnnotati
       url: "http://example.com/2",
       annos: TEST_ANNOTATIONS
     }],
   });
 
   let ids = await PlacesUtils.promiseManyItemIds(bms.map(bm => bm.guid));
 
   for (let i = 0; i < bms.length; i++) {
-    let annotations = PlacesUtils.getAnnotationsForItem(ids.get(bms[i].guid));
+    let annotations = await PlacesUtils.promiseAnnotationsForItem(ids.get(bms[i].guid));
     checkAnnotations(annotations, TEST_ANNOTATIONS.slice(0, i));
   }
 });
--- a/toolkit/components/places/tests/unit/test_async_transactions.js
+++ b/toolkit/components/places/tests/unit/test_async_transactions.js
@@ -1471,51 +1471,51 @@ add_task(async function test_annotate_mu
     this.flags = 0;
     this.expires = Ci.nsIAnnotationService.EXPIRE_NEVER;
   }
 
   function annos(a = null, b = null) {
     return [new AnnoObj("A", a), new AnnoObj("B", b)];
   }
 
-  function verifyAnnoValues(a = null, b = null) {
-    let currentAnnos = PlacesUtils.getAnnotationsForItem(itemId);
+  async function verifyAnnoValues(a = null, b = null) {
+    let currentAnnos = await PlacesUtils.promiseAnnotationsForItem(itemId);
     let expectedAnnos = [];
     if (a !== null)
       expectedAnnos.push(new AnnoObj("A", a));
     if (b !== null)
       expectedAnnos.push(new AnnoObj("B", b));
 
     Assert.deepEqual(currentAnnos, expectedAnnos);
   }
 
   await PT.Annotate({ guid, annotations: annos(1, 2) }).transact();
-  verifyAnnoValues(1, 2);
+  await verifyAnnoValues(1, 2);
   await PT.undo();
-  verifyAnnoValues();
+  await verifyAnnoValues();
   await PT.redo();
-  verifyAnnoValues(1, 2);
+  await verifyAnnoValues(1, 2);
 
   await PT.Annotate({ guid,
                       annotation: { name: "A" } }).transact();
-  verifyAnnoValues(null, 2);
+  await verifyAnnoValues(null, 2);
 
   await PT.Annotate({ guid,
                       annotation: { name: "B", value: 0 } }).transact();
-  verifyAnnoValues(null, 0);
+  await verifyAnnoValues(null, 0);
   await PT.undo();
-  verifyAnnoValues(null, 2);
+  await verifyAnnoValues(null, 2);
   await PT.redo();
-  verifyAnnoValues(null, 0);
+  await verifyAnnoValues(null, 0);
   await PT.undo();
-  verifyAnnoValues(null, 2);
+  await verifyAnnoValues(null, 2);
   await PT.undo();
-  verifyAnnoValues(1, 2);
+  await verifyAnnoValues(1, 2);
   await PT.undo();
-  verifyAnnoValues();
+  await verifyAnnoValues();
 
   // Cleanup
   await PT.undo();
   observer.reset();
 });
 
 add_task(async function test_sort_folder_by_name() {
   let folder_info = createTestFolderInfo();
--- a/toolkit/components/places/tests/unit/test_promiseBookmarksTree.js
+++ b/toolkit/components/places/tests/unit/test_promiseBookmarksTree.js
@@ -40,17 +40,17 @@ async function compareToNode(aItem, aNod
 
   if (aIsRootItem && aNode.itemId != PlacesUtils.placesRootId) {
     Assert.ok("parentGuid" in aItem);
     await check_has_child(aItem.parentGuid, aItem.guid);
   } else {
     check_unset("parentGuid");
   }
 
-  let expectedAnnos = PlacesUtils.getAnnotationsForItem(aItem.id);
+  let expectedAnnos = await PlacesUtils.promiseAnnotationsForItem(aItem.id);
   if (expectedAnnos.length > 0)
     Assert.deepEqual(aItem.annos, expectedAnnos);
   else
     check_unset("annos");
 
   const BOOKMARK_ONLY_PROPS = ["uri", "iconuri", "tags", "charset", "keyword"];
   const FOLDER_ONLY_PROPS = ["children", "root"];