[Merge M73][NTP] Add API for search suggestion selected

window.chrome.embeddedSearch.newTabPage.searchSuggestionSelected(task_version, task_id, hash)

When a search suggestion is selected clear the cached data and issue
a new request. For only this request we append the selected suggestion
to the blocklist. This prevents a race condition where the request
completes before the data server side is updated to reflect the
selection, resulting in the same suggestion appearing in the next set.

Bug: 904565
Change-Id: Ie322f0efdfc23618c0b8455695d6790e434a30b2
Reviewed-on: https://chromium-review.googlesource.com/c/1422397
Commit-Queue: Kyle Milka <[email protected]>
Reviewed-by: Chris Palmer <[email protected]>
Reviewed-by: Kristi Park <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#626757}(cherry picked from commit 9132e5f)
Reviewed-on: https://chromium-review.googlesource.com/c/1446557
Reviewed-by: Kyle Milka <[email protected]>
Cr-Commit-Position: refs/branch-heads/3683@{#67}
Cr-Branched-From: e510299-refs/heads/master@{#625896}

chrome/browser/search/search_suggest/search_suggest_loader.h

@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_SEARCH_SEARCH_SUGGEST_SEARCH_SUGGEST_LOADER_H_
 #define CHROME_BROWSER_SEARCH_SEARCH_SUGGEST_SEARCH_SUGGEST_LOADER_H_
 
+#include <string>
+
 #include "base/callback_forward.h"
 #include "base/optional.h"
 
@@ -41,8 +43,8 @@ class SearchSuggestLoader {
 
   // Initiates a load from the network. On completion (successful or not), the
   // callback will be called with the result, which will be nullopt on failure.
-  // |blacklist| will be appended to the request as the url param 'vtgb'.
-  virtual void Load(const std::string& blacklist,
+  // |blocklist| will be appended to the request as the url param 'vtgb'.
+  virtual void Load(const std::string& blocklist,
                     SearchSuggestionsCallback callback) = 0;
 
   // Retrieves the URL from which SearchSuggestData will be loaded.

chrome/browser/search/search_suggest/search_suggest_loader_impl.cc

@@ -211,34 +211,34 @@ SearchSuggestLoaderImpl::SearchSuggestLoaderImpl(
 
 SearchSuggestLoaderImpl::~SearchSuggestLoaderImpl() = default;
 
-void SearchSuggestLoaderImpl::Load(const std::string& blacklist,
+void SearchSuggestLoaderImpl::Load(const std::string& blocklist,
                                    SearchSuggestionsCallback callback) {
   callbacks_.push_back(std::move(callback));
 
   // Note: If there is an ongoing request, abandon it. It's possible that
   // something has changed in the meantime (e.g. signin state) that would make
   // the result obsolete.
   pending_request_ = std::make_unique<AuthenticatedURLLoader>(
-      url_loader_factory_, GetApiUrl(blacklist),
+      url_loader_factory_, GetApiUrl(blocklist),
       base::BindOnce(&SearchSuggestLoaderImpl::LoadDone,
                      base::Unretained(this)));
   pending_request_->Start();
 }
 
 GURL SearchSuggestLoaderImpl::GetLoadURLForTesting() const {
-  std::string blacklist;
-  return GetApiUrl(blacklist);
+  std::string blocklist;
+  return GetApiUrl(blocklist);
 }
 
-GURL SearchSuggestLoaderImpl::GetApiUrl(const std::string& blacklist) const {
+GURL SearchSuggestLoaderImpl::GetApiUrl(const std::string& blocklist) const {
   GURL google_base_url = google_util::CommandLineGoogleBaseURL();
   if (!google_base_url.is_valid()) {
     google_base_url = google_url_tracker_->google_url();
   }
 
   GURL api_url = google_base_url.Resolve(kNewTabSearchSuggestionsApiPath);
 
-  api_url = net::AppendQueryParameter(api_url, "vtgb", blacklist);
+  api_url = net::AppendQueryParameter(api_url, "vtgb", blocklist);
 
   return api_url;
 }

chrome/browser/search/search_suggest/search_suggest_loader_impl.h

@@ -35,15 +35,15 @@ class SearchSuggestLoaderImpl : public SearchSuggestLoader {
       const std::string& application_locale);
   ~SearchSuggestLoaderImpl() override;
 
-  void Load(const std::string& blacklist,
+  void Load(const std::string& blocklist,
             SearchSuggestionsCallback callback) override;
 
   GURL GetLoadURLForTesting() const override;
 
  private:
   class AuthenticatedURLLoader;
 
-  GURL GetApiUrl(const std::string& blacklist) const;
+  GURL GetApiUrl(const std::string& blocklist) const;
 
   void LoadDone(const network::SimpleURLLoader* simple_loader,
                 std::unique_ptr<std::string> response_body);

chrome/browser/search/search_suggest/search_suggest_loader_impl_unittest.cc

@@ -141,8 +141,8 @@ TEST_F(SearchSuggestLoaderImplTest, RequestReturns) {
   SetUpResponseWithData(kMinimalValidResponse);
 
   base::MockCallback<SearchSuggestLoader::SearchSuggestionsCallback> callback;
-  std::string blacklist;
-  search_suggest_loader()->Load(blacklist, callback.Get());
+  std::string blocklist;
+  search_suggest_loader()->Load(blocklist, callback.Get());
 
   base::Optional<SearchSuggestData> data;
   base::RunLoop loop;
@@ -159,8 +159,8 @@ TEST_F(SearchSuggestLoaderImplTest, HandlesResponsePreamble) {
   SetUpResponseWithData(std::string(")]}'") + kMinimalValidResponse);
 
   base::MockCallback<SearchSuggestLoader::SearchSuggestionsCallback> callback;
-  std::string blacklist;
-  search_suggest_loader()->Load(blacklist, callback.Get());
+  std::string blocklist;
+  search_suggest_loader()->Load(blocklist, callback.Get());
 
   base::Optional<SearchSuggestData> data;
   base::RunLoop loop;
@@ -179,8 +179,8 @@ TEST_F(SearchSuggestLoaderImplTest, ParsesFullResponse) {
             }}})json");
 
   base::MockCallback<SearchSuggestLoader::SearchSuggestionsCallback> callback;
-  std::string blacklist;
-  search_suggest_loader()->Load(blacklist, callback.Get());
+  std::string blocklist;
+  search_suggest_loader()->Load(blocklist, callback.Get());
 
   base::Optional<SearchSuggestData> data;
   base::RunLoop loop;
@@ -202,11 +202,11 @@ TEST_F(SearchSuggestLoaderImplTest, CoalescesMultipleRequests) {
   // Trigger two requests.
   base::MockCallback<SearchSuggestLoader::SearchSuggestionsCallback>
       first_callback;
-  std::string blacklist;
-  search_suggest_loader()->Load(blacklist, first_callback.Get());
+  std::string blocklist;
+  search_suggest_loader()->Load(blocklist, first_callback.Get());
   base::MockCallback<SearchSuggestLoader::SearchSuggestionsCallback>
       second_callback;
-  search_suggest_loader()->Load(blacklist, second_callback.Get());
+  search_suggest_loader()->Load(blocklist, second_callback.Get());
 
   // Make sure that a single response causes both callbacks to be called.
   base::Optional<SearchSuggestData> first_data;
@@ -228,8 +228,8 @@ TEST_F(SearchSuggestLoaderImplTest, NetworkErrorIsTransient) {
   SetUpResponseWithNetworkError();
 
   base::MockCallback<SearchSuggestLoader::SearchSuggestionsCallback> callback;
-  std::string blacklist;
-  search_suggest_loader()->Load(blacklist, callback.Get());
+  std::string blocklist;
+  search_suggest_loader()->Load(blocklist, callback.Get());
 
   base::RunLoop loop;
   EXPECT_CALL(callback, Run(SearchSuggestLoader::Status::TRANSIENT_ERROR,
@@ -243,8 +243,8 @@ TEST_F(SearchSuggestLoaderImplTest, DISABLED_InvalidJsonErrorIsFatal) {
   SetUpResponseWithData(kMinimalValidResponse + std::string(")"));
 
   base::MockCallback<SearchSuggestLoader::SearchSuggestionsCallback> callback;
-  std::string blacklist;
-  search_suggest_loader()->Load(blacklist, callback.Get());
+  std::string blocklist;
+  search_suggest_loader()->Load(blocklist, callback.Get());
 
   base::RunLoop loop;
   EXPECT_CALL(callback,
@@ -257,8 +257,8 @@ TEST_F(SearchSuggestLoaderImplTest, IncompleteJsonErrorIsFatal) {
   SetUpResponseWithData(R"json({"update": {}})json");
 
   base::MockCallback<SearchSuggestLoader::SearchSuggestionsCallback> callback;
-  std::string blacklist;
-  search_suggest_loader()->Load(blacklist, callback.Get());
+  std::string blocklist;
+  search_suggest_loader()->Load(blocklist, callback.Get());
 
   base::RunLoop loop;
   EXPECT_CALL(callback,