Added a timeout for platform verification key generation.

chrome/browser/chromeos/attestation/platform_verification_flow.h

Index: chrome/browser/chromeos/attestation/platform_verification_flow.h
diff --git a/chrome/browser/chromeos/attestation/platform_verification_flow.h b/chrome/browser/chromeos/attestation/platform_verification_flow.h
index 64f36274649b2ccc83f0daca72071a6823745d3e..f0581c8217945b76371b36fd0270504c586baf71 100644
--- a/chrome/browser/chromeos/attestation/platform_verification_flow.h
+++ b/chrome/browser/chromeos/attestation/platform_verification_flow.h
@@ -9,8 +9,8 @@
 
 #include "base/basictypes.h"
 #include "base/callback.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/memory/weak_ptr.h"
 #include "url/gurl.h"
 
 class PrefService;
@@ -43,7 +43,8 @@ class AttestationFlow;
 //   PlatformVerificationFlow::Callback callback = base::Bind(&MyCallback);
 //   verifier.ChallengePlatformKey(my_web_contents, "my_id", "some_challenge",
 //                                 callback);
-class PlatformVerificationFlow {
+class PlatformVerificationFlow
+    : public base::RefCountedThreadSafe<PlatformVerificationFlow> {

[pastarmovj, 2013/10/29 09:24:29]

From looking at this CL it is not obvious to me why do you need this class to be
RefCountedThreadSafe please put this explnation in the comment above this class
since this is a rather important detail IMO.

[Darren Krahn, 2013/10/29 17:23:04]

Done.  It is important -- please let me know if have a better way to solve this.

[pastarmovj, 2013/10/30 10:20:21]

Thanks this looks reasonable to me. I think your choice to use ref-counted is
reasonable. Especially the point that having this a singleton is not a good
choice because it isn't actually needed all the time so it doesn't have to stay
around all the time and pollute the global namespace/memory.

  public:
   enum Result {
     SUCCESS,                // The operation succeeded.
@@ -53,6 +54,7 @@ class PlatformVerificationFlow {
                             // - It is not running a verified OS image.
     USER_REJECTED,          // The user explicitly rejected the operation.
     POLICY_REJECTED,        // The operation is not allowed by policy/settings.
+    TIMEOUT,                // The operation timed out.
   };
 
   enum ConsentType {
@@ -111,8 +113,6 @@ class PlatformVerificationFlow {
                            UserManager* user_manager,
                            Delegate* delegate);
 
-  virtual ~PlatformVerificationFlow();
-
   // Invokes an asynchronous operation to challenge a platform key.  Any user
   // interaction will be associated with |web_contents|.  The |service_id| is an
   // arbitrary value but it should uniquely identify the origin of the request
@@ -138,52 +138,80 @@ class PlatformVerificationFlow {
     testing_url_ = testing_url;
   }
 
+  void set_timeout_delay(int timeout_delay) {
+    timeout_delay_ = timeout_delay;
+  }
+
  private:
+  // Tracks timeout status for an asynchronous operation.
+  struct TimeoutStatus : public base::RefCountedThreadSafe<TimeoutStatus> {
+    bool finished;
+    bool timed_out;
+
+    TimeoutStatus() : finished(false), timed_out(false) {}
+
+   private:
+    friend class base::RefCountedThreadSafe<TimeoutStatus>;
+    ~TimeoutStatus() {}
+  };
+
+  // Holds the arguments of a ChallengePlatformKey call.  This is convenient for
+  // use with base::Bind so we don't get too many arguments.
+  struct ChallengeContext {
+    content::WebContents* web_contents;
+    std::string service_id;
+    std::string challenge;
+    ChallengeCallback callback;
+  };
+
+  friend class base::RefCountedThreadSafe<PlatformVerificationFlow>;
+  ~PlatformVerificationFlow();
+
   // Checks whether we need to prompt the user for consent before proceeding and
-  // invokes the consent UI if so.  All parameters are the same as in
-  // ChallengePlatformKey except for the additional |attestation_enrolled| which
-  // specifies whether attestation has been enrolled for this device.
-  void CheckConsent(content::WebContents* web_contents,
-                    const std::string& service_id,
-                    const std::string& challenge,
-                    const ChallengeCallback& callback,
+  // invokes the consent UI if so.  The arguments to ChallengePlatformKey are
+  // in |context| and |attestation_enrolled| specifies whether attestation has
+  // been enrolled for this device.
+  void CheckConsent(const ChallengeContext& context,
                     bool attestation_enrolled);
 
-  // A callback called when the user has given their consent response.  All
-  // parameters are the same as in ChallengePlatformKey except for the
-  // additional |consent_type| and |consent_response| which indicate the consent
-  // type and user response, respectively.  If the response indicates that the
-  // operation should proceed, this method invokes a certificate request.
-  void OnConsentResponse(content::WebContents* web_contents,
-                         const std::string& service_id,
-                         const std::string& challenge,
-                         const ChallengeCallback& callback,
+  // A callback called when the user has given their consent response.  The
+  // arguments to ChallengePlatformKey are in |context|.  |consent_type| and
+  // |consent_response| indicate the consent type and user response,
+  // respectively.  If the response indicates that the operation should proceed,
+  // this method invokes a certificate request.
+  void OnConsentResponse(const ChallengeContext& context,
                          ConsentType consent_type,
                          ConsentResponse consent_response);
 
   // A callback called when an attestation certificate request operation
-  // completes.  |service_id|, |challenge|, and |callback| are the same as in
-  // ChallengePlatformKey.  |user_id| identifies the user for which the
-  // certificate was requested.  |operation_success| is true iff the certificate
-  // request operation succeeded.  |certificate| holds the certificate for the
-  // platform key on success.  If the certificate request was successful, this
-  // method invokes a request to sign the challenge.
-  void OnCertificateReady(const std::string& user_id,
-                          const std::string& service_id,
-                          const std::string& challenge,
-                          const ChallengeCallback& callback,
+  // completes.  The arguments to ChallengePlatformKey are in |context|.
+  // |user_id| identifies the user for which the certificate was requested.
+  // |operation_success| is true iff the certificate request operation
+  // succeeded.  |certificate| holds the certificate for the platform key on
+  // success.  If the certificate request was successful, this method invokes a
+  // request to sign the challenge.  If the operation timed out prior to this
+  // method being called, this method does nothing - notably, the callback is
+  // not invoked.
+  void OnCertificateReady(const ChallengeContext& context,
+                          const std::string& user_id,
+                          TimeoutStatus* timeout_status,
                           bool operation_success,
                           const std::string& certificate);
 
+  // A callback run after a constant delay to handle timeouts for lengthy
+  // certificate requests.  |context.callback| will be invoked with a TIMEOUT
+  // result if the certificate request has not already finished.
+  void OnCertificateTimeout(const ChallengeContext& context,
+                            TimeoutStatus* timeout_status);
+
   // A callback called when a challenge signing request has completed.  The
   // |certificate| is the platform certificate for the key which signed the
-  // |challenge|.  |callback| is the same as in ChallengePlatformKey.
+  // |challenge|.  The arguments to ChallengePlatformKey are in |context|.
   // |operation_success| is true iff the challenge signing operation was
   // successful.  If it was successful, |response_data| holds the challenge
-  // response and the method will invoke |callback|.
-  void OnChallengeReady(const std::string& certificate,
-                        const std::string& challenge,
-                        const ChallengeCallback& callback,
+  // response and the method will invoke |context.callback|.
+  void OnChallengeReady(const ChallengeContext& context,
+                        const std::string& certificate,
                         bool operation_success,
                         const std::string& response_data);
 
@@ -242,10 +270,7 @@ class PlatformVerificationFlow {
   scoped_ptr<Delegate> default_delegate_;
   PrefService* testing_prefs_;
   GURL testing_url_;
-
-  // Note: This should remain the last member so it'll be destroyed and
-  // invalidate the weak pointers before any other members are destroyed.
-  base::WeakPtrFactory<PlatformVerificationFlow> weak_factory_;
+  int timeout_delay_;
 
   DISALLOW_COPY_AND_ASSIGN(PlatformVerificationFlow);
 };