Move OnURLsDeleted from NTPSnippetsService to ContentSuggestionsService

components/ntp_snippets/content_suggestions_provider.h

 // Copyright 2016 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_NTP_SNIPPETS_CONTENT_SUGGESTIONS_PROVIDER_H_
 #define COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_PROVIDER_H_
 
 #include <string>
 #include <vector>
 
@@ skipping 83 matching lines @@
   virtual void DismissSuggestion(const std::string& suggestion_id) = 0;
 
   // Fetches the image for the suggestion with the given ID and returns it
   // through the callback. This fetch may occur locally or from the internet.
   // If that suggestion doesn't exist, doesn't have an image or if the fetch
   // fails, the callback gets a null image. The callback will not be called
   // synchronously.
   virtual void FetchSuggestionImage(const std::string& suggestion_id,
                                     const ImageFetchedCallback& callback) = 0;
 
+  // Removes the given list of URLs and their associated data (depends on the
+  // provider) from all providers. Providers should immediately clear any

[vitaliii, 2016/09/02 12:38:30]

I am not sure about the wording.
Should it be "from all providers' internal storage"?

[Marc Treib, 2016/09/02 12:57:10]

See below - if possible, I'd like to merge this with ClearHistory below, since
it's essentially the same thing: "something got deleted from history, do
whatever you have to do in response to that".
Or am I missing something? If these two actually are significantly different,
then the comments need to make very clear what the difference is, and what is
expected of a provider in either case.

[vitaliii, 2016/09/02 14:13:16]

Done.

+  // history-related data associated with |deleted_urls| or all data if
+  // |all_history| is true.
+  virtual void RemoveURLsFromHistory(bool all_history,

[Marc Treib, 2016/09/02 12:57:10]

Hm, with all_history==true, isn't this essentially the same as a ClearHistory
call with appropriate begin/end parameters? Can we maybe map one to the other?
(e.g. the service could construct an appropriate filter function from
deleted_urls, and pass that to ClearHistory).
I'd prefer if we didn't have multiple history-clearing-related methods on this
interface.

[vitaliii, 2016/09/02 14:13:16]

Done.

+                                     const std::vector<GURL>& deleted_urls) = 0;
+
   // Removes history from the specified time range where the URL matches the
   // |filter|. The data removed depends on the provider. Note that the
   // data outside the time range may be deleted, for example suggestions, which
   // are based on history from that time range. Providers should immediately
   // clear any data related to history from the specified time range where the
   // URL matches the |filter|.
   virtual void ClearHistory(
       base::Time begin,
       base::Time end,
       const base::Callback<bool(const GURL& url)>& filter) = 0;
@@ skipping 39 matching lines @@
   CategoryFactory* category_factory() const { return category_factory_; }
 
  private:
   Observer* observer_;
   CategoryFactory* category_factory_;
 };
 
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_PROVIDER_H_