diff --git a/toolkit/components/places/nsNavHistory.cpp b/toolkit/components/places/nsNavHistory.cpp index 095a0c6bcfab..3e66a3d5d9ce 100644 --- a/toolkit/components/places/nsNavHistory.cpp +++ b/toolkit/components/places/nsNavHistory.cpp @@ -193,10 +193,10 @@ NS_IMPL_CI_INTERFACE_GETTER(nsNavHistory, namespace { -static int64_t GetSimpleBookmarksQueryFolder(nsNavHistoryQuery* aQuery, - nsNavHistoryQueryOptions* aOptions); -static void ParseSearchTermsFromQueries(const nsCOMArray& aQueries, - nsTArray*>* aTerms); +static int64_t GetSimpleBookmarksQueryFolder(const RefPtr& aQuery, + const RefPtr& aOptions); +static void ParseSearchTermsFromQuery(const RefPtr& aQuery, + nsTArray* aTerms); void GetTagsSqlFragment(int64_t aTagsFolder, const nsACString& aRelation, @@ -790,47 +790,36 @@ nsNavHistory::NormalizeTime(uint32_t aRelative, PRTime aOffset) // change operations as simple instead of complex. uint32_t -nsNavHistory::GetUpdateRequirements(const nsCOMArray& aQueries, +nsNavHistory::GetUpdateRequirements(const RefPtr& aQuery, nsNavHistoryQueryOptions* aOptions, bool* aHasSearchTerms) { - NS_ASSERTION(aQueries.Count() > 0, "Must have at least one query"); - // first check if there are search terms *aHasSearchTerms = false; - int32_t i; - for (i = 0; i < aQueries.Count(); i ++) { - aQueries[i]->GetHasSearchTerms(aHasSearchTerms); - if (*aHasSearchTerms) - break; - } + aQuery->GetHasSearchTerms(aHasSearchTerms); bool nonTimeBasedItems = false; bool domainBasedItems = false; - for (i = 0; i < aQueries.Count(); i ++) { - nsNavHistoryQuery* query = aQueries[i]; - - bool hasSearchTerms = !query->SearchTerms().IsEmpty(); - if (query->Folders().Length() > 0 || - query->OnlyBookmarked() || - query->Tags().Length() > 0 || - (aOptions->QueryType() == nsINavHistoryQueryOptions::QUERY_TYPE_BOOKMARKS && - hasSearchTerms)) { - return QUERYUPDATE_COMPLEX_WITH_BOOKMARKS; - } - - // 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 (hasSearchTerms || - !query->Domain().IsVoid() || - query->Uri() != nullptr) - nonTimeBasedItems = true; - - if (!query->Domain().IsVoid()) - domainBasedItems = true; + bool hasSearchTerms = !aQuery->SearchTerms().IsEmpty(); + if (aQuery->Folders().Length() > 0 || + aQuery->OnlyBookmarked() || + aQuery->Tags().Length() > 0 || + (aOptions->QueryType() == nsINavHistoryQueryOptions::QUERY_TYPE_BOOKMARKS && + hasSearchTerms)) { + return QUERYUPDATE_COMPLEX_WITH_BOOKMARKS; } + // 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 (hasSearchTerms || + !aQuery->Domain().IsVoid() || + aQuery->Uri() != nullptr) + nonTimeBasedItems = true; + + if (!aQuery->Domain().IsVoid()) + domainBasedItems = true; + if (aOptions->ResultType() == nsINavHistoryQueryOptions::RESULTS_AS_TAG_QUERY) return QUERYUPDATE_COMPLEX_WITH_BOOKMARKS; @@ -850,9 +839,9 @@ nsNavHistory::GetUpdateRequirements(const nsCOMArray& aQuerie if (aOptions->MaxResults() > 0) return QUERYUPDATE_COMPLEX; - if (aQueries.Count() == 1 && domainBasedItems) + if (domainBasedItems) return QUERYUPDATE_HOST; - if (aQueries.Count() == 1 && !nonTimeBasedItems) + if (!nonTimeBasedItems) return QUERYUPDATE_TIME; return QUERYUPDATE_SIMPLE; @@ -861,7 +850,7 @@ nsNavHistory::GetUpdateRequirements(const nsCOMArray& aQuerie // nsNavHistory::EvaluateQueryForNode // -// This runs the node through the given queries to see if satisfies the +// This runs the node through the given query to see if satisfies the // query conditions. Not every query parameters are handled by this code, // but we handle the most common ones so that performance is better. // @@ -874,7 +863,7 @@ nsNavHistory::GetUpdateRequirements(const nsCOMArray& aQuerie // Returns true if node matches the query, false if not. bool -nsNavHistory::EvaluateQueryForNode(const nsCOMArray& aQueries, +nsNavHistory::EvaluateQueryForNode(const RefPtr& aQuery, nsNavHistoryQueryOptions* aOptions, nsNavHistoryResultNode* aNode) { @@ -885,101 +874,91 @@ nsNavHistory::EvaluateQueryForNode(const nsCOMArray& aQueries if (aNode->mHidden && !aOptions->IncludeHidden()) return false; - for (int32_t i = 0; i < aQueries.Count(); i ++) { - bool hasIt; - nsCOMPtr query = aQueries[i]; - - // --- begin time --- - query->GetHasBeginTime(&hasIt); - if (hasIt) { - PRTime beginTime = NormalizeTime(query->BeginTimeReference(), - query->BeginTime()); - if (aNode->mTime < beginTime) - continue; // before our time range - } - - // --- end time --- - query->GetHasEndTime(&hasIt); - if (hasIt) { - PRTime endTime = NormalizeTime(query->EndTimeReference(), - query->EndTime()); - if (aNode->mTime > endTime) - continue; // after our time range - } - - // --- search terms --- - if (! query->SearchTerms().IsEmpty()) { - // we can use the existing filtering code, just give it our one object in - // an array. - nsCOMArray inputSet; - inputSet.AppendObject(aNode); - nsCOMArray queries; - queries.AppendObject(query); - nsCOMArray filteredSet; - nsresult rv = FilterResultSet(nullptr, inputSet, &filteredSet, queries, aOptions); - if (NS_FAILED(rv)) - continue; - if (! filteredSet.Count()) - continue; // did not make it through the filter, doesn't match - } - - // --- domain/host matching --- - query->GetHasDomain(&hasIt); - if (hasIt) { - if (! nodeUri) { - // lazy creation of nodeUri, which might be checked for multiple queries - if (NS_FAILED(NS_NewURI(getter_AddRefs(nodeUri), aNode->mURI))) - continue; - } - nsAutoCString asciiRequest; - if (NS_FAILED(AsciiHostNameFromHostString(query->Domain(), asciiRequest))) - continue; - - if (query->DomainIsHost()) { - nsAutoCString host; - if (NS_FAILED(nodeUri->GetAsciiHost(host))) - continue; - - if (! asciiRequest.Equals(host)) - continue; // host names don't match - } - // check domain names - nsAutoCString domain; - DomainNameFromURI(nodeUri, domain); - if (! asciiRequest.Equals(domain)) - continue; // domain names don't match - } - - // --- URI matching --- - if (query->Uri()) { - if (! nodeUri) { // lazy creation of nodeUri - if (NS_FAILED(NS_NewURI(getter_AddRefs(nodeUri), aNode->mURI))) - continue; - } - - bool equals; - nsresult rv = query->Uri()->Equals(nodeUri, &equals); - NS_ENSURE_SUCCESS(rv, false); - if (! equals) - continue; - } - - // Transitions matching. - const nsTArray& 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; + bool hasIt; + // --- begin time --- + aQuery->GetHasBeginTime(&hasIt); + if (hasIt) { + PRTime beginTime = NormalizeTime(aQuery->BeginTimeReference(), + aQuery->BeginTime()); + if (aNode->mTime < beginTime) + return false; } - // didn't match any query - return false; + // --- end time --- + aQuery->GetHasEndTime(&hasIt); + if (hasIt) { + PRTime endTime = NormalizeTime(aQuery->EndTimeReference(), + aQuery->EndTime()); + if (aNode->mTime > endTime) + return false; + } + + // --- search terms --- + if (!aQuery->SearchTerms().IsEmpty()) { + // we can use the existing filtering code, just give it our one object in + // an array. + nsCOMArray inputSet; + inputSet.AppendObject(aNode); + nsCOMArray filteredSet; + nsresult rv = FilterResultSet(nullptr, inputSet, &filteredSet, aQuery, aOptions); + if (NS_FAILED(rv)) + return false; + if (!filteredSet.Count()) + return false; + } + + // --- domain/host matching --- + aQuery->GetHasDomain(&hasIt); + if (hasIt) { + if (!nodeUri) { + // lazy creation of nodeUri, which might be checked for multiple queries + if (NS_FAILED(NS_NewURI(getter_AddRefs(nodeUri), aNode->mURI))) + return false; + } + nsAutoCString asciiRequest; + if (NS_FAILED(AsciiHostNameFromHostString(aQuery->Domain(), asciiRequest))) + return false; + + if (aQuery->DomainIsHost()) { + nsAutoCString host; + if (NS_FAILED(nodeUri->GetAsciiHost(host))) + return false; + + if (!asciiRequest.Equals(host)) + return false; + } + // check domain names + nsAutoCString domain; + DomainNameFromURI(nodeUri, domain); + if (!asciiRequest.Equals(domain)) + return false; + } + + // --- URI matching --- + if (aQuery->Uri()) { + if (!nodeUri) { // lazy creation of nodeUri + if (NS_FAILED(NS_NewURI(getter_AddRefs(nodeUri), aNode->mURI))) + return false; + } + + bool equals; + nsresult rv = aQuery->Uri()->Equals(nodeUri, &equals); + NS_ENSURE_SUCCESS(rv, false); + if (!equals) + return false; + } + + // Transitions matching. + const nsTArray& transitions = aQuery->Transitions(); + if (aNode->mTransitionType > 0 && + transitions.Length() && + !transitions.Contains(aNode->mTransitionType)) { + return false; + } + + // If we ever make it to the bottom, that means it passed all the tests for + // the given query. + return true; } @@ -1244,16 +1223,18 @@ nsNavHistory::ExecuteQuery(nsINavHistoryQuery *aQuery, NS_ENSURE_ARG(aOptions); NS_ENSURE_ARG_POINTER(_retval); - nsresult rv; - // concrete options - nsCOMPtr options = do_QueryInterface(aOptions); - NS_ENSURE_TRUE(options, NS_ERROR_INVALID_ARG); - - // concrete queries array - nsCOMArray queries; - RefPtr query = do_QueryObject(aQuery); + // Clone the input query and options, because the caller might change the + // objects, but we always want to reflect the original parameters. + nsCOMPtr queryClone; + aQuery->Clone(getter_AddRefs(queryClone)); + NS_ENSURE_STATE(queryClone); + RefPtr query = do_QueryObject(queryClone); NS_ENSURE_STATE(query); - queries.AppendObject(query); + nsCOMPtr optionsClone; + aOptions->Clone(getter_AddRefs(optionsClone)); + NS_ENSURE_STATE(optionsClone); + RefPtr options = do_QueryObject(optionsClone); + NS_ENSURE_STATE(options); // Create the root node. RefPtr rootNode; @@ -1265,8 +1246,8 @@ nsNavHistory::ExecuteQuery(nsINavHistoryQuery *aQuery, nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService(); NS_ENSURE_TRUE(bookmarks, NS_ERROR_OUT_OF_MEMORY); RefPtr tempRootNode; - rv = bookmarks->ResultNodeForContainer(folderId, options, - getter_AddRefs(tempRootNode)); + nsresult rv = bookmarks->ResultNodeForContainer(folderId, options, + getter_AddRefs(tempRootNode)); if (NS_SUCCEEDED(rv)) { rootNode = tempRootNode->GetAsContainer(); } @@ -1281,16 +1262,13 @@ nsNavHistory::ExecuteQuery(nsINavHistoryQuery *aQuery, // Either this is not a folder shortcut, or is a broken one. In both cases // just generate a query node. rootNode = new nsNavHistoryQueryResultNode(EmptyCString(), - queries, options); + query, options); } // Create the result that will hold nodes. Inject batching status into it. - RefPtr result; - rv = nsNavHistoryResult::NewHistoryResult(queries, 1, options, - rootNode, isBatching(), - getter_AddRefs(result)); - NS_ENSURE_SUCCESS(rv, rv); - + RefPtr result = new nsNavHistoryResult(rootNode, + query, options, + isBatching()); result.forget(_retval); return NS_OK; } @@ -1304,15 +1282,10 @@ nsNavHistory::ExecuteQuery(nsINavHistoryQuery *aQuery, // "Smart Bookmarks" folder: place:sort=8&maxResults=10 // note, any maxResult > 0 will still be considered a Most Visited menu query static -bool IsOptimizableHistoryQuery(const nsCOMArray& aQueries, - nsNavHistoryQueryOptions *aOptions, - uint16_t aSortMode) +bool IsOptimizableHistoryQuery(const RefPtr& aQuery, + const RefPtr& aOptions, + uint16_t aSortMode) { - if (aQueries.Count() != 1) - return false; - - nsNavHistoryQuery *aQuery = aQueries[0]; - if (aOptions->QueryType() != nsINavHistoryQueryOptions::QUERY_TYPE_HISTORY) return false; @@ -1365,8 +1338,8 @@ bool IsOptimizableHistoryQuery(const nsCOMArray& aQueries, } static -bool NeedToFilterResultSet(const nsCOMArray& aQueries, - nsNavHistoryQueryOptions *aOptions) +bool NeedToFilterResultSet(const RefPtr& aQuery, + nsNavHistoryQueryOptions *aOptions) { uint16_t resultType = aOptions->ResultType(); return resultType == nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS || @@ -2185,8 +2158,8 @@ PlacesSQLQueryBuilder::Limit() nsresult nsNavHistory::ConstructQueryString( - const nsCOMArray& aQueries, - nsNavHistoryQueryOptions* aOptions, + const RefPtr& aQuery, + const RefPtr& aOptions, nsCString& queryString, bool& aParamsPresent, nsNavHistory::StringHash& aAddParams) @@ -2205,9 +2178,7 @@ nsNavHistory::ConstructQueryString( "Invalid sortingMode found while building query!"); bool hasSearchTerms = false; - for (int32_t i = 0; i < aQueries.Count() && !hasSearchTerms; i++) { - aQueries[i]->GetHasSearchTerms(&hasSearchTerms); - } + aQuery->GetHasSearchTerms(&hasSearchTerms); nsAutoCString tagsSqlFragment; GetTagsSqlFragment(GetTagsFolder(), @@ -2215,9 +2186,9 @@ nsNavHistory::ConstructQueryString( hasSearchTerms, tagsSqlFragment); - if (IsOptimizableHistoryQuery(aQueries, aOptions, + if (IsOptimizableHistoryQuery(aQuery, aOptions, nsINavHistoryQueryOptions::SORT_BY_DATE_DESCENDING) || - IsOptimizableHistoryQuery(aQueries, aOptions, + IsOptimizableHistoryQuery(aQuery, aOptions, nsINavHistoryQueryOptions::SORT_BY_VISITCOUNT_DESCENDING)) { // Generate an optimized query for the history menu and most visited // smart bookmark. @@ -2254,23 +2225,19 @@ nsNavHistory::ConstructQueryString( } nsAutoCString conditions; - for (int32_t i = 0; i < aQueries.Count(); i++) { - nsCString queryClause; - rv = QueryToSelectClause(aQueries[i], aOptions, i, &queryClause); - NS_ENSURE_SUCCESS(rv, rv); - if (! queryClause.IsEmpty()) { - aParamsPresent = true; - if (! conditions.IsEmpty()) // exists previous clause: multiple ones are ORed - conditions += NS_LITERAL_CSTRING(" OR "); - conditions += NS_LITERAL_CSTRING("(") + queryClause + - NS_LITERAL_CSTRING(")"); - } + nsCString queryClause; + rv = QueryToSelectClause(aQuery, aOptions, &queryClause); + NS_ENSURE_SUCCESS(rv, rv); + if (!queryClause.IsEmpty()) { + // TODO: This should be set on a case basis, not blindly. + aParamsPresent = true; + conditions += queryClause; } - // Determine whether we can push maxResults constraints into the queries + // Determine whether we can push maxResults constraints into the query // as LIMIT, or if we need to do result count clamping later // using FilterResultSet() - bool useLimitClause = !NeedToFilterResultSet(aQueries, aOptions); + bool useLimitClause = !NeedToFilterResultSet(aQuery, aOptions); PlacesSQLQueryBuilder queryStringBuilder(conditions, aOptions, useLimitClause, aAddParams, @@ -2297,19 +2264,19 @@ nsNavHistory::ConstructQueryString( nsresult nsNavHistory::GetQueryResults(nsNavHistoryQueryResultNode *aResultNode, - const nsCOMArray& aQueries, - nsNavHistoryQueryOptions *aOptions, + const RefPtr& aQuery, + const RefPtr& aOptions, nsCOMArray* aResults) { + NS_ENSURE_ARG_POINTER(aQuery); NS_ENSURE_ARG_POINTER(aOptions); NS_ASSERTION(aResults->Count() == 0, "Initial result array must be empty"); - if (! aQueries.Count()) - return NS_ERROR_INVALID_ARG; + nsCString queryString; bool paramsPresent = false; nsNavHistory::StringHash addParams(HISTORY_DATE_CONT_LENGTH); - nsresult rv = ConstructQueryString(aQueries, aOptions, queryString, + nsresult rv = ConstructQueryString(aQuery, aOptions, queryString, paramsPresent, addParams); NS_ENSURE_SUCCESS(rv,rv); @@ -2332,12 +2299,8 @@ nsNavHistory::GetQueryResults(nsNavHistoryQueryResultNode *aResultNode, mozStorageStatementScoper scoper(statement); if (paramsPresent) { - // bind parameters - int32_t i; - for (i = 0; i < aQueries.Count(); i++) { - rv = BindQueryClauseParameters(statement, i, aQueries[i], aOptions); - NS_ENSURE_SUCCESS(rv, rv); - } + rv = BindQueryClauseParameters(statement, aQuery, aOptions); + NS_ENSURE_SUCCESS(rv, rv); } for (auto iter = addParams.Iter(); !iter.Done(); iter.Next()) { @@ -2348,13 +2311,13 @@ nsNavHistory::GetQueryResults(nsNavHistoryQueryResultNode *aResultNode, } // Optimize the case where there is no need for any post-query filtering. - if (NeedToFilterResultSet(aQueries, aOptions)) { + if (NeedToFilterResultSet(aQuery, aOptions)) { // Generate the top-level results. nsCOMArray toplevel; rv = ResultsAsList(statement, aOptions, &toplevel); NS_ENSURE_SUCCESS(rv, rv); - FilterResultSet(aResultNode, toplevel, aResults, aQueries, aOptions); + FilterResultSet(aResultNode, toplevel, aResults, aQuery, aOptions); } else { rv = ResultsAsList(statement, aOptions, aResults); NS_ENSURE_SUCCESS(rv, rv); @@ -2904,18 +2867,15 @@ nsNavHistory::AsyncExecuteLegacyQuery(nsINavHistoryQuery* aQuery, NS_ENSURE_ARG(aCallback); NS_ENSURE_ARG_POINTER(_stmt); - nsCOMPtr query = do_QueryObject(aQuery); + RefPtr query = do_QueryObject(aQuery); NS_ENSURE_STATE(query); - nsCOMArray queries; - queries.AppendObject(query); - - nsCOMPtr options = do_QueryInterface(aOptions); + RefPtr options = do_QueryObject(aOptions); NS_ENSURE_ARG(options); nsCString queryString; bool paramsPresent = false; nsNavHistory::StringHash addParams(HISTORY_DATE_CONT_LENGTH); - nsresult rv = ConstructQueryString(queries, options, queryString, + nsresult rv = ConstructQueryString(query, options, queryString, paramsPresent, addParams); NS_ENSURE_SUCCESS(rv,rv); @@ -2939,12 +2899,8 @@ nsNavHistory::AsyncExecuteLegacyQuery(nsINavHistoryQuery* aQuery, NS_ENSURE_SUCCESS(rv, rv); if (paramsPresent) { - // bind parameters - int32_t i; - for (i = 0; i < queries.Count(); i++) { - rv = BindQueryClauseParameters(statement, i, queries[i], options); - NS_ENSURE_SUCCESS(rv, rv); - } + rv = BindQueryClauseParameters(statement, query, options); + NS_ENSURE_SUCCESS(rv, rv); } for (auto iter = addParams.Iter(); !iter.Done(); iter.Next()) { @@ -3146,18 +3102,11 @@ nsNavHistory::DecayFrecency() // Helper class for QueryToSelectClause // -// This class helps to build part of the WHERE clause. It supports -// multiple queries by appending the query index to the parameter name. -// For the query with index 0 the parameter name is not altered what -// allows using this parameter in other situations (see SelectAsSite). +// This class helps to build part of the WHERE clause. class ConditionBuilder { public: - - explicit ConditionBuilder(int32_t aQueryIndex): mQueryIndex(aQueryIndex) - { } - ConditionBuilder& Condition(const char* aStr) { if (!mClause.IsEmpty()) @@ -3177,11 +3126,7 @@ public: ConditionBuilder& Param(const char* aParam) { mClause.Append(' '); - if (!mQueryIndex) - mClause.Append(aParam); - else - mClause += nsPrintfCString("%s%d", aParam, mQueryIndex); - + mClause.Append(aParam); mClause.Append(' '); return *this; } @@ -3193,7 +3138,6 @@ public: private: - int32_t mQueryIndex; nsCString mClause; }; @@ -3206,9 +3150,8 @@ private: // no way for those to fail. nsresult -nsNavHistory::QueryToSelectClause(nsNavHistoryQuery* aQuery, // const - nsNavHistoryQueryOptions* aOptions, - int32_t aQueryIndex, +nsNavHistory::QueryToSelectClause(const RefPtr& aQuery, + const RefPtr& aOptions, nsCString* aClause) { bool hasIt; @@ -3216,7 +3159,7 @@ nsNavHistory::QueryToSelectClause(nsNavHistoryQuery* aQuery, // const // is set. bool excludeQueries = false; - ConditionBuilder clause(aQueryIndex); + ConditionBuilder clause; if ((NS_SUCCEEDED(aQuery->GetHasBeginTime(&hasIt)) && hasIt) || (NS_SUCCEEDED(aQuery->GetHasEndTime(&hasIt)) && hasIt)) { @@ -3379,26 +3322,17 @@ nsNavHistory::QueryToSelectClause(nsNavHistoryQuery* aQuery, // const nsresult nsNavHistory::BindQueryClauseParameters(mozIStorageBaseStatement* statement, - int32_t aQueryIndex, - nsNavHistoryQuery* aQuery, // const - nsNavHistoryQueryOptions* aOptions) + const RefPtr& aQuery, + const RefPtr& aOptions) { nsresult rv; bool hasIt; - // Append numbered index to param names, to replace them correctly in - // case of multiple queries. If we have just one query we don't change the - // param name though. - nsAutoCString qIndex; - if (aQueryIndex > 0) - qIndex.AppendInt(aQueryIndex); - // begin time if (NS_SUCCEEDED(aQuery->GetHasBeginTime(&hasIt)) && hasIt) { PRTime time = NormalizeTime(aQuery->BeginTimeReference(), aQuery->BeginTime()); - rv = statement->BindInt64ByName( - NS_LITERAL_CSTRING("begin_time") + qIndex, time); + rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("begin_time"), time); NS_ENSURE_SUCCESS(rv, rv); } @@ -3406,35 +3340,27 @@ nsNavHistory::BindQueryClauseParameters(mozIStorageBaseStatement* statement, if (NS_SUCCEEDED(aQuery->GetHasEndTime(&hasIt)) && hasIt) { PRTime time = NormalizeTime(aQuery->EndTimeReference(), aQuery->EndTime()); - rv = statement->BindInt64ByName( - NS_LITERAL_CSTRING("end_time") + qIndex, time - ); + rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("end_time"), time); NS_ENSURE_SUCCESS(rv, rv); } // search terms if (NS_SUCCEEDED(aQuery->GetHasSearchTerms(&hasIt)) && hasIt) { - rv = statement->BindStringByName( - NS_LITERAL_CSTRING("search_string") + qIndex, - aQuery->SearchTerms() - ); + rv = statement->BindStringByName(NS_LITERAL_CSTRING("search_string"), + aQuery->SearchTerms()); NS_ENSURE_SUCCESS(rv, rv); } // min and max visit count int32_t visits = aQuery->MinVisits(); if (visits >= 0) { - rv = statement->BindInt32ByName( - NS_LITERAL_CSTRING("min_visits") + qIndex, visits - ); + rv = statement->BindInt32ByName(NS_LITERAL_CSTRING("min_visits"), visits); NS_ENSURE_SUCCESS(rv, rv); } visits = aQuery->MaxVisits(); if (visits >= 0) { - rv = statement->BindInt32ByName( - NS_LITERAL_CSTRING("max_visits") + qIndex, visits - ); + rv = statement->BindInt32ByName(NS_LITERAL_CSTRING("max_visits"), visits); NS_ENSURE_SUCCESS(rv, rv); } @@ -3444,41 +3370,35 @@ nsNavHistory::BindQueryClauseParameters(mozIStorageBaseStatement* statement, GetReversedHostname(NS_ConvertUTF8toUTF16(aQuery->Domain()), revDomain); if (aQuery->DomainIsHost()) { - rv = statement->BindStringByName( - NS_LITERAL_CSTRING("domain_lower") + qIndex, revDomain - ); + rv = statement->BindStringByName(NS_LITERAL_CSTRING("domain_lower"), + revDomain); NS_ENSURE_SUCCESS(rv, rv); } else { // for "mozilla.org" do query >= "gro.allizom." AND < "gro.allizom/" // which will get everything starting with "gro.allizom." while using the // index (using SUBSTRING() causes indexes to be discarded). NS_ASSERTION(revDomain[revDomain.Length() - 1] == '.', "Invalid rev. host"); - rv = statement->BindStringByName( - NS_LITERAL_CSTRING("domain_lower") + qIndex, revDomain - ); + rv = statement->BindStringByName(NS_LITERAL_CSTRING("domain_lower"), + revDomain); NS_ENSURE_SUCCESS(rv, rv); revDomain.Truncate(revDomain.Length() - 1); revDomain.Append(char16_t('/')); - rv = statement->BindStringByName( - NS_LITERAL_CSTRING("domain_upper") + qIndex, revDomain - ); + rv = statement->BindStringByName(NS_LITERAL_CSTRING("domain_upper"), + revDomain); NS_ENSURE_SUCCESS(rv, rv); } } // URI if (aQuery->Uri()) { - rv = URIBinder::Bind( - statement, NS_LITERAL_CSTRING("uri") + qIndex, aQuery->Uri() - ); + rv = URIBinder::Bind(statement, NS_LITERAL_CSTRING("uri"), aQuery->Uri()); NS_ENSURE_SUCCESS(rv, rv); } // annotation if (!aQuery->Annotation().IsEmpty()) { - rv = statement->BindUTF8StringByName( - NS_LITERAL_CSTRING("anno") + qIndex, aQuery->Annotation() - ); + rv = statement->BindUTF8StringByName(NS_LITERAL_CSTRING("anno"), + aQuery->Annotation()); NS_ENSURE_SUCCESS(rv, rv); } @@ -3489,30 +3409,26 @@ nsNavHistory::BindQueryClauseParameters(mozIStorageBaseStatement* statement, nsPrintfCString paramName("tag%d_", i); NS_ConvertUTF16toUTF8 tag(tags[i]); ToLowerCase(tag); - rv = statement->BindUTF8StringByName(paramName + qIndex, tag); + rv = statement->BindUTF8StringByName(paramName, tag); NS_ENSURE_SUCCESS(rv, rv); } int64_t tagsFolder = GetTagsFolder(); - rv = statement->BindInt64ByName( - NS_LITERAL_CSTRING("tags_folder") + qIndex, tagsFolder - ); + rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("tags_folder"), + tagsFolder); NS_ENSURE_SUCCESS(rv, rv); if (!aQuery->TagsAreNot()) { - rv = statement->BindInt32ByName( - NS_LITERAL_CSTRING("tag_count") + qIndex, tags.Length() - ); + rv = statement->BindInt32ByName(NS_LITERAL_CSTRING("tag_count"), + tags.Length()); NS_ENSURE_SUCCESS(rv, rv); } } // transitions const nsTArray& transitions = aQuery->Transitions(); - if (transitions.Length() > 0) { - for (uint32_t i = 0; i < transitions.Length(); ++i) { - nsPrintfCString paramName("transition%d_", i); - rv = statement->BindInt64ByName(paramName + qIndex, transitions[i]); - NS_ENSURE_SUCCESS(rv, rv); - } + for (uint32_t i = 0; i < transitions.Length(); ++i) { + nsPrintfCString paramName("transition%d_", i); + rv = statement->BindInt64ByName(paramName, transitions[i]); + NS_ENSURE_SUCCESS(rv, rv); } return NS_OK; @@ -3601,7 +3517,7 @@ nsresult nsNavHistory::FilterResultSet(nsNavHistoryQueryResultNode* aQueryNode, const nsCOMArray& aSet, nsCOMArray* aFiltered, - const nsCOMArray& aQueries, + const RefPtr& aQuery, nsNavHistoryQueryOptions *aOptions) { // get the bookmarks service @@ -3609,8 +3525,8 @@ nsNavHistory::FilterResultSet(nsNavHistoryQueryResultNode* aQueryNode, NS_ENSURE_TRUE(bookmarks, NS_ERROR_OUT_OF_MEMORY); // parse the search terms - nsTArray*> terms; - ParseSearchTermsFromQueries(aQueries, &terms); + nsTArray terms; + ParseSearchTermsFromQuery(aQuery, &terms); uint16_t resultType = aOptions->ResultType(); bool excludeQueries = aOptions->ExcludeQueries(); @@ -3636,43 +3552,33 @@ nsNavHistory::FilterResultSet(nsNavHistoryQueryResultNode* aQueryNode, // If there are search terms, we are already getting only uri nodes, // thus we don't need to filter node types. Though, we must check for // matching terms. - bool appendNode = false; - for (int32_t queryIndex = 0; - queryIndex < aQueries.Count() && !appendNode; queryIndex++) { + if (terms.Length()) { + // Filter based on search terms. + // Convert title and url for the current node to UTF16 strings. + NS_ConvertUTF8toUTF16 nodeTitle(aSet[nodeIndex]->mTitle); + // Unescape the URL for search terms matching. + nsAutoCString cNodeURL(aSet[nodeIndex]->mURI); + NS_ConvertUTF8toUTF16 nodeURL(NS_UnescapeURL(cNodeURL)); - if (terms[queryIndex]->Length()) { - // Filter based on search terms. - // Convert title and url for the current node to UTF16 strings. - NS_ConvertUTF8toUTF16 nodeTitle(aSet[nodeIndex]->mTitle); - // Unescape the URL for search terms matching. - nsAutoCString cNodeURL(aSet[nodeIndex]->mURI); - NS_ConvertUTF8toUTF16 nodeURL(NS_UnescapeURL(cNodeURL)); - - // Determine if every search term matches anywhere in the title, url or - // tag. - bool matchAll = true; - for (int32_t termIndex = terms[queryIndex]->Length() - 1; - termIndex >= 0 && matchAll; - termIndex--) { - nsString& term = terms[queryIndex]->ElementAt(termIndex); - - // True if any of them match; false makes us quit the loop - matchAll = CaseInsensitiveFindInReadable(term, nodeTitle) || - CaseInsensitiveFindInReadable(term, nodeURL) || - CaseInsensitiveFindInReadable(term, aSet[nodeIndex]->mTags); - } - - // Skip the node if we don't match all terms in the title, url or tag - if (!matchAll) - continue; + // Determine if every search term matches anywhere in the title, url or + // tag. + bool matchAllTerms = true; + for (int32_t termIndex = terms.Length() - 1; + termIndex >= 0 && matchAllTerms; + termIndex--) { + nsString& term = terms.ElementAt(termIndex); + // True if any of them match; false makes us quit the loop + matchAllTerms = CaseInsensitiveFindInReadable(term, nodeTitle) || + CaseInsensitiveFindInReadable(term, nodeURL) || + CaseInsensitiveFindInReadable(term, aSet[nodeIndex]->mTags); + } + // Skip the node if we don't match all terms in the title, url or tag + if (!matchAllTerms) { + continue; } - - // We passed all filters, so we can append the node to filtered results. - appendNode = true; } - if (appendNode) - aFiltered->AppendObject(aSet[nodeIndex]); + aFiltered->AppendObject(aSet[nodeIndex]); // Stop once we have reached max results. if (aOptions->MaxResults() > 0 && @@ -3680,11 +3586,6 @@ nsNavHistory::FilterResultSet(nsNavHistoryQueryResultNode* aQueryNode, break; } - // De-allocate the temporary matrixes. - for (int32_t i = 0; i < aQueries.Count(); i++) { - delete terms[i]; - } - return NS_OK; } @@ -3997,9 +3898,7 @@ nsNavHistory::QueryRowToResult(int64_t itemId, } else { // This is a regular query. - nsCOMArray queries; - queries.AppendObject(queryObj); - resultNode = new nsNavHistoryQueryResultNode(aTitle, aTime, queries, optionsObj); + resultNode = new nsNavHistoryQueryResultNode(aTitle, aTime, queryObj, optionsObj); resultNode->mItemId = itemId; resultNode->mBookmarkGuid = aBookmarkGuid; } @@ -4280,8 +4179,8 @@ namespace { // // Returns the folder ID if it is a simple folder query, 0 if not. static int64_t -GetSimpleBookmarksQueryFolder(nsNavHistoryQuery* aQuery, - nsNavHistoryQueryOptions* aOptions) +GetSimpleBookmarksQueryFolder(const RefPtr& aQuery, + const RefPtr& aOptions) { if (aQuery->Folders().Length() != 1) return 0; @@ -4309,7 +4208,7 @@ GetSimpleBookmarksQueryFolder(nsNavHistoryQuery* aQuery, // RESULTS_AS_TAG_CONTENTS is quite similar to a folder shortcut, but it must // not be treated like that, since it needs all query options. - if(aOptions->ResultType() == nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS) + if (aOptions->ResultType() == nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS) return 0; // Don't care about onlyBookmarked flag, since specifying a bookmark @@ -4319,11 +4218,10 @@ GetSimpleBookmarksQueryFolder(nsNavHistoryQuery* aQuery, } -// ParseSearchTermsFromQueries +// ParseSearchTermsFromQuery // -// Construct a matrix of search terms from the given queries array. -// All of the query objects are ORed together. Within a query, all the terms -// are ANDed together. See nsINavHistoryService.idl. +// Construct an array of search terms from the given query. +// Within a query, all the terms are ANDed together. // // This just breaks the query up into words. We don't do anything fancy, // not even quoting. We do, however, strip quotes, because people might @@ -4335,37 +4233,32 @@ inline bool isQueryWhitespace(char16_t ch) return ch == ' '; } -void ParseSearchTermsFromQueries(const nsCOMArray& aQueries, - nsTArray*>* aTerms) +void ParseSearchTermsFromQuery(const RefPtr& aQuery, + nsTArray* aTerms) { int32_t lastBegin = -1; - for (int32_t i = 0; i < aQueries.Count(); i++) { - nsTArray *queryTerms = new nsTArray(); - bool hasSearchTerms; - if (NS_SUCCEEDED(aQueries[i]->GetHasSearchTerms(&hasSearchTerms)) && - hasSearchTerms) { - const nsString& searchTerms = aQueries[i]->SearchTerms(); - for (uint32_t j = 0; j < searchTerms.Length(); j++) { - if (isQueryWhitespace(searchTerms[j]) || - searchTerms[j] == '"') { - if (lastBegin >= 0) { - // found the end of a word - queryTerms->AppendElement(Substring(searchTerms, lastBegin, - j - lastBegin)); - lastBegin = -1; - } - } else { - if (lastBegin < 0) { - // found the beginning of a word - lastBegin = j; - } + bool hasSearchTerms; + if (NS_SUCCEEDED(aQuery->GetHasSearchTerms(&hasSearchTerms)) && + hasSearchTerms) { + const nsString& searchTerms = aQuery->SearchTerms(); + for (uint32_t j = 0; j < searchTerms.Length(); j++) { + if (isQueryWhitespace(searchTerms[j]) || + searchTerms[j] == '"') { + if (lastBegin >= 0) { + // found the end of a word + aTerms->AppendElement(Substring(searchTerms, lastBegin, j - lastBegin)); + lastBegin = -1; + } + } else { + if (lastBegin < 0) { + // found the beginning of a word + lastBegin = j; } } - // last word - if (lastBegin >= 0) - queryTerms->AppendElement(Substring(searchTerms, lastBegin)); } - aTerms->AppendElement(queryTerms); + // last word + if (lastBegin >= 0) + aTerms->AppendElement(Substring(searchTerms, lastBegin)); } } diff --git a/toolkit/components/places/nsNavHistory.h b/toolkit/components/places/nsNavHistory.h index b0a29c92b10e..e941ab9c09d5 100644 --- a/toolkit/components/places/nsNavHistory.h +++ b/toolkit/components/places/nsNavHistory.h @@ -258,8 +258,8 @@ public: // this actually executes a query and gives you results, it is used by // nsNavHistoryQueryResultNode nsresult GetQueryResults(nsNavHistoryQueryResultNode *aResultNode, - const nsCOMArray& aQueries, - nsNavHistoryQueryOptions *aOptions, + const RefPtr& aQuery, + const RefPtr& aOptions, nsCOMArray* aResults); // Take a row of kGetInfoIndex_* columns and construct a ResultNode. @@ -298,10 +298,10 @@ public: int32_t GetDaysOfHistory(); // used by query result nodes to update: see comment on body of CanLiveUpdateQuery - static uint32_t GetUpdateRequirements(const nsCOMArray& aQueries, + static uint32_t GetUpdateRequirements(const RefPtr& aQuery, nsNavHistoryQueryOptions* aOptions, bool* aHasSearchTerms); - bool EvaluateQueryForNode(const nsCOMArray& aQueries, + bool EvaluateQueryForNode(const RefPtr& aQuery, nsNavHistoryQueryOptions* aOptions, nsNavHistoryResultNode* aNode); @@ -539,20 +539,18 @@ protected: */ static void expireNowTimerCallback(nsITimer* aTimer, void* aClosure); - nsresult ConstructQueryString(const nsCOMArray& aQueries, - nsNavHistoryQueryOptions* aOptions, + nsresult ConstructQueryString(const RefPtr& aQuery, + const RefPtr& aOptions, nsCString& queryString, bool& aParamsPresent, StringHash& aAddParams); - nsresult QueryToSelectClause(nsNavHistoryQuery* aQuery, - nsNavHistoryQueryOptions* aOptions, - int32_t aQueryIndex, + nsresult QueryToSelectClause(const RefPtr& aQuery, + const RefPtr& aOptions, nsCString* aClause); nsresult BindQueryClauseParameters(mozIStorageBaseStatement* statement, - int32_t aQueryIndex, - nsNavHistoryQuery* aQuery, - nsNavHistoryQueryOptions* aOptions); + const RefPtr& aQuery, + const RefPtr& aOptions); nsresult ResultsAsList(mozIStorageStatement* statement, nsNavHistoryQueryOptions* aOptions, @@ -563,7 +561,7 @@ protected: nsresult FilterResultSet(nsNavHistoryQueryResultNode *aParentNode, const nsCOMArray& aSet, nsCOMArray* aFiltered, - const nsCOMArray& aQueries, + const RefPtr& aQuery, nsNavHistoryQueryOptions* aOptions); // observers diff --git a/toolkit/components/places/nsNavHistoryQuery.cpp b/toolkit/components/places/nsNavHistoryQuery.cpp index a67b6c9f215f..018d98bd16fc 100644 --- a/toolkit/components/places/nsNavHistoryQuery.cpp +++ b/toolkit/components/places/nsNavHistoryQuery.cpp @@ -807,16 +807,17 @@ nsNavHistoryQuery::nsNavHistoryQuery() nsNavHistoryQuery::nsNavHistoryQuery(const nsNavHistoryQuery& aOther) : mMinVisits(aOther.mMinVisits), mMaxVisits(aOther.mMaxVisits), - mBeginTime(aOther.mBeginTime), - mBeginTimeReference(aOther.mBeginTimeReference), + mBeginTime(aOther.mBeginTime), mBeginTimeReference(aOther.mBeginTimeReference), mEndTime(aOther.mEndTime), mEndTimeReference(aOther.mEndTimeReference), mSearchTerms(aOther.mSearchTerms), mOnlyBookmarked(aOther.mOnlyBookmarked), mDomainIsHost(aOther.mDomainIsHost), mDomain(aOther.mDomain), - mUri(aOther.mUri), - mAnnotationIsNot(aOther.mAnnotationIsNot), - mAnnotation(aOther.mAnnotation), mTags(aOther.mTags), - mTagsAreNot(aOther.mTagsAreNot), mTransitions(aOther.mTransitions) -{} + mUri(aOther.mUri), mAnnotationIsNot(aOther.mAnnotationIsNot), + mAnnotation(aOther.mAnnotation), + mFolders(aOther.mFolders), + mTags(aOther.mTags), mTagsAreNot(aOther.mTagsAreNot), + mTransitions(aOther.mTransitions) +{ +} NS_IMETHODIMP nsNavHistoryQuery::GetBeginTime(PRTime *aBeginTime) { @@ -1214,21 +1215,56 @@ NS_IMETHODIMP nsNavHistoryQuery::SetTransitions(const uint32_t* aTransitions, return NS_OK; } -NS_IMETHODIMP nsNavHistoryQuery::Clone(nsINavHistoryQuery** _retval) +NS_IMETHODIMP +nsNavHistoryQuery::Clone(nsINavHistoryQuery** _clone) { - *_retval = nullptr; - - RefPtr clone = new nsNavHistoryQuery(*this); - NS_ENSURE_TRUE(clone, NS_ERROR_OUT_OF_MEMORY); - - clone.forget(_retval); + nsNavHistoryQuery *clone = nullptr; + Unused << Clone(&clone); + *_clone = clone; return NS_OK; } +nsresult +nsNavHistoryQuery::Clone(nsNavHistoryQuery** _clone) +{ + *_clone = nullptr; + RefPtr clone = new nsNavHistoryQuery(*this); + clone.forget(_clone); + return NS_OK; +} // nsNavHistoryQueryOptions NS_IMPL_ISUPPORTS(nsNavHistoryQueryOptions, nsNavHistoryQueryOptions, nsINavHistoryQueryOptions) +nsNavHistoryQueryOptions::nsNavHistoryQueryOptions() +: mSort(0) +, mResultType(0) +, mExcludeItems(false) +, mExcludeQueries(false) +, mExcludeReadOnlyFolders(false) +, mExpandQueries(true) +, mIncludeHidden(false) +, mMaxResults(0) +, mQueryType(nsINavHistoryQueryOptions::QUERY_TYPE_HISTORY) +, mAsyncEnabled(false) +{ +} + +nsNavHistoryQueryOptions::nsNavHistoryQueryOptions(const nsNavHistoryQueryOptions& other) +: mSort(other.mSort) +, mSortingAnnotation(other.mSortingAnnotation) +, mResultType(other.mResultType) +, mExcludeItems(other.mExcludeItems) +, mExcludeQueries(other.mExcludeQueries) +, mExcludeReadOnlyFolders(other.mExcludeReadOnlyFolders) +, mExpandQueries(other.mExpandQueries) +, mIncludeHidden(other.mIncludeHidden) +, mMaxResults(other.mMaxResults) +, mQueryType(other.mQueryType) +, mAsyncEnabled(other.mAsyncEnabled) +{ +} + // sortingMode NS_IMETHODIMP nsNavHistoryQueryOptions::GetSortingMode(uint16_t* aMode) @@ -1398,37 +1434,23 @@ nsNavHistoryQueryOptions::SetAsyncEnabled(bool aAsyncEnabled) return NS_OK; } - NS_IMETHODIMP -nsNavHistoryQueryOptions::Clone(nsINavHistoryQueryOptions** aResult) +nsNavHistoryQueryOptions::Clone(nsINavHistoryQueryOptions** _clone) { nsNavHistoryQueryOptions *clone = nullptr; - nsresult rv = Clone(&clone); - *aResult = clone; - return rv; -} - -nsresult -nsNavHistoryQueryOptions::Clone(nsNavHistoryQueryOptions **aResult) -{ - *aResult = nullptr; - nsNavHistoryQueryOptions *result = new nsNavHistoryQueryOptions(); - - RefPtr resultHolder(result); - result->mSort = mSort; - result->mResultType = mResultType; - result->mExcludeItems = mExcludeItems; - result->mExcludeQueries = mExcludeQueries; - result->mExpandQueries = mExpandQueries; - result->mMaxResults = mMaxResults; - result->mQueryType = mQueryType; - result->mParentAnnotationToExclude = mParentAnnotationToExclude; - result->mAsyncEnabled = mAsyncEnabled; - - resultHolder.forget(aResult); + Unused << Clone(&clone); + *_clone = clone; return NS_OK; } +nsresult +nsNavHistoryQueryOptions::Clone(nsNavHistoryQueryOptions** _clone) +{ + *_clone = nullptr; + RefPtr clone = new nsNavHistoryQueryOptions(*this); + clone.forget(_clone); + return NS_OK; +} // AppendBoolKeyValueIfTrue diff --git a/toolkit/components/places/nsNavHistoryQuery.h b/toolkit/components/places/nsNavHistoryQuery.h index d1a8b759a14c..a60b3e474f2a 100644 --- a/toolkit/components/places/nsNavHistoryQuery.h +++ b/toolkit/components/places/nsNavHistoryQuery.h @@ -66,11 +66,15 @@ public: return NS_OK; } + nsresult Clone(nsNavHistoryQuery **_clone); + private: ~nsNavHistoryQuery() {} protected: + // IF YOU ADD MORE ITEMS: + // * Add to the copy constructor int32_t mMinVisits; int32_t mMaxVisits; PRTime mBeginTime; @@ -100,18 +104,8 @@ NS_DEFINE_STATIC_IID_ACCESSOR(nsNavHistoryQuery, NS_NAVHISTORYQUERY_IID) class nsNavHistoryQueryOptions final : public nsINavHistoryQueryOptions { public: - nsNavHistoryQueryOptions() - : mSort(0) - , mResultType(0) - , mExcludeItems(false) - , mExcludeQueries(false) - , mExcludeReadOnlyFolders(false) - , mExpandQueries(true) - , mIncludeHidden(false) - , mMaxResults(0) - , mQueryType(nsINavHistoryQueryOptions::QUERY_TYPE_HISTORY) - , mAsyncEnabled(false) - { } + nsNavHistoryQueryOptions(); + nsNavHistoryQueryOptions(const nsNavHistoryQueryOptions& other); NS_DECLARE_STATIC_IID_ACCESSOR(NS_NAVHISTORYQUERYOPTIONS_IID) @@ -129,21 +123,19 @@ public: uint16_t QueryType() const { return mQueryType; } bool AsyncEnabled() const { return mAsyncEnabled; } - nsresult Clone(nsNavHistoryQueryOptions **aResult); + nsresult Clone(nsNavHistoryQueryOptions **_clone); private: ~nsNavHistoryQueryOptions() {} - nsNavHistoryQueryOptions(const nsNavHistoryQueryOptions& other) {} // no copy // IF YOU ADD MORE ITEMS: + // * Add to the copy constructor // * Add a new getter for C++ above if it makes sense // * Add to the serialization code (see nsNavHistory::QueriesToQueryString()) // * Add to the deserialization code (see nsNavHistory::QueryStringToQueries) - // * Add to the nsNavHistoryQueryOptions::Clone() function // * Add to the nsNavHistory.cpp::GetSimpleBookmarksQueryFolder function if applicable uint16_t mSort; nsCString mSortingAnnotation; - nsCString mParentAnnotationToExclude; uint16_t mResultType; bool mExcludeItems; bool mExcludeQueries; diff --git a/toolkit/components/places/nsNavHistoryResult.cpp b/toolkit/components/places/nsNavHistoryResult.cpp index 97601bdb7b1b..5b748b89b202 100644 --- a/toolkit/components/places/nsNavHistoryResult.cpp +++ b/toolkit/components/places/nsNavHistoryResult.cpp @@ -1640,13 +1640,14 @@ nsNavHistoryContainerResultNode::GetChildCount(uint32_t* aChildCount) NS_IMETHODIMP nsNavHistoryContainerResultNode::GetChild(uint32_t aIndex, - nsINavHistoryResultNode** _retval) + nsINavHistoryResultNode** _child) { if (!mExpanded) return NS_ERROR_NOT_AVAILABLE; if (aIndex >= uint32_t(mChildren.Count())) return NS_ERROR_INVALID_ARG; - NS_ADDREF(*_retval = mChildren[aIndex]); + nsCOMPtr child = mChildren[aIndex]; + child.forget(_child); return NS_OK; } @@ -1701,67 +1702,43 @@ nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode( } nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode( - const nsACString& aTitle, const nsCOMArray& aQueries, + const nsACString& aTitle, const RefPtr& aQuery, nsNavHistoryQueryOptions* aOptions) : nsNavHistoryContainerResultNode(EmptyCString(), aTitle, nsNavHistoryResultNode::RESULT_TYPE_QUERY, aOptions), - mQueries(aQueries), + mQuery(aQuery), mContentsValid(false), mBatchChanges(0), - mTransitions(mQueries[0]->Transitions()) + mTransitions(aQuery->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, + mLiveUpdate = history->GetUpdateRequirements(mQuery, mOptions, &mHasSearchTerms); } - - // Collect transitions shared by all queries. - for (int32_t i = 1; i < mQueries.Count(); ++i) { - const nsTArray& 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, PRTime aTime, - const nsCOMArray& aQueries, + const RefPtr& aQuery, nsNavHistoryQueryOptions* aOptions) : nsNavHistoryContainerResultNode(EmptyCString(), aTitle, aTime, nsNavHistoryResultNode::RESULT_TYPE_QUERY, aOptions), - mQueries(aQueries), + mQuery(aQuery), mContentsValid(false), mBatchChanges(0), - mTransitions(mQueries[0]->Transitions()) + mTransitions(aQuery->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, + mLiveUpdate = history->GetUpdateRequirements(mQuery, mOptions, &mHasSearchTerms); } - - // Collect transitions shared by all queries. - for (int32_t i = 1; i < mQueries.Count(); ++i) { - const nsTArray& 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() { @@ -1934,7 +1911,7 @@ nsNavHistoryQueryResultNode::GetHasChildren(bool* aHasChildren) NS_IMETHODIMP nsNavHistoryQueryResultNode::GetUri(nsACString& aURI) { - nsresult rv = VerifyQueriesSerialized(); + nsresult rv = VerifyQuerySerialized(); NS_ENSURE_SUCCESS(rv, rv); aURI = mURI; return NS_OK; @@ -1957,32 +1934,31 @@ nsNavHistoryQueryResultNode::GetTargetFolderGuid(nsACString& aGuid) { NS_IMETHODIMP nsNavHistoryQueryResultNode::GetQuery(nsINavHistoryQuery** _query) { - nsresult rv = VerifyQueriesParsed(); + nsresult rv = VerifyQueryParsed(); NS_ENSURE_SUCCESS(rv, rv); - NS_ASSERTION(mQueries.Count() > 0, "Must have >= 1 query"); - nsCOMPtr query = mQueries[0]; + nsCOMPtr query = do_QueryInterface(mQuery); query.forget(_query); return NS_OK; } NS_IMETHODIMP -nsNavHistoryQueryResultNode::GetQueryOptions( - nsINavHistoryQueryOptions** aQueryOptions) +nsNavHistoryQueryResultNode::GetQueryOptions(nsINavHistoryQueryOptions** _options) { - *aQueryOptions = Options(); - NS_ADDREF(*aQueryOptions); + MOZ_ASSERT(mOptions, "Options should be valid"); + nsCOMPtr options = do_QueryInterface(mOptions); + options.forget(_options); return NS_OK; } /** - * Safe options getter, ensures queries are parsed first. + * Safe options getter, ensures query is parsed first. */ nsNavHistoryQueryOptions* nsNavHistoryQueryResultNode::Options() { - nsresult rv = VerifyQueriesParsed(); + nsresult rv = VerifyQueryParsed(); if (NS_FAILED(rv)) return nullptr; MOZ_ASSERT(mOptions, "Options invalid, cannot generate from URI"); @@ -1991,11 +1967,11 @@ nsNavHistoryQueryResultNode::Options() nsresult -nsNavHistoryQueryResultNode::VerifyQueriesParsed() +nsNavHistoryQueryResultNode::VerifyQueryParsed() { - if (mQueries.Count() > 0) { + if (mQuery) { MOZ_ASSERT(mOriginalOptions && mOptions, - "If a result has queries, it also needs options"); + "A result with a parsed query must have options"); return NS_OK; } NS_ASSERTION(!mURI.IsEmpty(), @@ -2011,27 +1987,26 @@ nsNavHistoryQueryResultNode::VerifyQueriesParsed() NS_ENSURE_SUCCESS(rv, rv); mOptions = do_QueryObject(options); NS_ENSURE_STATE(mOptions); - RefPtr queryObj = do_QueryObject(query); - NS_ENSURE_STATE(queryObj); - mQueries.AppendObject(queryObj); - mLiveUpdate = history->GetUpdateRequirements(mQueries, mOptions, + mQuery = do_QueryObject(query); + NS_ENSURE_STATE(mQuery); + mLiveUpdate = history->GetUpdateRequirements(mQuery, mOptions, &mHasSearchTerms); return NS_OK; } nsresult -nsNavHistoryQueryResultNode::VerifyQueriesSerialized() +nsNavHistoryQueryResultNode::VerifyQuerySerialized() { if (!mURI.IsEmpty()) { return NS_OK; } - MOZ_ASSERT(mQueries.Count() > 0 && mOptions, + MOZ_ASSERT(mQuery && mOptions, "Query nodes must have either a URI or query/options"); nsNavHistory* history = nsNavHistory::GetHistoryService(); NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY); - nsCOMPtr query = do_QueryInterface(mQueries[0]); + nsCOMPtr query = do_QueryInterface(mQuery); nsresult rv = history->QueryToQueryString(query, mOriginalOptions, mURI); NS_ENSURE_SUCCESS(rv, rv); NS_ENSURE_STATE(!mURI.IsEmpty()); @@ -2051,9 +2026,9 @@ nsNavHistoryQueryResultNode::FillChildren() NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY); // get the results from the history service - nsresult rv = VerifyQueriesParsed(); + nsresult rv = VerifyQueryParsed(); NS_ENSURE_SUCCESS(rv, rv); - rv = history->GetQueryResults(this, mQueries, mOptions, &mChildren); + rv = history->GetQueryResults(this, mQuery, mOptions, &mChildren); NS_ENSURE_SUCCESS(rv, rv); // it is important to call FillStats to fill in the parents on all @@ -2372,12 +2347,8 @@ nsNavHistoryQueryResultNode::OnVisit(nsIURI* aURI, int64_t aVisitId, case QUERYUPDATE_HOST: { // For these simple yet common cases we can check the host ourselves // before doing the overhead of creating a new result node. - MOZ_ASSERT(mQueries.Count() == 1, - "Host updated queries can have only one object"); - RefPtr query = do_QueryObject(mQueries[0]); - bool hasDomain; - query->GetHasDomain(&hasDomain); + mQuery->GetHasDomain(&hasDomain); if (!hasDomain) return NS_OK; @@ -2385,7 +2356,7 @@ nsNavHistoryQueryResultNode::OnVisit(nsIURI* aURI, int64_t aVisitId, if (NS_FAILED(aURI->GetAsciiHost(host))) return NS_OK; - if (!query->Domain().Equals(host)) + if (!mQuery->Domain().Equals(host)) return NS_OK; // Fall through to check the time, if the time is not present it will @@ -2396,22 +2367,18 @@ nsNavHistoryQueryResultNode::OnVisit(nsIURI* aURI, int64_t aVisitId, case QUERYUPDATE_TIME: { // For these simple yet common cases we can check the time ourselves // before doing the overhead of creating a new result node. - MOZ_ASSERT(mQueries.Count() == 1, - "Time updated queries can have only one object"); - RefPtr query = do_QueryObject(mQueries[0]); - bool hasIt; - query->GetHasBeginTime(&hasIt); + mQuery->GetHasBeginTime(&hasIt); if (hasIt) { - PRTime beginTime = history->NormalizeTime(query->BeginTimeReference(), - query->BeginTime()); + PRTime beginTime = history->NormalizeTime(mQuery->BeginTimeReference(), + mQuery->BeginTime()); if (aTime < beginTime) return NS_OK; // before our time range } - query->GetHasEndTime(&hasIt); + mQuery->GetHasEndTime(&hasIt); if (hasIt) { - PRTime endTime = history->NormalizeTime(query->EndTimeReference(), - query->EndTime()); + PRTime endTime = history->NormalizeTime(mQuery->EndTimeReference(), + mQuery->EndTime()); if (aTime > endTime) return NS_OK; // after our time range } @@ -2421,7 +2388,7 @@ nsNavHistoryQueryResultNode::OnVisit(nsIURI* aURI, int64_t aVisitId, } case QUERYUPDATE_SIMPLE: { - // If all of the queries are filtered by some transitions, skip the + // If the query is 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; @@ -2437,7 +2404,7 @@ nsNavHistoryQueryResultNode::OnVisit(nsIURI* aURI, int64_t aVisitId, return NS_OK; } addition->mTransitionType = aTransitionType; - if (!history->EvaluateQueryForNode(mQueries, mOptions, addition)) + if (!history->EvaluateQueryForNode(mQuery, mOptions, addition)) return NS_OK; // don't need to include in our query if (mOptions->ResultType() == nsNavHistoryQueryOptions::RESULTS_AS_VISIT) { @@ -2523,7 +2490,7 @@ nsNavHistoryQueryResultNode::OnTitleChanged(nsIURI* aURI, mOptions->ResultType() == nsINavHistoryQueryOptions::RESULTS_AS_URI || mOptions->ResultType() == nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS; - // See if our queries have any search term matching. + // See if our query has any search term matching. if (mHasSearchTerms) { // Find all matching URI nodes. nsCOMArray matches; @@ -2539,7 +2506,7 @@ nsNavHistoryQueryResultNode::OnTitleChanged(nsIURI* aURI, NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY); rv = history->URIToResultNode(aURI, mOptions, getter_AddRefs(node)); NS_ENSURE_SUCCESS(rv, rv); - if (history->EvaluateQueryForNode(mQueries, mOptions, node)) { + if (history->EvaluateQueryForNode(mQuery, mOptions, node)) { rv = InsertSortedChild(node); NS_ENSURE_SUCCESS(rv, rv); } @@ -2554,7 +2521,7 @@ nsNavHistoryQueryResultNode::OnTitleChanged(nsIURI* aURI, nsNavHistory* history = nsNavHistory::GetHistoryService(); NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY); - if (!history->EvaluateQueryForNode(mQueries, mOptions, node)) { + if (!history->EvaluateQueryForNode(mQuery, mOptions, node)) { nsNavHistoryContainerResultNode* parent = node->mParent; // URI nodes should always have parents NS_ENSURE_TRUE(parent, NS_ERROR_UNEXPECTED); @@ -2748,7 +2715,7 @@ nsNavHistoryQueryResultNode::NotifyIfTagsChanged(nsIURI* aURI) NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY); rv = history->URIToResultNode(aURI, mOptions, getter_AddRefs(node)); NS_ENSURE_SUCCESS(rv, rv); - if (history->EvaluateQueryForNode(mQueries, mOptions, node)) { + if (history->EvaluateQueryForNode(mQuery, mOptions, node)) { rv = InsertSortedChild(node); NS_ENSURE_SUCCESS(rv, rv); } @@ -2764,7 +2731,7 @@ nsNavHistoryQueryResultNode::NotifyIfTagsChanged(nsIURI* aURI) // It's possible now this node does not respect anymore the conditions. // In such a case it should be removed. if (mHasSearchTerms && - !history->EvaluateQueryForNode(mQueries, mOptions, node)) { + !history->EvaluateQueryForNode(mQuery, mOptions, node)) { nsNavHistoryContainerResultNode* parent = node->mParent; // URI nodes should always have parents NS_ENSURE_TRUE(parent, NS_ERROR_UNEXPECTED); @@ -3130,13 +3097,11 @@ nsNavHistoryFolderResultNode::GetQuery(nsINavHistoryQuery** _query) * the options for the folder with the current folder ID set. */ NS_IMETHODIMP -nsNavHistoryFolderResultNode::GetQueryOptions( - nsINavHistoryQueryOptions** aQueryOptions) +nsNavHistoryFolderResultNode::GetQueryOptions(nsINavHistoryQueryOptions** _options) { - NS_ASSERTION(mOptions, "Options invalid"); - - *aQueryOptions = mOptions; - NS_ADDREF(*aQueryOptions); + MOZ_ASSERT(mOptions, "Options should be valid"); + nsCOMPtr options = do_QueryInterface(mOptions); + options.forget(_options); return NS_OK; } @@ -4009,18 +3974,29 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsNavHistoryResult) NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference) NS_INTERFACE_MAP_END -nsNavHistoryResult::nsNavHistoryResult(nsNavHistoryContainerResultNode* aRoot) - : mRootNode(aRoot) +nsNavHistoryResult::nsNavHistoryResult(nsNavHistoryContainerResultNode* aRoot, + const RefPtr& aQuery, + const RefPtr& aOptions, + bool aBatchInProgress +) : mRootNode(aRoot) + , mQuery(aQuery) + , mOptions(aOptions) , mNeedsToApplySortingMode(false) , mIsHistoryObserver(false) , mIsBookmarkFolderObserver(false) , mIsAllBookmarksObserver(false) , mIsMobilePrefObserver(false) , mBookmarkFolderObservers(64) - , mBatchInProgress(false) + , mBatchInProgress(aBatchInProgress) , mSuppressNotifications(false) { + mSortingMode = aOptions->SortingMode(); + MOZ_ALWAYS_SUCCEEDS(aOptions->GetSortingAnnotation(mSortingAnnotation)); + mRootNode->mResult = this; + MOZ_ASSERT(mRootNode->mIndentLevel == -1, + "Root node's indent level initialized wrong"); + mRootNode->FillStats(); } nsNavHistoryResult::~nsNavHistoryResult() @@ -4058,71 +4034,6 @@ nsNavHistoryResult::StopObserving() } } -/** - * @note you must call AddRef before this, since we may do things like - * register ourselves. - */ -nsresult -nsNavHistoryResult::Init(nsCOMArray& aQueries, - uint32_t aQueryCount, - nsNavHistoryQueryOptions *aOptions) -{ - nsresult rv; - NS_ASSERTION(aOptions, "Must have valid options"); - NS_ASSERTION(aQueries.Count() == aQueryCount && aQueryCount > 0, "Must have >1 query in result"); - - // Fill saved source queries with copies of the original (the caller might - // change their original objects, and we always want to reflect the source - // parameters). - for (uint32_t i = 0; i < aQueryCount; ++i) { - nsCOMPtr queryClone; - rv = aQueries[i]->Clone(getter_AddRefs(queryClone)); - NS_ENSURE_SUCCESS(rv, rv); - if (!mQueries.AppendObject(queryClone)) - return NS_ERROR_OUT_OF_MEMORY; - } - rv = aOptions->Clone(getter_AddRefs(mOptions)); - NS_ENSURE_SUCCESS(rv, rv); - mSortingMode = aOptions->SortingMode(); - rv = aOptions->GetSortingAnnotation(mSortingAnnotation); - NS_ENSURE_SUCCESS(rv, rv); - - NS_ASSERTION(mRootNode->mIndentLevel == -1, - "Root node's indent level initialized wrong"); - mRootNode->FillStats(); - - return NS_OK; -} - - -/** - * Constructs a new history result object. - */ -nsresult // static -nsNavHistoryResult::NewHistoryResult(nsCOMArray& aQueries, - uint32_t aQueryCount, - nsNavHistoryQueryOptions* aOptions, - nsNavHistoryContainerResultNode* aRoot, - bool aBatchInProgress, - nsNavHistoryResult** result) -{ - *result = new nsNavHistoryResult(aRoot); - if (!*result) - return NS_ERROR_OUT_OF_MEMORY; - NS_ADDREF(*result); // must happen before Init - // Correctly set mBatchInProgress for the result based on the root node value. - (*result)->mBatchInProgress = aBatchInProgress; - nsresult rv = (*result)->Init(aQueries, aQueryCount, aOptions); - if (NS_FAILED(rv)) { - NS_RELEASE(*result); - *result = nullptr; - return rv; - } - - return NS_OK; -} - - void nsNavHistoryResult::AddHistoryObserver(nsNavHistoryQueryResultNode* aNode) { diff --git a/toolkit/components/places/nsNavHistoryResult.h b/toolkit/components/places/nsNavHistoryResult.h index fdc88b9afb2d..ca3f1804972b 100644 --- a/toolkit/components/places/nsNavHistoryResult.h +++ b/toolkit/components/places/nsNavHistoryResult.h @@ -101,13 +101,6 @@ class nsNavHistoryResult final : public nsSupportsWeakReference, public nsINavHistoryObserver { public: - static nsresult NewHistoryResult(nsCOMArray& aQueries, - uint32_t aQueryCount, - nsNavHistoryQueryOptions* aOptions, - nsNavHistoryContainerResultNode* aRoot, - bool aBatchInProgress, - nsNavHistoryResult** result); - NS_DECLARE_STATIC_IID_ACCESSOR(NS_NAVHISTORYRESULT_IID) NS_DECL_CYCLE_COLLECTING_ISUPPORTS @@ -133,16 +126,15 @@ public: const nsAString& aLastKnownTitle); public: - // two-stage init, use NewHistoryResult to construct - explicit nsNavHistoryResult(nsNavHistoryContainerResultNode* mRoot); - nsresult Init(nsCOMArray& aQueries, - uint32_t aQueryCount, - nsNavHistoryQueryOptions *aOptions); + explicit nsNavHistoryResult(nsNavHistoryContainerResultNode* mRoot, + const RefPtr& aQuery, + const RefPtr& aOptions, + bool aBatchInProgress); RefPtr mRootNode; - nsCOMArray mQueries; - nsCOMPtr mOptions; + RefPtr mQuery; + RefPtr mOptions; // One of nsNavHistoryQueryOptions.SORY_BY_* This is initialized to mOptions.sortingMode, // but may be overridden if the user clicks on one of the columns. @@ -233,7 +225,7 @@ NS_DEFINE_STATIC_IID_ACCESSOR(nsNavHistoryResult, NS_NAVHISTORYRESULT_IID) // short and we can save a virtual function call. // // (GetUri is redefined only by QueryResultNode and FolderResultNode because -// the queries might not necessarily be parsed. The rest just return the node's +// the query might not necessarily be parsed. The rest just return the node's // buffer.) #define NS_FORWARD_COMMON_RESULTNODE_TO_BASE \ NS_IMPLEMENT_SIMPLE_RESULTNODE \ @@ -488,8 +480,8 @@ public: // the direct parent of this container node, see SetAsParentOfNode. For // example, if the parent has excludeItems, options will have it too, even if // originally this object was not defined with that option. - nsCOMPtr mOriginalOptions; - nsCOMPtr mOptions; + RefPtr mOriginalOptions; + RefPtr mOptions; void FillStats(); // Sets this container as parent of aNode, propagating the appropriate options. @@ -629,11 +621,11 @@ public: nsNavHistoryQueryResultNode(const nsACString& aTitle, const nsACString& aQueryURI); nsNavHistoryQueryResultNode(const nsACString& aTitle, - const nsCOMArray& aQueries, + const RefPtr& aQuery, nsNavHistoryQueryOptions* aOptions); nsNavHistoryQueryResultNode(const nsACString& aTitle, PRTime aTime, - const nsCOMArray& aQueries, + const RefPtr& aQuery, nsNavHistoryQueryOptions* aOptions); NS_DECL_ISUPPORTS_INHERITED @@ -663,18 +655,18 @@ public: virtual void OnRemoving() override; public: - // this constructs lazily mURI from mQueries and mOptions, call - // VerifyQueriesSerialized either this or mQueries/mOptions should be valid - nsresult VerifyQueriesSerialized(); + // This constructs lazily mURI from mQuery and mOptions. + // Either this or mQuery/mOptions should be valid. + nsresult VerifyQuerySerialized(); - // these may be constructed lazily from mURI, call VerifyQueriesParsed - // either this or mURI should be valid - nsCOMArray mQueries; + // This may be constructed lazily from mURI, call VerifyQueryParsed. + // Either this or mURI should be valid. + RefPtr mQuery; uint32_t mLiveUpdate; // one of QUERYUPDATE_* in nsNavHistory.h bool mHasSearchTerms; - nsresult VerifyQueriesParsed(); + nsresult VerifyQueryParsed(); - // safe options getter, ensures queries are parsed + // safe options getter, ensures query is parsed nsNavHistoryQueryOptions* Options(); // this indicates whether the query contents are valid, they don't go away @@ -694,7 +686,7 @@ public: uint32_t mBatchChanges; - // Tracks transition type filters shared by all mQueries. + // Tracks transition type filters. nsTArray mTransitions; protected: diff --git a/toolkit/components/places/tests/unit/test_annotations.js b/toolkit/components/places/tests/unit/test_annotations.js index d131baa5e295..dec2c728c2e0 100644 --- a/toolkit/components/places/tests/unit/test_annotations.js +++ b/toolkit/components/places/tests/unit/test_annotations.js @@ -297,7 +297,7 @@ add_task(async function test_execute() { info("verify that removing an annotation updates the last modified date"); info("lastModified3 = " + lastModified3); info("lastModified4 = " + lastModified4); - Assert.ok(lastModified4 > lastModified3); + Assert.ok(is_time_ordered(lastModified3, lastModified4)); Assert.equal(annoObserver.PAGE_lastRemoved_URI, testURI.spec); Assert.equal(annoObserver.PAGE_lastRemoved_AnnoName, int32Key);