Copy challenge*Key methods to enterprise.platformKeys.

chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.h

Index: chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.h
diff --git a/chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.h b/chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.h
index 49f334f69f8300e2c114aab2e21c526754ae4037..6196f0ddff0d72272ffbd2f6c1d84b22052d46d3 100644
--- a/chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.h
+++ b/chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.h
@@ -10,15 +10,16 @@
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
-#include "chrome/browser/extensions/chrome_extension_function.h"
 #include "chrome/common/extensions/api/enterprise_platform_keys_private.h"
 #include "chromeos/attestation/attestation_constants.h"
 #include "chromeos/attestation/attestation_flow.h"
 #include "chromeos/dbus/cryptohome_client.h"
 #include "chromeos/dbus/dbus_method_call_status.h"
+#include "extensions/browser/extension_function.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 class PrefService;
+class Profile;
 
 namespace chromeos {
 class CryptohomeClient;
@@ -38,7 +39,13 @@ class PrefRegistrySyncable;
 
 namespace extensions {
 
-class EPKPChallengeKeyBase : public ChromeAsyncExtensionFunction {
+// A callback for challenge key operations. If the operation succeeded,
+// |success| is true and |data| is the challenge response. Otherwise, |success|
+// is false and |data| is an error message.
+using ChallengeKeyCallback =
+    base::Callback<void(bool success, const std::string& data)>;
+
+class EPKPChallengeKeyBase {

[emaxx, 2016/02/18 23:35:18]

Considering the old functions in enterprise.platformKeysPrivate API as becoming
sort-of deprecated, this class and its descendants should probably be renamed
("EPKP"=>"EPK") and moved out of this header.
This IMO would leave the code in a clearer state: first, the public API
implementation part won't depend on the private part, and second, the private
part will contain only the thin wrapper around the common code.

[Darren Krahn, 2016/02/19 16:45:23]

I agree. In this CL I was going for minimal diffs and leaving the refactor for
another CL. Is that acceptable or do you want it all in this CL?

  public:
   static const char kChallengeBadBase64Error[];
   static const char kDevicePolicyDisabledError[];
@@ -62,7 +69,7 @@ class EPKPChallengeKeyBase : public ChromeAsyncExtensionFunction {
       cryptohome::AsyncMethodCaller* async_caller,
       chromeos::attestation::AttestationFlow* attestation_flow,
       policy::EnterpriseInstallAttributes* install_attributes);
-  ~EPKPChallengeKeyBase() override;
+  virtual ~EPKPChallengeKeyBase();
 
   // Returns a trusted value from CroSettings indicating if the device
   // attestation is enabled.
@@ -103,6 +110,9 @@ class EPKPChallengeKeyBase : public ChromeAsyncExtensionFunction {
   cryptohome::AsyncMethodCaller* async_caller_;
   chromeos::attestation::AttestationFlow* attestation_flow_;
   scoped_ptr<chromeos::attestation::AttestationFlow> default_attestation_flow_;
+  ChallengeKeyCallback callback_;
+  Profile* profile_;
+  std::string extension_id_;
 
  private:
   // Holds the context of a PrepareKey() operation.
@@ -156,29 +166,24 @@ class EPKPChallengeMachineKey : public EPKPChallengeKeyBase {
       cryptohome::AsyncMethodCaller* async_caller,
       chromeos::attestation::AttestationFlow* attestation_flow,
       policy::EnterpriseInstallAttributes* install_attributes);
+  ~EPKPChallengeMachineKey() override;
 
- protected:
-  bool RunAsync() override;
+  // Asynchronously run the flow to challenge a machine key in the |caller|
+  // context.
+  void Run(scoped_refptr<UIThreadExtensionFunction> caller,
+           const ChallengeKeyCallback& callback,
+           const std::string& encoded_challenge);
 
  private:
   static const char kKeyName[];
 
-  ~EPKPChallengeMachineKey() override;
-
   void GetDeviceAttestationEnabledCallback(const std::string& challenge,
                                            bool enabled);
   void PrepareKeyCallback(const std::string& challenge,
                           PrepareKeyResult result);
   void SignChallengeCallback(bool success, const std::string& response);
-
-  DECLARE_EXTENSION_FUNCTION(
-      "enterprise.platformKeysPrivate.challengeMachineKey",
-      ENTERPRISE_PLATFORMKEYSPRIVATE_CHALLENGEMACHINEKEY);
 };
 
-typedef EPKPChallengeMachineKey
-    EnterprisePlatformKeysPrivateChallengeMachineKeyFunction;
-
 class EPKPChallengeUserKey : public EPKPChallengeKeyBase {
  public:
   static const char kGetCertificateFailedError[];
@@ -191,17 +196,20 @@ class EPKPChallengeUserKey : public EPKPChallengeKeyBase {
       cryptohome::AsyncMethodCaller* async_caller,
       chromeos::attestation::AttestationFlow* attestation_flow,
       policy::EnterpriseInstallAttributes* install_attributes);
+  ~EPKPChallengeUserKey() override;
 
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 
- protected:
-  bool RunAsync() override;
+  // Asynchronously run the flow to challenge a user key in the |caller|
+  // context.
+  void Run(scoped_refptr<UIThreadExtensionFunction> caller,
+           const ChallengeKeyCallback& callback,
+           const std::string& encoded_challenge,
+           bool register_key);
 
  private:
   static const char kKeyName[];
 
-  ~EPKPChallengeUserKey() override;
-
   void GetDeviceAttestationEnabledCallback(const std::string& challenge,
                                            bool register_key,
                                            bool require_user_consent,
@@ -217,15 +225,48 @@ class EPKPChallengeUserKey : public EPKPChallengeKeyBase {
                            cryptohome::MountError return_code);
 
   bool IsRemoteAttestationEnabledForUser() const;
+};
+
+class EnterprisePlatformKeysPrivateChallengeMachineKeyFunction
+    : public UIThreadExtensionFunction {
+ public:
+  EnterprisePlatformKeysPrivateChallengeMachineKeyFunction();
+  EnterprisePlatformKeysPrivateChallengeMachineKeyFunction(
+      EPKPChallengeMachineKey* impl_for_testing);

[Devlin, 2016/02/19 18:05:02]

same comments as other file :)

[Darren Krahn, 2016/02/23 23:39:46]

Done.

+
+ private:
+  ~EnterprisePlatformKeysPrivateChallengeMachineKeyFunction() override;
+  ResponseAction Run() override;
+  void OnChallengedKey(bool success, const std::string& data);
+
+  scoped_ptr<EPKPChallengeMachineKey> default_impl_;
+  EPKPChallengeMachineKey* impl_;
+
+  DECLARE_EXTENSION_FUNCTION(
+      "enterprise.platformKeysPrivate.challengeMachineKey",
+      ENTERPRISE_PLATFORMKEYSPRIVATE_CHALLENGEMACHINEKEY);
+};
+
+class EnterprisePlatformKeysPrivateChallengeUserKeyFunction
+    : public UIThreadExtensionFunction {
+ public:
+  EnterprisePlatformKeysPrivateChallengeUserKeyFunction();
+  EnterprisePlatformKeysPrivateChallengeUserKeyFunction(
+      EPKPChallengeUserKey* impl_for_testing);
+
+ private:
+  ~EnterprisePlatformKeysPrivateChallengeUserKeyFunction() override;
+  ResponseAction Run() override;
+  void OnChallengedKey(bool success, const std::string& data);
+
+  scoped_ptr<EPKPChallengeUserKey> default_impl_;
+  EPKPChallengeUserKey* impl_;
 
   DECLARE_EXTENSION_FUNCTION(
       "enterprise.platformKeysPrivate.challengeUserKey",
       ENTERPRISE_PLATFORMKEYSPRIVATE_CHALLENGEUSERKEY);
 };
 
-typedef EPKPChallengeUserKey
-    EnterprisePlatformKeysPrivateChallengeUserKeyFunction;
-
 }  // namespace extensions
 
 #endif  // CHROME_BROWSER_EXTENSIONS_API_ENTERPRISE_PLATFORM_KEYS_PRIVATE_ENTERPRISE_PLATFORM_KEYS_PRIVATE_API_H__