[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}

components/omnibox/browser/search_suggestion_parser.cc

@@ -149,12 +149,15 @@ operator=(const SuggestResult& rhs) = default;
 void SearchSuggestionParser::SuggestResult::ClassifyMatchContents(
     const bool allow_bolding_all,
     const base::string16& input_text) {
-  if (input_text.empty()) {
-    // In case of zero-suggest results, do not highlight matches.
-    match_contents_class_.push_back(
-        ACMatchClassification(0, ACMatchClassification::NONE));
+  // Start with the trivial nothing-bolded classification.
+  DCHECK(!match_contents_.empty());
+  match_contents_class_.clear();
+  match_contents_class_.push_back(
+      ACMatchClassification(0, ACMatchClassification::NONE));
+
+  // Leave trivial classification alone in the ZeroSuggest case.
+  if (input_text.empty())
     return;
-  }
 
   base::string16 lookup_text = input_text;
   if (type_ == AutocompleteMatchType::SEARCH_SUGGEST_TAIL) {
@@ -181,6 +184,7 @@ void SearchSuggestionParser::SuggestResult::ClassifyMatchContents(
     return;
   }
 
+  // Note we discard our existing match_contents_class_ with this call.
   match_contents_class_ = AutocompleteProvider::ClassifyAllMatchesInString(
       input_text, match_contents_, true);
 }
@@ -244,12 +248,15 @@ void
 SearchSuggestionParser::NavigationResult::CalculateAndClassifyMatchContents(
     const bool allow_bolding_nothing,
     const base::string16& input_text) {
-  if (input_text.empty()) {
-    // In case of zero-suggest results, do not highlight matches.
-    match_contents_class_.push_back(
-        ACMatchClassification(0, ACMatchClassification::NONE));
+  // Start with the trivial nothing-bolded classification.
+  DCHECK(url_.is_valid());
+  match_contents_class_.clear();
+  match_contents_class_.push_back(
+      ACMatchClassification(0, ACMatchClassification::NONE));
+
+  // Leave trivial classification alone in the ZeroSuggest case.
+  if (input_text.empty())
     return;
-  }
 
   // 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

components/omnibox/browser/search_suggestion_parser_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/json/json_reader.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
+#include "components/omnibox/browser/autocomplete_match.h"
 #include "components/omnibox/browser/test_scheme_classifier.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -154,3 +155,32 @@ TEST(SearchSuggestionParserTest, ParseSuggestResults) {
     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