Bug 827268 - Avoid useless onVisit work on transition filtered queries.
authorMarco Bonardo <mbonardo@mozilla.com>
Mon, 07 Jan 2013 18:54:38 +0100
changeset 127052 f5e3e40ab3c9704459e122393be05083cb8353d8
parent 127051 bef5ca4854d6061bbfc8682e444853b06680fcf2
child 127053 6ec8ebb4f02e3efe94d948c5837b1b54c67e55a2
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs827268
milestone20.0a2
Bug 827268 - Avoid useless onVisit work on transition filtered queries. r=Mano a=gavin
toolkit/components/places/nsNavHistory.cpp
toolkit/components/places/nsNavHistoryResult.cpp
toolkit/components/places/nsNavHistoryResult.h
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -889,59 +889,53 @@ nsNavHistory::GetUpdateRequirements(cons
   for (i = 0; i < aQueries.Count(); i ++) {
     aQueries[i]->GetHasSearchTerms(aHasSearchTerms);
     if (*aHasSearchTerms)
       break;
   }
 
   bool nonTimeBasedItems = false;
   bool domainBasedItems = false;
-  bool queryContainsTransitions = false;
 
   for (i = 0; i < aQueries.Count(); i ++) {
     nsNavHistoryQuery* query = aQueries[i];
 
     if (query->Folders().Length() > 0 ||
         query->OnlyBookmarked() ||
         query->Tags().Length() > 0) {
       return QUERYUPDATE_COMPLEX_WITH_BOOKMARKS;
     }
 
-    if (query->Transitions().Length() > 0)
-      queryContainsTransitions = true;
-
     // Note: we don't currently have any complex non-bookmarked items, but these
     // are expected to be added. Put detection of these items here.
     if (!query->SearchTerms().IsEmpty() ||
         !query->Domain().IsVoid() ||
         query->Uri() != nullptr)
       nonTimeBasedItems = true;
 
     if (! query->Domain().IsVoid())
       domainBasedItems = true;
   }
 
   if (aOptions->ResultType() ==
       nsINavHistoryQueryOptions::RESULTS_AS_TAG_QUERY)
     return QUERYUPDATE_COMPLEX_WITH_BOOKMARKS;
 
-  if (queryContainsTransitions)
-    return QUERYUPDATE_COMPLEX;
-
   // Whenever there is a maximum number of results, 
   // and we are not a bookmark query we must requery. This
   // is because we can't generally know if any given addition/change causes
   // the item to be in the top N items in the database.
   if (aOptions->MaxResults() > 0)
     return QUERYUPDATE_COMPLEX;
 
   if (aQueries.Count() == 1 && domainBasedItems)
     return QUERYUPDATE_HOST;
   if (aQueries.Count() == 1 && !nonTimeBasedItems)
     return QUERYUPDATE_TIME;
+
   return QUERYUPDATE_SIMPLE;
 }
 
 
 // nsNavHistory::EvaluateQueryForNode
 //
 //    This runs the node through the given queries to see if satisfies the
 //    query conditions. Not every query parameters are handled by this code,
@@ -1056,16 +1050,24 @@ nsNavHistory::EvaluateQueryForNode(const
         if (queryUriString.Length() > nodeUriString.Length())
           continue; // not long enough to match as prefix
         nodeUriString.SetLength(queryUriString.Length());
         if (! nodeUriString.Equals(queryUriString))
           continue; // prefixes don't match
       }
     }
 
+    // Transitions matching.
+    const nsTArray<uint32_t>& transitions = query->Transitions();
+    if (aNode->mTransitionType > 0 &&
+        transitions.Length() &&
+        !transitions.Contains(aNode->mTransitionType)) {
+      continue; // transition doesn't match.
+    }
+
     // If we ever make it to the bottom of this loop, that means it passed all
     // tests for the given query. Since queries are ORed together, that means
     // it passed everything and we are done.
     return true;
   }
 
   // didn't match any query
   return false;
--- a/toolkit/components/places/nsNavHistoryResult.cpp
+++ b/toolkit/components/places/nsNavHistoryResult.cpp
@@ -101,17 +101,18 @@ nsNavHistoryResultNode::nsNavHistoryResu
   mFaviconURI(aIconURI),
   mBookmarkIndex(-1),
   mItemId(-1),
   mFolderId(-1),
   mDateAdded(0),
   mLastModified(0),
   mIndentLevel(-1),
   mFrecency(0),
-  mHidden(false)
+  mHidden(false),
+  mTransitionType(0)
 {
   mTags.SetIsVoid(true);
 }
 
 
 NS_IMETHODIMP
 nsNavHistoryResultNode::GetIcon(nsACString& aIcon)
 {
@@ -1947,48 +1948,70 @@ nsNavHistoryQueryResultNode::nsNavHistor
     const nsACString& aTitle, const nsACString& aIconURI,
     const nsCOMArray<nsNavHistoryQuery>& aQueries,
     nsNavHistoryQueryOptions* aOptions) :
   nsNavHistoryContainerResultNode(EmptyCString(), aTitle, aIconURI,
                                   nsNavHistoryResultNode::RESULT_TYPE_QUERY,
                                   true, aOptions),
   mQueries(aQueries),
   mContentsValid(false),
-  mBatchChanges(0)
+  mBatchChanges(0),
+  mTransitions(mQueries[0]->Transitions())
 {
   NS_ASSERTION(aQueries.Count() > 0, "Must have at least one query");
 
   nsNavHistory* history = nsNavHistory::GetHistoryService();
   NS_ASSERTION(history, "History service missing");
   if (history) {
     mLiveUpdate = history->GetUpdateRequirements(mQueries, mOptions,
                                                  &mHasSearchTerms);
   }
+
+  // Collect transitions shared by all queries.
+  for (int32_t i = 1; i < mQueries.Count(); ++i) {
+    const nsTArray<uint32_t>& queryTransitions = mQueries[i]->Transitions();
+    for (uint32_t j = 0; j < mTransitions.Length() ; ++j) {
+      uint32_t transition = mTransitions.SafeElementAt(j, 0);
+      if (transition && !queryTransitions.Contains(transition))
+        mTransitions.RemoveElement(transition);
+    }
+  }
 }
 
 nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode(
     const nsACString& aTitle, const nsACString& aIconURI,
     PRTime aTime,
     const nsCOMArray<nsNavHistoryQuery>& aQueries,
     nsNavHistoryQueryOptions* aOptions) :
   nsNavHistoryContainerResultNode(EmptyCString(), aTitle, aTime, aIconURI,
                                   nsNavHistoryResultNode::RESULT_TYPE_QUERY,
                                   true, aOptions),
   mQueries(aQueries),
   mContentsValid(false),
-  mBatchChanges(0)
+  mBatchChanges(0),
+  mTransitions(mQueries[0]->Transitions())
 {
   NS_ASSERTION(aQueries.Count() > 0, "Must have at least one query");
 
   nsNavHistory* history = nsNavHistory::GetHistoryService();
   NS_ASSERTION(history, "History service missing");
   if (history) {
     mLiveUpdate = history->GetUpdateRequirements(mQueries, mOptions,
                                                  &mHasSearchTerms);
   }
+
+  // Collect transitions shared by all queries.
+  for (int32_t i = 1; i < mQueries.Count(); ++i) {
+    const nsTArray<uint32_t>& queryTransitions = mQueries[i]->Transitions();
+    for (uint32_t j = 0; j < mTransitions.Length() ; ++j) {
+      uint32_t transition = mTransitions.SafeElementAt(j, 0);
+      if (transition && !queryTransitions.Contains(transition))
+        mTransitions.RemoveElement(transition);
+    }
+  }
 }
 
 nsNavHistoryQueryResultNode::~nsNavHistoryQueryResultNode() {
   // Remove this node from result's observers.  We don't need to be notified
   // anymore.
   if (mResult && mResult->mAllBookmarksObservers.IndexOf(this) !=
                    mResult->mAllBookmarksObservers.NoIndex)
     mResult->RemoveAllBookmarksObserver(this);
@@ -2598,22 +2621,29 @@ nsNavHistoryQueryResultNode::OnVisit(nsI
                                                 query->EndTime());
         if (aTime > endTime)
           return NS_OK; // after our time range
       }
       // Now we know that our visit satisfies the time range, fallback to the
       // QUERYUPDATE_SIMPLE case.
     }
     case QUERYUPDATE_SIMPLE: {
+      // If all of the queries are filtered by some transitions, skip the
+      // update if aTransitionType doesn't match any of them.
+      if (mTransitions.Length() > 0 && !mTransitions.Contains(aTransitionType))
+        return NS_OK;
+
       // The history service can tell us whether the new item should appear
       // in the result.  We first have to construct a node for it to check.
       rv = history->VisitIdToResultNode(aVisitId, mOptions,
                                         getter_AddRefs(addition));
-      if (NS_FAILED(rv) || !addition ||
-          !history->EvaluateQueryForNode(mQueries, mOptions, addition))
+      NS_ENSURE_SUCCESS(rv, rv);
+      NS_ENSURE_STATE(addition);
+      addition->mTransitionType = aTransitionType;
+      if (!history->EvaluateQueryForNode(mQueries, mOptions, addition))
         return NS_OK; // don't need to include in our query
       break;
     }
     case QUERYUPDATE_COMPLEX:
     case QUERYUPDATE_COMPLEX_WITH_BOOKMARKS:
       // need to requery in complex cases
       return Refresh();
     default:
@@ -2868,17 +2898,17 @@ nsNavHistoryQueryResultNode::OnDeleteVis
     // query this is equivalent to a onDeleteURI notification.
     nsresult rv = OnDeleteURI(aURI, aGUID, aReason);
     NS_ENSURE_SUCCESS(rv, rv);
   }
   if (aTransitionType > 0) {
     // All visits for aTransitionType have been removed, if the query is
     // filtering on such transition type, this is equivalent to an onDeleteURI
     // notification.
-    if ((mQueries[0]->Transitions()).Contains(aTransitionType)) {
+    if (mTransitions.Length() > 0 && mTransitions.Contains(aTransitionType)) {
       nsresult rv = OnDeleteURI(aURI, aGUID, aReason);
       NS_ENSURE_SUCCESS(rv, rv);
     }
   }
 
   return NS_OK;
 }
 
--- a/toolkit/components/places/nsNavHistoryResult.h
+++ b/toolkit/components/places/nsNavHistoryResult.h
@@ -377,16 +377,19 @@ public:
   // root's children will have a value of 0, and so on.
   int32_t mIndentLevel;
 
   // Frecency of the page.  Valid only for URI nodes.
   int32_t mFrecency;
 
   // Hidden status of the page.  Valid only for URI nodes.
   bool mHidden;
+
+  // Transition type used when this node represents a single visit.
+  int32_t mTransitionType;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsNavHistoryResultNode, NS_NAVHISTORYRESULTNODE_IID)
 
 // nsNavHistoryVisitResultNode
 
 #define NS_IMPLEMENT_VISITRESULT \
   NS_IMETHOD GetUri(nsACString& aURI) { aURI = mURI; return NS_OK; } \
@@ -744,16 +747,19 @@ public:
   virtual void GetSortingAnnotation(nsACString& aSortingAnnotation);
   virtual void RecursiveSort(const char* aData,
                              SortComparator aComparator);
 
   nsCOMPtr<nsIURI> mRemovingURI;
   nsresult NotifyIfTagsChanged(nsIURI* aURI);
 
   uint32_t mBatchChanges;
+
+  // Tracks transition type filters shared by all mQueries.
+  nsTArray<uint32_t> mTransitions;
 };
 
 
 // nsNavHistoryFolderResultNode
 //
 //    Overridden container type for bookmark folders. It will keep the contents
 //    of the folder in sync with the bookmark service.