FetchMore functionality backend

components/ntp_snippets/remote/ntp_snippets_service.h

Index: components/ntp_snippets/remote/ntp_snippets_service.h
diff --git a/components/ntp_snippets/remote/ntp_snippets_service.h b/components/ntp_snippets/remote/ntp_snippets_service.h
index af9c017bafbe1f0903c0e174b8afc768348eedfa..6ecea335c9093ecfb39048c41a269368f7158bac 100644
--- a/components/ntp_snippets/remote/ntp_snippets_service.h
+++ b/components/ntp_snippets/remote/ntp_snippets_service.h
@@ -116,6 +116,8 @@ class NTPSnippetsService final : public ContentSuggestionsProvider,
   void DismissSuggestion(const ContentSuggestion::ID& suggestion_id) override;
   void FetchSuggestionImage(const ContentSuggestion::ID& suggestion_id,
                             const ImageFetchedCallback& callback) override;
+  void FetchMore(const Category& category,
+                 FetchedMoreCallback callback) override;
   void ClearHistory(
       base::Time begin,
       base::Time end,
@@ -149,6 +151,10 @@ class NTPSnippetsService final : public ContentSuggestionsProvider,
 
  private:
   friend class NTPSnippetsServiceTest;
+
+  // Forward declaration of the Strategy used to process fetch requests.
+  class NTPSnippetsFetchingStrategy;

[Marc Treib, 2016/10/20 16:51:40]

Doesn't exist anymore

[Marc Treib, 2016/10/28 14:49:49]

Done.

+
   FRIEND_TEST_ALL_PREFIXES(NTPSnippetsServiceTest,
                            RemoveExpiredDismissedContent);
   FRIEND_TEST_ALL_PREFIXES(NTPSnippetsServiceTest, RescheduleOnStateChange);
@@ -208,6 +214,8 @@ class NTPSnippetsService final : public ContentSuggestionsProvider,
 
   // Callback for the NTPSnippetsFetcher.
   void OnFetchFinished(
+      bool fetched_more,
+      FetchedMoreCallback fetched_more_callback,

[Marc Treib, 2016/10/20 16:51:40]

Hm. Should we maybe have two different FetchFinished methods for more vs.
regular fetches? (I dislike parameters that are used only sometimes)

[fhorschig, 2016/11/02 05:05:27]

I would make that change as soon as we decided how to handle the callbacks
within the SnippetsFetcher.

       NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories);
 
   // Moves all snippets from |to_archive| into the archive of the |category|.
@@ -215,8 +223,15 @@ class NTPSnippetsService final : public ContentSuggestionsProvider,
   // short.
   void ArchiveSnippets(Category category, NTPSnippet::PtrVector* to_archive);
 
-  // Replace old snippets in |category| by newly available snippets.
-  void ReplaceSnippets(Category category, NTPSnippet::PtrVector new_snippets);
+  // Add newly available snippets in |category| to the provided content.
+  // If |replace_snippets| is set, archive old snippets.

[Marc Treib, 2016/10/20 16:51:40]

s/old/existing/ ?

[Marc Treib, 2016/10/28 14:49:49]

Done.

+  void IncludeSnippets(const Category& category,
+                       NTPSnippet::PtrVector new_snippets,
+                       bool replace_snippets);
+
+  void SaveSnippets(const Category& category,

[Marc Treib, 2016/10/20 16:51:40]

What's the difference between IncludeSnippets and SaveSnippets?

[Marc Treib, 2016/10/28 14:49:50]

Done. (Merged SaveSnippets into IncludeSnippets)

+                    NTPSnippet::PtrVector new_snippets,
+                    bool replace_snippets);
 
   // Removes expired dismissed snippets from the service and the database.
   void ClearExpiredDismissedSnippets();
@@ -272,7 +287,8 @@ class NTPSnippetsService final : public ContentSuggestionsProvider,
 
   // Converts the cached snippets to article content suggestions and notifies
   // the observers.
-  void NotifyNewSuggestions();
+  void NotifyNewSuggestions(bool fetched_more,

[Marc Treib, 2016/10/20 16:51:40]

Heads-up: This will conflict with
https://chromiumcodereview.appspot.com/2438543003/

[Marc Treib, 2016/10/28 14:49:50]

Done.

+                            FetchedMoreCallback fetched_more_callback);
 
   // Updates the internal status for |category| to |category_status_| and
   // notifies the content suggestions observer if it changed.
@@ -283,6 +299,22 @@ class NTPSnippetsService final : public ContentSuggestionsProvider,
   void RestoreCategoriesFromPrefs();
   void StoreCategoriesToPrefs();
 
+  // Implementation for |FetchSnippets| and |FetchMore| that calls the snippet
+  // fetcher and replaces or adds the fetched snippets depending on the passed
+  // |strategy|.
+  void FetchSnippetsFromHostsImpl(const std::set<std::string>& hosts,
+                                  bool interactive_request,
+                                  bool fetch_more,
+                                  FetchedMoreCallback fetched_more_callback,
+                                  base::Optional<Category> exclusive_category);
+
+  // Returns a set of snippet ids that are should not be fetched in subsequent
+  // requests. These IDs akways include dismissed snippets.
+  // For |fetch more| requests, all known snippets are returned.

[Marc Treib, 2016/10/20 16:51:40]

IDs (capitalize)
"are should"
"akways"
fetch_more (underscore)
s/all known snippets are returned/returns all known snippet IDs/

[Marc Treib, 2016/10/28 14:49:50]

Done.

+  std::set<std::string> CollectIdsToExclude(bool fetch_more);

[Marc Treib, 2016/10/20 16:51:40]

const

[Marc Treib, 2016/10/28 14:49:50]

Done.

+
+  void MarkEmptyCategoriesAsLoading();
+
   State state_;
 
   PrefService* pref_service_;
@@ -323,7 +355,9 @@ class NTPSnippetsService final : public ContentSuggestionsProvider,
     ~CategoryContent();
     CategoryContent& operator=(CategoryContent&&);
   };
-  std::map<Category, CategoryContent, Category::CompareByID> categories_;
+  using CategoryContentMap =
+      std::map<Category, CategoryContent, Category::CompareByID>;
+  CategoryContentMap categories_;

[Marc Treib, 2016/10/20 16:51:40]

Any reason for this change?

[Marc Treib, 2016/10/28 14:49:50]

Done. (reverted)

 
   // The ISO 639-1 code of the language used by the application.
   const std::string application_language_code_;