From 10c13b9b18ca1e385de3933d20dc18c8bd256dba Mon Sep 17 00:00:00 2001 From: "dbaron@dbaron.org" Date: Fri, 28 Sep 2007 18:04:31 -0700 Subject: [PATCH] Backing out bug 394508 due to tinderbox orange (test failure of places unit tests). --- .../components/places/src/nsNavHistory.cpp | 115 ++++++++---------- toolkit/components/places/src/nsNavHistory.h | 3 +- 2 files changed, 50 insertions(+), 68 deletions(-) diff --git a/toolkit/components/places/src/nsNavHistory.cpp b/toolkit/components/places/src/nsNavHistory.cpp index b74bf495ccc4..f4cf4c6b9d67 100644 --- a/toolkit/components/places/src/nsNavHistory.cpp +++ b/toolkit/components/places/src/nsNavHistory.cpp @@ -1558,7 +1558,7 @@ nsNavHistory::EvaluateQueryForNode(const nsCOMArray& aQueries nsCOMArray queries; queries.AppendObject(query); nsCOMArray filteredSet; - nsresult rv = FilterResultSet(nsnull, inputSet, &filteredSet, queries, aOptions); + nsresult rv = FilterResultSet(nsnull, inputSet, &filteredSet, queries); if (NS_FAILED(rv)) continue; if (! filteredSet.Count()) @@ -2144,32 +2144,6 @@ PRBool IsHistoryMenuQuery(const nsCOMArray& aQueries, nsNavHi return PR_TRUE; } -static -PRBool NeedToFilterResultSet(const nsCOMArray& aQueries, - nsNavHistoryQueryOptions *aOptions) -{ - // optimize the case where we just want a list with no grouping: this - // directly fills in the results and we avoid a copy of the whole list - PRUint32 groupCount; - const PRUint16 *groupings = aOptions->GroupingMode(&groupCount); - - if (groupCount != 0 || aOptions->ExcludeQueries()) - return PR_TRUE; - - PRInt32 i; - for (i = 0; i < aQueries.Count(); i ++) { - if (aQueries[i]->Folders().Length() != 0) { - return PR_TRUE; - } else { - PRBool hasSearchTerms; - nsresult rv = aQueries[i]->GetHasSearchTerms(&hasSearchTerms); - if (NS_FAILED(rv) || hasSearchTerms) - return PR_TRUE; - } - } - return PR_FALSE; -} - nsresult nsNavHistory::ConstructQueryString(const nsCOMArray& aQueries, nsNavHistoryQueryOptions *aOptions, @@ -2181,11 +2155,6 @@ nsNavHistory::ConstructQueryString(const nsCOMArray& aQueries return NS_ERROR_INVALID_ARG; } - // determine whether we can push maxResults constraints - // into the queries as LIMIT, or if we need to do result count clamping later - // using FilterResultSet() - PRBool canLimitInSQL = !NeedToFilterResultSet(aQueries, aOptions); - // for the very special query for the history menu // we generate a super-optimized SQL query if (IsHistoryMenuQuery(aQueries, aOptions)) { @@ -2200,11 +2169,8 @@ nsNavHistory::ConstructQueryString(const nsCOMArray& aQueries "(h.id IN (SELECT DISTINCT h.id FROM moz_historyvisits, " " moz_places h WHERE place_id = " " h.id AND hidden <> 1 AND visit_type <> 4 AND visit_type <> 0 " - " ORDER BY visit_date DESC"); - if (canLimitInSQL) { - queryString += NS_LITERAL_CSTRING(" LIMIT "); - queryString.AppendInt(aOptions->MaxResults()); - } + " ORDER BY visit_date DESC LIMIT "); + queryString.AppendInt(aOptions->MaxResults()); queryString += NS_LITERAL_CSTRING(")) GROUP BY h.id ORDER BY 6 DESC"); // v.visit_date return NS_OK; } @@ -2272,7 +2238,8 @@ nsNavHistory::ConstructQueryString(const nsCOMArray& aQueries "LEFT OUTER JOIN moz_historyvisits v ON b.fk = v.place_id " "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id "); groupBy = NS_LITERAL_CSTRING(" GROUP BY b.id"); - } else { + } + else { // XXX: implement support for nsINavHistoryQueryOptions::QUERY_TYPE_UNIFIED return NS_ERROR_NOT_IMPLEMENTED; } @@ -2367,8 +2334,8 @@ nsNavHistory::ConstructQueryString(const nsCOMArray& aQueries NS_NOTREACHED("Invalid sorting mode"); } - // limit clause if there are 'maxResults' and can use limit in our SQL - if (canLimitInSQL && aOptions->MaxResults() > 0) { + // limit clause if there are 'maxResults' + if (aOptions->MaxResults() > 0) { queryString += NS_LITERAL_CSTRING(" LIMIT "); queryString.AppendInt(aOptions->MaxResults()); queryString.AppendLiteral(" "); @@ -2423,30 +2390,55 @@ nsNavHistory::GetQueryResults(nsNavHistoryQueryResultNode *aResultNode, // bind parameters PRInt32 numParameters = 0; - // optimize the case where we just use the results as is - // and we don't need to do any post-query filtering - if (NeedToFilterResultSet(aQueries, aOptions)) { + // optimize the case where we just want a list with no grouping: this + // directly fills in the results and we avoid a copy of the whole list + PRBool resultAsList = PR_TRUE; + PRUint32 groupCount; + const PRUint16 *groupings = aOptions->GroupingMode(&groupCount); + + if (groupCount != 0 || aOptions->ExcludeQueries()) { + resultAsList = PR_FALSE; + } + + PRInt32 i; + for (i = 0; i < aQueries.Count(); i ++) { + PRInt32 clauseParameters = 0; + rv = BindQueryClauseParameters(statement, numParameters, + aQueries[i], aOptions, &clauseParameters); + NS_ENSURE_SUCCESS(rv, rv); + numParameters += clauseParameters; + if (resultAsList) { + if (aQueries[i]->Folders().Length() != 0) { + resultAsList = PR_FALSE; + } else { + PRBool hasSearchTerms; + rv = aQueries[i]->GetHasSearchTerms(&hasSearchTerms); + if (hasSearchTerms) + resultAsList = PR_FALSE; + NS_ENSURE_SUCCESS(rv, rv); + } + } + } + + if (resultAsList) { + rv = ResultsAsList(statement, aOptions, aResults); + NS_ENSURE_SUCCESS(rv, rv); + } else { // generate the toplevel results nsCOMArray toplevel; rv = ResultsAsList(statement, aOptions, &toplevel); NS_ENSURE_SUCCESS(rv, rv); - PRUint32 groupCount; - const PRUint16 *groupings = aOptions->GroupingMode(&groupCount); - if (groupCount == 0) { - FilterResultSet(aResultNode, toplevel, aResults, aQueries, aOptions); + FilterResultSet(aResultNode, toplevel, aResults, aQueries); } else { nsCOMArray filteredResults; - FilterResultSet(aResultNode, toplevel, &filteredResults, aQueries, aOptions); + FilterResultSet(aResultNode, toplevel, &filteredResults, aQueries); rv = RecursiveGroup(aResultNode, filteredResults, groupings, groupCount, aResults); NS_ENSURE_SUCCESS(rv, rv); } - } else { - rv = ResultsAsList(statement, aOptions, aResults); - NS_ENSURE_SUCCESS(rv, rv); - } + } return NS_OK; } @@ -4135,22 +4127,16 @@ nsNavHistory::URIHasTag(nsIURI* aURI, const nsAString& aTag) // nsNavHistory::FilterResultSet // -// This does some post-query-execution filtering: -// - searching on title & url -// - parent folder (recursively) -// - excludeQueries -// - tags -// - limit count -// -// Note: changes to filtering in FilterResultSet() -// may require changes to NeedToFilterResultSet() +// This does some post-query-execution filtering: +// - searching on title & url +// - parent folder (recursively) +// - excludeQueries nsresult nsNavHistory::FilterResultSet(nsNavHistoryQueryResultNode* aQueryNode, const nsCOMArray& aSet, nsCOMArray* aFiltered, - const nsCOMArray& aQueries, - nsNavHistoryQueryOptions *aOptions) + const nsCOMArray& aQueries) { nsresult rv; @@ -4274,9 +4260,6 @@ nsNavHistory::FilterResultSet(nsNavHistoryQueryResultNode* aQueryNode, } if (appendNode) aFiltered->AppendObject(aSet[nodeIndex]); - - if (aOptions->MaxResults() > 0 && aFiltered->Count() >= aOptions->MaxResults()) - break; } // de-allocate the matrixes diff --git a/toolkit/components/places/src/nsNavHistory.h b/toolkit/components/places/src/nsNavHistory.h index d6f4fbdc6bc8..650b0c318028 100644 --- a/toolkit/components/places/src/nsNavHistory.h +++ b/toolkit/components/places/src/nsNavHistory.h @@ -516,8 +516,7 @@ protected: nsresult FilterResultSet(nsNavHistoryQueryResultNode *aParentNode, const nsCOMArray& aSet, nsCOMArray* aFiltered, - const nsCOMArray& aQueries, - nsNavHistoryQueryOptions* aOptions); + const nsCOMArray& aQueries); // observers nsMaybeWeakPtrArray mObservers;