[NTP Snippets] Refactor background scheduling for remote suggestions

components/ntp_snippets/remote/persistent_scheduler.h

Index: components/ntp_snippets/remote/persistent_scheduler.h
diff --git a/components/ntp_snippets/remote/persistent_scheduler.h b/components/ntp_snippets/remote/persistent_scheduler.h
new file mode 100644
index 0000000000000000000000000000000000000000..4e1b570f07932f03e35e6270b2c18e644a683626
--- /dev/null
+++ b/components/ntp_snippets/remote/persistent_scheduler.h
@@ -0,0 +1,48 @@
+// 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_PERSISTENT_SCHEDULER_H_
+#define COMPONENTS_NTP_SNIPPETS_REMOTE_PERSISTENT_SCHEDULER_H_
+
+#include "base/macros.h"
+#include "base/time/time.h"
+
+namespace ntp_snippets {
+
+// Interface to schedule persistent periodic fetches for remote suggestions.
+// These persistent fetches must get triggered according to their schedule
+// independent of whether Chrome is running at that moment.
+class PersistentScheduler {
+ public:
+  // Interface to access from platform dependend persistent scheduling code.
+  class Listener {

[tschumann, 2016/12/15 14:12:11]

this feels like the wrong place to implement this interface.
In my mind, the Android system scheduler and possibly the tbd soft scheduler are
like clock generators (that's why I'd call it PerodicTask or the like). In fact,
they are not scheduling a thing -- the NTPSnippetsLauncher *gets* scheduled by
the system and it triggers a fetch. What we want to introduce is a smarter fetch
logic so that it only maybe triggers a fetch -- and that logic is in
SchedulingRemoteSuggestionsProvider which actually maintains the schedule (and
hence is "Scheduling").

That said, I'd move the Listener interfac either into
scheduling_remote_suggestions_provider.h or content_suggestions_service.h
We can call it RemoteSuggestionsScheduler. I think for now a simply "DoFetch()"
would be sufficient -- later we might introduce more ones to better distinguish
between optional fetches etc.

The RescheduleFetching() seems odd -- in the case of the NTPSnippetsLauncher, it
would ultimately result in a call on the NTPSnippetsLauncher which feels like a
weird round-trip. Can we just leave this one out and make sure the
SchedulingRemoteSuggestionsProvider has access to the scheduler?

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

Done.

+   public:
+    // Fetches content suggestions.
+    virtual void OnFetchDue() = 0;
+
+    // Forces rescheduling of fetching.
+    virtual void RescheduleFetching() = 0;
+  };
+
+  // Schedule periodic fetching of remote suggestions, with different periods
+  // depending on network state. Once per period, the concrete implementation
+  // should call PersitentScheduler::Listener::OnFetchDue() where the listener
+  // is obtained from ContentSuggestionsService. Any of the periods can be zero
+  // to indicate that the corresponding task should not be scheduled.
+  virtual bool Schedule(base::TimeDelta period_wifi,
+                        base::TimeDelta period_fallback) = 0;
+
+  // Cancel any scheduled tasks. Equivalent to Schedule(0, 0).
+  virtual bool Unschedule() = 0;
+
+ protected:
+  PersistentScheduler() = default;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PersistentScheduler);
+};
+
+}  // namespace ntp_snippets
+
+#endif  // COMPONENTS_NTP_SNIPPETS_REMOTE_PERSISTENT_SCHEDULER_H_