📰 Persist category dismissals

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 <map>
 #include <string>
 #include <vector>
 
 #include "base/callback_forward.h"
 #include "components/ntp_snippets/category.h"
 #include "components/ntp_snippets/category_info.h"
 #include "components/ntp_snippets/category_status.h"
 #include "components/ntp_snippets/content_suggestion.h"
 
 namespace gfx {
@@ skipping 10 matching lines @@
 // shut down by the ContentSuggestionsService.
 class ContentSuggestionsProvider {
  public:
   using ImageFetchedCallback = base::Callback<void(const gfx::Image&)>;
   using DismissedSuggestionsCallback = base::Callback<void(
       std::vector<ContentSuggestion> dismissed_suggestions)>;
 
   // 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
     // without clearing the UI.
     // If |new_status| is any other value, it must match the value that is
     // currently returned from the provider's |GetCategoryStatus(category)|. In
     // case the given |category| is not known yet, the calling |provider| will
     // be registered as its provider. Whenever the status changes to an
     // unavailable status, all suggestions in that category must immediately be
     // removed from all caches and from the UI, but the provider remains
@@ skipping 78 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_