Wait for the attestation to be ready (TPM being prepared for attestation) before trying to enroll.

chromeos/attestation/attestation_flow.h

Index: chromeos/attestation/attestation_flow.h
diff --git a/chromeos/attestation/attestation_flow.h b/chromeos/attestation/attestation_flow.h
index 26a7292d60a9d4eab6901f21e268e31b6a799846..a72562621773bb6827512e21eaab612eb07fcd5f 100644
--- a/chromeos/attestation/attestation_flow.h
+++ b/chromeos/attestation/attestation_flow.h
@@ -11,6 +11,7 @@
 #include "base/callback_forward.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/time/time.h"
 #include "chromeos/attestation/attestation_constants.h"
 #include "chromeos/chromeos_export.h"
 #include "chromeos/dbus/dbus_method_call_status.h"
@@ -18,6 +19,12 @@
 
 class AccountId;
 
+namespace base {
+

[achuithb, 2016/12/05 19:53:59]

remove newline

+class TickClock;
+

[achuithb, 2016/12/05 19:53:59]

remove newline

+}  // namespace base

[achuithb, 2016/12/05 19:53:59]

drop comment.

+
 namespace cryptohome {
 
 class AsyncMethodCaller;
@@ -48,7 +55,8 @@ class CHROMEOS_EXPORT ServerProxy {
 // and the Chrome OS Privacy CA server.  Sample usage:
 //    AttestationFlow flow(AsyncMethodCaller::GetInstance(),
 //                         DBusThreadManager::Get().GetCryptohomeClient(),
-//                         std::move(my_server_proxy));
+//                         std::move(my_server_proxy),
+//                         base::TimeDelta::Max());
 //    AttestationFlow::CertificateCallback callback = base::Bind(&MyCallback);
 //    flow.GetCertificate(ENTERPRISE_USER_CERTIFICATE, false, callback);
 class CHROMEOS_EXPORT AttestationFlow {
@@ -79,7 +87,8 @@ class CHROMEOS_EXPORT AttestationFlow {
 
   AttestationFlow(cryptohome::AsyncMethodCaller* async_caller,
                   CryptohomeClient* cryptohome_client,
-                  std::unique_ptr<ServerProxy> server_proxy);
+                  std::unique_ptr<ServerProxy> server_proxy,
+                  base::TimeDelta preparedness_timeout);
   virtual ~AttestationFlow();
 
   // Gets an attestation certificate for a hardware-protected key.  If a key for
@@ -88,6 +97,11 @@ class CHROMEOS_EXPORT AttestationFlow {
   // the key.  If the key already exists and |force_new_key| is false, the
   // existing certificate is returned.
   //
+  // If the TPM has not been prepared for attestation yet, this method will poll
+  // the attestation preparedness within the flow's |preparedness_timeout|.
+  // There is no guarantee than a retry will be made if the timeout is too
+  // short (e.g. less than 10 seconds).

[achuithb, 2016/12/05 19:53:59]

It's unusual to reference a param defined in one function in the comments of
another function like this. Is there a way you can move this verbiage to a
function comment for the ctor?

+  //
   // Parameters
   //   certificate_profile - Specifies what kind of certificate should be
   //                         requested from the CA.
@@ -108,8 +122,42 @@ class CHROMEOS_EXPORT AttestationFlow {
                               bool force_new_key,
                               const CertificateCallback& callback);
 
+  // Sets the tick clock for tests.
+  void SetTickClockForTest(base::TickClock* tick_clock);
+
  private:
+  struct RetryData;
+
   // Asynchronously initiates the attestation enrollment flow.
+  // If attestation is not ready yet, retry as needed.
+  //
+  // Parameters
+  //   on_failure - Called if any failure occurs.
+  //   next_task - Called on successful enrollment.
+  void InitiateEnroll(const base::Closure& on_failure,
+                      const base::Closure& next_task);
+
+  // Asynchronously tries to initiate the attestation enrollment flow.
+  // If attestation is not ready yet, retry as needed.
+  //
+  // Parameters
+  //   retry_data - Data to manage retries.
+  //   on_failure - Called if any failure occurs.
+  //   next_task - Called on successful enrollment.
+  void TryInitiateEnroll(RetryData* retry_data,
+                         const base::Closure& on_failure,
+                         const base::Closure& next_task);
+
+  // Called when atestation is not prepared yet, to re-initiate enrollment

[achuithb, 2016/12/05 19:53:59]

attestation spelling

+  // after a delay.
+  //
+  // Parameters
+  //   on_failure - Called if any failure occurs.
+  //   next_task - Called on successful enrollment.
+  void RetryInitiateEnroll(const base::Closure& on_failure,
+                           const base::Closure& next_task);
+
+  // Called when attestation is prepared, to start the actual enrollment flow.
   //
   // Parameters
   //   on_failure - Called if any failure occurs.
@@ -223,10 +271,21 @@ class CHROMEOS_EXPORT AttestationFlow {
                               const std::string& key_name,
                               const CertificateCallback& callback);
 
+  // Handles retries. If |retry_data| indicates that we are done retrying,
+  // runs |on_giving_up|, otherwise runs |on_retrying| after a delay.
+  void StillRetrying(RetryData* retry_data,
+                     const base::Closure& on_giving_up,
+                     const base::Closure& on_retrying);
+  // Handles the end of retries. Deletes |retry_data| and runs |continuation|.
+  void DoneRetrying(RetryData* retry_data, const base::Closure& continuation);
+
   cryptohome::AsyncMethodCaller* async_caller_;
   CryptohomeClient* cryptohome_client_;
   std::unique_ptr<ServerProxy> server_proxy_;
 
+  base::TimeDelta preparedness_timeout_;
+  base::TickClock* tick_clock_ = nullptr;
+
   base::WeakPtrFactory<AttestationFlow> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(AttestationFlow);