Remove validation retry logic from DeviceSettingsService

chrome/browser/chromeos/settings/session_manager_operation.cc

Index: chrome/browser/chromeos/settings/session_manager_operation.cc
diff --git a/chrome/browser/chromeos/settings/session_manager_operation.cc b/chrome/browser/chromeos/settings/session_manager_operation.cc
index 47d600774f9bbcc54b27a8a60e45f9e828575027..973eb8feb63793fa12e3de749d4098daa170c1ee 100644
--- a/chrome/browser/chromeos/settings/session_manager_operation.cc
+++ b/chrome/browser/chromeos/settings/session_manager_operation.cc
@@ -151,14 +151,14 @@ void SessionManagerOperation::BlockingRetrieveDeviceSettings() {
 void SessionManagerOperation::ValidateDeviceSettings(
     const std::string& policy_blob,
     RetrievePolicyResponseType response_type) {
-  std::unique_ptr<em::PolicyFetchResponse> policy(
-      new em::PolicyFetchResponse());
   if (policy_blob.empty()) {
     ReportResult(DeviceSettingsService::STORE_NO_POLICY);
     return;
   }
 
-  if (!policy->ParseFromString(policy_blob) || !policy->IsInitialized()) {

[Thiemo Nagel, 2017/06/01 11:55:26]

Funny enough, IsInitialized() maps to true [1].  Thus dropping it for clearer
code.

[1]
https://cs.chromium.org/chromium/src/out/Debug/gen/components/policy/proto/de...

[emaxx, 2017/06/01 18:51:17]

Not sure about this change. It's identical to "true" only whenever there are no
required fields in the protobuf message. Even though it's very unlikely that a
required field will be introduced into PolicyFetchResponse now, it's still a bit
cleaner to still catch this in the code.

[Thiemo Nagel, 2017/06/02 09:30:53]

Thanks for explaining the intention of IsInitialized()!  I don't feel very
strongly about this, but given that the proto docs recommend against required
fields, I don't think we'll ever add any and thus the code seems to be clearer
without.

Anyways, I'll drop it, probably not worth to discuss further.

+  std::unique_ptr<em::PolicyFetchResponse> policy =
+      base::MakeUnique<em::PolicyFetchResponse>();
+  if (!policy->ParseFromString(policy_blob)) {
     ReportResult(DeviceSettingsService::STORE_INVALID_POLICY);
     return;
   }
@@ -210,23 +210,14 @@ void SessionManagerOperation::ValidateDeviceSettings(
 
 void SessionManagerOperation::ReportValidatorStatus(
     policy::DeviceCloudPolicyValidator* validator) {
-  DeviceSettingsService::Status status =
-      DeviceSettingsService::STORE_VALIDATION_ERROR;
   if (validator->success()) {
-    status = DeviceSettingsService::STORE_SUCCESS;
     policy_data_ = std::move(validator->policy_data());
     device_settings_ = std::move(validator->payload());
+    ReportResult(DeviceSettingsService::STORE_SUCCESS);
   } else {
     LOG(ERROR) << "Policy validation failed: " << validator->status();
-
-    // Those are mostly caused by RTC loss and are recoverable.
-    if (validator->status() ==
-        policy::DeviceCloudPolicyValidator::VALIDATION_BAD_TIMESTAMP) {
-      status = DeviceSettingsService::STORE_TEMP_VALIDATION_ERROR;
-    }
+    ReportResult(DeviceSettingsService::STORE_VALIDATION_ERROR);
   }
-
-  ReportResult(status);
 }
 
 LoadSettingsOperation::LoadSettingsOperation(bool force_key_load,