[NTP][RQ] Adds support for deletion of repeatable queries

Repeatable Queries Service now populates the destination URL for the
query suggestions which is later used from the MostVisitedSites to
delete a repeatable query.

Depending on whether the query is provided by the server or the local
history, the service issues a deletion request to the server or deletes
the suggestion from the in-memory URL DB. In both cases, all eligible
search URLs from the default search provider which produce that query
also get deleted.

Fixed: 1147119
Change-Id: I4fad9eaf209c72e735d2260c7e22b1f0af5db4ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2527605
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Reviewed-by: Ramya Nagarajan <ramyan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825945}

components/search/BUILD.gn

@@ -51,6 +51,7 @@ source_set("unit_tests") {
     "//components/history/core/test",
     "//components/omnibox/browser:test_support",
     "//components/search_engines:search_engines",
+    "//components/search_engines:test_support",
     "//components/signin/public/base:test_support",
     "//components/signin/public/identity_manager:test_support",
     "//components/variations",

components/search/repeatable_queries/repeatable_queries_service.cc

@@ -10,8 +10,10 @@
 #include "base/bind.h"
 #include "base/json/json_writer.h"
 #include "base/scoped_observer.h"
+#include "base/stl_util.h"
 #include "base/values.h"
 #include "components/history/core/browser/history_service.h"
+#include "components/history/core/browser/history_types.h"
 #include "components/history/core/browser/keyword_search_term.h"
 #include "components/history/core/browser/url_database.h"
 #include "components/search/ntp_features.h"
@@ -44,8 +46,8 @@ bool JsonToRepeatableQueriesData(const base::Value& root_value,
   // the Suggest server containing the deletion URLs.
   const base::DictionaryValue* extras = nullptr;
   const base::ListValue* suggestion_details = nullptr;
-  if (root_list->GetDictionary(4, &extras) &&
-      extras->GetList("google:suggestdetail", &suggestion_details) &&
+  if (!root_list->GetDictionary(4, &extras) ||
+      !extras->GetList("google:suggestdetail", &suggestion_details) ||
       suggestion_details->GetSize() != results_list->GetSize()) {
     return false;
   }
@@ -150,6 +152,37 @@ void RepeatableQueriesService::Refresh() {
   }
 }
 
+void RepeatableQueriesService::DeleteQueryWithDestinationURL(const GURL& url) {
+  auto it = std::find_if(repeatable_queries_.begin(), repeatable_queries_.end(),
+                         [&url](const auto& repeatable_query) {
+                           return repeatable_query.destination_url == url;
+                         });
+
+  // Return if no repeatable query with a matching destination URL exists.
+  if (it == repeatable_queries_.end()) {
+    // Still notify observers of the deletion attempt.
+    NotifyObservers();
+    return;
+  }
+
+  if (it->deletion_url.empty()) {
+    DeleteRepeatableQueryFromURLDatabase(it->query);
+  } else {
+    DeleteRepeatableQueryFromServer(it->deletion_url);
+  }
+
+  // Delete all the Google search URLs for the given query from history.
+  history_service_->DeleteMatchingURLsForKeyword(
+      template_url_service_->GetDefaultSearchProvider()->id(), it->query);
+
+  // Make sure the query is not suggested again.
+  MarkQueryAsDeleted(it->query);
+
+  // Update the repeatable queries and notify the observers.
+  repeatable_queries_.erase(it);
+  NotifyObservers();
+}
+
 void RepeatableQueriesService::AddObserver(
     RepeatableQueriesServiceObserver* observer) {
   observers_.AddObserver(observer);
@@ -181,6 +214,28 @@ void RepeatableQueriesService::SigninStatusChanged() {
   Refresh();
 }
 
+GURL RepeatableQueriesService::GetQueryDestinationURL(
+    const base::string16& query) {
+  TemplateURLRef::SearchTermsArgs search_terms_args(query);
+  const TemplateURLRef& search_url_ref =
+      template_url_service_->GetDefaultSearchProvider()->url_ref();
+  const SearchTermsData& search_terms_data =
+      template_url_service_->search_terms_data();
+  DCHECK(search_url_ref.SupportsReplacement(search_terms_data));
+  return GURL(
+      search_url_ref.ReplaceSearchTerms(search_terms_args, search_terms_data));
+}
+
+GURL RepeatableQueriesService::GetQueryDeletionURL(
+    const std::string& deletion_url) {
+  const auto* default_provider =
+      template_url_service_->GetDefaultSearchProvider();
+  const SearchTermsData& search_terms_data =
+      template_url_service_->search_terms_data();
+  GURL request_url = default_provider->GenerateSearchURL(search_terms_data);
+  return request_url.GetOrigin().Resolve(deletion_url);
+}
+
 GURL RepeatableQueriesService::GetRequestURL() {
   TemplateURLRef::SearchTermsArgs search_terms_args;
   search_terms_args.request_source = TemplateURLRef::NON_SEARCHBOX_NTP;
@@ -188,6 +243,7 @@ GURL RepeatableQueriesService::GetRequestURL() {
       template_url_service_->GetDefaultSearchProvider()->suggestions_url_ref();
   const SearchTermsData& search_terms_data =
       template_url_service_->search_terms_data();
+  DCHECK(suggestion_url_ref.SupportsReplacement(search_terms_data));
   return GURL(suggestion_url_ref.ReplaceSearchTerms(search_terms_args,
                                                     search_terms_data));
 }
@@ -235,18 +291,23 @@ void RepeatableQueriesService::GetRepeatableQueriesFromServer() {
   resource_request->request_initiator =
       url::Origin::Create(request_initiator_url_);
 
-  simple_loader_ = network::SimpleURLLoader::Create(std::move(resource_request),
-                                                    traffic_annotation);
-  simple_loader_->DownloadToString(
+  loaders_.push_back(network::SimpleURLLoader::Create(
+      std::move(resource_request), traffic_annotation));
+  loaders_.back()->DownloadToString(
       url_loader_factory_.get(),
       base::BindOnce(&RepeatableQueriesService::RepeatableQueriesResponseLoaded,
-                     weak_ptr_factory_.GetWeakPtr()),
+                     weak_ptr_factory_.GetWeakPtr(), loaders_.back().get()),
       network::SimpleURLLoader::kMaxBoundedStringDownloadSize);
 }
 
 void RepeatableQueriesService::RepeatableQueriesResponseLoaded(
+    network::SimpleURLLoader* loader,
     std::unique_ptr<std::string> response) {
-  auto net_error = simple_loader_->NetError();
+  auto net_error = loader->NetError();
+  base::EraseIf(loaders_, [loader](const auto& loader_ptr) {
+    return loader == loader_ptr.get();
+  });
+
   if (net_error != net::OK || !response) {
     // In the case of network errors, keep the cached data, if any, but still
     // notify observers of the finished load attempt.
@@ -267,12 +328,18 @@ void RepeatableQueriesService::RepeatableQueriesResponseLoaded(
 
 void RepeatableQueriesService::RepeatableQueriesParsed(
     data_decoder::DataDecoder::ValueOrError result) {
-  std::vector<RepeatableQuery> data;
-  if (result.value && JsonToRepeatableQueriesData(*result.value, &data)) {
-    repeatable_queries_ = std::vector<RepeatableQuery>(
-        data.begin(), data.begin() + std::min(data.size(), kMaxQueries));
-  } else {
-    repeatable_queries_ = data;
+  repeatable_queries_.clear();
+
+  std::vector<RepeatableQuery> queries;
+  if (result.value && JsonToRepeatableQueriesData(*result.value, &queries)) {
+    for (auto& query : queries) {
+      if (IsQueryDeleted(query.query))
+        continue;
+      query.destination_url = GetQueryDestinationURL(query.query);
+      repeatable_queries_.push_back(query);
+      if (repeatable_queries_.size() >= kMaxQueries)
+        break;
+    }
   }
 
   NotifyObservers();
@@ -304,6 +371,10 @@ void RepeatableQueriesService::GetRepeatableQueriesFromURLDatabase() {
   for (const auto& result : results) {
     RepeatableQuery repeatable_query;
     repeatable_query.query = result.normalized_term;
+    if (IsQueryDeleted(repeatable_query.query))
+      continue;
+    repeatable_query.destination_url =
+        GetQueryDestinationURL(repeatable_query.query);
     repeatable_queries_.push_back(repeatable_query);
     if (repeatable_queries_.size() >= kMaxQueries)
       break;
@@ -312,8 +383,92 @@ void RepeatableQueriesService::GetRepeatableQueriesFromURLDatabase() {
   NotifyObservers();
 }
 
+void RepeatableQueriesService::DeleteRepeatableQueryFromServer(
+    const std::string& deletion_url) {
+  net::NetworkTrafficAnnotationTag traffic_annotation =
+      net::DefineNetworkTrafficAnnotation("repeatable_queries_deletion", R"(
+        semantics {
+          sender: "Repeatable Queries Service"
+          description:
+            "When users attempt to delete a server-provided repeatable search "
+            "query from the Most Visited section of New Tab Page, Chrome sends "
+            "a request to the server requesting deletion of that suggestion."
+          trigger:
+            "User attempts to delete a server-provided repeatable search "
+            "query for which the server provided a custom deletion URL from "
+            "the Most Visited section of New Tab Page, if Google is the "
+            "configured search provider, and the user is signed in."
+          data: "Google credentials if user is signed in."
+          destination: GOOGLE_OWNED_SERVICE
+        }
+        policy {
+          cookies_allowed: YES
+          cookies_store: "user"
+          setting:
+            "Users can control this feature by selecting a non-Google default "
+            "search engine in Chrome settings under 'Search Engine', or by "
+            "signing out of the browser on the New Tab Page. Users can opt "
+            "out of this feature by switching to custom shortcuts."
+          chrome_policy {
+            DefaultSearchProviderEnabled {
+              policy_options {mode: MANDATORY}
+              DefaultSearchProviderEnabled: false
+            }
+            BrowserSignin {
+              policy_options {mode: MANDATORY}
+              BrowserSignin: 0
+            }
+          }
+        })");
+
+  GURL request_url = GetQueryDeletionURL(deletion_url);
+  auto deletion_request = std::make_unique<network::ResourceRequest>();
+  variations::AppendVariationsHeaderUnknownSignedIn(
+      request_url, variations::InIncognito::kNo, deletion_request.get());
+  deletion_request->url = request_url;
+  deletion_request->request_initiator =
+      url::Origin::Create(request_initiator_url_);
+
+  loaders_.push_back(network::SimpleURLLoader::Create(
+      std::move(deletion_request), traffic_annotation));
+  loaders_.back()->DownloadToString(
+      url_loader_factory_.get(),
+      base::BindOnce(&RepeatableQueriesService::DeletionResponseLoaded,
+                     weak_ptr_factory_.GetWeakPtr(), loaders_.back().get()),
+      network::SimpleURLLoader::kMaxBoundedStringDownloadSize);
+}
+
+void RepeatableQueriesService::DeletionResponseLoaded(
+    network::SimpleURLLoader* loader,
+    std::unique_ptr<std::string> response) {
+  base::EraseIf(loaders_, [loader](const auto& loader_ptr) {
+    return loader == loader_ptr.get();
+  });
+}
+
+void RepeatableQueriesService::DeleteRepeatableQueryFromURLDatabase(
+    const base::string16& query) {
+  // Fail if the in-memory URL database is not available.
+  history::URLDatabase* url_db = history_service_->InMemoryDatabase();
+  if (!url_db)
+    return;
+
+  // Delete all the search terms matching the repeatable query suggestion from
+  // the in-memory URL database.
+  url_db->DeleteKeywordSearchTermForNormalizedTerm(
+      template_url_service_->GetDefaultSearchProvider()->id(), query);
+}
+
 void RepeatableQueriesService::NotifyObservers() {
   for (auto& observer : observers_) {
     observer.OnRepeatableQueriesUpdated();
   }
 }
+
+bool RepeatableQueriesService::IsQueryDeleted(const base::string16& query) {
+  return base::Contains(deleted_repeatable_queries_, query);
+}
+
+void RepeatableQueriesService::MarkQueryAsDeleted(const base::string16& query) {
+  deleted_repeatable_queries_.insert(query);
+}

components/search/repeatable_queries/repeatable_queries_service.h

@@ -39,7 +39,8 @@ class RepeatableQuery {
   ~RepeatableQuery() = default;
 
   bool operator==(const RepeatableQuery& other) const {
-    return query == other.query && deletion_url == other.deletion_url;
+    return query == other.query && destination_url == other.destination_url &&
+           deletion_url == other.deletion_url;
   }
   bool operator!=(const RepeatableQuery& other) const {
     return !(this == &other);
@@ -48,7 +49,10 @@ class RepeatableQuery {
   // Repeatable query suggestion.
   base::string16 query;
 
-  // The endpoint used for deleting the query suggestion on the server.
+  // The URL to navigate to when the suggestion is selected.
+  GURL destination_url;
+
+  // The relative endpoint used for deleting the query suggestion on the server.
   // Populated for server provided queries only.
   std::string deletion_url;
 };
@@ -82,6 +86,13 @@ class RepeatableQueriesService : public KeyedService {
   // RepeatableQueriesServiceObserver::OnRepeatableQueriesUpdated.
   void Refresh();
 
+  // Deletes the records of the repeatable query suggestion with the given
+  // destination URL on the server as well as on the device, whichever is
+  // applicable. Prevents the suggestion from being offered again by
+  // blocklisting it. Updates the current set of suggestions and notifies the
+  // observers.
+  void DeleteQueryWithDestinationURL(const GURL& url);
+
   // Add/remove observers.
   void AddObserver(RepeatableQueriesServiceObserver* observer);
   void RemoveObserver(RepeatableQueriesServiceObserver* observer);
@@ -98,21 +109,39 @@ class RepeatableQueriesService : public KeyedService {
   // Called when the signin status changes.
   void SigninStatusChanged();
 
+  // Returns the server destination URL for |query|.
+  GURL GetQueryDestinationURL(const base::string16& query);
+
+  // Returns the resolved deletion URL for the given relative deletion URL.
+  GURL GetQueryDeletionURL(const std::string& deletion_url);
+
   // Returns the server request URL.
   GURL GetRequestURL();
 
  private:
   // Requests repeatable queries from the server. Called for signed-in users.
   void GetRepeatableQueriesFromServer();
-  void RepeatableQueriesResponseLoaded(std::unique_ptr<std::string> response);
+  void RepeatableQueriesResponseLoaded(network::SimpleURLLoader* loader,
+                                       std::unique_ptr<std::string> response);
   void RepeatableQueriesParsed(data_decoder::DataDecoder::ValueOrError result);
 
   // Queries the in-memory URLDatabase for the repeatable queries submitted
   // to the default search provider. Called for unauthenticated users.
   void GetRepeatableQueriesFromURLDatabase();
 
+  // Deletes |query| from the in-memory URLDatabase.
+  void DeleteRepeatableQueryFromURLDatabase(const base::string16& query);
+
+  // Deletes the query with |deletion_url| from the server.
+  void DeleteRepeatableQueryFromServer(const std::string& deletion_url);
+  void DeletionResponseLoaded(network::SimpleURLLoader* loader,
+                              std::unique_ptr<std::string> response);
+
   void NotifyObservers();
 
+  bool IsQueryDeleted(const base::string16& query);
+  void MarkQueryAsDeleted(const base::string16& query);
+
   history::HistoryService* history_service_;
 
   TemplateURLService* template_url_service_;
@@ -129,7 +158,12 @@ class RepeatableQueriesService : public KeyedService {
 
   std::vector<RepeatableQuery> repeatable_queries_;
 
-  std::unique_ptr<network::SimpleURLLoader> simple_loader_;
+  // Used to ensure the deleted repeatable queries won't be suggested again.
+  // This does not need to be persisted across sessions as the queries do get
+  // deleted on the server as well as on the device, whichever is applicable.
+  std::set<base::string16> deleted_repeatable_queries_;
+
+  std::vector<std::unique_ptr<network::SimpleURLLoader>> loaders_;
 
   base::WeakPtrFactory<RepeatableQueriesService> weak_ptr_factory_{this};
 };

components/search/repeatable_queries/repeatable_queries_service_unittest.cc

@@ -22,6 +22,7 @@
 #include "components/omnibox/browser/in_memory_url_index_test_util.h"
 #include "components/search/search.h"
 #include "components/search/search_provider_observer.h"
+#include "components/search_engines/search_engines_test_util.h"
 #include "components/search_engines/template_url_service.h"
 #include "components/signin/public/base/test_signin_client.h"
 #include "components/signin/public/identity_manager/identity_test_environment.h"
@@ -62,7 +63,23 @@ std::string GoodServerResponse() {
 ])";
 }
 
-std::string BadServerResponse() {
+std::string BadServerResponse1() {
+  return R"()]}'
+[
+   "",
+   [
+      "server query 1",
+      "server query 2",
+      "server query 3"
+   ],
+   [],
+   [],
+   {
+   }
+])";
+}
+
+std::string BadServerResponse2() {
   return R"()]}'
 [
    "",
@@ -128,6 +145,14 @@ class TestRepeatableQueriesService : public RepeatableQueriesService {
     return &search_provider_observer_;
   }
 
+  GURL GetQueryDestinationURL(const base::string16& query) {
+    return RepeatableQueriesService::GetQueryDestinationURL(query);
+  }
+
+  GURL GetQueryDeletionURL(const std::string& deletion_url) {
+    return RepeatableQueriesService::GetQueryDeletionURL(deletion_url);
+  }
+
   GURL GetRequestURL() { return RepeatableQueriesService::GetRequestURL(); }
 
   void SearchProviderChanged() {
@@ -218,6 +243,10 @@ class RepeatableQueriesServiceTest : public ::testing::Test,
 
   void SignOut() { identity_env_->SetCookieAccounts({}); }
 
+  GURL GetQueryDestinationURL(const std::string& query) {
+    return service_->GetQueryDestinationURL(base::ASCIIToUTF16(query));
+  }
+
   void RefreshAndMaybeWaitForService() {
     service_is_done_ = false;
     service_->Refresh();
@@ -305,8 +334,10 @@ TEST_F(RepeatableQueriesServiceTest, SignedIn) {
   RefreshAndMaybeWaitForService();
   // The first two server suggestions are kept as repeatable queries.
   std::vector<RepeatableQuery> expected_server_queries{
-      {base::ASCIIToUTF16("server query 1"), "/delete?server+query+1"},
-      {base::ASCIIToUTF16("server query 2"), "/delete?server+query+2"}};
+      {base::ASCIIToUTF16("server query 1"),
+       GetQueryDestinationURL("server query 1"), "/delete?server+query+1"},
+      {base::ASCIIToUTF16("server query 2"),
+       GetQueryDestinationURL("server query 2"), "/delete?server+query+2"}};
   EXPECT_EQ(expected_server_queries, service()->repeatable_queries());
 }
 
@@ -321,17 +352,27 @@ TEST_F(RepeatableQueriesServiceTest, SignedIn_BadResponse) {
   // Request a refresh.
   RefreshAndMaybeWaitForService();
   std::vector<RepeatableQuery> expected_server_queries{
-      {base::ASCIIToUTF16("server query 1"), "/delete?server+query+1"},
-      {base::ASCIIToUTF16("server query 2"), "/delete?server+query+2"}};
+      {base::ASCIIToUTF16("server query 1"),
+       GetQueryDestinationURL("server query 1"), "/delete?server+query+1"},
+      {base::ASCIIToUTF16("server query 2"),
+       GetQueryDestinationURL("server query 2"), "/delete?server+query+2"}};
   EXPECT_EQ(expected_server_queries, service()->repeatable_queries());
 
   test_url_loader_factory()->AddResponse(service()->GetRequestURL().spec(),
-                                         BadServerResponse());
+                                         BadServerResponse1());
 
   // Request a refresh.
   RefreshAndMaybeWaitForService();
   // Cached data is cleared.
   EXPECT_TRUE(service()->repeatable_queries().empty());
+
+  test_url_loader_factory()->AddResponse(service()->GetRequestURL().spec(),
+                                         BadServerResponse2());
+
+  // Request a refresh.
+  RefreshAndMaybeWaitForService();
+  // Cached data is still empty.
+  EXPECT_TRUE(service()->repeatable_queries().empty());
 }
 
 TEST_F(RepeatableQueriesServiceTest, SignedIn_ErrorResponse) {
@@ -345,8 +386,10 @@ TEST_F(RepeatableQueriesServiceTest, SignedIn_ErrorResponse) {
   // Request a refresh.
   RefreshAndMaybeWaitForService();
   std::vector<RepeatableQuery> expected_server_queries{
-      {base::ASCIIToUTF16("server query 1"), "/delete?server+query+1"},
-      {base::ASCIIToUTF16("server query 2"), "/delete?server+query+2"}};
+      {base::ASCIIToUTF16("server query 1"),
+       GetQueryDestinationURL("server query 1"), "/delete?server+query+1"},
+      {base::ASCIIToUTF16("server query 2"),
+       GetQueryDestinationURL("server query 2"), "/delete?server+query+2"}};
   EXPECT_EQ(expected_server_queries, service()->repeatable_queries());
 
   test_url_loader_factory()->AddResponse(
@@ -371,8 +414,10 @@ TEST_F(RepeatableQueriesServiceTest, SignedIn_DefaultSearchProviderChanged) {
   // Request a refresh.
   RefreshAndMaybeWaitForService();
   std::vector<RepeatableQuery> expected_server_queries{
-      {base::ASCIIToUTF16("server query 1"), "/delete?server+query+1"},
-      {base::ASCIIToUTF16("server query 2"), "/delete?server+query+2"}};
+      {base::ASCIIToUTF16("server query 1"),
+       GetQueryDestinationURL("server query 1"), "/delete?server+query+1"},
+      {base::ASCIIToUTF16("server query 2"),
+       GetQueryDestinationURL("server query 2"), "/delete?server+query+2"}};
   EXPECT_EQ(expected_server_queries, service()->repeatable_queries());
 
   set_service_is_done(false);
@@ -394,8 +439,10 @@ TEST_F(RepeatableQueriesServiceTest, SignedIn_SigninStatusChanged) {
   // Request a refresh.
   RefreshAndMaybeWaitForService();
   std::vector<RepeatableQuery> expected_server_queries{
-      {base::ASCIIToUTF16("server query 1"), "/delete?server+query+1"},
-      {base::ASCIIToUTF16("server query 2"), "/delete?server+query+2"}};
+      {base::ASCIIToUTF16("server query 1"),
+       GetQueryDestinationURL("server query 1"), "/delete?server+query+1"},
+      {base::ASCIIToUTF16("server query 2"),
+       GetQueryDestinationURL("server query 2"), "/delete?server+query+2"}};
   EXPECT_EQ(expected_server_queries, service()->repeatable_queries());
 
   int original_query_age =
@@ -418,11 +465,65 @@ TEST_F(RepeatableQueriesServiceTest, SignedIn_SigninStatusChanged) {
   MaybeWaitForService();
   // Cached data is updated to local results.
   std::vector<RepeatableQuery> expected_local_queries{
-      {base::ASCIIToUTF16("more recent local query"), ""},
-      {base::ASCIIToUTF16("less recent local query"), ""}};
+      {base::ASCIIToUTF16("more recent local query"),
+       GetQueryDestinationURL("more recent local query"), ""},
+      {base::ASCIIToUTF16("less recent local query"),
+       GetQueryDestinationURL("less recent local query"), ""}};
   EXPECT_EQ(expected_local_queries, service()->repeatable_queries());
 }
 
+TEST_F(RepeatableQueriesServiceTest, SignedIn_Deletion) {
+  SignIn();
+  test_url_loader_factory()->AddResponse(service()->GetRequestURL().spec(),
+                                         GoodServerResponse());
+
+  EXPECT_CALL(*service()->search_provider_observer(), is_google())
+      .WillRepeatedly(testing::Return(true));
+
+  // Request a refresh.
+  RefreshAndMaybeWaitForService();
+  std::vector<RepeatableQuery> expected_server_queries{
+      {base::ASCIIToUTF16("server query 1"),
+       GetQueryDestinationURL("server query 1"), "/delete?server+query+1"},
+      {base::ASCIIToUTF16("server query 2"),
+       GetQueryDestinationURL("server query 2"), "/delete?server+query+2"}};
+  EXPECT_EQ(expected_server_queries, service()->repeatable_queries());
+
+  // Try to delete a query suggestion not provided by the service.
+  set_service_is_done(false);
+  service()->DeleteQueryWithDestinationURL(GetQueryDestinationURL("blah"));
+  // No request to delete the suggestion was sent.
+  EXPECT_TRUE(test_url_loader_factory()->pending_requests()->empty());
+  MaybeWaitForService();
+  // Suggestions should not change.
+  EXPECT_EQ(expected_server_queries, service()->repeatable_queries());
+
+  // Delete the query suggestion provided by the service.
+  set_service_is_done(false);
+  service()->DeleteQueryWithDestinationURL(
+      GetQueryDestinationURL("server query 1"));
+  // A request to delete the suggestion was sent.
+  EXPECT_EQ(1u, test_url_loader_factory()->pending_requests()->size());
+  EXPECT_EQ(test_url_loader_factory()->GetPendingRequest(0)->request.url,
+            service()->GetQueryDeletionURL("/delete?server+query+1"));
+  MaybeWaitForService();
+  expected_server_queries = {{base::ASCIIToUTF16("server query 2"),
+                              GetQueryDestinationURL("server query 2"),
+                              "/delete?server+query+2"}};
+  // The deleted suggestion is not offered anymore.
+  EXPECT_EQ(expected_server_queries, service()->repeatable_queries());
+
+  expected_server_queries = {
+      {base::ASCIIToUTF16("server query 2"),
+       GetQueryDestinationURL("server query 2"), "/delete?server+query+2"},
+      {base::ASCIIToUTF16("server query 3"),
+       GetQueryDestinationURL("server query 3"), "/delete?server+query+3"}};
+  // Request a refresh.
+  RefreshAndMaybeWaitForService();
+  // The deleted suggestion will not be offered again.
+  EXPECT_EQ(expected_server_queries, service()->repeatable_queries());
+}
+
 TEST_F(RepeatableQueriesServiceTest, SignedOut_DefaultSearchProviderChanged) {
   int original_query_age =
       history::kAutocompleteDuplicateVisitIntervalThreshold.InSeconds() + 3;
@@ -446,8 +547,10 @@ TEST_F(RepeatableQueriesServiceTest, SignedOut_DefaultSearchProviderChanged) {
   // Request a refresh.
   RefreshAndMaybeWaitForService();
   std::vector<RepeatableQuery> expected_local_queries{
-      {base::ASCIIToUTF16("more recent local query"), ""},
-      {base::ASCIIToUTF16("less recent local query"), ""}};
+      {base::ASCIIToUTF16("more recent local query"),
+       GetQueryDestinationURL("more recent local query"), ""},
+      {base::ASCIIToUTF16("less recent local query"),
+       GetQueryDestinationURL("less recent local query"), ""}};
   EXPECT_EQ(expected_local_queries, service()->repeatable_queries());
 
   set_service_is_done(false);
@@ -480,8 +583,10 @@ TEST_F(RepeatableQueriesServiceTest, SignedOut_SigninStatusChanged) {
   // Request a refresh.
   RefreshAndMaybeWaitForService();
   std::vector<RepeatableQuery> expected_local_queries{
-      {base::ASCIIToUTF16("more recent local query"), ""},
-      {base::ASCIIToUTF16("less recent local query"), ""}};
+      {base::ASCIIToUTF16("more recent local query"),
+       GetQueryDestinationURL("more recent local query"), ""},
+      {base::ASCIIToUTF16("less recent local query"),
+       GetQueryDestinationURL("less recent local query"), ""}};
   EXPECT_EQ(expected_local_queries, service()->repeatable_queries());
 
   test_url_loader_factory()->AddResponse(service()->GetRequestURL().spec(),
@@ -492,7 +597,56 @@ TEST_F(RepeatableQueriesServiceTest, SignedOut_SigninStatusChanged) {
   MaybeWaitForService();
   // Cached data is updated to server results.
   std::vector<RepeatableQuery> expected_server_queries{
-      {base::ASCIIToUTF16("server query 1"), "/delete?server+query+1"},
-      {base::ASCIIToUTF16("server query 2"), "/delete?server+query+2"}};
+      {base::ASCIIToUTF16("server query 1"),
+       GetQueryDestinationURL("server query 1"), "/delete?server+query+1"},
+      {base::ASCIIToUTF16("server query 2"),
+       GetQueryDestinationURL("server query 2"), "/delete?server+query+2"}};
   EXPECT_EQ(expected_server_queries, service()->repeatable_queries());
 }
+
+TEST_F(RepeatableQueriesServiceTest, SignedOut_Deletion) {
+  FillURLDatabase({{default_search_provider(), "local query 1",
+                    /*age_in_seconds=*/1},
+                   {default_search_provider(), "local query 2",
+                    /*age_in_seconds=*/2},
+                   {default_search_provider(), "local query 3",
+                    /*age_in_seconds=*/3}});
+
+  EXPECT_CALL(*service()->search_provider_observer(), is_google())
+      .WillRepeatedly(testing::Return(true));
+
+  // Request a refresh.
+  RefreshAndMaybeWaitForService();
+  std::vector<RepeatableQuery> expected_local_queries{
+      {base::ASCIIToUTF16("local query 1"),
+       GetQueryDestinationURL("local query 1"), ""},
+      {base::ASCIIToUTF16("local query 2"),
+       GetQueryDestinationURL("local query 2"), ""}};
+  EXPECT_EQ(expected_local_queries, service()->repeatable_queries());
+
+  // Try to delete a query suggestion not provided by the service.
+  set_service_is_done(false);
+  service()->DeleteQueryWithDestinationURL(GetQueryDestinationURL("blah"));
+  MaybeWaitForService();
+  // Suggestions should not change.
+  EXPECT_EQ(expected_local_queries, service()->repeatable_queries());
+
+  // Delete the query suggestion provided by the service.
+  set_service_is_done(false);
+  service()->DeleteQueryWithDestinationURL(
+      GetQueryDestinationURL("local query 1"));
+  MaybeWaitForService();
+  expected_local_queries = {{base::ASCIIToUTF16("local query 2"),
+                             GetQueryDestinationURL("local query 2"), ""}};
+  // The deleted suggestion is not offered anymore.
+  EXPECT_EQ(expected_local_queries, service()->repeatable_queries());
+
+  // Request a refresh.
+  RefreshAndMaybeWaitForService();
+  expected_local_queries = {{base::ASCIIToUTF16("local query 2"),
+                             GetQueryDestinationURL("local query 2"), ""},
+                            {base::ASCIIToUTF16("local query 3"),
+                             GetQueryDestinationURL("local query 3"), ""}};
+  // The deleted suggestion will not be offered again.
+  EXPECT_EQ(expected_local_queries, service()->repeatable_queries());
+}

tools/traffic_annotation/summary/annotations.xml

@@ -277,6 +277,7 @@ Refer to README.md for content description and update process.
  <item id="remoting_telemetry_log_writer" added_in_milestone="86" hash_code="107268760" type="0" content_hash_code="81741595" os_list="linux,windows" file_path="remoting/base/telemetry_log_writer.cc"/>
  <item id="render_view_context_menu" added_in_milestone="62" hash_code="25844439" type="0" content_hash_code="69471170" os_list="linux,windows" file_path="chrome/browser/renderer_context_menu/render_view_context_menu.cc"/>
  <item id="renderer_initiated_download" added_in_milestone="62" hash_code="116443055" type="0" content_hash_code="37846436" os_list="linux,windows" file_path="content/browser/renderer_host/render_frame_host_impl.cc"/>
+ <item id="repeatable_queries_deletion" added_in_milestone="88" hash_code="59980744" type="0" content_hash_code="9022058" os_list="linux,windows" file_path="components/search/repeatable_queries/repeatable_queries_service.cc"/>
  <item id="repeatable_queries_service" added_in_milestone="88" hash_code="5394442" type="0" content_hash_code="115294794" os_list="linux,windows" file_path="components/search/repeatable_queries/repeatable_queries_service.cc"/>
  <item id="reporting" added_in_milestone="62" hash_code="109891200" type="0" content_hash_code="125758928" os_list="linux,windows" file_path="net/reporting/reporting_uploader.cc"/>
  <item id="resource_dispatcher_host" added_in_milestone="62" hash_code="81157007" type="0" deprecated="2019-07-30" content_hash_code="35725167" file_path=""/>

tools/traffic_annotation/summary/grouping.xml

@@ -393,6 +393,7 @@ hidden="true" so that these annotations don't show up in the document.
       <traffic_annotation unique_id="popular_sites_fetch"/>
       <traffic_annotation unique_id="search_suggest_service"/>
       <traffic_annotation unique_id="remote_suggestions_provider"/>
+      <traffic_annotation unique_id="repeatable_queries_deletion"/>
       <traffic_annotation unique_id="repeatable_queries_service"/>
       <traffic_annotation unique_id="promo_service"/>
       <traffic_annotation unique_id="task_module_service"/>