Bug 827268 - Avoid useless onVisit work on transition filtered queries.
authorMarco Bonardo <mbonardo@mozilla.com>
Mon, 07 Jan 2013 18:54:38 +0100
changeset 118134 63cbf8539eac85de59f81a0ab2641b5762d39b01
parent 118133 303b3bb5f27fe7af58ec9bc689ad8325bac5779a
child 118135 cb1f63ecc1cd690fe9c4a46fcbdce34c62e1fe5a
push id20864
push usermak77@bonardo.net
push dateTue, 08 Jan 2013 20:36:00 +0000
treeherdermozilla-inbound@63cbf8539eac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs827268
milestone21.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 827268 - Avoid useless onVisit work on transition filtered queries. r=Mano
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.