Refactor CryptAuth component to be more testable.

components/proximity_auth/cryptauth/cryptauth_client_impl.h

 // Copyright 2014 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_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"
+
+class OAuth2TokenService;
 
 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
   // request and response types. The client first fetches the access token and
   // then makes the HTTP request.
   template <class RequestProto, class ResponseProto>
   void MakeApiCall(
       const std::string& request_path,
       const RequestProto& request_proto,
       const base::Callback<void(const ResponseProto&)>& response_callback,
       const ErrorCallback& error_callback);
 
   // Called when the access token is obtained so the API request can be made.
   template <class ResponseProto>
   void OnAccessTokenFetched(
       const std::string& serialized_request,
       const base::Callback<void(const ResponseProto&)>& response_callback,
       const std::string& access_token);
 
   // Called with CryptAuthApiCallFlow completes successfully to deserialize and
   // return the result.
   template <class ResponseProto>
   void OnFlowSuccess(
       const base::Callback<void(const ResponseProto&)>& result_callback,
       const std::string& serialized_response);
 
   // 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 has_call_started_;
 
   // URL path of the current request.
   std::string request_path_;
 
   // 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);
+};
+
+// Implementation of CryptAuthClientFactory.
+class CryptAuthClientFactoryImpl : public CryptAuthClientFactory {
+ public:
+  CryptAuthClientFactoryImpl(
+      OAuth2TokenService* token_service,

[Ilya Sherman, 2015/04/06 23:48:02]

Please document lifetime expectations.  Actually, it would be nice to have some
docs for all of the parameters.

[Tim Song, 2015/04/07 02:22:57]

Done.

+      const std::string& account_id,
+      scoped_refptr<net::URLRequestContextGetter> url_request_context,
+      const cryptauth::DeviceClassifier& device_classifier);
+  ~CryptAuthClientFactoryImpl() override;
+
+  // CryptAuthClientFactory:
+  scoped_ptr<CryptAuthClient> CreateInstance() override;
+
+ private:
+  OAuth2TokenService* token_service_;
+  const std::string account_id_;
+  const scoped_refptr<net::URLRequestContextGetter> url_request_context_;
+  const cryptauth::DeviceClassifier device_classifier_;
+
+  DISALLOW_COPY_AND_ASSIGN(CryptAuthClientFactoryImpl);
 };
 
 }  // namespace proximity_auth
 
-#endif  // COMPONENTS_PROXIMITY_AUTH_CRYPT_AUTH_CLIENT_H
+#endif  // COMPONENTS_PROXIMITY_AUTH_CRYPT_AUTH_CLIENT_IMPL_H