Bug 827268 - Avoid useless onVisit work on transition filtered queries.

r=Mano

toolkit/components/places/nsNavHistory.cpp

@@ -894,7 +894,6 @@ nsNavHistory::GetUpdateRequirements(const nsCOMArray<nsNavHistoryQuery>& aQuerie
 
   bool nonTimeBasedItems = false;
   bool domainBasedItems = false;
-  bool queryContainsTransitions = false;
 
   for (i = 0; i < aQueries.Count(); i ++) {
     nsNavHistoryQuery* query = aQueries[i];
@@ -905,9 +904,6 @@ nsNavHistory::GetUpdateRequirements(const nsCOMArray<nsNavHistoryQuery>& aQuerie
       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() ||
@@ -923,9 +919,6 @@ nsNavHistory::GetUpdateRequirements(const nsCOMArray<nsNavHistoryQuery>& aQuerie
       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
@@ -937,6 +930,7 @@ nsNavHistory::GetUpdateRequirements(const nsCOMArray<nsNavHistoryQuery>& aQuerie
     return QUERYUPDATE_HOST;
   if (aQueries.Count() == 1 && !nonTimeBasedItems)
     return QUERYUPDATE_TIME;
+
   return QUERYUPDATE_SIMPLE;
 }
 
@@ -1061,6 +1055,14 @@ nsNavHistory::EvaluateQueryForNode(const nsCOMArray<nsNavHistoryQuery>& aQueries
       }
     }
 
+    // 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.

toolkit/components/places/nsNavHistoryResult.cpp

@@ -106,7 +106,8 @@ nsNavHistoryResultNode::nsNavHistoryResultNode(
   mLastModified(0),
   mIndentLevel(-1),
   mFrecency(0),
-  mHidden(false)
+  mHidden(false),
+  mTransitionType(0)
 {
   mTags.SetIsVoid(true);
 }
@@ -1952,7 +1953,8 @@ nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode(
                                   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");
 
@@ -1962,6 +1964,16 @@ nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode(
     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(
@@ -1974,7 +1986,8 @@ nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode(
                                   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");
 
@@ -1984,6 +1997,16 @@ nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode(
     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() {
@@ -2603,12 +2626,19 @@ nsNavHistoryQueryResultNode::OnVisit(nsIURI* aURI, int64_t aVisitId,
       // 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;
     }
@@ -2873,7 +2903,7 @@ nsNavHistoryQueryResultNode::OnDeleteVisits(nsIURI* aURI,
     // 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);
     }

toolkit/components/places/nsNavHistoryResult.h

@@ -382,6 +382,9 @@ public:
 
   // 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)
@@ -749,6 +752,9 @@ public:
   nsresult NotifyIfTagsChanged(nsIURI* aURI);
 
   uint32_t mBatchChanges;
+
+  // Tracks transition type filters shared by all mQueries.
+  nsTArray<uint32_t> mTransitions;
 };