📰 Persist category dismissals

components/ntp_snippets/content_suggestions_provider.h

Index: components/ntp_snippets/content_suggestions_provider.h
diff --git a/components/ntp_snippets/content_suggestions_provider.h b/components/ntp_snippets/content_suggestions_provider.h
index f6741356d66e6cfc0a3773c83c8afe25a6d63d83..476385468b52a38dd219cea5ddcb891af09590fe 100644
--- a/components/ntp_snippets/content_suggestions_provider.h
+++ b/components/ntp_snippets/content_suggestions_provider.h
@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_PROVIDER_H_
 #define COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_PROVIDER_H_
 
+#include <map>
 #include <string>
 #include <vector>
 
@@ -35,19 +36,34 @@ class ContentSuggestionsProvider {
   // The observer of a provider is notified when new data is available.
   class Observer {
    public:
+    typedef std::
+        map<Category, std::vector<ContentSuggestion>, Category::CompareByID>
+            SuggestionBatch;

[Marc Treib, 2016/10/11 07:52:55]

nit: using SuggestionBatch = ...

+
     // Called when the available content changed.
-    // If a provider provides suggestions for multiple categories, this callback
-    // is called once per category. The |suggestions| parameter always contains
-    // the full list of currently available suggestions for that category, i.e.,
-    // an empty list will remove all suggestions from the given category. Note
-    // that to clear them from the UI immediately, the provider needs to change
-    // the status of the respective category. If the given |category| is not
-    // known yet, the calling |provider| will be registered as its provider.
+    // If a provider provides suggestions for multiple categories, it should
+    // use OnNewSuggestionBatch() instead. The |suggestions| parameter always
+    // contains the full list of currently available suggestions for that
+    // category, i.e., an empty list will remove all suggestions from the given
+    // category. Note that to clear them from the UI immediately, the provider
+    // needs to change the status of the respective category. If the given
+    // |category| is not known yet, the calling |provider| will be registered as
+    // its provider.
     virtual void OnNewSuggestions(
         ContentSuggestionsProvider* provider,
         Category category,
         std::vector<ContentSuggestion> suggestions) = 0;
 
+    // Called when the available content for multiple categories changed.
+    // The |suggestions| parameter always contains the full list of currently
+    // available suggestions for that category, i.e., an empty list will remove
+    // all suggestions from the given category. Note that to clear them from the
+    // UI immediately, the provider needs to change the status of the respective
+    // category. If the given |category| is not known yet, the calling
+    // |provider| will be registered as its provider.
+    virtual void OnNewSuggestionBatch(ContentSuggestionsProvider* provider,
+                                      SuggestionBatch suggestion_batch) = 0;

[Marc Treib, 2016/10/11 07:52:55]

Like Tim, I'm not happy with having two methods here which do almost (but not
quite) exactly the same thing. IIUC, you need this so that you can detect when a
category isn't there anymore. But shouldn't it be the responsibility of the
provider to set that category's status to something unavailable?

[dgn, 2016/10/11 19:14:01]

As discussed offline, I'm going to remove this then, we'll figure out what to do
about orphaned categories later, it's not a big concern.

+
     // Called when the status of a category changed, including when it is added.
     // If |new_status| is NOT_PROVIDED, the calling provider must be the one
     // that currently provides the |category|, and the category is unregistered