[Background fetching] Background fetching when opening an NTP.

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 <utility>
 #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 base {
+class Clock;
+}
+
 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.
@@ skipping 56 matching lines @@
       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;
 
+  // Overrides internal clock for testing purposes.
+  void SetClockForTesting(std::unique_ptr<base::Clock> clock) {
+    clock_ = std::move(clock);

[tschumann, 2017/01/04 10:43:52]

can we dependency inject this into the constructor? I don't think we have many
places to update and that would be the superior approach.
(ForTesting() methods are crutches applicable as temporary solutions where the
actual cleanup is too costly for the moment).

[jkrcal, 2017/01/04 14:19:03]

Done.

+  }
+
  private:
   // Abstract description of the fetching schedule.
-  struct FetchingSchedule;
+  struct FetchingSchedule {
+      base::TimeDelta interval_persistent_wifi;

[tschumann, 2017/01/04 10:43:52]

members go after methods.

[jkrcal, 2017/01/04 14:19:03]

Done.

+      base::TimeDelta interval_persistent_fallback;
+      base::TimeDelta interval_soft_on_usage_event;
+
+      static FetchingSchedule Empty();
+      bool operator==(const FetchingSchedule& other) const;
+      bool operator!=(const FetchingSchedule& other) const;
+      bool is_empty() const;
+  };
 
   // Callback that is notified whenever the status of |provider_| changes.
   void OnProviderStatusChanged(
       RemoteSuggestionsProvider::ProviderStatus status);
 
   // 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();
 
+  // Checks whether it is time to perform a soft background fetch, according to
+  // |schedule|.
+  bool ShouldRefetchInTheBackgroundNow();
+
   // Callback after Fetch is completed.
   void FetchFinished(const FetchDoneCallback& callback,
-                     Status status_code,
+                     Status fetch_status,
                      std::vector<ContentSuggestion> suggestions);
 
+  // Callback after RefetchInTheBackground is completed.
+  void RefetchInTheBackgroundFinished(
+      std::unique_ptr<FetchStatusCallback> callback,
+      Status fetch_status);
+
+  // Common function to call after a fetch of any type is finished.
+  void OnFetchCompleted(Status fetch_status);
+
   FetchingSchedule GetDesiredFetchingSchedule() const;
-  FetchingSchedule GetLastFetchingSchedule() const;
-  void StoreLastFetchingSchedule(const FetchingSchedule& schedule);
 
-  // Common function to call after each fetch.
-  void OnFetchCompleted(Status status);
+  // Load and store |schedule_|.
+  void LoadLastFetchingSchedule();
+  void StoreFetchingSchedule();
 
-  // Applies the provided |schedule|.
-  void ApplyFetchingSchedule(const FetchingSchedule& schedule);
+  // Applies the schedule given by |schedule_|.
+  void ApplyFetchingSchedule(bool also_apply_persistent_schedule);
 
   // Interface for doing all the actual work (apart from scheduling).
   std::unique_ptr<RemoteSuggestionsProvider> provider_;
 
   // Interface for scheduling hard fetches, OS dependent. Not owned, may be
   // null.
   PersistentScheduler* persistent_scheduler_;
 
+  FetchingSchedule schedule_;
+

[tschumann, 2017/01/04 10:43:52]

nit: in general, we should aim to limit vertical white space. In the members
section, there's no comment addressing multiple members, so I'd vote for
removing the blank lines between them.

[jkrcal, 2017/01/04 14:19:03]

Done.

+  bool background_fetch_in_progress_;
+
   // Used to adapt the schedule based on usage activity of the user. Not owned.
   const UserClassifier* user_classifier_;
 
   PrefService* pref_service_;
 
+  // Allow for an injectable clock for testing.

[tschumann, 2017/01/04 10:43:52]

this comment is not necessary.

[jkrcal, 2017/01/04 14:19:03]

Done.

+  std::unique_ptr<base::Clock> clock_;
+
   DISALLOW_COPY_AND_ASSIGN(SchedulingRemoteSuggestionsProvider);
 };
 
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_REMOTE_SCHEDULING_REMOTE_SUGGESTIONS_PROVIDER_H_