Migrate DataMethodCallback into DBusMethodCallback.

BUG=739622
TEST=Build.

Change-Id: I196aea8c5d66c020a22c6eceb899783da3b2cd3a
Reviewed-on: https://chromium-review.googlesource.com/697047
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506829}

chrome/browser/chromeos/attestation/attestation_policy_observer.cc

@@ -76,17 +76,15 @@ void DBusStringCallback(
     const base::Callback<void(const std::string&)> on_success,
     const base::Closure& on_failure,
     const base::Location& from_here,
-    chromeos::DBusMethodCallStatus status,
-    bool result,
-    const std::string& data) {
-  if (status != chromeos::DBUS_METHOD_CALL_SUCCESS || !result) {
-    LOG(ERROR) << "Cryptohome DBus method failed: " << from_here.ToString()
-               << " - " << status << " - " << result;
+    base::Optional<chromeos::CryptohomeClient::TpmAttestationDataResult>
+        result) {
+  if (!result.has_value() || !result->success) {
+    LOG(ERROR) << "Cryptohome DBus method failed: " << from_here.ToString();
     if (!on_failure.is_null())
       on_failure.Run();
     return;
   }
-  on_success.Run(data);
+  on_success.Run(result->data);
 }
 
 }  // namespace
@@ -189,12 +187,19 @@ void AttestationPolicyObserver::GetNewCertificate() {
       EmptyAccountId(),  // Not used.
       std::string(),     // Not used.
       true,              // Force a new key to be generated.
-      base::Bind(DBusStringCallback,
-                 base::Bind(&AttestationPolicyObserver::UploadCertificate,
-                            weak_factory_.GetWeakPtr()),
-                 base::Bind(&AttestationPolicyObserver::Reschedule,
-                            weak_factory_.GetWeakPtr()),
-                 FROM_HERE, DBUS_METHOD_CALL_SUCCESS));
+      base::Bind(
+          [](const base::Callback<void(const std::string&)> on_success,
+             const base::Closure& on_failure, const base::Location& from_here,
+             bool success, const std::string& data) {
+            DBusStringCallback(on_success, on_failure, from_here,
+                               CryptohomeClient::TpmAttestationDataResult{
+                                   success, std::move(data)});
+          },
+          base::Bind(&AttestationPolicyObserver::UploadCertificate,
+                     weak_factory_.GetWeakPtr()),
+          base::Bind(&AttestationPolicyObserver::Reschedule,
+                     weak_factory_.GetWeakPtr()),
+          FROM_HERE));
 }
 
 void AttestationPolicyObserver::GetExistingCertificate() {

chromeos/attestation/attestation_flow.cc

@@ -10,6 +10,7 @@
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
+#include "base/optional.h"
 #include "base/timer/timer.h"
 #include "chromeos/cryptohome/async_method_caller.h"
 #include "chromeos/cryptohome/cryptohome_parameters.h"
@@ -55,17 +56,15 @@ void DBusBoolRedirectCallback(const base::Closure& on_true,
 
 void DBusDataMethodCallback(
     const AttestationFlow::CertificateCallback& callback,
-    DBusMethodCallStatus status,
-    bool result,
-    const std::string& data) {
-  if (status != DBUS_METHOD_CALL_SUCCESS) {
+    base::Optional<CryptohomeClient::TpmAttestationDataResult> result) {
+  if (!result.has_value()) {
     LOG(ERROR) << "Attestation: DBus data operation failed.";
     if (!callback.is_null())
       callback.Run(false, "");
     return;
   }
   if (!callback.is_null())
-    callback.Run(result, data);
+    callback.Run(result->success, result->data);
 }
 
 }  // namespace

chromeos/dbus/cryptohome_client.cc

@@ -587,7 +587,7 @@ class CryptohomeClientImpl : public CryptohomeClient {
       attestation::AttestationKeyType key_type,
       const cryptohome::Identification& cryptohome_id,
       const std::string& key_name,
-      const DataMethodCallback& callback) override {
+      DBusMethodCallback<TpmAttestationDataResult> callback) override {
     dbus::MethodCall method_call(
         cryptohome::kCryptohomeInterface,
         cryptohome::kCryptohomeTpmAttestationGetCertificate);
@@ -598,8 +598,8 @@ class CryptohomeClientImpl : public CryptohomeClient {
     writer.AppendString(key_name);
     proxy_->CallMethod(
         &method_call, kTpmDBusTimeoutMs,
-        base::BindOnce(&CryptohomeClientImpl::OnDataMethod,
-                       weak_ptr_factory_.GetWeakPtr(), callback));
+        base::BindOnce(&CryptohomeClientImpl::OnTpmAttestationDataMethod,
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
   }
 
   // CryptohomeClient override.
@@ -607,7 +607,7 @@ class CryptohomeClientImpl : public CryptohomeClient {
       attestation::AttestationKeyType key_type,
       const cryptohome::Identification& cryptohome_id,
       const std::string& key_name,
-      const DataMethodCallback& callback) override {
+      DBusMethodCallback<TpmAttestationDataResult> callback) override {
     dbus::MethodCall method_call(
         cryptohome::kCryptohomeInterface,
         cryptohome::kCryptohomeTpmAttestationGetPublicKey);
@@ -618,8 +618,8 @@ class CryptohomeClientImpl : public CryptohomeClient {
     writer.AppendString(key_name);
     proxy_->CallMethod(
         &method_call, kTpmDBusTimeoutMs,
-        base::BindOnce(&CryptohomeClientImpl::OnDataMethod,
-                       weak_ptr_factory_.GetWeakPtr(), callback));
+        base::BindOnce(&CryptohomeClientImpl::OnTpmAttestationDataMethod,
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
   }
 
   // CryptohomeClient override.
@@ -702,7 +702,7 @@ class CryptohomeClientImpl : public CryptohomeClient {
       attestation::AttestationKeyType key_type,
       const cryptohome::Identification& cryptohome_id,
       const std::string& key_name,
-      const DataMethodCallback& callback) override {
+      DBusMethodCallback<TpmAttestationDataResult> callback) override {
     dbus::MethodCall method_call(
         cryptohome::kCryptohomeInterface,
         cryptohome::kCryptohomeTpmAttestationGetKeyPayload);
@@ -713,8 +713,8 @@ class CryptohomeClientImpl : public CryptohomeClient {
     writer.AppendString(key_name);
     proxy_->CallMethod(
         &method_call, kTpmDBusTimeoutMs,
-        base::BindOnce(&CryptohomeClientImpl::OnDataMethod,
-                       weak_ptr_factory_.GetWeakPtr(), callback));
+        base::BindOnce(&CryptohomeClientImpl::OnTpmAttestationDataMethod,
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
   }
 
   // CryptohomeClient override.
@@ -1112,23 +1112,24 @@ class CryptohomeClientImpl : public CryptohomeClient {
   }
 
   // Handles responses for methods with a bool result and data.
-  void OnDataMethod(const DataMethodCallback& callback,
-                    dbus::Response* response) {
+  void OnTpmAttestationDataMethod(
+      DBusMethodCallback<TpmAttestationDataResult> callback,
+      dbus::Response* response) {
     if (!response) {
-      callback.Run(DBUS_METHOD_CALL_FAILURE, false, std::string());
+      std::move(callback).Run(base::nullopt);
       return;
     }
     dbus::MessageReader reader(response);
-    const uint8_t* data_buffer = NULL;
+    TpmAttestationDataResult result;
+    const uint8_t* data_buffer = nullptr;
     size_t data_length = 0;
-    bool result = false;
     if (!reader.PopArrayOfBytes(&data_buffer, &data_length) ||
-        !reader.PopBool(&result)) {
-      callback.Run(DBUS_METHOD_CALL_FAILURE, false, std::string());
+        !reader.PopBool(&result.success)) {
+      std::move(callback).Run(base::nullopt);
       return;
     }
-    std::string data(reinterpret_cast<const char*>(data_buffer), data_length);
-    callback.Run(DBUS_METHOD_CALL_SUCCESS, result, data);
+    result.data.assign(reinterpret_cast<const char*>(data_buffer), data_length);
+    std::move(callback).Run(std::move(result));
   }
 
   // Handles responses for methods with a BaseReply protobuf method.

chromeos/dbus/cryptohome_client.h

@@ -65,10 +65,6 @@ class CHROMEOS_EXPORT CryptohomeClient : public DBusClient {
 
   // A callback to handle LowDiskSpace signals.
   typedef base::Callback<void(uint64_t disk_free_bytes)> LowDiskSpaceHandler;
-  // A callback for methods which return both a bool result and data.
-  typedef base::Callback<void(DBusMethodCallStatus call_status,
-                              bool result,
-                              const std::string& data)> DataMethodCallback;
 
   // A callback to handle DircryptoMigrationProgress signals.
   typedef base::Callback<void(cryptohome::DircryptoMigrationStatus status,
@@ -76,6 +72,15 @@ class CHROMEOS_EXPORT CryptohomeClient : public DBusClient {
                               uint64_t total)>
       DircryptoMigrationProgessHandler;
 
+  // Represents the result to obtain the data related to TPM attestation.
+  struct TpmAttestationDataResult {
+    // True when it is succeeded to obtain the data.
+    bool success = false;
+
+    // The returned content. Available iff |success| is true.
+    std::string data;
+  };
+
   // TPM Token Information retrieved from cryptohome.
   // For invalid token |label| and |user_pin| will be empty, while |slot| will
   // be set to -1.
@@ -394,7 +399,7 @@ class CHROMEOS_EXPORT CryptohomeClient : public DBusClient {
       attestation::AttestationKeyType key_type,
       const cryptohome::Identification& cryptohome_id,
       const std::string& key_name,
-      const DataMethodCallback& callback) = 0;
+      DBusMethodCallback<TpmAttestationDataResult> callback) = 0;
 
   // Gets the public key for the key specified by |key_type| and |key_name|.
   // |callback| will be called when the operation completes.  If the key does
@@ -405,7 +410,7 @@ class CHROMEOS_EXPORT CryptohomeClient : public DBusClient {
       attestation::AttestationKeyType key_type,
       const cryptohome::Identification& cryptohome_id,
       const std::string& key_name,
-      const DataMethodCallback& callback) = 0;
+      DBusMethodCallback<TpmAttestationDataResult> callback) = 0;
 
   // Asynchronously registers an attestation key with the current user's
   // PKCS #11 token.  The |callback| will be called when the dbus call
@@ -463,7 +468,7 @@ class CHROMEOS_EXPORT CryptohomeClient : public DBusClient {
       attestation::AttestationKeyType key_type,
       const cryptohome::Identification& cryptohome_id,
       const std::string& key_name,
-      const DataMethodCallback& callback) = 0;
+      DBusMethodCallback<TpmAttestationDataResult> callback) = 0;
 
   // Sets the |payload| associated with the key specified by |key_type| and
   // |key_name|.  The |callback| will be called when the operation completes.

chromeos/dbus/fake_cryptohome_client.cc

@@ -450,41 +450,39 @@ void FakeCryptohomeClient::TpmAttestationGetCertificate(
     attestation::AttestationKeyType key_type,
     const cryptohome::Identification& cryptohome_id,
     const std::string& key_name,
-    const DataMethodCallback& callback) {
-  bool result = false;
-  std::string certificate;
+    DBusMethodCallback<TpmAttestationDataResult> callback) {
+  TpmAttestationDataResult result;
   switch (key_type) {
     case attestation::KEY_DEVICE: {
       const auto it = device_certificate_map_.find(key_name);
       if (it != device_certificate_map_.end()) {
-        result = true;
-        certificate = it->second;
+        result.success = true;
+        result.data = it->second;
       }
       break;
     }
     case attestation::KEY_USER: {
       const auto it = user_certificate_map_.find({cryptohome_id, key_name});
       if (it != user_certificate_map_.end()) {
-        result = true;
-        certificate = it->second;
+        result.success = true;
+        result.data = it->second;
       }
       break;
     }
   }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::BindOnce(callback, DBUS_METHOD_CALL_SUCCESS, result, certificate));
+      FROM_HERE, base::BindOnce(std::move(callback), std::move(result)));
 }
 
 void FakeCryptohomeClient::TpmAttestationGetPublicKey(
     attestation::AttestationKeyType key_type,
     const cryptohome::Identification& cryptohome_id,
     const std::string& key_name,
-    const DataMethodCallback& callback) {
+    DBusMethodCallback<TpmAttestationDataResult> callback) {
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
-      base::Bind(callback, DBUS_METHOD_CALL_SUCCESS, false, std::string()));
+      base::BindOnce(std::move(callback), TpmAttestationDataResult{}));
 }
 
 void FakeCryptohomeClient::TpmAttestationRegisterKey(
@@ -523,20 +521,18 @@ void FakeCryptohomeClient::TpmAttestationGetKeyPayload(
     attestation::AttestationKeyType key_type,
     const cryptohome::Identification& cryptohome_id,
     const std::string& key_name,
-    const DataMethodCallback& callback) {
-  bool result = false;
-  std::string payload;
+    DBusMethodCallback<TpmAttestationDataResult> callback) {
+  TpmAttestationDataResult result;
   if (key_type == attestation::KEY_DEVICE) {
     const auto it = device_key_payload_map_.find(key_name);
     if (it != device_key_payload_map_.end()) {
-      result = true;
-      payload = it->second;
+      result.success = true;
+      result.data = it->second;
     }
   }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::BindOnce(callback, DBUS_METHOD_CALL_SUCCESS, result, payload));
+      FROM_HERE, base::BindOnce(std::move(callback), std::move(result)));
 }
 
 void FakeCryptohomeClient::TpmAttestationSetKeyPayload(

chromeos/dbus/fake_cryptohome_client.h

@@ -127,12 +127,12 @@ class CHROMEOS_EXPORT FakeCryptohomeClient : public CryptohomeClient {
       attestation::AttestationKeyType key_type,
       const cryptohome::Identification& cryptohome_id,
       const std::string& key_name,
-      const DataMethodCallback& callback) override;
+      DBusMethodCallback<TpmAttestationDataResult> callback) override;
   void TpmAttestationGetPublicKey(
       attestation::AttestationKeyType key_type,
       const cryptohome::Identification& cryptohome_id,
       const std::string& key_name,
-      const DataMethodCallback& callback) override;
+      DBusMethodCallback<TpmAttestationDataResult> callback) override;
   void TpmAttestationRegisterKey(
       attestation::AttestationKeyType key_type,
       const cryptohome::Identification& cryptohome_id,
@@ -157,7 +157,7 @@ class CHROMEOS_EXPORT FakeCryptohomeClient : public CryptohomeClient {
       attestation::AttestationKeyType key_type,
       const cryptohome::Identification& cryptohome_id,
       const std::string& key_name,
-      const DataMethodCallback& callback) override;
+      DBusMethodCallback<TpmAttestationDataResult> callback) override;
   void TpmAttestationSetKeyPayload(
       attestation::AttestationKeyType key_type,
       const cryptohome::Identification& cryptohome_id,