Restrict verbatim match to valid URL texts only.

This change addresses the case where a native URL is presented
with a clear omnibox text on mobile devices, eg.
- chrome://history or
- chrome://bookmarks
and ensures that no suggestion is offered when the text is cleared.

The default match is produced by AutocompleteProviderClient, which
does not use the current URL, but instead the text presented in the
omnibox, and with the Omnibox text being empty, the default match
carries no URL.

Bug: 1125046
Change-Id: I56c11def20ed79a9852a578bbf75dd6578d4e26d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2401331
Commit-Queue: Tomasz Wiszkowski <ender@google.com>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805538}

components/omnibox/browser/zero_suggest_verbatim_match_provider.cc

@@ -54,16 +54,20 @@ void ZeroSuggestVerbatimMatchProvider::Start(const AutocompleteInput& input,
   if (!input.current_url().is_valid())
     return;
 
-  const auto& current_query = input.current_url().spec();
-  const auto& current_title = input.current_title();
-
   AutocompleteInput verbatim_input = input;
   verbatim_input.set_prevent_inline_autocomplete(true);
   verbatim_input.set_allow_exact_keyword_match(false);
 
-  AutocompleteMatch match =
-      VerbatimMatchForURL(client_, verbatim_input, GURL(current_query),
-                          current_title, nullptr, kVerbatimMatchRelevanceScore);
+  AutocompleteMatch match = VerbatimMatchForURL(
+      client_, verbatim_input, input.current_url(), input.current_title(),
+      nullptr, kVerbatimMatchRelevanceScore);
+
+  // In the case of native pages, the classifier may replace the URL with an
+  // empty content, resulting with a verbatim match that does not point
+  // anywhere.
+  if (!match.destination_url.is_valid())
+    return;
+
   match.provider = this;
   matches_.push_back(match);
 }

components/omnibox/browser/zero_suggest_verbatim_match_provider_unittest.cc

@@ -45,6 +45,13 @@ bool ZeroSuggestVerbatimMatchProviderTest::IsVerbatimMatchEligible() const {
 void ZeroSuggestVerbatimMatchProviderTest::SetUp() {
   provider_ = new ZeroSuggestVerbatimMatchProvider(&mock_client_);
   ON_CALL(mock_client_, IsOffTheRecord()).WillByDefault([] { return false; });
+  ON_CALL(mock_client_, Classify)
+      .WillByDefault(
+          [](const base::string16& text, bool prefer_keyword,
+             bool allow_exact_keyword_match,
+             metrics::OmniboxEventProto::PageClassification page_classification,
+             AutocompleteMatch* match,
+             GURL* alternate_nav_url) { match->destination_url = GURL(text); });
 }
 
 TEST_P(ZeroSuggestVerbatimMatchProviderTest,
@@ -106,15 +113,14 @@ TEST_P(ZeroSuggestVerbatimMatchProviderTest,
   // test. As a result, the test would validate what the mocks fill in.
 }
 
-TEST_P(ZeroSuggestVerbatimMatchProviderTest,
-       OffersVerbatimMatchWithEmptyInput) {
+TEST_P(ZeroSuggestVerbatimMatchProviderTest, NoVerbatimMatchWithEmptyInput) {
   std::string url("https://www.wired.com/");
   AutocompleteInput input(base::string16(),  // Note: empty input.
                           GetParam(), TestSchemeClassifier());
   input.set_current_url(GURL(url));
   input.set_focus_type(OmniboxFocusType::DEFAULT);
   provider_->Start(input, false);
-  ASSERT_EQ(IsVerbatimMatchEligible(), provider_->matches().size() > 0);
+  ASSERT_TRUE(provider_->matches().empty());
   // Note: we intentionally do not validate the match content here.
   // The content is populated either by HistoryURLProvider or
   // AutocompleteProviderClient both of which we would have to mock for this
@@ -122,7 +128,7 @@ TEST_P(ZeroSuggestVerbatimMatchProviderTest,
 }
 
 TEST_P(ZeroSuggestVerbatimMatchProviderTest,
-       OffersVerbatimMatchWithEmptyInputInIncognito) {
+       NoVerbatimMatchWithEmptyInputInIncognito) {
   std::string url("https://www.wired.com/");
   AutocompleteInput input(base::string16(),  // Note: empty input.
                           GetParam(), TestSchemeClassifier());
@@ -130,21 +136,21 @@ TEST_P(ZeroSuggestVerbatimMatchProviderTest,
   input.set_focus_type(OmniboxFocusType::DEFAULT);
   ON_CALL(mock_client_, IsOffTheRecord()).WillByDefault([] { return true; });
   provider_->Start(input, false);
-  ASSERT_EQ(IsVerbatimMatchEligible(), provider_->matches().size() > 0);
+  ASSERT_TRUE(provider_->matches().empty());
   // Note: we intentionally do not validate the match content here.
   // The content is populated either by HistoryURLProvider or
   // AutocompleteProviderClient both of which we would have to mock for this
   // test. As a result, the test would validate what the mocks fill in.
 }
 
-TEST_P(ZeroSuggestVerbatimMatchProviderTest, OffersVerbatimMatchOnClearInput) {
+TEST_P(ZeroSuggestVerbatimMatchProviderTest, NoVerbatimMatchOnClearInput) {
   std::string url("https://www.wired.com/");
   AutocompleteInput input(base::string16(),  // Note: empty input.
                           GetParam(), TestSchemeClassifier());
   input.set_current_url(GURL(url));
   input.set_focus_type(OmniboxFocusType::DELETED_PERMANENT_TEXT);
   provider_->Start(input, false);
-  ASSERT_EQ(IsVerbatimMatchEligible(), provider_->matches().size() > 0);
+  ASSERT_TRUE(provider_->matches().empty());
   // Note: we intentionally do not validate the match content here.
   // The content is populated either by HistoryURLProvider or
   // AutocompleteProviderClient both of which we would have to mock for this
@@ -152,7 +158,7 @@ TEST_P(ZeroSuggestVerbatimMatchProviderTest, OffersVerbatimMatchOnClearInput) {
 }
 
 TEST_P(ZeroSuggestVerbatimMatchProviderTest,
-       OffersVerbatimMatchOnClearInputInIncognito) {
+       NoVerbatimMatchOnClearInputInIncognito) {
   std::string url("https://www.wired.com/");
   AutocompleteInput input(base::string16(),  // Note: empty input.
                           GetParam(), TestSchemeClassifier());
@@ -160,7 +166,7 @@ TEST_P(ZeroSuggestVerbatimMatchProviderTest,
   input.set_focus_type(OmniboxFocusType::DELETED_PERMANENT_TEXT);
   ON_CALL(mock_client_, IsOffTheRecord()).WillByDefault([] { return true; });
   provider_->Start(input, false);
-  ASSERT_EQ(IsVerbatimMatchEligible(), provider_->matches().size() > 0);
+  ASSERT_TRUE(provider_->matches().empty());
   // Note: we intentionally do not validate the match content here.
   // The content is populated either by HistoryURLProvider or
   // AutocompleteProviderClient both of which we would have to mock for this