[Enrollment v2] Refactor CryptAuth v2 Enrollment schedulers

See design at http://go/cros-devicesync-v2-scheduling

In preparation for DeviceSync v2, we refactor the v2 Enrollment
schedulers. DeviceSync v2 scheduling will be added to these schedulers
in a subsequent CL.

In this CL, we
  * Move ownership of the scheduler from the Enrollment manager to
    DeviceSyncImpl. This way the scheduler can be shared between the
    v2 Enrollment and DeviceSync managers.
  * Move all caching and invocation reason determination out of the
    Enrollment manager and into the scheduler.
  * Create a pending-request caching mechanism in the scheduler. This
    allows us store an Enrollment request that is made before
    scheduling has been started, while an Enrollment attempt is in
    progress, or while we are offline.
  * The latter point enables us to easily merge the "persistent" and
    "network-aware" schedulers into a single implementation.

Note: M76 branch is May 30th, so these changes will make it into the v2
Enrollment rollout.

Bug: 899080
Change-Id: Ia994141f7f245ff316f0df93b2f979c3756d94ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1606244
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#661367}

chromeos/services/device_sync/BUILD.gn

@@ -67,8 +67,6 @@ static_library("device_sync") {
     "device_sync_service.h",
     "device_sync_type_converters.cc",
     "device_sync_type_converters.h",
-    "network_aware_enrollment_scheduler.cc",
-    "network_aware_enrollment_scheduler.h",
     "network_request_error.cc",
     "network_request_error.h",
     "pref_names.cc",
@@ -187,7 +185,6 @@ source_set("unit_tests") {
     "cryptauth_v2_enroller_impl_unittest.cc",
     "cryptauth_v2_enrollment_manager_impl_unittest.cc",
     "device_sync_service_unittest.cc",
-    "network_aware_enrollment_scheduler_unittest.cc",
     "remote_device_loader_unittest.cc",
     "remote_device_provider_impl_unittest.cc",
     "software_feature_manager_impl_unittest.cc",

chromeos/services/device_sync/cryptauth_scheduler.cc

@@ -8,19 +8,39 @@ namespace chromeos {
 
 namespace device_sync {
 
-CryptAuthScheduler::CryptAuthScheduler(Delegate* delegate)
-    : delegate_(delegate) {
-  DCHECK(delegate);
-}
+CryptAuthScheduler::CryptAuthScheduler() = default;
 
 CryptAuthScheduler::~CryptAuthScheduler() = default;
 
+void CryptAuthScheduler::StartEnrollmentScheduling(
+    const base::WeakPtr<EnrollmentDelegate>& enrollment_delegate) {
+  // Ensure this is only called once.
+  DCHECK(!enrollment_delegate_);
+
+  DCHECK(enrollment_delegate);
+  enrollment_delegate_ = enrollment_delegate;
+
+  OnEnrollmentSchedulingStarted();
+}
+
+bool CryptAuthScheduler::HasEnrollmentSchedulingStarted() {
+  return enrollment_delegate_.get();
+}
+
 void CryptAuthScheduler::NotifyEnrollmentRequested(
+    const cryptauthv2::ClientMetadata& client_metadata,
     const base::Optional<cryptauthv2::PolicyReference>&
         client_directive_policy_reference) const {
-  delegate_->OnEnrollmentRequested(client_directive_policy_reference);
+  // Do nothing if weak pointer was invalidated.
+  if (!enrollment_delegate_)
+    return;
+
+  enrollment_delegate_->OnEnrollmentRequested(
+      client_metadata, client_directive_policy_reference);
 }
 
+void CryptAuthScheduler::OnEnrollmentSchedulingStarted() {}
+
 }  // namespace device_sync
 
 }  // namespace chromeos

chromeos/services/device_sync/cryptauth_scheduler.h

@@ -5,7 +5,10 @@
 #ifndef CHROMEOS_SERVICES_DEVICE_SYNC_CRYPTAUTH_SCHEDULER_H_
 #define CHROMEOS_SERVICES_DEVICE_SYNC_CRYPTAUTH_SCHEDULER_H_
 
+#include <string>
+
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/optional.h"
 #include "base/time/time.h"
 #include "chromeos/services/device_sync/cryptauth_enrollment_result.h"
@@ -17,34 +20,45 @@ namespace device_sync {
 
 // Schedules periodic enrollments, alerting the delegate when an enrollment
 // attempt is due via Delegate::OnEnrollmentRequested(). The periodic scheduling
-// begins on construction. The client can bypass the periodic schedule and
-// immediately trigger an enrollment request via RequestEnrollmentNow(). When an
+// begins on StartEnrollmentScheduling(). The client can bypass the periodic
+// schedule and trigger an enrollment request via RequestEnrollment(). When an
 // enrollment attempt has completed, successfully or not, the client should
 // invoke HandleEnrollmentResult() so the scheduler can process the enrollment
 // attempt outcome.
 class CryptAuthScheduler {
  public:
-  class Delegate {
+  class EnrollmentDelegate {
    public:
-    Delegate() = default;
-    virtual ~Delegate() = default;
+    EnrollmentDelegate() = default;
+    virtual ~EnrollmentDelegate() = default;
 
     // Called to alert the delegate that an enrollment attempt has been
     // requested by the scheduler.
-    // |client_directive_policy_reference|: Identifies the CryptAuth policy
-    // associated with the ClientDirective parameters used to schedule this
-    // enrollment attempt. If no ClientDirective was used by the scheduler,
-    // base::nullopt is passed.
+    //   |client_metadata|: Contains the retry count, invocation reason, and
+    //       possible session ID of the enrollment request.
+    //   |client_directive_policy_reference|: Identifies the CryptAuth policy
+    //       associated with the ClientDirective parameters used to schedule
+    //       this enrollment attempt. If no ClientDirective was used by the
+    //       scheduler, base::nullopt is passed.
     virtual void OnEnrollmentRequested(
+        const cryptauthv2::ClientMetadata& client_metadata,
         const base::Optional<cryptauthv2::PolicyReference>&
             client_directive_policy_reference) = 0;
   };
 
   virtual ~CryptAuthScheduler();
 
-  // Cancels the currently scheduled enrollment, and requests an enrollment
-  // immediately.
-  virtual void RequestEnrollmentNow() = 0;
+  // Note: This should only be called once.
+  void StartEnrollmentScheduling(
+      const base::WeakPtr<EnrollmentDelegate>& enrollment_delegate);
+
+  bool HasEnrollmentSchedulingStarted();
+
+  // Requests an enrollment with the desired |invocation_reason| and, if
+  // relevant, the |session_id| of the GCM message that requested enrollment.
+  virtual void RequestEnrollment(
+      const cryptauthv2::ClientMetadata::InvocationReason& invocation_reason,
+      const base::Optional<std::string>& session_id) = 0;
 
   // Processes the result of the previous enrollment attempt.
   virtual void HandleEnrollmentResult(
@@ -55,10 +69,10 @@ class CryptAuthScheduler {
   virtual base::Optional<base::Time> GetLastSuccessfulEnrollmentTime()
       const = 0;
 
-  // Returns the scheduler's time period between a successful enrollment and
-  // its next enrollment request. Note that this period may not be strictly
-  // adhered to due to RequestEnrollmentNow() calls or the system being offline,
-  // for instance.
+  // Returns the scheduler's time period between a successful enrollment and its
+  // next enrollment request. Note that this period may not be strictly adhered
+  // to due to RequestEnrollment() calls or the system being offline, for
+  // instance.
   virtual base::TimeDelta GetRefreshPeriod() const = 0;
 
   // Returns the time until the next scheduled enrollment request.
@@ -68,20 +82,24 @@ class CryptAuthScheduler {
   // has not been processed yet.
   virtual bool IsWaitingForEnrollmentResult() const = 0;
 
-  // The number of consecutive failed enrollment attempts. Once an enrollment
-  // attempt succeeds, this counter is reset.
-  virtual size_t GetNumConsecutiveFailures() const = 0;
+  // The number of times the current enrollment request has failed. Once the
+  // enrollment request succeeds or a fresh request is made--for example, via a
+  // forced enrollment--this counter is reset.
+  virtual size_t GetNumConsecutiveEnrollmentFailures() const = 0;
 
  protected:
-  explicit CryptAuthScheduler(Delegate* delegate);
+  CryptAuthScheduler();
+
+  virtual void OnEnrollmentSchedulingStarted();
 
   // Alerts the delegate that an enrollment has been requested.
   void NotifyEnrollmentRequested(
+      const cryptauthv2::ClientMetadata& client_metadata,
       const base::Optional<cryptauthv2::PolicyReference>&
           client_directive_policy_reference) const;
 
  private:
-  Delegate* delegate_;
+  base::WeakPtr<EnrollmentDelegate> enrollment_delegate_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(CryptAuthScheduler);
 };