CloudPolicyClient: wrap error into base::Optional

Wrap the error into base::Optional to distinguish between "no error"
state and "unknown error".

Bug: 1045895
Test: CloudPolicyClientCertProvisioning*
Change-Id: Icf6f22f127c46c1604632520cf4026a1760b6d69
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2077993Reviewed-by: Pavol Marko <pmarko@chromium.org>
Commit-Queue: Michael Ershov <miersh@google.com>
Cr-Commit-Position: refs/heads/master@{#748158}

components/policy/core/common/cloud/cloud_policy_client.cc

@@ -1162,7 +1162,7 @@ namespace {
 bool CheckCommonClientCertProvisioningResponse(
     const em::DeviceManagementResponse& response,
     policy::DeviceManagementStatus* status,
-    CertProvisioningResponseErrorType* response_error,
+    base::Optional<CertProvisioningResponseErrorType>* response_error,
     base::Optional<int64_t>* try_later) {
   if (*status != DM_STATUS_SUCCESS) {
     return false;
@@ -1201,8 +1201,7 @@ void CloudPolicyClient::OnClientCertProvisioningStartCsrResponse(
       &CloudPolicyClient::RemoveJob, base::Unretained(this), job));
 
   status_ = status;
-  CertProvisioningResponseErrorType response_error =
-      CertProvisioningResponseError::UNDEFINED;
+  base::Optional<CertProvisioningResponseErrorType> response_error;
   base::Optional<int64_t> try_later;
 
   // Single step loop for convenience.
@@ -1274,8 +1273,7 @@ void CloudPolicyClient::OnClientCertProvisioningFinishCsrResponse(
       &CloudPolicyClient::RemoveJob, base::Unretained(this), job));
 
   status_ = status;
-  CertProvisioningResponseErrorType response_error =
-      CertProvisioningResponseError::UNDEFINED;
+  base::Optional<CertProvisioningResponseErrorType> response_error;
   base::Optional<int64_t> try_later;
 
   // Single step loop for convenience.
@@ -1308,8 +1306,7 @@ void CloudPolicyClient::OnClientCertProvisioningDownloadCertResponse(
       &CloudPolicyClient::RemoveJob, base::Unretained(this), job));
 
   status_ = status;
-  CertProvisioningResponseErrorType response_error =
-      CertProvisioningResponseError::UNDEFINED;
+  base::Optional<CertProvisioningResponseErrorType> response_error;
   base::Optional<int64_t> try_later;
 
   // Single step loop for convenience.

components/policy/core/common/cloud/cloud_policy_client.h

@@ -75,7 +75,8 @@ class POLICY_EXPORT CloudPolicyClient {
 
   using ClientCertProvisioningStartCsrCallback = base::OnceCallback<void(
       DeviceManagementStatus,
-      enterprise_management::ClientCertificateProvisioningResponse::Error,
+      base::Optional<
+          enterprise_management::ClientCertificateProvisioningResponse::Error>,
       base::Optional<int64_t> try_later,
       const std::string& invalidation_topic,
       const std::string& va_challenge,
@@ -84,12 +85,14 @@ class POLICY_EXPORT CloudPolicyClient {
 
   using ClientCertProvisioningFinishCsrCallback = base::OnceCallback<void(
       DeviceManagementStatus,
-      enterprise_management::ClientCertificateProvisioningResponse::Error,
+      base::Optional<
+          enterprise_management::ClientCertificateProvisioningResponse::Error>,
       base::Optional<int64_t> try_later)>;
 
   using ClientCertProvisioningDownloadCertCallback = base::OnceCallback<void(
       DeviceManagementStatus,
-      enterprise_management::ClientCertificateProvisioningResponse::Error,
+      base::Optional<
+          enterprise_management::ClientCertificateProvisioningResponse::Error>,
       base::Optional<int64_t> try_later,
       const std::string& pem_encoded_certificate)>;
 

components/policy/core/common/cloud/cloud_policy_client_unittest.cc

@@ -1909,7 +1909,7 @@ class MockClientCertProvisioningStartCsrCallbackObserver {
   MOCK_METHOD(void,
               Callback,
               (DeviceManagementStatus,
-               CertProvisioningResponseErrorType,
+               base::Optional<CertProvisioningResponseErrorType>,
                base::Optional<int64_t> try_later,
                const std::string& invalidation_topic,
                const std::string& va_challenge,
@@ -1992,11 +1992,11 @@ TEST_F(CloudPolicyClientCertProvisioningStartCsrTest,
   }
 
   MockClientCertProvisioningStartCsrCallbackObserver callback_observer;
-  EXPECT_CALL(callback_observer,
-              Callback(DeviceManagementStatus::DM_STATUS_SUCCESS,
-                       CertProvisioningResponseError::UNDEFINED,
-                       testing::Eq(base::nullopt), invalidation_topic,
-                       va_challenge, hash_algorithm, data_to_sign))
+  EXPECT_CALL(
+      callback_observer,
+      Callback(DeviceManagementStatus::DM_STATUS_SUCCESS,
+               testing::Eq(base::nullopt), testing::Eq(base::nullopt),
+               invalidation_topic, va_challenge, hash_algorithm, data_to_sign))
       .Times(1);
 
   RunTest(fake_response, callback_observer);
@@ -2018,8 +2018,8 @@ TEST_F(CloudPolicyClientCertProvisioningStartCsrTest,
   MockClientCertProvisioningStartCsrCallbackObserver callback_observer;
   EXPECT_CALL(callback_observer,
               Callback(DeviceManagementStatus::DM_STATUS_SUCCESS,
-                       CertProvisioningResponseError::UNDEFINED,
-                       testing::Eq(try_later), std::string(), std::string(),
+                       testing::Eq(base::nullopt), testing::Eq(try_later),
+                       std::string(), std::string(),
                        em::HashingAlgorithm::HASHING_ALGORITHM_UNSPECIFIED,
                        std::string()))
       .Times(1);
@@ -2042,11 +2042,12 @@ TEST_F(CloudPolicyClientCertProvisioningStartCsrTest,
   }
 
   MockClientCertProvisioningStartCsrCallbackObserver callback_observer;
-  EXPECT_CALL(callback_observer,
-              Callback(DeviceManagementStatus::DM_STATUS_SUCCESS, error,
-                       testing::Eq(base::nullopt), std::string(), std::string(),
-                       em::HashingAlgorithm::HASHING_ALGORITHM_UNSPECIFIED,
-                       std::string()))
+  EXPECT_CALL(
+      callback_observer,
+      Callback(DeviceManagementStatus::DM_STATUS_SUCCESS, testing::Eq(error),
+               testing::Eq(base::nullopt), std::string(), std::string(),
+               em::HashingAlgorithm::HASHING_ALGORITHM_UNSPECIFIED,
+               std::string()))
       .Times(1);
 
   RunTest(fake_response, callback_observer);
@@ -2059,7 +2060,7 @@ class MockClientCertProvisioningFinishCsrCallbackObserver {
   MOCK_METHOD(void,
               Callback,
               (DeviceManagementStatus,
-               CertProvisioningResponseErrorType,
+               base::Optional<CertProvisioningResponseErrorType>,
                base::Optional<int64_t> try_later),
               (const));
 };
@@ -2134,8 +2135,7 @@ TEST_F(CloudPolicyClientCertProvisioningFinishCsrTest,
   MockClientCertProvisioningFinishCsrCallbackObserver callback_observer;
   EXPECT_CALL(callback_observer,
               Callback(DeviceManagementStatus::DM_STATUS_SUCCESS,
-                       CertProvisioningResponseError::UNDEFINED,
-                       testing::Eq(base::nullopt)))
+                       testing::Eq(base::nullopt), testing::Eq(base::nullopt)))
       .Times(1);
 
   RunTest(fake_response, callback_observer);
@@ -2157,8 +2157,8 @@ TEST_F(CloudPolicyClientCertProvisioningFinishCsrTest,
 
   MockClientCertProvisioningFinishCsrCallbackObserver callback_observer;
   EXPECT_CALL(callback_observer,
-              Callback(DeviceManagementStatus::DM_STATUS_SUCCESS, error,
-                       testing::Eq(base::nullopt)))
+              Callback(DeviceManagementStatus::DM_STATUS_SUCCESS,
+                       testing::Eq(error), testing::Eq(base::nullopt)))
       .Times(1);
 
   RunTest(fake_response, callback_observer);
@@ -2171,7 +2171,7 @@ class MockClientCertProvisioningDownloadCertCallbackObserver {
   MOCK_METHOD(void,
               Callback,
               (DeviceManagementStatus,
-               CertProvisioningResponseErrorType,
+               base::Optional<CertProvisioningResponseErrorType>,
                base::Optional<int64_t> try_later,
                const std::string& pem_encoded_certificate),
               (const));
@@ -2246,8 +2246,8 @@ TEST_F(CloudPolicyClientCertProvisioningDownloadCertTest,
   MockClientCertProvisioningDownloadCertCallbackObserver callback_observer;
   EXPECT_CALL(callback_observer,
               Callback(DeviceManagementStatus::DM_STATUS_SUCCESS,
-                       CertProvisioningResponseError::UNDEFINED,
-                       testing::Eq(base::nullopt), pem_encoded_cert))
+                       testing::Eq(base::nullopt), testing::Eq(base::nullopt),
+                       pem_encoded_cert))
       .Times(1);
 
   RunTest(fake_response, callback_observer);
@@ -2269,10 +2269,10 @@ TEST_F(CloudPolicyClientCertProvisioningDownloadCertTest,
   }
 
   MockClientCertProvisioningDownloadCertCallbackObserver callback_observer;
-  EXPECT_CALL(callback_observer,
-              Callback(DeviceManagementStatus::DM_STATUS_SUCCESS,
-                       CertProvisioningResponseError::CA_ERROR,
-                       testing::Eq(base::nullopt), std::string()))
+  EXPECT_CALL(
+      callback_observer,
+      Callback(DeviceManagementStatus::DM_STATUS_SUCCESS, testing::Eq(error),
+               testing::Eq(base::nullopt), std::string()))
       .Times(1);
 
   RunTest(fake_response, callback_observer);