[NTP Snippets] Refactor background scheduling for remote suggestions

components/ntp_snippets/content_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_CONTENT_SUGGESTIONS_PROVIDER_H_
 #define COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_PROVIDER_H_
 
 #include <set>
 #include <string>
 #include <vector>
@@ skipping 86 matching lines @@
 
   // Fetches more suggestions for the given category. The new suggestions
   // will not include any suggestion of the |known_suggestion_ids| sets.
   // The given |callback| is called with these suggestions, along with all
   // existing suggestions. It has to be invoked exactly once as the front-end
   // might wait for its completion.
   virtual void Fetch(const Category& category,
                      const std::set<std::string>& known_suggestion_ids,
                      const FetchDoneCallback& callback) = 0;
 
+  // Reloads suggestions from all categories. Makes sense to call only after

[tschumann, 2016/12/15 19:27:00]

Can we find a better name for this method? To me it conveys pretty much no
semantics.
I find this comment confusing, why doesn't it make sense to call if no cards are
dismissed? It would still fetch new ones, right?

Isn't this just a FetchForAllCategories() functionality?

[jkrcal, 2016/12/19 09:40:24]

Removed the comment. 

As regards naming, this is a mess :) It is kind-of FetchForAllCategories but it
has a different API so I did not want to name the function analogously. Fetch
has fetch-and-return-by-callback semantics (and has not a clear semantics about
whether the additional suggestions should be cached in the provider). This is
fetch-for-all-and-notify-the-observer semantics.

Markus is about to clean things up a bit so this function may become redundant
and get renamed.

Do you feel strongly about some name?

+  // some suggestions have been dismissed. The provider may fill in the vacant
+  // space by new suggestions that haven't been included previously due to space
+  // limits. If this happens, the observer must be notified via
+  // OnNewSuggestions();
+  virtual void ReloadSuggestions() {}

[tschumann, 2016/12/15 19:27:00]

hm... we should make this pure virtual -- add a TODO()?

[jkrcal, 2016/12/19 09:40:24]

Done.

+
   // Removes history from the specified time range where the URL matches the
   // |filter|. The data removed depends on the provider. Note that the
   // data outside the time range may be deleted, for example suggestions, which
   // are based on history from that time range. Providers should immediately
   // clear any data related to history from the specified time range where the
   // URL matches the |filter|.
   virtual void ClearHistory(
       base::Time begin,
       base::Time end,
       const base::Callback<bool(const GURL& url)>& filter) = 0;
@@ skipping 32 matching lines @@
   CategoryFactory* category_factory() const { return category_factory_; }
 
  private:
   Observer* observer_;
   CategoryFactory* category_factory_;
 };
 
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_PROVIDER_H_