FetchMore functionality backend

components/ntp_snippets/remote/ntp_snippets_service.h

 // Copyright 2015 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_REMOTE_NTP_SNIPPETS_SERVICE_H_
 #define COMPONENTS_NTP_SNIPPETS_REMOTE_NTP_SNIPPETS_SERVICE_H_
 
 #include <cstddef>
 #include <map>
 #include <memory>
@@ skipping 98 matching lines @@
   // tasks will be re-scheduled even if they already exist and have the correct
   // periods.
   void RescheduleFetching(bool force);
 
   // ContentSuggestionsProvider implementation
   CategoryStatus GetCategoryStatus(Category category) override;
   CategoryInfo GetCategoryInfo(Category category) override;
   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,
       const base::Callback<bool(const GURL& url)>& filter) override;
   void ClearCachedSuggestions(Category category) override;
   void GetDismissedSuggestionsForDebugging(
       Category category,
       const DismissedSuggestionsCallback& callback) override;
   void ClearDismissedSuggestionsForDebugging(Category category) override;
 
@@ skipping 13 matching lines @@
   }
 
   // Dismissed snippets, only for unit tests.
   const NTPSnippet::PtrVector& GetDismissedSnippetsForTesting(
       Category category) const {
     return categories_.find(category)->second.dismissed;
   }
 
  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);
   FRIEND_TEST_ALL_PREFIXES(NTPSnippetsServiceTest, StatusChanges);
   FRIEND_TEST_ALL_PREFIXES(NTPSnippetsServiceTest,
                            SuggestionsFetchedOnSignInAndSignOut);
 
   // Possible state transitions:
   //       NOT_INITED --------+
   //       /       \          |
@@ skipping 39 matching lines @@
   // image_fetcher::ImageFetcherDelegate implementation.
   void OnImageDataFetched(const std::string& id_within_category,
                           const std::string& image_data) override;
 
   // Callbacks for the NTPSnippetsDatabase.
   void OnDatabaseLoaded(NTPSnippet::PtrVector snippets);
   void OnDatabaseError();
 
   // 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|.
   // It also deletes the snippets from the DB and keeps the archive reasonably
   // 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();
 
   // Removes images from the DB that are not referenced from any known snippet.
   // Needs to iterate the whole snippet database -- so do it often enough to
   // keep it small but not too often as it still iterates over the file system.
   void ClearOrphanedImages();
 
   // Clears all stored snippets and updates the observer.
@@ skipping 35 matching lines @@
 
   // Disables the service. Do not call directly, use |EnterState| instead.
   void EnterStateDisabled();
 
   // Disables the service permanently because an unrecoverable error occurred.
   // Do not call directly, use |EnterState| instead.
   void EnterStateError();
 
   // 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.
   void UpdateCategoryStatus(Category category, CategoryStatus status);
   // Calls UpdateCategoryStatus() for all provided categories.
   void UpdateAllCategoryStatus(CategoryStatus status);
 
   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_;
 
   const Category articles_category_;
 
   // TODO(sfiera): Reduce duplication of CategoryContent with CategoryInfo.
   struct CategoryContent {
     CategoryStatus status = CategoryStatus::INITIALIZING;
 
@@ skipping 20 matching lines @@
 
     // Returns a non-dismissed snippet with the given |id_within_category|, or
     // null if none exist.
     const NTPSnippet* FindSnippet(const std::string& id_within_category) const;
 
     CategoryContent();
     CategoryContent(CategoryContent&&);
     ~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_;
 
   // Classifier that tells us how active the user is. Not owned.
   const UserClassifier* user_classifier_;
 
   // Scheduler for fetching snippets. Not owned.
   NTPSnippetsScheduler* scheduler_;
 
@@ skipping 20 matching lines @@
 
   // Request throttler for limiting requests to thumbnail images.
   RequestThrottler thumbnail_requests_throttler_;
 
   DISALLOW_COPY_AND_ASSIGN(NTPSnippetsService);
 };
 
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_REMOTE_NTP_SNIPPETS_SERVICE_H_