Refactor CryptAuth component to be more testable.

components/proximity_auth/cryptauth/cryptauth_client_impl.h

Index: components/proximity_auth/cryptauth/cryptauth_client_impl.h
diff --git a/components/proximity_auth/cryptauth/cryptauth_client.h b/components/proximity_auth/cryptauth/cryptauth_client_impl.h
similarity index 56%
copy from components/proximity_auth/cryptauth/cryptauth_client.h
copy to components/proximity_auth/cryptauth/cryptauth_client_impl.h
index 0e77e5025b857f37379a82fc4cdf8657d64c64d7..0a2a0056685b5eace022d6fcbc35a56e20990faa 100644
--- a/components/proximity_auth/cryptauth/cryptauth_client.h
+++ b/components/proximity_auth/cryptauth/cryptauth_client_impl.h
@@ -2,93 +2,57 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef COMPONENTS_PROXIMITY_AUTH_CRYPT_AUTH_CLIENT_H
-#define COMPONENTS_PROXIMITY_AUTH_CRYPT_AUTH_CLIENT_H
+#ifndef COMPONENTS_PROXIMITY_AUTH_CRYPT_AUTH_CLIENT_IMPL_H
+#define COMPONENTS_PROXIMITY_AUTH_CRYPT_AUTH_CLIENT_IMPL_H
 
-#include "base/callback.h"
-#include "base/macros.h"
-#include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
-#include "components/proximity_auth/cryptauth/proto/cryptauth_api.pb.h"
+#include "components/proximity_auth/cryptauth/cryptauth_access_token_fetcher.h"
+#include "components/proximity_auth/cryptauth/cryptauth_api_call_flow.h"
+#include "components/proximity_auth/cryptauth/cryptauth_client.h"
 #include "net/url_request/url_request_context_getter.h"
-#include "url/gurl.h"
 
 namespace proximity_auth {
 
-class CryptAuthAccessTokenFetcher;
-class CryptAuthApiCallFlow;
-
-// Use CryptAuthClient to make API requests to the CryptAuth service, which
-// manages cryptographic credentials (ie. public keys) for a user's devices.
-// CryptAuthClient only processes one request, so create a new instance for each
-// request you make. DO NOT REUSE.
-// For documentation on each API call, see
-// components/proximity_auth/cryptauth/proto/cryptauth_api.proto
+// Implementation of CryptAuthClient.
 // Note: There is no need to set the |device_classifier| field in request
 // messages. CryptAuthClient will fill this field for all requests.
-class CryptAuthClient {
+class CryptAuthClientImpl : public CryptAuthClient {
  public:
   typedef base::Callback<void(const std::string&)> ErrorCallback;
 
-  // Creates the client using |url_request_context| to make the HTTP request.
-  // CryptAuthClient takes ownership of |access_token_fetcher|, which provides
-  // the access token authorizing CryptAuth requests.
-  // The |device_classifier| argument contains basic device information of the
-  // caller (e.g. version and device type).
-  CryptAuthClient(
+  // Creates the client using |url_request_context| to make the HTTP request
+  // through |api_call_flow|. CryptAuthClientImpl takes ownership of
+  // |access_token_fetcher|, which provides the access token authorizing
+  // CryptAuth requests. The |device_classifier| argument contains basic device
+  // information of the caller (e.g. version and device type).
+  CryptAuthClientImpl(
+      scoped_ptr<CryptAuthApiCallFlow> api_call_flow,
       scoped_ptr<CryptAuthAccessTokenFetcher> access_token_fetcher,
       scoped_refptr<net::URLRequestContextGetter> url_request_context,
       const cryptauth::DeviceClassifier& device_classifier);
-  virtual ~CryptAuthClient();
+  ~CryptAuthClientImpl() override;
 
-  // GetMyDevices
-  typedef base::Callback<void(const cryptauth::GetMyDevicesResponse&)>
-      GetMyDevicesCallback;
+  // CryptAuthClient:
   void GetMyDevices(const cryptauth::GetMyDevicesRequest& request,
                     const GetMyDevicesCallback& callback,
-                    const ErrorCallback& error_callback);
-
-  // FindEligibleUnlockDevices
-  typedef base::Callback<void(
-      const cryptauth::FindEligibleUnlockDevicesResponse&)>
-      FindEligibleUnlockDevicesCallback;
+                    const ErrorCallback& error_callback) override;
   void FindEligibleUnlockDevices(
       const cryptauth::FindEligibleUnlockDevicesRequest& request,
       const FindEligibleUnlockDevicesCallback& callback,
-      const ErrorCallback& error_callback);
-
-  // SendDeviceSyncTickle
-  typedef base::Callback<void(const cryptauth::SendDeviceSyncTickleResponse&)>
-      SendDeviceSyncTickleCallback;
+      const ErrorCallback& error_callback) override;
   void SendDeviceSyncTickle(
       const cryptauth::SendDeviceSyncTickleRequest& request,
       const SendDeviceSyncTickleCallback& callback,
-      const ErrorCallback& error_callback);
-
-  // ToggleEasyUnlock
-  typedef base::Callback<void(const cryptauth::ToggleEasyUnlockResponse&)>
-      ToggleEasyUnlockCallback;
+      const ErrorCallback& error_callback) override;
   void ToggleEasyUnlock(const cryptauth::ToggleEasyUnlockRequest& request,
                         const ToggleEasyUnlockCallback& callback,
-                        const ErrorCallback& error_callback);
-
-  // SetupEnrollment
-  typedef base::Callback<void(const cryptauth::SetupEnrollmentResponse&)>
-      SetupEnrollmentCallback;
+                        const ErrorCallback& error_callback) override;
   void SetupEnrollment(const cryptauth::SetupEnrollmentRequest& request,
                        const SetupEnrollmentCallback& callback,
-                       const ErrorCallback& error_callback);
-
-  // FinishEnrollment
-  typedef base::Callback<void(const cryptauth::FinishEnrollmentResponse&)>
-      FinishEnrollmentCallback;
+                       const ErrorCallback& error_callback) override;
   void FinishEnrollment(const cryptauth::FinishEnrollmentRequest& request,
                         const FinishEnrollmentCallback& callback,
-                        const ErrorCallback& error_callback);
-
- protected:
-  // Creates a CryptAuthApiCallFlow object. Exposed for testing.
-  virtual scoped_ptr<CryptAuthApiCallFlow> CreateFlow(const GURL& request_url);
+                        const ErrorCallback& error_callback) override;
 
  private:
   // Starts a call to the API given by |request_path|, with the templated
@@ -118,18 +82,22 @@ class CryptAuthClient {
   // Called when the current API call fails at any step.
   void OnApiCallFailed(const std::string& error_message);
 
-  // The context for network requests.
-  scoped_refptr<net::URLRequestContextGetter> url_request_context_;
+  // Constructs and executes the actual HTTP request.
+  scoped_ptr<CryptAuthApiCallFlow> api_call_flow_;
 
   // Fetches the access token authorizing the API calls.
   scoped_ptr<CryptAuthAccessTokenFetcher> access_token_fetcher_;
 
+  // The context for network requests.
+  scoped_refptr<net::URLRequestContextGetter> url_request_context_;
+
   // Contains basic device info of the client making the request that is sent to
   // CryptAuth with each API call.
   const cryptauth::DeviceClassifier device_classifier_;
 
-  // Handles the current API call.
-  scoped_ptr<CryptAuthApiCallFlow> flow_;
+  // True if an API call has been started. Remains true even after the API call
+  // completes.
+  bool call_started_;

[Ilya Sherman, 2015/04/06 18:53:34]

nit: I'd add a "has_" prefix, to emphasize that this is a boolean variable.

[Tim Song, 2015/04/06 23:21:17]

Done.

 
   // URL path of the current request.
   std::string request_path_;
@@ -137,11 +105,11 @@ class CryptAuthClient {
   // Called when the current request fails.
   ErrorCallback error_callback_;
 
-  base::WeakPtrFactory<CryptAuthClient> weak_ptr_factory_;
+  base::WeakPtrFactory<CryptAuthClientImpl> weak_ptr_factory_;
 
-  DISALLOW_COPY_AND_ASSIGN(CryptAuthClient);
+  DISALLOW_COPY_AND_ASSIGN(CryptAuthClientImpl);
 };
 
 }  // namespace proximity_auth
 
-#endif  // COMPONENTS_PROXIMITY_AUTH_CRYPT_AUTH_CLIENT_H
+#endif  // COMPONENTS_PROXIMITY_AUTH_CRYPT_AUTH_CLIENT_IMPL_H