Bug 1450223 - Remove unnecessary synchronous annotation lookups from the places context menu. r=mak

MozReview-Commit-ID: Ck2eaeVAjXs

--HG--
extra : rebase_source : 8de9f419570773d922a6d73c6f44069ffe403376

browser/components/places/content/controller.js

@@ -400,7 +400,6 @@ PlacesController.prototype = {
       var nodeData = {};
       var node = nodes[i];
       var nodeType = node.type;
-      var uri = null;
 
       // We don't use the nodeIs* methods here to avoid going through the type
       // property way too often
@@ -422,13 +421,15 @@ PlacesController.prototype = {
         case Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER:
         case Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT:
           nodeData.folder = true;
+          if (this.hasCachedLivemarkInfo(node)) {
+            nodeData.livemark = true;
+          }
           break;
         case Ci.nsINavHistoryResultNode.RESULT_TYPE_SEPARATOR:
           nodeData.separator = true;
           break;
         case Ci.nsINavHistoryResultNode.RESULT_TYPE_URI:
           nodeData.link = true;
-          uri = Services.io.newURI(node.uri);
           if (PlacesUtils.nodeIsBookmark(node)) {
             nodeData.bookmark = true;
             var parentNode = node.parent;
@@ -442,20 +443,6 @@ PlacesController.prototype = {
           break;
       }
 
-      // annotations
-      if (uri) {
-        let names = PlacesUtils.annotations.getPageAnnotationNames(uri);
-        for (let j = 0; j < names.length; ++j)
-          nodeData[names[j]] = true;
-      }
-
-      // For items also include the item-specific annotations
-      if (node.itemId != -1) {
-        let names = PlacesUtils.annotations
-                               .getItemAnnotationNames(node.itemId);
-        for (let j = 0; j < names.length; ++j)
-          nodeData[names[j]] = true;
-      }
       metadata.push(nodeData);
     }
 

browser/components/places/content/placesContextMenu.inc.xul

@@ -125,7 +125,7 @@
             label="&cmd.reloadLivebookmark.label;"
             accesskey="&cmd.reloadLivebookmark.accesskey;"
             closemenu="single"
-            selection="livemark/feedURI"/>
+            selection="livemark"/>
   <menuseparator id="placesContext_sortSeparator"/>
   <menuitem id="placesContext_show:info"
             command="placesCmd_show:info"
@@ -133,4 +133,4 @@
             accesskey="&cmd.properties.accesskey;"
             selection="bookmark|folder|query"
             forcehideselection="livemarkChild"/>
-</menupopup>
+</menupopup>

toolkit/components/places/nsAnnotationService.cpp

@@ -1250,82 +1250,23 @@ nsAnnotationService::GetItemsWithAnnotationTArray(const nsACString& aName,
 }
 
 
-NS_IMETHODIMP
-nsAnnotationService::GetPageAnnotationNames(nsIURI* aURI,
-                                            uint32_t* _count,
-                                            nsIVariant*** _result)
-{
-  NS_ENSURE_ARG(aURI);
-  NS_ENSURE_ARG_POINTER(_count);
-  NS_ENSURE_ARG_POINTER(_result);
-
-  *_count = 0;
-  *_result = nullptr;
-
-  nsTArray<nsCString> names;
-  nsresult rv = GetAnnotationNamesTArray(aURI, 0, &names);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (names.Length() == 0)
-    return NS_OK;
-
-  *_result = static_cast<nsIVariant**>
-                        (moz_xmalloc(sizeof(nsIVariant*) * names.Length()));
-  NS_ENSURE_TRUE(*_result, NS_ERROR_OUT_OF_MEMORY);
-
-  for (uint32_t i = 0; i < names.Length(); i ++) {
-    nsCOMPtr<nsIWritableVariant> var = new nsVariant();
-    if (!var) {
-      // need to release all the variants we've already created
-      for (uint32_t j = 0; j < i; j ++)
-        NS_RELEASE((*_result)[j]);
-      free(*_result);
-      *_result = nullptr;
-      return NS_ERROR_OUT_OF_MEMORY;
-    }
-    var->SetAsAUTF8String(names[i]);
-    NS_ADDREF((*_result)[i] = var);
-  }
-  *_count = names.Length();
-
-  return NS_OK;
-}
-
-
 nsresult
-nsAnnotationService::GetAnnotationNamesTArray(nsIURI* aURI,
-                                              int64_t aItemId,
-                                              nsTArray<nsCString>* _result)
+nsAnnotationService::GetItemAnnotationNamesTArray(int64_t aItemId,
+                                                  nsTArray<nsCString>* _result)
 {
   _result->Clear();
 
-  bool isItemAnnotation = (aItemId > 0);
   nsCOMPtr<mozIStorageStatement> statement;
-  if (isItemAnnotation) {
-    statement = mDB->GetStatement(
-      "SELECT n.name "
-      "FROM moz_anno_attributes n "
-      "JOIN moz_items_annos a ON a.anno_attribute_id = n.id "
-      "WHERE a.item_id = :item_id"
-    );
-  }
-  else {
-    statement = mDB->GetStatement(
-      "SELECT n.name "
-      "FROM moz_anno_attributes n "
-      "JOIN moz_annos a ON a.anno_attribute_id = n.id "
-      "JOIN moz_places h ON h.id = a.place_id "
-      "WHERE h.url_hash = hash(:page_url) AND h.url = :page_url"
-    );
-  }
+  statement = mDB->GetStatement(
+    "SELECT n.name "
+    "FROM moz_anno_attributes n "
+    "JOIN moz_items_annos a ON a.anno_attribute_id = n.id "
+    "WHERE a.item_id = :item_id"
+  );
   NS_ENSURE_STATE(statement);
   mozStorageStatementScoper scoper(statement);
 
-  nsresult rv;
-  if (isItemAnnotation)
-    rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("item_id"), aItemId);
-  else
-    rv = URIBinder::Bind(statement, NS_LITERAL_CSTRING("page_url"), aURI);
+  nsresult rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("item_id"), aItemId);
   NS_ENSURE_SUCCESS(rv, rv);
 
   bool hasResult = false;
@@ -1355,7 +1296,7 @@ nsAnnotationService::GetItemAnnotationNames(int64_t aItemId,
   *_result = nullptr;
 
   nsTArray<nsCString> names;
-  nsresult rv = GetAnnotationNamesTArray(nullptr, aItemId, &names);
+  nsresult rv = GetItemAnnotationNamesTArray(aItemId, &names);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (names.Length() == 0)

toolkit/components/places/nsAnnotationService.h

@@ -164,9 +164,8 @@ public:
                                           nsCOMArray<nsIURI>* _results);
   nsresult GetItemsWithAnnotationTArray(const nsACString& aName,
                                         nsTArray<int64_t>* _result);
-  nsresult GetAnnotationNamesTArray(nsIURI* aURI,
-                                    int64_t aItemId,
-                                    nsTArray<nsCString>* _result);
+  nsresult GetItemAnnotationNamesTArray(int64_t aItemId,
+                                        nsTArray<nsCString>* _result);
   nsresult RemoveItemAnnotationsWithoutNotifying(int64_t aItemId);
 };
 

toolkit/components/places/nsIAnnotationService.idl

@@ -322,10 +322,6 @@ interface nsIAnnotationService : nsISupports
      * example JS:
      *   var annotations = annotator.getPageAnnotations(myURI, {});
      */
-    void getPageAnnotationNames(
-      in nsIURI aURI,
-      [optional] out unsigned long count,
-      [retval, array, size_is(count)] out nsIVariant result);
     void getItemAnnotationNames(
       in long long aItemId,
       [optional] out unsigned long count,

toolkit/components/places/tests/unit/test_annotations.js

@@ -149,13 +149,8 @@ add_task(async function test_execute() {
   Assert.equal(exp.value, 0);
   Assert.equal(storageType.value, Ci.nsIAnnotationService.TYPE_STRING);
 
-  // get annotation names for a uri
-  var annoNames = annosvc.getPageAnnotationNames(testURI);
-  Assert.equal(annoNames.length, 1);
-  Assert.equal(annoNames[0], "moz-test-places/annotations");
-
   // get annotation names for an item
-  annoNames = annosvc.getItemAnnotationNames(testItemId);
+  let annoNames = annosvc.getItemAnnotationNames(testItemId);
   Assert.equal(annoNames.length, 1);
   Assert.equal(annoNames[0], "moz-test-places/annotations");