Commit 3646277b authored by manuk's avatar manuk Committed by Commit Bot

Revert "[omnibox] Dedup history matches HQP matches."

This reverts commit 14f12b97.

Reason for revert: As mpearson pointed out, this CL has an unintended affect on HQP suggestions.

This is a partial revert. A full revert would fail tests due to other
changes since the original CL landed. This revert keeps test changes
from the original CL and updates another test. Additionally, it retains
a variable rename and formatting change since those have no functional
effect and we prefer the newer versions.

Original change's description:
> [omnibox] Dedup history matches HQP matches.
>
> This CL filters duplicate history matches in
> URLIndexPrivateData::HistoryItemsForTerms used by the HQP. Previously,
> we relied on the autocomplete controller's deduping which
> 1) may affect performance since it occured after additional processing
> of each suggestion (I would guess it's insignificant), and
> 2) prevented the HQP provider from making full use of it's allocated
> per-provider-limit.
>
> Also update a few HQP unit tests.
>
> Bug: 591981
> Change-Id: I1c6ce02050ba1b674e3507368c1a095f5a5b0c6d
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2219027
> Commit-Queue: manuk hovanesian <manukh@chromium.org>
> Reviewed-by: Tommy Li <tommycli@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#773185}

TBR=tommycli@chromium.org,jdonnelly@chromium.org,manukh@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 591981
Change-Id: I00ad1ebdcb40163b0eb488dd5771f1b703566aeb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2327955Reviewed-by: default avatarMark Pearson <mpearson@chromium.org>
Reviewed-by: default avatarmanuk hovanesian <manukh@chromium.org>
Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Commit-Queue: manuk hovanesian <manukh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796553}
parent f4113df1
......@@ -407,11 +407,11 @@ TEST_F(HistoryQuickProviderTest, SingleMatchWithCursor) {
TEST_F(HistoryQuickProviderTest, MatchWithAndWithoutCursorWordBreak) {
std::vector<std::string> expected_urls;
expected_urls.push_back("https://twitter.com/fungoodtimes");
// With cursor after "good", we should retrieve the desired result but it
// With cursor after ".com", we should retrieve the desired result but it
// should not be allowed to be the default match.
RunTestWithCursor(ASCIIToUTF16("fungoodtimes"), 7, false, expected_urls,
false, ASCIIToUTF16("https://twitter.com/fungoodtimes"),
base::string16());
RunTestWithCursor(
ASCIIToUTF16("twitter.comfungoodtimes"), 11, false, expected_urls, false,
ASCIIToUTF16("https://twitter.com/fungoodtimes"), base::string16());
}
TEST_F(HistoryQuickProviderTest, WordBoundariesWithPunctuationMatch) {
......
......@@ -185,8 +185,6 @@ ScoredHistoryMatches URLIndexPrivateData::HistoryItemsForTerms(
// A set containing the list of words extracted from each search string,
// used to prevent running duplicate searches.
std::set<String16Vector> seen_search_words;
// Likewise, a set of seen history_ids to prevent creating duplicate matches.
std::set<HistoryID> seen_history_ids;
for (const base::string16& search_string : search_strings) {
// The search string we receive may contain escaped characters. For reducing
// the index we need individual, lower-cased words, ignoring escapings. For
......@@ -215,14 +213,6 @@ ScoredHistoryMatches URLIndexPrivateData::HistoryItemsForTerms(
seen_search_words.insert(lower_words);
HistoryIDVector history_ids = HistoryIDsFromWords(lower_words);
// Filter history IDs found previously. Otherwise, we'll end up creating and
// pushing duplicate matches into |scored_items|. Besides being wasteful,
// this could result in not making full use of max_matches.
base::EraseIf(history_ids, [seen_history_ids](const auto& history_id) {
return seen_history_ids.count(history_id);
});
seen_history_ids.insert(history_ids.begin(), history_ids.end());
history_ids_were_trimmed |= TrimHistoryIdsPool(&history_ids);
HistoryIdsToScoredMatches(std::move(history_ids), lower_raw_string,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment