[NTP Snippets] Refactor background scheduling for remote suggestions

components/ntp_snippets/remote/scheduling_remote_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_REMOTE_SCHEDULING_REMOTE_SUGGESTIONS_PROVIDER_H_
+#define COMPONENTS_NTP_SNIPPETS_REMOTE_SCHEDULING_REMOTE_SUGGESTIONS_PROVIDER_H_
+
+#include <memory>
+#include <set>
+#include <string>
+#include <vector>
+
+#include "base/macros.h"
+#include "base/time/time.h"
+#include "components/ntp_snippets/content_suggestions_provider.h"
+#include "components/ntp_snippets/remote/persistent_scheduler.h"
+#include "components/ntp_snippets/remote/remote_suggestions_provider.h"
+#include "components/ntp_snippets/remote/remote_suggestions_scheduler.h"
+
+class PrefRegistrySimple;
+class PrefService;
+
+namespace ntp_snippets {
+
+struct Status;
+class UserClassifier;
+
+// A wrapper around RemoteSuggestionsProvider that introduces periodic fetching.
+//
+// The class initiates fetches on its own in these situations:
+//  - initial fetch when the provider is constructed and we have no suggestions;
+//  - regular fetches according to its schedule (when notified via

[tschumann, 2016/12/19 11:07:19]

i'd drop the part in parenthesis. It's an implementation detail which might
change soon. It's fine to have comments like that at the function definition,
but I'd consider them noise in headers.

[jkrcal, 2016/12/20 16:39:47]

Done.

+//  PersistentScheduler::Listener::OnFetchDue).
+// TODO(jkrcal): After soft fetch on Chrome startup is introduced, remove
+// the initial fetch completely.
+//
+// The class also needs to understand when last fetch trials and successful
+// fetches happen and thus it intercepts following interactive fetch requests:
+//  - Fetch() - after "More" button of a remote section is pressed in the UI;
+// TODO(jkrcal): Clarify what Fetch() should do for this provider and maybe stop
+// intercepting it.
+// TODO(jkrcal): Intercept also ReloadSuggestions() call (after the user swipes
+// away everything incl. all empty sections and presses "More"); Not done in the
+// first shot because  it implements a public interface function without any
+// callback.
+// This class is final because it does things in its constructor which make it
+// unsafe to derive from it.
+// TODO(jkrcal): Introduce two-phase initialization and make the class not
+// final? (see the same comment for RemoteSuggestionsProvider)
+class SchedulingRemoteSuggestionsProvider final
+    : public ContentSuggestionsProvider,
+      public RemoteSuggestionsScheduler {
+ public:
+  // The passed in |updater| is called when an update is due according to the

[Marc Treib, 2016/12/19 12:59:24]

There's no |updater|

[jkrcal, 2016/12/20 16:39:47]

Done.

+  // schedule. Note that hard fetches get access to the |updater| via the keyed
+  // ContentSuggestionService because the concrete instance passed to
+  // RemoteSuggestionsScheduler when the hard fetch was scheduled may not exist
+  // any more when the hard update is due.
+  SchedulingRemoteSuggestionsProvider(
+      Observer* observer,
+      CategoryFactory* category_factory,
+      std::unique_ptr<RemoteSuggestionsProvider> provider,
+      PersistentScheduler* persistent_scheduler,
+      const UserClassifier* user_classifier,
+      PrefService* pref_service);
+
+  ~SchedulingRemoteSuggestionsProvider() override;
+
+  static void RegisterProfilePrefs(PrefRegistrySimple* registry);
+
+  // RemoteSuggestionsScheduler implementation

[tschumann, 2016/12/19 11:07:19]

nit: Comments should end with a period.

[jkrcal, 2016/12/20 16:39:47]

Done.

+  void RescheduleFetching() override;
+  void OnFetchDue() override;
+
+  // 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 Fetch(const Category& category,
+             const std::set<std::string>& known_suggestion_ids,
+             const FetchDoneCallback& callback) override;
+  void ReloadSuggestions() override;
+  void ClearHistory(
+      base::Time begin,
+      base::Time end,
+      const base::Callback<bool(const GURL& url)>& filter) override;
+  void ClearCachedSuggestions(Category category) override;
+  void OnSignInStateChanged() override;
+  void GetDismissedSuggestionsForDebugging(
+      Category category,
+      const DismissedSuggestionsCallback& callback) override;
+  void ClearDismissedSuggestionsForDebugging(Category category) override;
+
+ private:
+  // Abstract description of the fetching schedule.
+  struct FetchingSchedule;
+
+  void OnProviderStatusChanged(

[Marc Treib, 2016/12/19 12:59:24]

Comment?

[jkrcal, 2016/12/20 16:39:47]

Done.

+      RemoteSuggestionsProvider::ProviderStatus status);
+
+  // One of the following two functions must be called on startup of Chrome for
+  // the scheduler to work correctly.
+  // After the call, updates will be scheduled in the future. Idempotent, can be
+  // run any time later without impacting the current schedule.
+  // If you want to enforce rescheduling, call Unschedule() and then Schedule().
+  void StartScheduling();
+
+  // After the call, no updates will happen before another call to Schedule().
+  // Idempotent, can be run any time later without impacting the current
+  // schedule.
+  void StopScheduling();
+
+  // Callback after Fetch is completed.
+  void FetchFinished(const FetchDoneCallback& callback,
+                     Status status_code,
+                     std::vector<ContentSuggestion> suggestions);
+
+  FetchingSchedule GetDesiredFetchingSchedule();

[Marc Treib, 2016/12/19 12:59:24]

Can this be const?

+  FetchingSchedule GetLastFetchingSchedule() const;
+  void StoreLastFetchingSchedule(const FetchingSchedule& schedule);
+
+    // Common function to call after each fetch.

[Marc Treib, 2016/12/19 12:59:24]

misaligned

[jkrcal, 2016/12/20 16:39:47]

Done.

+  void OnFetchCompleted(Status status);
+
+  // Applies the provided |schedule|.
+  void ApplyFetchingSchedule(const FetchingSchedule& schedule);
+
+  // Interface for doing the actual updates, when they are due. Not owned.

[Marc Treib, 2016/12/19 12:59:24]

If it's a unique_ptr, it's owned by definition.

[jkrcal, 2016/12/20 16:39:47]

Done (outdated comment).

+  std::unique_ptr<RemoteSuggestionsProvider> provider_;
+
+  // Interface for scheduling hard fetches, OS dependent. Not owned, may be
+  // null.
+  PersistentScheduler* persistent_scheduler_;
+
+  // Used to adapt the schedule based on usage activity of the user. Not owned.
+  const UserClassifier* user_classifier_;
+
+  PrefService* pref_service_;
+
+  DISALLOW_COPY_AND_ASSIGN(SchedulingRemoteSuggestionsProvider);
+};
+
+}  // namespace ntp_snippets
+
+#endif  // COMPONENTS_NTP_SNIPPETS_REMOTE_SCHEDULING_REMOTE_SUGGESTIONS_PROVIDER_H_