Finalize backend for fetching more NTPSnippets.

components/ntp_snippets/bookmarks/bookmark_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_BOOKMARKS_BOOKMARK_SUGGESTIONS_PROVIDER_H_
 #define COMPONENTS_NTP_SNIPPETS_BOOKMARKS_BOOKMARK_SUGGESTIONS_PROVIDER_H_
 
 #include <set>
 #include <string>
 
@@ skipping 24 matching lines @@
 
   static void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
  private:
   // 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 Fetch(const Category& category,
+             std::set<std::string> known_suggestion_ids,

[tschumann, 2016/11/02 11:23:22]

@dgn (to get quicker feedback as Friedrich is not available).

we should pass the ids per const reference. Any reason why we use per value and
rely on std::move().
The problem with per-value and std::move() is that if you forget std::move()
you'll make a copy without noticing *and* you have zombie-state data on the
caller side (moved-out variables). Their state is usually well defined with STL
data types, but accessing them rings a few alarm bells.

[dgn, 2016/11/02 12:04:33]

Yes, I also would prefer having const references rather than duplicating or
moving things around manually. I see no specific reason why it is that way
currently.

[fhorschig, 2016/11/03 01:53:14]

Done.

BTW: I used the ownership of the set to std::move all elements in
NTPSnippetService::CollectIdsToExclude() into another set. Now, the inserted
strings have to be copied. What I could have used is an rvalue-reference as
parameter but copying strings is rather cheap compared to accidentally copying
the whole set, so I will go with your solution.

+             FetchingCallback 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 58 matching lines @@
   // For six weeks after first installing M54, this is true and the
   // fallback implemented in BookmarkLastVisitUtils is activated.
   bool creation_date_fallback_;
 
   DISALLOW_COPY_AND_ASSIGN(BookmarkSuggestionsProvider);
 };
 
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_BOOKMARKS_BOOKMARK_SUGGESTIONS_PROVIDER_H_