Extract MostVisited from ZeroSuggestProvider.

This change splits the ZeroSuggestProvider into
- ZeroSuggestProvider serving the Search suggestions and
- ZeroSuggestMostVisitedSitesProvider, serving the MostVisited
  suggestions.

The Most Visited Sites are served specifically on mobile devices
and are therefore wired specifically for iOS and Android.
This change enables MostVisited suggestions to be served
alongside the zero-prefix REMOTE_SEND_URL suggestions.

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

components/omnibox/browser/BUILD.gn

@@ -134,6 +134,8 @@ static_library("browser") {
     "local_history_zero_suggest_provider.cc",
     "local_history_zero_suggest_provider.h",
     "match_compare.h",
+    "most_visited_sites_provider.cc",
+    "most_visited_sites_provider.h",
     "omnibox_client.cc",
     "omnibox_client.h",
     "omnibox_controller.cc",
@@ -493,6 +495,7 @@ source_set("unit_tests") {
     "keyword_provider_unittest.cc",
     "local_history_zero_suggest_provider_unittest.cc",
     "location_bar_model_impl_unittest.cc",
+    "most_visited_sites_provider_unittest.cc",
     "omnibox_controller_unittest.cc",
     "omnibox_edit_model_unittest.cc",
     "omnibox_field_trial_unittest.cc",

components/omnibox/browser/autocomplete_classifier.cc

@@ -45,6 +45,8 @@ int AutocompleteClassifier::DefaultOmniboxProviders() {
       AutocompleteProvider::TYPE_KEYWORD |
 #else
       AutocompleteProvider::TYPE_CLIPBOARD |
+      AutocompleteProvider::TYPE_MOST_VISITED_SITES |
+      AutocompleteProvider::TYPE_VERBATIM_MATCH |
 #endif
       AutocompleteProvider::TYPE_ZERO_SUGGEST |
       AutocompleteProvider::TYPE_ZERO_SUGGEST_LOCAL_HISTORY |

components/omnibox/browser/autocomplete_controller.cc

@@ -38,6 +38,7 @@
 #include "components/omnibox/browser/history_url_provider.h"
 #include "components/omnibox/browser/keyword_provider.h"
 #include "components/omnibox/browser/local_history_zero_suggest_provider.h"
+#include "components/omnibox/browser/most_visited_sites_provider.h"
 #include "components/omnibox/browser/omnibox_field_trial.h"
 #include "components/omnibox/browser/omnibox_pedal_provider.h"
 #include "components/omnibox/browser/on_device_head_provider.h"
@@ -290,22 +291,22 @@ AutocompleteController::AutocompleteController(
         ZeroSuggestProvider::Create(provider_client_.get(), this);
     if (zero_suggest_provider_)
       providers_.push_back(zero_suggest_provider_);
-#if defined(OS_ANDROID)
+  }
+  if (provider_types & AutocompleteProvider::TYPE_ZERO_SUGGEST_LOCAL_HISTORY) {
+    providers_.push_back(
+        LocalHistoryZeroSuggestProvider::Create(provider_client_.get(), this));
+  }
+  if (provider_types & AutocompleteProvider::TYPE_MOST_VISITED_SITES) {
+    providers_.push_back(
+        new MostVisitedSitesProvider(provider_client_.get(), this));
     // Note: the need for the always-present verbatim match originates from the
     // OmniboxSearchReadyIncognito feature.
     // The feature aims at showing SRO in an Incognito mode, where the
     // ZeroSuggestProvider intentionally never gets invoked.
     // The gating flag here should be removed when the SRO Incognito is
     // launched.
-    if (base::FeatureList::IsEnabled(omnibox::kOmniboxSearchReadyIncognito)) {
-      providers_.push_back(
-          new ZeroSuggestVerbatimMatchProvider(provider_client_.get()));
-    }
-#endif
-  }
-  if (provider_types & AutocompleteProvider::TYPE_ZERO_SUGGEST_LOCAL_HISTORY) {
     providers_.push_back(
-        LocalHistoryZeroSuggestProvider::Create(provider_client_.get(), this));
+        new ZeroSuggestVerbatimMatchProvider(provider_client_.get()));
   }
   if (provider_types & AutocompleteProvider::TYPE_DOCUMENT) {
     document_provider_ = DocumentProvider::Create(provider_client_.get(), this);

components/omnibox/browser/autocomplete_provider.cc

@@ -62,6 +62,10 @@ const char* AutocompleteProvider::TypeToString(Type type) {
       return "LocalHistoryZeroSuggest";
     case TYPE_QUERY_TILE:
       return "QueryTile";
+    case TYPE_MOST_VISITED_SITES:
+      return "MostVisitedSites";
+    case TYPE_VERBATIM_MATCH:
+      return "VerbatimMatch";
     default:
       NOTREACHED() << "Unhandled AutocompleteProvider::Type " << type;
       return "Unknown";
@@ -134,6 +138,10 @@ metrics::OmniboxEventProto_ProviderType AutocompleteProvider::
       return metrics::OmniboxEventProto::ZERO_SUGGEST_LOCAL_HISTORY;
     case TYPE_QUERY_TILE:
       return metrics::OmniboxEventProto::QUERY_TILE;
+    case TYPE_MOST_VISITED_SITES:
+      return metrics::OmniboxEventProto::ZERO_SUGGEST;
+    case TYPE_VERBATIM_MATCH:
+      return metrics::OmniboxEventProto::ZERO_SUGGEST;
     default:
       NOTREACHED() << "Unhandled AutocompleteProvider::Type " << type_;
       return metrics::OmniboxEventProto::UNKNOWN_PROVIDER;

components/omnibox/browser/autocomplete_provider.h

@@ -151,6 +151,8 @@ class AutocompleteProvider
     TYPE_ON_DEVICE_HEAD = 1 << 10,
     TYPE_ZERO_SUGGEST_LOCAL_HISTORY = 1 << 11,
     TYPE_QUERY_TILE = 1 << 12,
+    TYPE_MOST_VISITED_SITES = 1 << 13,
+    TYPE_VERBATIM_MATCH = 1 << 14,
   };
 
   explicit AutocompleteProvider(Type type);

components/omnibox/browser/most_visited_sites_provider.cc

+// Copyright (c) 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/omnibox/browser/most_visited_sites_provider.h"
+
+#include <string>
+
+#include "base/bind.h"
+#include "base/feature_list.h"
+#include "base/strings/string16.h"
+#include "components/history/core/browser/top_sites.h"
+#include "components/omnibox/browser/autocomplete_input.h"
+#include "components/omnibox/browser/autocomplete_match.h"
+#include "components/omnibox/browser/autocomplete_match_classification.h"
+#include "components/omnibox/common/omnibox_features.h"
+#include "components/url_formatter/url_formatter.h"
+#include "net/base/escape.h"
+#include "third_party/metrics_proto/omnibox_event.pb.h"
+#include "third_party/metrics_proto/omnibox_input_type.pb.h"
+#include "url/gurl.h"
+
+namespace {
+// The relevance score for navsuggest tiles.
+// Navsuggest tiles should be positioned below the Query Tiles object.
+constexpr const int kMostVisitedTilesRelevance = 1500;
+}  // namespace
+
+void MostVisitedSitesProvider::Start(const AutocompleteInput& input,
+                                     bool minimal_changes) {
+  Stop(true, false);
+  if (!AllowMostVisitedSitesSuggestions(input))
+    return;
+
+  scoped_refptr<history::TopSites> top_sites = client_->GetTopSites();
+  if (!top_sites)
+    return;
+
+  top_sites->GetMostVisitedURLs(
+      base::BindRepeating(&MostVisitedSitesProvider::OnMostVisitedUrlsAvailable,
+                          request_weak_ptr_factory_.GetWeakPtr()));
+}
+
+void MostVisitedSitesProvider::Stop(bool clear_cached_results,
+                                    bool due_to_user_inactivity) {
+  request_weak_ptr_factory_.InvalidateWeakPtrs();
+  matches_.clear();
+}
+
+MostVisitedSitesProvider::MostVisitedSitesProvider(
+    AutocompleteProviderClient* client,
+    AutocompleteProviderListener* listener)
+    : AutocompleteProvider(TYPE_MOST_VISITED_SITES),
+      client_{client},
+      listener_{listener} {}
+
+MostVisitedSitesProvider::~MostVisitedSitesProvider() = default;
+
+AutocompleteMatch MostVisitedSitesProvider::BuildMatch(
+    const base::string16& description,
+    const GURL& url,
+    int relevance,
+    AutocompleteMatchType::Type type) {
+  AutocompleteMatch match(this, relevance, false, type);
+  match.destination_url = url;
+
+  match.fill_into_edit +=
+      AutocompleteInput::FormattedStringWithEquivalentMeaning(
+          url, url_formatter::FormatUrl(url), client_->GetSchemeClassifier(),
+          nullptr);
+
+  // Zero suggest results should always omit protocols and never appear bold.
+  auto format_types = AutocompleteMatch::GetFormatTypes(false, false);
+  match.contents = url_formatter::FormatUrl(
+      url, format_types, net::UnescapeRule::SPACES, nullptr, nullptr, nullptr);
+  match.contents_class = ClassifyTermMatches({}, match.contents.length(), 0,
+                                             ACMatchClassification::URL);
+
+  match.description = AutocompleteMatch::SanitizeString(description);
+  match.description_class = ClassifyTermMatches({}, match.description.length(),
+                                                0, ACMatchClassification::NONE);
+
+  return match;
+}
+
+void MostVisitedSitesProvider::OnMostVisitedUrlsAvailable(
+    const history::MostVisitedURLList& urls) {
+  if (urls.empty())
+    return;
+
+  if (base::FeatureList::IsEnabled(omnibox::kMostVisitedTiles)) {
+    AutocompleteMatch match = BuildMatch(
+        base::string16(), GURL::EmptyGURL(), kMostVisitedTilesRelevance,
+        AutocompleteMatchType::TILE_NAVSUGGEST);
+    match.navsuggest_tiles.reserve(urls.size());
+
+    for (const auto& url : urls) {
+      match.navsuggest_tiles.push_back({url.url, url.title});
+    }
+    matches_.push_back(std::move(match));
+  } else {
+    int relevance = 600;
+    for (const auto& url : urls) {
+      matches_.emplace_back(BuildMatch(url.title, url.url, relevance,
+                                       AutocompleteMatchType::NAVSUGGEST));
+      --relevance;
+    }
+  }
+  listener_->OnProviderUpdate(true);
+}
+
+bool MostVisitedSitesProvider::AllowMostVisitedSitesSuggestions(
+    const AutocompleteInput& input) const {
+  const auto& page_url = input.current_url();
+  const auto page_class = input.current_page_classification();
+  const auto input_type = input.type();
+
+  if (input.focus_type() == OmniboxFocusType::DEFAULT)
+    return false;
+
+  if (client_->IsOffTheRecord())
+    return false;
+
+  // Only serve Most Visited suggestions when the current context is page visit.
+  if (page_class != metrics::OmniboxEventProto::OTHER)
+    return false;
+
+  // When omnibox contains pre-populated content, only show zero suggest for
+  // pages with URLs the user will recognize.
+  //
+  // This list intentionally does not include items such as ftp: and file:
+  // because (a) these do not work on Android and iOS, where most visited
+  // zero suggest is launched and (b) on desktop, where contextual zero suggest
+  // is running, these types of schemes aren't eligible to be sent to the
+  // server to ask for suggestions (and thus in practice we won't display zero
+  // suggest for them).
+  if (input_type != metrics::OmniboxInputType::EMPTY &&
+      !(page_url.is_valid() &&
+        ((page_url.scheme() == url::kHttpScheme) ||
+         (page_url.scheme() == url::kHttpsScheme) ||
+         (page_url.scheme() == url::kAboutScheme) ||
+         (page_url.scheme() ==
+          client_->GetEmbedderRepresentationOfAboutScheme())))) {
+    return false;
+  }
+
+  return true;
+}

components/omnibox/browser/most_visited_sites_provider.h

+// Copyright (c) 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_OMNIBOX_BROWSER_MOST_VISITED_SITES_PROVIDER_H_
+#define COMPONENTS_OMNIBOX_BROWSER_MOST_VISITED_SITES_PROVIDER_H_
+
+#include <memory>
+#include <string>
+
+#include "base/compiler_specific.h"
+#include "base/memory/weak_ptr.h"
+#include "components/omnibox/browser/autocomplete_input.h"
+#include "components/omnibox/browser/autocomplete_provider.h"
+#include "components/omnibox/browser/autocomplete_provider_client.h"
+#include "components/omnibox/browser/autocomplete_provider_listener.h"
+#include "third_party/metrics_proto/omnibox_event.pb.h"
+
+// Autocomplete provider serving Most Visited Sites in zero-prefix context.
+// Serves most frequently visited URLs in a form of either individual- or
+// aggregate suggestions.
+class MostVisitedSitesProvider : public AutocompleteProvider {
+ public:
+  MostVisitedSitesProvider(AutocompleteProviderClient* client,
+                           AutocompleteProviderListener* listener);
+
+  void Start(const AutocompleteInput& input, bool minimal_changes) override;
+  void Stop(bool clear_cached_results, bool due_to_user_inactivity) override;
+
+ private:
+  FRIEND_TEST_ALL_PREFIXES(MostVisitedSitesProviderTest,
+                           AllowMostVisitedSitesSuggestions);
+
+  ~MostVisitedSitesProvider() override;
+
+  // Constructs an AutocompleteMatch from supplied details.
+  AutocompleteMatch BuildMatch(const base::string16& description,
+                               const GURL& url,
+                               int relevance,
+                               AutocompleteMatchType::Type type);
+
+  // When the TopSites service serves the most visited URLs, this function
+  // converts those urls to AutocompleteMatches and adds them to |matches_|.
+  void OnMostVisitedUrlsAvailable(const history::MostVisitedURLList& urls);
+
+  // Whether zero suggest suggestions are allowed in the given context.
+  // Invoked early, confirms all the external conditions for ZeroSuggest are
+  // met.
+  bool AllowMostVisitedSitesSuggestions(const AutocompleteInput& input) const;
+
+  AutocompleteProviderClient* const client_;
+  AutocompleteProviderListener* const listener_;
+  // Note: used to cancel requests - not a general purpose WeakPtr factory.
+  base::WeakPtrFactory<MostVisitedSitesProvider> request_weak_ptr_factory_{
+      this};
+};
+
+#endif  // COMPONENTS_OMNIBOX_BROWSER_MOST_VISITED_SITES_PROVIDER_H_

components/omnibox/browser/zero_suggest_provider.cc

@@ -91,9 +91,6 @@ void LogOmniboxZeroSuggestRequest(
 
 // Relevance value to use if it was not set explicitly by the server.
 const int kDefaultZeroSuggestRelevance = 100;
-// The relevance score for navsuggest tiles.
-// Navsuggest tiles should be positioned below the Query Tiles object.
-const int kMostVisitedTilesRelevance = 1500;
 
 // Used for testing whether zero suggest is ever available.
 constexpr char kArbitraryInsecureUrlString[] = "http://www.google.com/";
@@ -183,21 +180,6 @@ void ZeroSuggestProvider::Start(const AutocompleteInput& input,
 
   MaybeUseCachedSuggestions();
 
-  if (result_type_running_ == MOST_VISITED) {
-    most_visited_urls_.clear();
-    scoped_refptr<history::TopSites> ts = client()->GetTopSites();
-    if (!ts) {
-      done_ = true;
-      result_type_running_ = NONE;
-      return;
-    }
-
-    ts->GetMostVisitedURLs(base::BindRepeating(
-        &ZeroSuggestProvider::OnMostVisitedUrlsAvailable,
-        weak_ptr_factory_.GetWeakPtr(), most_visited_request_num_));
-    return;
-  }
-
   search_terms_args.current_page_url =
       result_type_running_ == REMOTE_SEND_URL ? current_query_ : std::string();
   // Create a request for suggestions, routing completion to
@@ -224,7 +206,6 @@ void ZeroSuggestProvider::Stop(bool clear_cached_results,
   // the TopSites::GetMostVisitedURLs request.
   done_ = true;
   result_type_running_ = NONE;
-  ++most_visited_request_num_;
 
   if (clear_cached_results) {
     // We do not call Clear() on |results_| to retain |verbatim_relevance|
@@ -237,7 +218,6 @@ void ZeroSuggestProvider::Stop(bool clear_cached_results,
     results_.headers_map.clear();
     current_query_.clear();
     current_title_.clear();
-    most_visited_urls_.clear();
   }
 }
 
@@ -259,9 +239,7 @@ void ZeroSuggestProvider::DeleteMatch(const AutocompleteMatch& match) {
 
 void ZeroSuggestProvider::AddProviderInfo(ProvidersInfo* provider_info) const {
   BaseSearchProvider::AddProviderInfo(provider_info);
-  if (!results_.suggest_results.empty() ||
-      !results_.navigation_results.empty() ||
-      !most_visited_urls_.empty())
+  if (!results_.suggest_results.empty() || !results_.navigation_results.empty())
     provider_info->back().set_times_returned_results_in_session(1);
 }
 
@@ -352,7 +330,6 @@ void ZeroSuggestProvider::OnURLLoadComplete(
   loader_.reset();
   done_ = true;
   result_type_running_ = NONE;
-  ++most_visited_request_num_;
   listener_->OnProviderUpdate(results_updated);
 }
 
@@ -415,21 +392,6 @@ AutocompleteMatch ZeroSuggestProvider::NavigationToMatch(
   return match;
 }
 
-void ZeroSuggestProvider::OnMostVisitedUrlsAvailable(
-    size_t orig_request_num,
-    const history::MostVisitedURLList& urls) {
-  if (result_type_running_ != MOST_VISITED ||
-      orig_request_num != most_visited_request_num_) {
-    return;
-  }
-  most_visited_urls_ = urls;
-  done_ = true;
-  ConvertResultsToAutocompleteMatches();
-  result_type_running_ = NONE;
-  ++most_visited_request_num_;
-  listener_->OnProviderUpdate(true);
-}
-
 void ZeroSuggestProvider::OnRemoteSuggestionsLoaderAvailable(
     std::unique_ptr<network::SimpleURLLoader> loader) {
   // RemoteSuggestionsService has already started |loader|, so here it's
@@ -468,63 +430,9 @@ void ZeroSuggestProvider::ConvertResultsToAutocompleteMatches() {
   UMA_HISTOGRAM_COUNTS_1M("ZeroSuggest.URLResults", num_nav_results);
   UMA_HISTOGRAM_COUNTS_1M("ZeroSuggest.AllResults", num_results);
 
-  // Show Most Visited results after ZeroSuggest response is received.
-  if (result_type_running_ == MOST_VISITED) {
-    // Ensure we don't show most visited URL suggestions on NTP.
-    // This allows us to prevent undesired side outcome of presenting
-    // URL suggestions to users who are not in the personalized field trial for
-    // zero query suggestions.
-    if (IsNTPPage(current_page_classification_) ||
-        !current_text_match_.destination_url.is_valid()) {
-      return;
-    }
-    matches_.push_back(current_text_match_);
-
-    // Short-circuit in case we have no MOST_VISITED urls to show.
-    if (most_visited_urls_.empty())
-      return;
-
-    if (base::FeatureList::IsEnabled(omnibox::kMostVisitedTiles)) {
-      AutocompleteMatch match =
-          NavigationToMatch(SearchSuggestionParser::NavigationResult(
-              client()->GetSchemeClassifier(), GURL::EmptyGURL(),
-              AutocompleteMatchType::TILE_NAVSUGGEST, {}, base::string16(),
-              std::string(), false, kMostVisitedTilesRelevance, true,
-              base::ASCIIToUTF16(current_query_)));
-      match.navsuggest_tiles.reserve(most_visited_urls_.size());
-
-      for (const auto& url : most_visited_urls_) {
-        match.navsuggest_tiles.push_back({url.url, url.title});
-      }
-      matches_.push_back(std::move(match));
-    } else {
-      int relevance = 600;
-      for (const auto& url : most_visited_urls_) {
-        SearchSuggestionParser::NavigationResult nav(
-            client()->GetSchemeClassifier(), url.url,
-            AutocompleteMatchType::NAVSUGGEST, {}, url.title, std::string(),
-            false, relevance, true, base::ASCIIToUTF16(current_query_));
-        matches_.push_back(NavigationToMatch(nav));
-        --relevance;
-      }
-    }
-    return;
-  }
-
   if (num_results == 0)
     return;
 
-#if defined(OS_ANDROID) || defined(OS_IOS)
-  // Android needs the verbatim match on non-NTP surfaces to properly present
-  // the Search Ready Omnibox URL edit widget. Desktop specifically does NOT
-  // want to show verbatim matches in remotely-fetched ZeroSuggest anymore.
-  // iOS we are keeping the same as Android for now. No strong reason to change.
-  if (!IsNTPPage(current_page_classification_) &&
-      current_text_match_.destination_url.is_valid()) {
-    matches_.push_back(current_text_match_);
-  }
-#endif
-
   for (MatchMap::const_iterator it(map.begin()); it != map.end(); ++it)
     matches_.push_back(it->second);
 
@@ -713,12 +621,5 @@ ZeroSuggestProvider::ResultType ZeroSuggestProvider::TypeOfResultToRun(
   if (IsNTPPage(current_page_classification) && remote_no_url_allowed)
     return REMOTE_NO_URL;
 
-#if defined(OS_ANDROID) || defined(OS_IOS)
-  // For Android and iOS, default to MOST_VISITED everywhere except on the SERP.
-  if (!IsSearchResultsPage(current_page_classification)) {
-    return MOST_VISITED;
-  }
-#endif
-
   return NONE;
 }

components/omnibox/browser/zero_suggest_provider.h

@@ -55,9 +55,6 @@ class ZeroSuggestProvider : public BaseSearchProvider {
     // suggestions. The endpoint is sent the user's authentication state and
     // the current URL.
     REMOTE_SEND_URL,
-
-    // Gets the most visited sites from local history.
-    MOST_VISITED,
   };
 
   // Creates and returns an instance of this provider.
@@ -152,12 +149,6 @@ class ZeroSuggestProvider : public BaseSearchProvider {
   // page.
   AutocompleteMatch MatchForCurrentText();
 
-  // When the user is in the Most Visited field trial, we ask the TopSites
-  // service for the most visited URLs. It then calls back to this function to
-  // return those |urls|.
-  void OnMostVisitedUrlsAvailable(size_t request_num,
-                                  const history::MostVisitedURLList& urls);
-
   // When the user is in the remote omnibox suggestions field trial, we ask
   // the RemoteSuggestionsService for a loader to retrieve recommendations.
   // When the loader has started, the remote suggestion service then calls
@@ -191,9 +182,6 @@ class ZeroSuggestProvider : public BaseSearchProvider {
   // When the provider is not running, the result type is set to NONE.
   ResultType result_type_running_;
 
-  // For reconciling asynchronous requests for most visited URLs.
-  size_t most_visited_request_num_ = 0;
-
   // The URL for which a suggestion fetch is pending.
   std::string current_query_;
 
@@ -218,8 +206,6 @@ class ZeroSuggestProvider : public BaseSearchProvider {
   // the response for the most recent zero suggest input URL.
   SearchSuggestionParser::Results results_;
 
-  history::MostVisitedURLList most_visited_urls_;
-
   // For callbacks that may be run after destruction.
   base::WeakPtrFactory<ZeroSuggestProvider> weak_ptr_factory_{this};
 };

components/omnibox/browser/zero_suggest_provider_unittest.cc

@@ -36,59 +36,10 @@
 #include "third_party/metrics_proto/omnibox_event.pb.h"
 
 namespace {
-class FakeEmptyTopSites : public history::TopSites {
- public:
-  FakeEmptyTopSites() {
-  }
-  FakeEmptyTopSites(const FakeEmptyTopSites&) = delete;
-  FakeEmptyTopSites& operator=(const FakeEmptyTopSites&) = delete;
-
-  // history::TopSites:
-  void GetMostVisitedURLs(GetMostVisitedURLsCallback callback) override;
-  void SyncWithHistory() override {}
-  bool HasBlockedUrls() const override { return false; }
-  void AddBlockedUrl(const GURL& url) override {}
-  void RemoveBlockedUrl(const GURL& url) override {}
-  bool IsBlocked(const GURL& url) override { return false; }
-  void ClearBlockedUrls() override {}
-  bool IsFull() override { return false; }
-  bool loaded() const override {
-    return false;
-  }
-  history::PrepopulatedPageList GetPrepopulatedPages() override {
-    return history::PrepopulatedPageList();
-  }
-  void OnNavigationCommitted(const GURL& url) override {}
-
-  // RefcountedKeyedService:
-  void ShutdownOnUIThread() override {}
-
-  // Only runs a single callback, so that the test can specify a different
-  // set per call.
-  void RunACallback(const history::MostVisitedURLList& urls) {
-    DCHECK(!callbacks.empty());
-    std::move(callbacks.front()).Run(urls);
-    callbacks.pop_front();
-  }
-
- protected:
-  // A test-specific field for controlling when most visited callback is run
-  // after top sites have been requested.
-  std::list<GetMostVisitedURLsCallback> callbacks;
-
-  ~FakeEmptyTopSites() override {}
-};
-
-void FakeEmptyTopSites::GetMostVisitedURLs(
-    GetMostVisitedURLsCallback callback) {
-  callbacks.push_back(std::move(callback));
-}
-
 class FakeAutocompleteProviderClient : public MockAutocompleteProviderClient {
  public:
   FakeAutocompleteProviderClient()
-      : template_url_service_(new TemplateURLService(nullptr, 0)),
-        top_sites_(new FakeEmptyTopSites()) {
+      : template_url_service_(new TemplateURLService(nullptr, 0)) {
     pref_service_.registry()->RegisterStringPref(
         omnibox::kZeroSuggestCachedResults, std::string());
   }
@@ -99,8 +50,6 @@ class FakeAutocompleteProviderClient : public MockAutocompleteProviderClient {
 
   bool SearchSuggestEnabled() const override { return true; }
 
-  scoped_refptr<history::TopSites> GetTopSites() override { return top_sites_; }
-
   TemplateURLService* GetTemplateURLService() override {
     return template_url_service_.get();
   }
@@ -131,11 +80,9 @@ class FakeAutocompleteProviderClient : public MockAutocompleteProviderClient {
 
  private:
   std::unique_ptr<TemplateURLService> template_url_service_;
-  scoped_refptr<history::TopSites> top_sites_;
   TestingPrefServiceSimple pref_service_;
   TestSchemeClassifier scheme_classifier_;
 };
-
 }  // namespace
 
 class ZeroSuggestProviderTest : public testing::Test,
@@ -222,9 +169,6 @@ TEST_F(ZeroSuggestProviderTest, AllowZeroSuggestSuggestions) {
   // ZeroSuggest should never deal with prefix suggestions.
   EXPECT_FALSE(provider_->AllowZeroSuggestSuggestions(prefix_input));
 
-  // This should always be true, as otherwise we will break MostVisited.
-  // TODO(tommycli): We should split this into its own provider to avoid
-  // breaking it again.
   EXPECT_TRUE(provider_->AllowZeroSuggestSuggestions(on_focus_input));
 
   EXPECT_FALSE(provider_->AllowZeroSuggestSuggestions(on_clobber_input));
@@ -263,22 +207,11 @@ TEST_F(ZeroSuggestProviderTest, TypeOfResultToRun) {
         GURL suggest_url = GetSuggestURL(current_page_classification);
         const auto result_type = ZeroSuggestProvider::TypeOfResultToRun(
             client_.get(), input, suggest_url);
-#if !defined(OS_ANDROID) && !defined(OS_IOS)  // Desktop
         EXPECT_EQ(BaseSearchProvider::IsNTPPage(current_page_classification) &&
                           remote_no_url_allowed
                       ? ZeroSuggestProvider::ResultType::REMOTE_NO_URL
                       : ZeroSuggestProvider::ResultType::NONE,
                   result_type);
-#else                                         // Android and iOS
-        EXPECT_EQ(BaseSearchProvider::IsNTPPage(current_page_classification) &&
-                          remote_no_url_allowed
-                      ? ZeroSuggestProvider::ResultType::REMOTE_NO_URL
-                      : !BaseSearchProvider::IsSearchResultsPage(
-                            current_page_classification)
-                            ? ZeroSuggestProvider::ResultType::MOST_VISITED
-                            : ZeroSuggestProvider::ResultType::NONE,
-                  result_type);
-#endif
       };
 
   // Verify OTHER defaults (contextual web).
@@ -352,13 +285,8 @@ TEST_F(ZeroSuggestProviderTest, TypeOfResultToRunForContextualWeb) {
   on_clobber_input.set_current_url(GURL(input_url));
   on_clobber_input.set_focus_type(OmniboxFocusType::DELETED_PERMANENT_TEXT);
 
-#if defined(OS_ANDROID) || defined(OS_IOS)
-  const ZeroSuggestProvider::ResultType kDefaultContextualWebResultType =
-      ZeroSuggestProvider::ResultType::MOST_VISITED;
-#else
   const ZeroSuggestProvider::ResultType kDefaultContextualWebResultType =
       ZeroSuggestProvider::ResultType::NONE;
-#endif
 
   EXPECT_EQ(kDefaultContextualWebResultType,
             ZeroSuggestProvider::TypeOfResultToRun(
@@ -455,87 +383,6 @@ TEST_F(ZeroSuggestProviderTest, TestStartWillStopForSomeInput) {
   EXPECT_TRUE(provider_->done_);
 }
 
-// MostVisited in only ever enabled on Mobile platforms.
-#if defined(OS_IOS) || defined(OS_ANDROID)
-TEST_F(ZeroSuggestProviderTest, TestMostVisitedCallback) {
-  std::string current_url("http://www.foxnews.com/");
-  std::string input_url("http://www.cnn.com/");
-  AutocompleteInput input(base::ASCIIToUTF16(input_url),
-                          metrics::OmniboxEventProto::OTHER,
-                          TestSchemeClassifier());
-  input.set_current_url(GURL(current_url));
-  input.set_focus_type(OmniboxFocusType::ON_FOCUS);
-  history::MostVisitedURLList urls;
-  history::MostVisitedURL url(GURL("http://foo.com/"),
-                              base::ASCIIToUTF16("Foo"));
-  urls.push_back(url);
-
-  provider_->Start(input, false);
-  EXPECT_TRUE(provider_->matches().empty());
-  scoped_refptr<history::TopSites> top_sites = client_->GetTopSites();
-  static_cast<FakeEmptyTopSites*>(top_sites.get())->RunACallback(urls);
-  // Should have verbatim match + most visited url match.
-  EXPECT_EQ(2U, provider_->matches().size());
-  provider_->Stop(false, false);
-
-  provider_->Start(input, false);
-  provider_->Stop(false, false);
-  EXPECT_TRUE(provider_->matches().empty());
-  // Most visited results arriving after Stop() has been called, ensure they
-  // are not displayed.
-  static_cast<FakeEmptyTopSites*>(top_sites.get())->RunACallback(urls);
-  EXPECT_TRUE(provider_->matches().empty());
-
-  history::MostVisitedURLList urls2;
-  urls2.push_back(history::MostVisitedURL(GURL("http://bar.com/"),
-                                          base::ASCIIToUTF16("Bar")));
-  urls2.push_back(history::MostVisitedURL(GURL("http://zinga.com/"),
-                                          base::ASCIIToUTF16("Zinga")));
-  provider_->Start(input, false);
-  provider_->Stop(false, false);
-  provider_->Start(input, false);
-  static_cast<FakeEmptyTopSites*>(top_sites.get())->RunACallback(urls);
-  // Stale results should get rejected.
-  EXPECT_TRUE(provider_->matches().empty());
-  static_cast<FakeEmptyTopSites*>(top_sites.get())->RunACallback(urls2);
-  EXPECT_FALSE(provider_->matches().empty());
-  provider_->Stop(false, false);
-}
-
-TEST_F(ZeroSuggestProviderTest, TestMostVisitedNavigateToSearchPage) {
-  std::string current_url("http://www.foxnews.com/");
-  std::string input_url("http://www.cnn.com/");
-  AutocompleteInput input(base::ASCIIToUTF16(input_url),
-                          metrics::OmniboxEventProto::OTHER,
-                          TestSchemeClassifier());
-  input.set_current_url(GURL(current_url));
-  input.set_focus_type(OmniboxFocusType::ON_FOCUS);
-  history::MostVisitedURLList urls;
-  history::MostVisitedURL url(GURL("http://foo.com/"),
-                              base::ASCIIToUTF16("Foo"));
-  urls.push_back(url);
-
-  provider_->Start(input, false);
-  EXPECT_TRUE(provider_->matches().empty());
-  // Stop() doesn't always get called.
-
-  std::string search_url("https://www.google.com/?q=flowers");
-  AutocompleteInput srp_input(
-      base::ASCIIToUTF16(search_url),
-      metrics::OmniboxEventProto::SEARCH_RESULT_PAGE_NO_SEARCH_TERM_REPLACEMENT,
-      TestSchemeClassifier());
-  srp_input.set_current_url(GURL(search_url));
-  srp_input.set_focus_type(OmniboxFocusType::ON_FOCUS);
-
-  provider_->Start(srp_input, false);
-  EXPECT_TRUE(provider_->matches().empty());
-  // Most visited results arriving after a new request has been started.
-  scoped_refptr<history::TopSites> top_sites = client_->GetTopSites();
-  static_cast<FakeEmptyTopSites*>(top_sites.get())->RunACallback(urls);
-  EXPECT_TRUE(provider_->matches().empty());
-}
-#endif  // defined(OS_IOS) || defined(OS_ANDROID)
-
 TEST_F(ZeroSuggestProviderTest, TestPsuggestZeroSuggestCachingFirstRun) {
   EXPECT_CALL(*client_, IsAuthenticated())
       .WillRepeatedly(testing::Return(true));

components/omnibox/browser/zero_suggest_verbatim_match_provider.cc

@@ -4,9 +4,11 @@
 
 #include "components/omnibox/browser/zero_suggest_verbatim_match_provider.h"
 
+#include "base/feature_list.h"
 #include "components/omnibox/browser/autocomplete_provider_client.h"
 #include "components/omnibox/browser/autocomplete_provider_listener.h"
 #include "components/omnibox/browser/verbatim_match.h"
+#include "components/omnibox/common/omnibox_features.h"
 
 namespace {
 // The relevance score for verbatim match.
@@ -23,6 +25,8 @@ bool IsVerbatimMatchEligible(
         SEARCH_RESULT_PAGE_DOING_SEARCH_TERM_REPLACEMENT:
     case metrics::OmniboxEventProto::
         SEARCH_RESULT_PAGE_NO_SEARCH_TERM_REPLACEMENT:
+      return base::FeatureList::IsEnabled(
+          omnibox::kOmniboxSearchReadyIncognito);
     case metrics::OmniboxEventProto::OTHER:
       return true;
     default:
@@ -34,7 +38,7 @@ bool IsVerbatimMatchEligible(
 
 ZeroSuggestVerbatimMatchProvider::ZeroSuggestVerbatimMatchProvider(
     AutocompleteProviderClient* client)
-    : AutocompleteProvider(TYPE_ZERO_SUGGEST), client_(client) {}
+    : AutocompleteProvider(TYPE_VERBATIM_MATCH), client_(client) {}
 
 ZeroSuggestVerbatimMatchProvider::~ZeroSuggestVerbatimMatchProvider() = default;
 
@@ -46,21 +50,28 @@ void ZeroSuggestVerbatimMatchProvider::Start(const AutocompleteInput& input,
 
   // Only offer verbatim match after the user just focused the Omnibox,
   // or if the input field is empty.
-  if (!((input.focus_type() == OmniboxFocusType::ON_FOCUS) ||
-        (input.type() == metrics::OmniboxInputType::EMPTY))) {
+  if (input.focus_type() == OmniboxFocusType::DEFAULT)
     return;
-  }
-  // Do not offer verbatim match, if the Omnibox does not contain a valid URL.
-  if (!input.current_url().is_valid())
+
+  // For consistency with other zero-prefix providers.
+  const auto& page_url = input.current_url();
+  if (input.type() != metrics::OmniboxInputType::EMPTY &&
+      !(page_url.is_valid() &&
+        ((page_url.scheme() == url::kHttpScheme) ||
+         (page_url.scheme() == url::kHttpsScheme) ||
+         (page_url.scheme() == url::kAboutScheme) ||
+         (page_url.scheme() ==
+          client_->GetEmbedderRepresentationOfAboutScheme())))) {
     return;
+  }
 
   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, input.current_url(), input.current_title(),
-      nullptr, kVerbatimMatchRelevanceScore);
+      client_, verbatim_input, page_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
@@ -75,4 +86,4 @@ void ZeroSuggestVerbatimMatchProvider::Start(const AutocompleteInput& input,
 void ZeroSuggestVerbatimMatchProvider::Stop(bool clear_cached_results,
                                             bool due_to_user_inactivity) {
   matches_.clear();
-}
+}
\ No newline at end of file

components/omnibox/browser/zero_suggest_verbatim_match_provider_unittest.cc

@@ -9,9 +9,11 @@
 #include <memory>
 #include <string>
 
+#include "base/feature_list.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/omnibox/browser/mock_autocomplete_provider_client.h"
 #include "components/omnibox/browser/test_scheme_classifier.h"
+#include "components/omnibox/common/omnibox_features.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/metrics_proto/omnibox_event.pb.h"
@@ -32,11 +34,13 @@ class ZeroSuggestVerbatimMatchProviderTest
 bool ZeroSuggestVerbatimMatchProviderTest::IsVerbatimMatchEligible() const {
   switch (GetParam()) {
     case metrics::OmniboxEventProto::OTHER:
+      return true;
     case metrics::OmniboxEventProto::
         SEARCH_RESULT_PAGE_DOING_SEARCH_TERM_REPLACEMENT:
     case metrics::OmniboxEventProto::
         SEARCH_RESULT_PAGE_NO_SEARCH_TERM_REPLACEMENT:
-      return true;
+      return base::FeatureList::IsEnabled(
+          omnibox::kOmniboxSearchReadyIncognito);
     default:
       return false;
   }