Bug 494371. Only GetItemType once per itemAdded notification, instead of once per per observer per notification. r=dietrich, a=shaver.
authorMarco Bonardo <mak77@bonardo.net>
Thu, 04 Jun 2009 21:00:55 -0400
changeset 25876 cf3f07dadf75a1014b165ab9cdcfb8e8660aa905
parent 25875 783767cb0d7e9fa71e1e438b2d64a53d7e1222c0
child 25877 010761f9d36ba24af6cc70ca47cff3e11b909390
push id1649
push userbzbarsky@mozilla.com
push dateFri, 05 Jun 2009 02:55:33 +0000
reviewersdietrich, shaver
bugs494371
milestone1.9.1pre
Bug 494371. Only GetItemType once per itemAdded notification, instead of once per per observer per notification. r=dietrich, a=shaver.
toolkit/components/places/src/nsNavHistoryResult.cpp
toolkit/components/places/src/nsNavHistoryResult.h
--- a/toolkit/components/places/src/nsNavHistoryResult.cpp
+++ b/toolkit/components/places/src/nsNavHistoryResult.cpp
@@ -2890,26 +2890,40 @@ nsNavHistoryQueryResultNode::OnPageChang
 
 NS_IMETHODIMP
 nsNavHistoryQueryResultNode::OnPageExpired(nsIURI* aURI, PRTime aVisitTime,
                                            PRBool aWholeEntry)
 {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsNavHistoryQueryResultNode::OnItemAdded(PRInt64 aItemId,
+                                         PRInt64 aFolder,
+                                         PRInt32 aIndex)
+{
+  nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
+  NS_ENSURE_TRUE(bookmarks, NS_ERROR_OUT_OF_MEMORY);
+  PRUint16 itemType;
+  nsresult rv = bookmarks->GetItemType(aItemId, &itemType);
+  NS_ENSURE_SUCCESS(rv, rv);
+  return OnItemAdded(aItemId, aFolder, aIndex, itemType);
+}
 
 // nsNavHistoryQueryResultNode bookmark observers
 //
 //    These are the bookmark observer functions for query nodes. They listen
 //    for bookmark events and refresh the results if we have any dependence on
 //    the bookmark system.
 
 NS_IMETHODIMP
-nsNavHistoryQueryResultNode::OnItemAdded(PRInt64 aItemId, PRInt64 aFolder,
-                                          PRInt32 aIndex)
+nsNavHistoryQueryResultNode::OnItemAdded(PRInt64 aItemId,
+                                         PRInt64 aFolder,
+                                         PRInt32 aIndex,
+                                         PRUint16 aItemType)
 {
   if (mLiveUpdate == QUERYUPDATE_COMPLEX_WITH_BOOKMARKS)
     return Refresh();
   return NS_OK;
 }
 NS_IMETHODIMP
 nsNavHistoryQueryResultNode::OnItemRemoved(PRInt64 aItemId, PRInt64 aFolder,
                                             PRInt32 aIndex)
@@ -3407,126 +3421,156 @@ nsNavHistoryFolderResultNode::OnBeginUpd
 
 NS_IMETHODIMP
 nsNavHistoryFolderResultNode::OnEndUpdateBatch()
 {
   return NS_OK;
 }
 
 
+NS_IMETHODIMP
+nsNavHistoryFolderResultNode::OnItemAdded(PRInt64 aItemId,
+                                         PRInt64 aFolder,
+                                         PRInt32 aIndex)
+{
+  nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
+  NS_ENSURE_TRUE(bookmarks, NS_ERROR_OUT_OF_MEMORY);
+  PRUint16 itemType;
+  nsresult rv = bookmarks->GetItemType(aItemId, &itemType);
+  NS_ENSURE_SUCCESS(rv, rv);
+  return OnItemAdded(aItemId, aFolder, aIndex, itemType);
+}
+
+
 // nsNavHistoryFolderResultNode::OnItemAdded (nsINavBookmarkObserver)
 
 NS_IMETHODIMP
 nsNavHistoryFolderResultNode::OnItemAdded(PRInt64 aItemId,
                                           PRInt64 aParentFolder,
-                                          PRInt32 aIndex)
+                                          PRInt32 aIndex,
+                                          PRUint16 aItemType)
 {
   NS_ASSERTION(aParentFolder == mItemId, "Got wrong bookmark update");
 
   // here, try to do something reasonable if the bookmark service gives us
   // a bogus index.
   if (aIndex < 0) {
     NS_NOTREACHED("Invalid index for item adding: <0");
     aIndex = 0;
-  } else if (aIndex > mChildren.Count()) {
+  }
+  else if (aIndex > mChildren.Count()) {
+    PRBool excludeItems = (mResult && mResult->mRootNode->mOptions->ExcludeItems()) ||
+                          (mParent && mParent->mOptions->ExcludeItems()) ||
+                          mOptions->ExcludeItems();
+    if (excludeItems &&
+        (aItemType == nsINavBookmarksService::TYPE_BOOKMARK ||
+         aItemType == nsINavBookmarksService::TYPE_SEPARATOR))
+      return NS_OK;
+
     NS_NOTREACHED("Invalid index for item adding: greater than count");
     aIndex = mChildren.Count();
   }
 
   nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
   NS_ENSURE_TRUE(bookmarks, NS_ERROR_OUT_OF_MEMORY);
 
-  PRUint16 itemType;
-  nsresult rv = bookmarks->GetItemType(aItemId, &itemType);
-  NS_ENSURE_SUCCESS(rv, rv);
+  nsresult rv;
 
   // check for query URIs, which are bookmarks, but treated as containers
   // in results and views.
   PRBool isQuery = PR_FALSE;
-  if (itemType == nsINavBookmarksService::TYPE_BOOKMARK) {
+  if (aItemType == nsINavBookmarksService::TYPE_BOOKMARK) {
     nsCOMPtr<nsIURI> itemURI;
     rv = bookmarks->GetBookmarkURI(aItemId, getter_AddRefs(itemURI));
     NS_ENSURE_SUCCESS(rv, rv);
     nsCAutoString itemURISpec;
     rv = itemURI->GetSpec(itemURISpec);
     NS_ENSURE_SUCCESS(rv, rv);
     isQuery = IsQueryURI(itemURISpec);
   }
 
-  if (itemType != nsINavBookmarksService::TYPE_FOLDER &&
+  if (aItemType != nsINavBookmarksService::TYPE_FOLDER &&
       !isQuery && mOptions->ExcludeItems()) {
     // don't update items when we aren't displaying them, but we still need
     // to adjust bookmark indices to account for the insertion
     ReindexRange(aIndex, PR_INT32_MAX, 1);
     return NS_OK; 
   }
 
   if (!StartIncrementalUpdate())
     return NS_OK; // folder was completely refreshed for us
 
   // adjust indices to account for insertion
   ReindexRange(aIndex, PR_INT32_MAX, 1);
 
   nsRefPtr<nsNavHistoryResultNode> node;
-  if (itemType == nsINavBookmarksService::TYPE_BOOKMARK) {
+  if (aItemType == nsINavBookmarksService::TYPE_BOOKMARK) {
     nsNavHistory* history = nsNavHistory::GetHistoryService();
     NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
     rv = history->BookmarkIdToResultNode(aItemId, mOptions, getter_AddRefs(node));
     NS_ENSURE_SUCCESS(rv, rv);
     // Correctly set batch status for new query nodes.
     if (mResult && node->IsQuery())
       node->GetAsQuery()->mBatchInProgress = mResult->mBatchInProgress;
   }
-  else if (itemType == nsINavBookmarksService::TYPE_FOLDER ||
-           itemType == nsINavBookmarksService::TYPE_DYNAMIC_CONTAINER) {
+  else if (aItemType == nsINavBookmarksService::TYPE_FOLDER ||
+           aItemType == nsINavBookmarksService::TYPE_DYNAMIC_CONTAINER) {
     rv = bookmarks->ResultNodeForContainer(aItemId, mOptions, getter_AddRefs(node));
     NS_ENSURE_SUCCESS(rv, rv);
   }
-  else if (itemType == nsINavBookmarksService::TYPE_SEPARATOR) {
+  else if (aItemType == nsINavBookmarksService::TYPE_SEPARATOR) {
     node = new nsNavHistorySeparatorResultNode();
     NS_ENSURE_TRUE(node, NS_ERROR_OUT_OF_MEMORY);
     node->mItemId = aItemId;
   }
   node->mBookmarkIndex = aIndex;
 
-  if (itemType == nsINavBookmarksService::TYPE_SEPARATOR ||
+  if (aItemType == nsINavBookmarksService::TYPE_SEPARATOR ||
       GetSortType() == nsINavHistoryQueryOptions::SORT_BY_NONE) {
     // insert at natural bookmarks position
     return InsertChildAt(node, aIndex);
   }
   // insert at sorted position
   return InsertSortedChild(node, PR_FALSE);
 }
 
 
 // nsNavHistoryFolderResultNode::OnItemRemoved (nsINavBookmarkObserver)
 
 NS_IMETHODIMP
 nsNavHistoryFolderResultNode::OnItemRemoved(PRInt64 aItemId,
-                                            PRInt64 aParentFolder, PRInt32 aIndex)
+                                            PRInt64 aParentFolder,
+                                            PRInt32 aIndex)
 {
   // We only care about notifications when a child changes. When the deleted
   // item is us, our parent should also be registered and will remove us from
   // its list.
   if (mItemId == aItemId)
     return NS_OK;
 
+  NS_ASSERTION(aParentFolder == mItemId, "Got wrong bookmark update");
+
+  PRBool excludeItems = (mResult && mResult->mRootNode->mOptions->ExcludeItems()) ||
+                        (mParent && mParent->mOptions->ExcludeItems()) ||
+                        mOptions->ExcludeItems();
+
   // don't trust the index from the bookmark service, find it ourselves. The
   // sorting could be different, or the bookmark services indices and ours might
   // be out of sync somehow.
   PRUint32 index;
   nsNavHistoryResultNode* node = FindChildById(aItemId, &index);
   if (!node) {
+    if (excludeItems)
+      return NS_OK;
+
     NS_NOTREACHED("Removing item we don't have");
     return NS_ERROR_FAILURE;
   }
 
-  NS_ASSERTION(aParentFolder == mItemId, "Got wrong bookmark update");
-
-  if ((node->IsURI() || node->IsSeparator()) && mOptions->ExcludeItems()) {
+  if ((node->IsURI() || node->IsSeparator()) && excludeItems) {
     // don't update items when we aren't displaying them, but we do need to
     // adjust everybody's bookmark indices to account for the removal
     ReindexRange(aIndex, PR_INT32_MAX, -1);
     return NS_OK;
   }
 
   if (!StartIncrementalUpdate())
     return NS_OK; // we are completely refreshed
@@ -4174,20 +4218,28 @@ nsNavHistoryResult::OnEndUpdateBatch()
 
 // nsNavHistoryResult::OnItemAdded (nsINavBookmarkObserver)
 
 NS_IMETHODIMP
 nsNavHistoryResult::OnItemAdded(PRInt64 aItemId,
                                 PRInt64 aFolder,
                                 PRInt32 aIndex)
 {
+  nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
+  NS_ENSURE_TRUE(bookmarks, NS_ERROR_OUT_OF_MEMORY);
+
+  PRUint16 itemType;
+  nsresult rv = bookmarks->GetItemType(aItemId, &itemType);
+  NS_ENSURE_SUCCESS(rv, rv);
+
   ENUMERATE_BOOKMARK_FOLDER_OBSERVERS(aFolder,
-      OnItemAdded(aItemId, aFolder, aIndex));
-  ENUMERATE_HISTORY_OBSERVERS(OnItemAdded(aItemId, aFolder, aIndex));
-  ENUMERATE_ALL_BOOKMARKS_OBSERVERS(OnItemAdded(aItemId, aFolder, aIndex));
+      OnItemAdded(aItemId, aFolder, aIndex, itemType));
+  ENUMERATE_HISTORY_OBSERVERS(OnItemAdded(aItemId, aFolder, aIndex, itemType));
+  ENUMERATE_ALL_BOOKMARKS_OBSERVERS(OnItemAdded(aItemId, aFolder, aIndex,
+                                                itemType));
   return NS_OK;
 }
 
 
 // nsNavHistoryResult::OnItemRemoved (nsINavBookmarkObserver)
 
 NS_IMETHODIMP
 nsNavHistoryResult::OnItemRemoved(PRInt64 aItemId,
@@ -4206,48 +4258,46 @@ nsNavHistoryResult::OnItemRemoved(PRInt6
 // nsNavHistoryResult::OnItemChanged (nsINavBookmarkObserver)
 
 NS_IMETHODIMP
 nsNavHistoryResult::OnItemChanged(PRInt64 aItemId,
                                   const nsACString &aProperty,
                                   PRBool aIsAnnotationProperty,
                                   const nsACString &aValue)
 {
-  nsNavBookmarks* bookmarkService = nsNavBookmarks::GetBookmarksService();
-  NS_ENSURE_TRUE(bookmarkService, NS_ERROR_OUT_OF_MEMORY);
-
   ENUMERATE_ALL_BOOKMARKS_OBSERVERS(
     OnItemChanged(aItemId, aProperty, aIsAnnotationProperty, aValue));
 
-  PRUint16 itemType;
-  nsresult rv = bookmarkService->GetItemType(aItemId, &itemType);
-  NS_ENSURE_SUCCESS(rv, rv);
-
   // Note: folder-nodes set their own bookmark observer only once they're
   // opened, meaning we cannot optimize this code path for changes done to
   // folder-nodes.
 
+  nsNavBookmarks* bookmarkService = nsNavBookmarks::GetBookmarksService();
+  NS_ENSURE_TRUE(bookmarkService, NS_ERROR_OUT_OF_MEMORY);
+
   // Find the changed items under the folders list
   PRInt64 folderId;
-  rv = bookmarkService->GetFolderIdForItem(aItemId, &folderId);
+  nsresult rv = bookmarkService->GetFolderIdForItem(aItemId, &folderId);
   NS_ENSURE_SUCCESS(rv, rv);
 
   FolderObserverList* list = BookmarkFolderObserversForId(folderId, PR_FALSE);
   if (!list)
     return NS_OK;
 
   for (PRUint32 i = 0; i < list->Length(); i++) {
     nsRefPtr<nsNavHistoryFolderResultNode> folder = list->ElementAt(i);
     if (folder) {
       PRUint32 nodeIndex;
-      nsRefPtr<nsNavHistoryResultNode> node = folder->FindChildById(aItemId, &nodeIndex);
+      nsRefPtr<nsNavHistoryResultNode> node =
+        folder->FindChildById(aItemId, &nodeIndex);
       // if ExcludeItems is true we don't update non visible items
+      PRBool excludeItems = (mRootNode->mOptions->ExcludeItems()) ||
+                             folder->mOptions->ExcludeItems();
       if (node &&
-          (!folder->mOptions->ExcludeItems() ||
-           !(node->IsURI() || node->IsSeparator())) &&
+          (!excludeItems || !(node->IsURI() || node->IsSeparator())) &&
           folder->StartIncrementalUpdate()) {
         node->OnItemChanged(aItemId, aProperty, aIsAnnotationProperty, aValue);
       }
     }
   }
 
   // Note: we do NOT call history observers in this case. This notification is
   // the same as other history notification, except that here we know the item
--- a/toolkit/components/places/src/nsNavHistoryResult.h
+++ b/toolkit/components/places/src/nsNavHistoryResult.h
@@ -92,25 +92,28 @@ private:
 
 
 // Declare methods for implementing nsINavBookmarkObserver
 // and nsINavHistoryObserver (some methods, such as BeginUpdateBatch overlap)
 #define NS_DECL_BOOKMARK_HISTORY_OBSERVER                               \
   NS_DECL_NSINAVBOOKMARKOBSERVER                                        \
   NS_IMETHOD OnVisit(nsIURI* aURI, PRInt64 aVisitId, PRTime aTime,      \
                      PRInt64 aSessionId, PRInt64 aReferringId,          \
-                     PRUint32 aTransitionType, PRUint32* aAdded);      \
+                     PRUint32 aTransitionType, PRUint32* aAdded);       \
   NS_IMETHOD OnTitleChanged(nsIURI* aURI, const nsAString& aPageTitle); \
   NS_IMETHOD OnDeleteURI(nsIURI *aURI);                                 \
   NS_IMETHOD OnClearHistory();                                          \
   NS_IMETHOD OnPageChanged(nsIURI *aURI, PRUint32 aWhat,                \
                            const nsAString &aValue);                    \
   NS_IMETHOD OnPageExpired(nsIURI* aURI, PRTime aVisitTime,             \
                            PRBool aWholeEntry);
 
+#define NS_DECL_EXTENDED_BOOKMARK_OBSERVER                              \
+  NS_IMETHOD OnItemAdded(PRInt64 aItemId, PRInt64 aFolder,              \
+                         PRInt32 aIndex, PRUint16 aItemType);
 
 // nsNavHistoryResult
 //
 //    nsNavHistory creates this object and fills in mChildren (by getting
 //    it through GetTopLevel()). Then FilledAllResults() is called to finish
 //    object initialization.
 //
 //    This object implements nsITreeView so you can just set it to a tree
@@ -711,16 +714,17 @@ public:
   NS_DECL_NSINAVHISTORYQUERYRESULTNODE
 
   PRBool CanExpand();
   PRBool IsContainersQuery();
 
   virtual nsresult OpenContainer();
 
   NS_DECL_BOOKMARK_HISTORY_OBSERVER
+  NS_DECL_EXTENDED_BOOKMARK_OBSERVER
   virtual void OnRemoving();
 
 public:
   // this constructs lazily mURI from mQueries and mOptions, call
   // VerifyQueriesSerialized either this or mQueries/mOptions should be valid
   nsresult VerifyQueriesSerialized();
 
   // these may be constructed lazily from mURI, call VerifyQueriesParsed
@@ -784,19 +788,19 @@ public:
   NS_DECL_NSINAVHISTORYQUERYRESULTNODE
 
   virtual nsresult OpenContainer();
 
   // This object implements a bookmark observer interface without deriving from
   // the bookmark observers. This is called from the result's actual observer
   // and it knows all observers are FolderResultNodes
   NS_DECL_NSINAVBOOKMARKOBSERVER
+  NS_DECL_EXTENDED_BOOKMARK_OBSERVER
 
   virtual void OnRemoving();
-
 public:
 
   // this indicates whether the folder contents are valid, they don't go away
   // after the container is closed until a notification comes in
   PRBool mContentsValid;
 
   // If the node is generated from a place:folder=X query, this is the query's
   // itemId.