Commit b4aa685c authored by Aaron Gable's avatar Aaron Gable Committed by Commit Bot

Revert "[omnibox] Fix ZeroSuggestProvider duplicate classifications bug"

This reverts commit ebc60412.

Reason for revert: Broke compile on iOS
https://ci.chromium.org/p/chromium/builders/ci/ios-device/143201

Original change's description:
> [omnibox] Fix ZeroSuggestProvider duplicate classifications bug
> 
> The below CL introduces a bug, where ZeroSuggestProvider can create
> a match with duplicate text classifications. This triggers a DCHECK
> as described in the linked bug.
> 
> https://chromium-review.googlesource.com/c/chromium/src/+/1947765
> 
> This CL fixes it and adds a test.
> 
> Bug: 1030741
> Change-Id: If59fbd320d720e2a7ff8d2f66d9ee92e11bb9a68
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1951557
> Commit-Queue: Tommy Li <tommycli@chromium.org>
> Reviewed-by: manuk hovanesian <manukh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#722497}

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

Change-Id: Idba8826447487022b4149a310bdb67e8ae560268
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1030741
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1954941Reviewed-by: default avatarAaron Gable <agable@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
Cr-Commit-Position: refs/heads/master@{#722501}
parent f9181ba6
...@@ -149,15 +149,12 @@ operator=(const SuggestResult& rhs) = default; ...@@ -149,15 +149,12 @@ operator=(const SuggestResult& rhs) = default;
void SearchSuggestionParser::SuggestResult::ClassifyMatchContents( void SearchSuggestionParser::SuggestResult::ClassifyMatchContents(
const bool allow_bolding_all, const bool allow_bolding_all,
const base::string16& input_text) { const base::string16& input_text) {
// Start with the trivial nothing-bolded classification. if (input_text.empty()) {
DCHECK(!match_contents_.empty()); // In case of zero-suggest results, do not highlight matches.
match_contents_class_.clear(); match_contents_class_.push_back(
match_contents_class_.push_back( ACMatchClassification(0, ACMatchClassification::NONE));
ACMatchClassification(0, ACMatchClassification::NONE));
// Leave trivial classification alone in the ZeroSuggest case.
if (input_text.empty())
return; return;
}
base::string16 lookup_text = input_text; base::string16 lookup_text = input_text;
if (type_ == AutocompleteMatchType::SEARCH_SUGGEST_TAIL) { if (type_ == AutocompleteMatchType::SEARCH_SUGGEST_TAIL) {
...@@ -184,7 +181,6 @@ void SearchSuggestionParser::SuggestResult::ClassifyMatchContents( ...@@ -184,7 +181,6 @@ void SearchSuggestionParser::SuggestResult::ClassifyMatchContents(
return; return;
} }
// Note we discard our existing match_contents_class_ with this call.
match_contents_class_ = AutocompleteProvider::ClassifyAllMatchesInString( match_contents_class_ = AutocompleteProvider::ClassifyAllMatchesInString(
input_text, match_contents_, true); input_text, match_contents_, true);
} }
...@@ -248,15 +244,12 @@ void ...@@ -248,15 +244,12 @@ void
SearchSuggestionParser::NavigationResult::CalculateAndClassifyMatchContents( SearchSuggestionParser::NavigationResult::CalculateAndClassifyMatchContents(
const bool allow_bolding_nothing, const bool allow_bolding_nothing,
const base::string16& input_text) { const base::string16& input_text) {
// Start with the trivial nothing-bolded classification. if (input_text.empty()) {
DCHECK(url_.is_valid()); // In case of zero-suggest results, do not highlight matches.
match_contents_class_.clear(); match_contents_class_.push_back(
match_contents_class_.push_back( ACMatchClassification(0, ACMatchClassification::NONE));
ACMatchClassification(0, ACMatchClassification::NONE));
// Leave trivial classification alone in the ZeroSuggest case.
if (input_text.empty())
return; return;
}
// Set contents to the formatted URL while ensuring the scheme and subdomain // Set contents to the formatted URL while ensuring the scheme and subdomain
// are kept if the user text seems to include them. E.g., for the user text // are kept if the user text seems to include them. E.g., for the user text
......
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
#include "base/json/json_reader.h" #include "base/json/json_reader.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/values.h" #include "base/values.h"
#include "components/omnibox/browser/autocomplete_match.h"
#include "components/omnibox/browser/test_scheme_classifier.h" #include "components/omnibox/browser/test_scheme_classifier.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -155,32 +154,3 @@ TEST(SearchSuggestionParserTest, ParseSuggestResults) { ...@@ -155,32 +154,3 @@ TEST(SearchSuggestionParserTest, ParseSuggestResults) {
ASSERT_EQ("http://example.com/a.png", suggestion_result.image_url()); ASSERT_EQ("http://example.com/a.png", suggestion_result.image_url());
} }
} }
TEST(SearchSuggestionParserTest, SuggestClassification) {
AutocompleteMatch::ACMatchClassification none_classification(
0, AutocompleteMatch::ACMatchClassification::NONE);
SearchSuggestionParser::SuggestResult result(
base::ASCIIToUTF16("foobar"), AutocompleteMatchType::SEARCH_SUGGEST, 0,
false, 400, true, base::string16());
AutocompleteMatch dummy_match;
dummy_match.ValidateClassifications(result.match_contents(),
result.match_contents_class());
// Re-classify the match contents, as the ZeroSuggestProvider does.
result.ClassifyMatchContents(true, base::string16());
dummy_match.ValidateClassifications(result.match_contents(),
result.match_contents_class());
// Make sure that searching text-not-found still gives valid classifications,
// if we don't allow the code to bold everything.
result.ClassifyMatchContents(false, base::ASCIIToUTF16("apple"));
dummy_match.ValidateClassifications(result.match_contents(),
result.match_contents_class());
// Make sure that searching text-not-found still gives valid classifications,
// if we don't allow the code to bold everything.
result.ClassifyMatchContents(true, base::ASCIIToUTF16("foobar"));
dummy_match.ValidateClassifications(result.match_contents(),
result.match_contents_class());
}
\ No newline at end of file
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