Make OfflinePage ContentSuggestions dismissable

components/ntp_snippets/offline_pages/offline_page_suggestions_provider.h

Index: components/ntp_snippets/offline_pages/offline_page_suggestions_provider.h
diff --git a/components/ntp_snippets/offline_pages/offline_page_suggestions_provider.h b/components/ntp_snippets/offline_pages/offline_page_suggestions_provider.h
index 6266256f7fce6ff29055f2551e47067dd32b760a..40ce31ad41090363421bc5dbb7ff710c2e574152 100644
--- a/components/ntp_snippets/offline_pages/offline_page_suggestions_provider.h
+++ b/components/ntp_snippets/offline_pages/offline_page_suggestions_provider.h
@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_NTP_SNIPPETS_OFFLINE_PAGES_OFFLINE_PAGE_SUGGESTIONS_PROVIDER_H_
 #define COMPONENTS_NTP_SNIPPETS_OFFLINE_PAGES_OFFLINE_PAGE_SUGGESTIONS_PROVIDER_H_
 
+#include <set>
 #include <string>
 #include <vector>
 
@@ -17,6 +18,9 @@
 #include "components/offline_pages/offline_page_model.h"
 #include "components/offline_pages/offline_page_types.h"
 
+class PrefRegistrySimple;
+class PrefService;
+
 namespace gfx {
 class Image;
 }
@@ -33,9 +37,12 @@ class OfflinePageSuggestionsProvider
   OfflinePageSuggestionsProvider(
       ContentSuggestionsProvider::Observer* observer,
       CategoryFactory* category_factory,
-      offline_pages::OfflinePageModel* offline_page_model);
+      offline_pages::OfflinePageModel* offline_page_model,
+      PrefService* pref_service);
   ~OfflinePageSuggestionsProvider() override;
 
+  static void RegisterProfilePrefs(PrefRegistrySimple* registry);
+
  private:
   // ContentSuggestionsProvider implementation.
   std::vector<Category> GetProvidedCategories() override;
@@ -65,12 +72,26 @@ class OfflinePageSuggestionsProvider
   // Updates the |category_status_| and notifies the |observer_|, if necessary.
   void NotifyStatusChanged(CategoryStatus new_status);
 
+  // Converts an OfflinePageItem to a ContentSuggestion.
+  ContentSuggestion ConvertOfflinePage(
+      const offline_pages::OfflinePageItem& offline_page) const;
+
+  // Reads |dismissed_ids_| from the prefs.
+  void ReadDismissedIDsFromPrefs();
+
+  // Writes |dismissed_ids_| to the prefs.
+  void StoreDismissedIDsToPrefs();
+
   CategoryStatus category_status_;
 
   offline_pages::OfflinePageModel* offline_page_model_;
 
   const Category provided_category_;
 
+  PrefService* pref_service_;
+
+  std::set<std::string> dismissed_ids_;

[Marc Treib, 2016/08/11 10:28:12]

We could also get rid of this member, and just read from prefs on demand. Then
there's no problem with keeping the two things in sync. WDYT?

[Philipp Keck, 2016/08/11 12:53:19]

I don't know -- how slow are prefs? Since we're doing this on every navigation,
it might be a relevant difference.

[Marc Treib, 2016/08/11 13:13:23]

They're in memory at least, so it can't be too bad. Certainly not compared to
all the other stuff we're doing per navigation.
So if you agree it makes things simpler, then I'd go with removing the member.

[Philipp Keck, 2016/08/11 13:34:04]

I tried it out and it doesn't make things easier.
The operations are: "Add one" (which becomes "load all, add one, store all") and
"remove all" (which becomes "re-init empty, store all"). The first operation is
more weird code, the second is the same amount of code. And once we'll have the
selective clean-up, that will also be a bit more code.
To be precise, instead of having the member around, you need to put
dismissed_ids = Read...() in every place you want to use the member. Or you
could have specific "AddToDismissedPref" methods that directly operator on the
base::Values, but that's not simpler either.

[Marc Treib, 2016/08/11 13:41:56]

For "remove all", there's a ClearPref method.
But it's not a big deal either way.

+
   DISALLOW_COPY_AND_ASSIGN(OfflinePageSuggestionsProvider);
 };