Adding a call from ChromeACProvider to Search Prefetch service

This is called when the AC matches change. The result will be used in
the future to check top result, all results gone (i.e., we can cancel
ongoing prefetches).

Bug: 1138631
Change-Id: I7b6bad212888eac36756f41f2f534233df596b04
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2493461
Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Robert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820409}

chrome/browser/autocomplete/chrome_autocomplete_provider_client.cc

@@ -25,6 +25,8 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/history/top_sites_factory.h"
+#include "chrome/browser/prefetch/search_prefetch/search_prefetch_service.h"
+#include "chrome/browser/prefetch/search_prefetch/search_prefetch_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_key.h"
 #include "chrome/browser/query_tiles/tile_service_factory.h"
@@ -461,6 +463,16 @@ bool ChromeAutocompleteProviderClient::IsBrowserUpdateAvailable() const {
 #endif
 }
 
+void ChromeAutocompleteProviderClient::OnAutocompleteControllerResultReady(
+    AutocompleteController* controller) {
+  auto* search_prefetch_service =
+      SearchPrefetchServiceFactory::GetForProfile(profile_);
+
+  // Prefetches result pages that the search provider marked as prefetchable.
+  if (search_prefetch_service)
+    search_prefetch_service->OnResultChanged(controller);
+}
+
 bool ChromeAutocompleteProviderClient::StrippedURLsAreEqual(
     const GURL& url1,
     const GURL& url2,

chrome/browser/autocomplete/chrome_autocomplete_provider_client.h

@@ -84,6 +84,8 @@ class ChromeAutocompleteProviderClient : public AutocompleteProviderClient {
   bool IsTabOpenWithURL(const GURL& url,
                         const AutocompleteInput* input) override;
   bool IsBrowserUpdateAvailable() const override;
+  void OnAutocompleteControllerResultReady(
+      AutocompleteController* controller) override;
 
   // For testing.
   void set_storage_partition(content::StoragePartition* storage_partition) {

chrome/browser/prefetch/search_prefetch/search_prefetch_service.cc

@@ -11,6 +11,8 @@
 #include "chrome/browser/prefetch/search_prefetch/prefetched_response_container.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
+#include "components/omnibox/browser/autocomplete_controller.h"
+#include "components/omnibox/browser/base_search_provider.h"
 #include "components/search_engines/template_url_service.h"
 #include "components/variations/net/variations_http_headers.h"
 #include "content/public/browser/browser_context.h"
@@ -247,3 +249,15 @@ void SearchPrefetchService::DeletePrefetch(base::string16 search_terms) {
 void SearchPrefetchService::ReportError() {
   last_error_time_ticks_ = base::TimeTicks::Now();
 }
+
+void SearchPrefetchService::OnResultChanged(
+    AutocompleteController* controller) {
+  const auto& result = controller->result();
+  for (const auto& match : result) {
+    // TODO(ryansturm): Pass a bool for IsTopResult to limit prefetch to when
+    // the match is the first item in the omnibox. https://crbug.com/1138649
+    if (BaseSearchProvider::ShouldPrefetch(match)) {
+      MaybePrefetchURL(match.destination_url);
+    }
+  }
+}

chrome/browser/prefetch/search_prefetch/search_prefetch_service.h

@@ -19,6 +19,8 @@ class Profile;
 class GURL;
 class PrefetchedResponseContainer;
 
+class AutocompleteController;
+
 enum class SearchPrefetchStatus {
   // The request is on the network and may move to any other state.
   kInFlight = 1,
@@ -36,6 +38,9 @@ class SearchPrefetchService : public KeyedService {
   SearchPrefetchService(const SearchPrefetchService&) = delete;
   SearchPrefetchService& operator=(const SearchPrefetchService&) = delete;
 
+  // Called when |controller| has updated information.
+  void OnResultChanged(AutocompleteController* controller);
+
   // Returns whether the prefetch started or not.
   bool MaybePrefetchURL(const GURL& url);
 

chrome/browser/prefetch/search_prefetch/search_prefetch_service_browsertest.cc

@@ -6,15 +6,23 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/time/time.h"
+#include "chrome/browser/autocomplete/chrome_autocomplete_scheme_classifier.h"
 #include "chrome/browser/prefetch/search_prefetch/field_trial_settings.h"
 #include "chrome/browser/prefetch/search_prefetch/search_prefetch_service.h"
 #include "chrome/browser/prefetch/search_prefetch/search_prefetch_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/location_bar/location_bar.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/search_test_utils.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/omnibox/browser/autocomplete_input.h"
+#include "components/omnibox/browser/autocomplete_match.h"
+#include "components/omnibox/browser/autocomplete_provider.h"
+#include "components/omnibox/browser/omnibox_popup_model.h"
+#include "components/omnibox/browser/omnibox_view.h"
 #include "components/prefs/pref_service.h"
 #include "components/search_engines/template_url_service.h"
 #include "content/public/browser/browser_task_traits.h"
@@ -29,6 +37,13 @@
 #include "net/test/embedded_test_server/http_response.h"
 #include "url/gurl.h"
 
+namespace {
+constexpr char kSuggestDomain[] = "suggest.com";
+constexpr char kSearchDomain[] = "search.com";
+constexpr char kOmniboxSuggestPrefetchQuery[] = "porgs";
+constexpr char kOmniboxSuggestNonPrefetchQuery[] = "puffins";
+}  // namespace
+
 class SearchPrefetchBaseBrowserTest : public InProcessBrowserTest {
  public:
   SearchPrefetchBaseBrowserTest() {
@@ -39,12 +54,21 @@ class SearchPrefetchBaseBrowserTest : public InProcessBrowserTest {
         base::BindRepeating(&SearchPrefetchBaseBrowserTest::HandleSearchRequest,
                             base::Unretained(this)));
     EXPECT_TRUE(search_server_->Start());
+
+    search_suggest_server_ = std::make_unique<net::EmbeddedTestServer>(
+        net::EmbeddedTestServer::TYPE_HTTPS);
+    search_suggest_server_->ServeFilesFromSourceDirectory("chrome/test/data");
+    search_suggest_server_->RegisterRequestHandler(base::BindRepeating(
+        &SearchPrefetchBaseBrowserTest::HandleSearchSuggestRequest,
+        base::Unretained(this)));
+    EXPECT_TRUE(search_suggest_server_->Start());
   }
 
   void SetUpOnMainThread() override {
     InProcessBrowserTest::SetUpOnMainThread();
 
-    host_resolver()->AddRule("search.test", "127.0.0.1");
+    host_resolver()->AddRule(kSearchDomain, "127.0.0.1");
+    host_resolver()->AddRule(kSuggestDomain, "127.0.0.1");
 
     TemplateURLService* model =
         TemplateURLServiceFactory::GetForProfile(browser()->profile());
@@ -53,9 +77,12 @@ class SearchPrefetchBaseBrowserTest : public InProcessBrowserTest {
     ASSERT_TRUE(model->loaded());
 
     TemplateURLData data;
-    data.SetShortName(base::ASCIIToUTF16("search.test"));
+    data.SetShortName(base::ASCIIToUTF16(kSearchDomain));
     data.SetKeyword(data.short_name());
     data.SetURL(GetSearchServerQueryURL("{searchTerms}").spec());
+    data.suggestions_url =
+        search_suggest_server_->GetURL(kSuggestDomain, "/?q={searchTerms}")
+            .spec();
 
     TemplateURL* template_url = model->Add(std::make_unique<TemplateURL>(data));
     ASSERT_TRUE(template_url);
@@ -83,11 +110,11 @@ class SearchPrefetchBaseBrowserTest : public InProcessBrowserTest {
   }
 
   GURL GetSearchServerQueryURL(const std::string& path) const {
-    return search_server_->GetURL("search.test", "/search_page.html?q=" + path);
+    return search_server_->GetURL(kSearchDomain, "/search_page.html?q=" + path);
   }
 
   GURL GetSearchServerQueryURLWithNoQuery(const std::string& path) const {
-    return search_server_->GetURL("search.test", path);
+    return search_server_->GetURL(kSearchDomain, path);
   }
 
   void WaitUntilStatusChanges(base::string16 search_terms) {
@@ -102,6 +129,21 @@ class SearchPrefetchBaseBrowserTest : public InProcessBrowserTest {
     }
   }
 
+  void WaitUntilStatusChangesTo(base::string16 search_terms,
+                                SearchPrefetchStatus status) {
+    auto* search_prefetch_service =
+        SearchPrefetchServiceFactory::GetForProfile(browser()->profile());
+    while (!search_prefetch_service
+                ->GetSearchPrefetchStatusForTesting(search_terms)
+                .has_value() ||
+           status != search_prefetch_service
+                         ->GetSearchPrefetchStatusForTesting(search_terms)
+                         .value()) {
+      base::RunLoop run_loop;
+      run_loop.RunUntilIdle();
+    }
+  }
+
   content::WebContents* GetWebContents() const {
     return browser()->tab_strip_model()->GetActiveWebContents();
   }
@@ -173,9 +215,52 @@ class SearchPrefetchBaseBrowserTest : public InProcessBrowserTest {
     }
   }
 
+  std::unique_ptr<net::test_server::HttpResponse> HandleSearchSuggestRequest(
+      const net::test_server::HttpRequest& request) {
+    // |content| is a json request that contains the search suggest response.
+    // The first item is the query (not used), the second is the results list,
+    // the third is descriptions, fifth is an extra data dictionary. The
+    // google:clientdata contains "phi" which is the prefetch index (i.e., which
+    // suggest can be prefetched).
+    std::string content = "";
+
+    if (request.GetURL().spec().find(kOmniboxSuggestPrefetchQuery) !=
+        std::string::npos) {
+      content = R"([
+      "porgs",
+      ["porgs","porgsandwich"],
+      ["", ""],
+      [],
+      {
+        "google:clientdata": {
+          "phi": 0
+        }
+      }])";
+    }
+
+    if (request.GetURL().spec().find(kOmniboxSuggestNonPrefetchQuery) !=
+        std::string::npos) {
+      content = R"([
+      "puffins",
+      ["puffins","puffinsalad"],
+      ["", ""],
+      [],
+      {}])";
+    }
+
+    std::unique_ptr<net::test_server::BasicHttpResponse> resp =
+        std::make_unique<net::test_server::BasicHttpResponse>();
+    resp->set_code(net::HTTP_OK);
+    resp->set_content_type("application/json");
+    resp->set_content(content);
+    return resp;
+  }
+
   std::vector<net::test_server::HttpRequest> search_server_requests_;
   std::unique_ptr<net::EmbeddedTestServer> search_server_;
 
+  std::unique_ptr<net::EmbeddedTestServer> search_suggest_server_;
+
   bool should_hang_requests_ = false;
 
   size_t search_server_request_count_ = 0;
@@ -494,6 +579,85 @@ IN_PROC_BROWSER_TEST_F(SearchPrefetchServiceEnabledBrowserTest,
       GetSearchServerQueryURL("other_query")));
 }
 
+IN_PROC_BROWSER_TEST_F(SearchPrefetchServiceEnabledBrowserTest,
+                       OmniboxEditTriggersPrefetch) {
+  auto* search_prefetch_service =
+      SearchPrefetchServiceFactory::GetForProfile(browser()->profile());
+  std::string search_terms = kOmniboxSuggestPrefetchQuery;
+
+  // Trigger an omnibox suggest fetch that has a prefetch hint.
+  AutocompleteInput input(
+      base::ASCIIToUTF16(search_terms), metrics::OmniboxEventProto::BLANK,
+      ChromeAutocompleteSchemeClassifier(browser()->profile()));
+  LocationBar* location_bar = browser()->window()->GetLocationBar();
+  OmniboxView* omnibox = location_bar->GetOmniboxView();
+  AutocompleteController* autocomplete_controller =
+      omnibox->model()->autocomplete_controller();
+
+  // Prevent the stop timer from killing the hints fetch early.
+  autocomplete_controller->SetStartStopTimerDurationForTesting(
+      base::TimeDelta::FromSeconds(10));
+  autocomplete_controller->Start(input);
+
+  ui_test_utils::WaitForAutocompleteDone(browser());
+  EXPECT_TRUE(autocomplete_controller->done());
+
+  WaitUntilStatusChangesTo(base::ASCIIToUTF16(search_terms),
+                           SearchPrefetchStatus::kSuccessfullyCompleted);
+  auto prefetch_status =
+      search_prefetch_service->GetSearchPrefetchStatusForTesting(
+          base::ASCIIToUTF16(search_terms));
+  ASSERT_TRUE(prefetch_status.has_value());
+  EXPECT_EQ(SearchPrefetchStatus::kSuccessfullyCompleted,
+            prefetch_status.value());
+
+  ui_test_utils::NavigateToURL(browser(),
+                               GetSearchServerQueryURL(search_terms));
+
+  auto inner_html = GetDocumentInnerHTML();
+
+  EXPECT_FALSE(base::Contains(inner_html, "regular"));
+  EXPECT_TRUE(base::Contains(inner_html, "prefetch"));
+}
+
+IN_PROC_BROWSER_TEST_F(SearchPrefetchServiceEnabledBrowserTest,
+                       OmniboxEditDoesNotTriggersPrefetch) {
+  auto* search_prefetch_service =
+      SearchPrefetchServiceFactory::GetForProfile(browser()->profile());
+  std::string search_terms = kOmniboxSuggestNonPrefetchQuery;
+
+  // Trigger an omnibox suggest fetch that does not have a prefetch hint.
+  AutocompleteInput input(
+      base::ASCIIToUTF16(search_terms), metrics::OmniboxEventProto::BLANK,
+      ChromeAutocompleteSchemeClassifier(browser()->profile()));
+  LocationBar* location_bar = browser()->window()->GetLocationBar();
+  OmniboxView* omnibox = location_bar->GetOmniboxView();
+  AutocompleteController* autocomplete_controller =
+      omnibox->model()->autocomplete_controller();
+
+  // Prevent the stop timer from killing the hints fetch early.
+  autocomplete_controller->SetStartStopTimerDurationForTesting(
+      base::TimeDelta::FromSeconds(10));
+  autocomplete_controller->Start(input);
+
+  ui_test_utils::WaitForAutocompleteDone(browser());
+  EXPECT_TRUE(autocomplete_controller->done());
+
+  WaitForDuration(base::TimeDelta::FromMilliseconds(100));
+
+  auto prefetch_status =
+      search_prefetch_service->GetSearchPrefetchStatusForTesting(
+          base::ASCIIToUTF16(search_terms));
+  EXPECT_FALSE(prefetch_status.has_value());
+  ui_test_utils::NavigateToURL(browser(),
+                               GetSearchServerQueryURL(search_terms));
+
+  auto inner_html = GetDocumentInnerHTML();
+
+  EXPECT_TRUE(base::Contains(inner_html, "regular"));
+  EXPECT_FALSE(base::Contains(inner_html, "prefetch"));
+}
+
 class SearchPrefetchServiceZeroCacheTimeBrowserTest
     : public SearchPrefetchBaseBrowserTest {
  public:

components/omnibox/browser/autocomplete_controller.cc

@@ -1003,3 +1003,8 @@ bool AutocompleteController::OnMemoryDump(
                   base::trace_event::MemoryAllocatorDump::kUnitsBytes, res);
   return true;
 }
+
+void AutocompleteController::SetStartStopTimerDurationForTesting(
+    base::TimeDelta duration) {
+  stop_timer_duration_ = duration;
+}

components/omnibox/browser/autocomplete_controller.h

@@ -172,6 +172,9 @@ class AutocompleteController : public AutocompleteProviderListener,
     return last_time_default_match_changed_;
   }
 
+  // Sets the provider timeout duration for future calls to |Start()|.
+  void SetStartStopTimerDurationForTesting(base::TimeDelta duration);
+
  private:
   friend class AutocompleteProviderTest;
   friend class OmniboxSuggestionButtonRowBrowserTest;
@@ -320,12 +323,11 @@ class AutocompleteController : public AutocompleteProviderListener,
   // Timer used to tell the providers to Stop() searching for matches.
   base::OneShotTimer stop_timer_;
 
-  // Amount of time (in ms) between when the user stops typing and
-  // when we send Stop() to every provider.  This is intended to avoid
-  // the disruptive effect of belated omnibox updates, updates that
-  // come after the user has had to time to read the whole dropdown
-  // and doesn't expect it to change.
-  const base::TimeDelta stop_timer_duration_;
+  // Amount of time between when the user stops typing and when we send Stop()
+  // to every provider.  This is intended to avoid the disruptive effect of
+  // belated omnibox updates, updates that come after the user has had to time
+  // to read the whole dropdown and doesn't expect it to change.
+  base::TimeDelta stop_timer_duration_;
 
   // True if a query is not currently running.
   bool done_;

components/omnibox/browser/autocomplete_provider_client.h

@@ -150,8 +150,8 @@ class AutocompleteProviderClient {
   virtual void StartServiceWorker(const GURL& destination_url) {}
 
   // Called by |controller| when its results have changed and all providers are
-  // done processing the autocomplete request. Chrome ignores this. It's only
-  // used in components unit tests. TODO(blundell): remove it.
+  // done processing the autocomplete request. Used by chrome to inform the
+  // prefetch service of updated results.
   virtual void OnAutocompleteControllerResultReady(
       AutocompleteController* controller) {}