Make OfflinePage ContentSuggestions dismissable

components/ntp_snippets/offline_pages/offline_page_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_OFFLINE_PAGES_OFFLINE_PAGE_SUGGESTIONS_PROVIDER_H_
 #define COMPONENTS_NTP_SNIPPETS_OFFLINE_PAGES_OFFLINE_PAGE_SUGGESTIONS_PROVIDER_H_
 
+#include <set>
 #include <string>
 #include <vector>
 
 #include "base/callback_forward.h"
 #include "components/ntp_snippets/category.h"
 #include "components/ntp_snippets/category_factory.h"
 #include "components/ntp_snippets/category_status.h"
 #include "components/ntp_snippets/content_suggestion.h"
 #include "components/ntp_snippets/content_suggestions_provider.h"
 #include "components/offline_pages/offline_page_model.h"
 #include "components/offline_pages/offline_page_types.h"
 
+class PrefRegistrySimple;
+class PrefService;
+
 namespace gfx {
 class Image;
 }
 
 namespace ntp_snippets {
 
 // Provides content suggestions from the offline pages model.
 // Currently, those are only the pages that the user last navigated to in an
 // open tab and offlined bookmarks.
 class OfflinePageSuggestionsProvider
     : public ContentSuggestionsProvider,
       public offline_pages::OfflinePageModel::Observer {
  public:
   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;
   CategoryStatus GetCategoryStatus(Category category) override;
   CategoryInfo GetCategoryInfo(Category category) override;
   void DismissSuggestion(const std::string& suggestion_id) override;
   void FetchSuggestionImage(const std::string& suggestion_id,
                             const ImageFetchedCallback& callback) override;
   void ClearCachedSuggestionsForDebugging(Category category) override;
   std::vector<ContentSuggestion> GetDismissedSuggestionsForDebugging(
       Category category) override;
   void ClearDismissedSuggestionsForDebugging(Category category) override;
 
   // OfflinePageModel::Observer implementation.
   void OfflinePageModelLoaded(offline_pages::OfflinePageModel* model) override;
   void OfflinePageModelChanged(offline_pages::OfflinePageModel* model) override;
   void OfflinePageDeleted(int64_t offline_id,
                           const offline_pages::ClientId& client_id) override;
 
   // Queries the OfflinePageModel for offline pages.
   void FetchOfflinePages();
 
   // Callback from the OfflinePageModel.
   void OnOfflinePagesLoaded(
       const offline_pages::MultipleOfflinePageItemResult& result);
 
   // 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);
 };
 
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_OFFLINE_PAGES_OFFLINE_PAGE_SUGGESTIONS_PROVIDER_H_