Update FWMP in TPM

chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc

Index: chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc
diff --git a/chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc b/chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc
index c4bcf11f3d016e291ce611035e442083827f8090..ae9da61a051e81ce1d663726c10a27ce0ed91ef5 100644
--- a/chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc
+++ b/chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc
@@ -393,10 +393,50 @@ void EnrollmentHandlerChromeOS::OnGetTokensResponse(
 
   robot_refresh_token_ = refresh_token;
 
+  SetStep(STEP_SET_FWMP_DATA);
+  SetFwmpData();
+}
+
+void EnrollmentHandlerChromeOS::SetFwmpData() {
+  DCHECK_EQ(STEP_SET_FWMP_DATA, enrollment_step_);

[Daniel Erat, 2017/03/06 21:18:25]

nit: i think the order used for DCHECK_EQ is typically (actual, expected), e.g.

  DCHECK_EQ(enrollment_step_, STEP_SET_FWMP_DATA);

[igorcov, 2017/03/09 12:22:57]

This would make my code inconsistent with the rest of the code from this class.
Why would DCHECK_EQ be different from the standard we use in EXPECT_EQ from
unit tests?
(https://cs.chromium.org/chromium/src/testing/gtest/include/gtest/gtest.h?dr=C...)

[Daniel Erat, 2017/03/09 14:52:37]

because they come from completely different places. :-/ DCHECK_EQ is from
chrome's base/logging.h, while EXPECT_EQ and ASSERT_EQ are from googletest,
which is modeled on junit.

leaving it like this is fine if this file already lists things in the opposite
order, though.

+
+  install_attributes_->SetBlockDevmodeInTpm(
+      GetBlockDevmode(), base::Bind(&EnrollmentHandlerChromeOS::OnFwmpDataSet,
+                                    weak_ptr_factory_.GetWeakPtr()));
+}
+
+void EnrollmentHandlerChromeOS::OnFwmpDataSet(
+    chromeos::DBusMethodCallStatus call_status,
+    bool result,
+    const cryptohome::BaseReply& reply) {
+  DCHECK_EQ(STEP_SET_FWMP_DATA, enrollment_step_);

[Daniel Erat, 2017/03/06 21:18:26]

nit: same here

+  if (!result) {
+    LOG(ERROR) << "Failed to update firmware management parameters in TPM";
+  }
+
+  // get devmode and set the data.

[Daniel Erat, 2017/03/06 21:18:25]

nit: s/get/Get/

[igorcov, 2017/03/09 12:22:57]

Done.

   SetStep(STEP_AD_DOMAIN_JOIN);
   StartJoinAdDomain();
 }
 
+bool EnrollmentHandlerChromeOS::GetBlockDevmode() {
+  bool block_devmode = false;
+  std::unique_ptr<em::PolicyData> policy_data(new em::PolicyData());

[Daniel Erat, 2017/03/06 21:18:25]

nit: use auto and MakeUnique

[igorcov, 2017/03/09 12:22:57]

Done.

+  policy_data->ParseFromString(policy_->policy_data());

[Daniel Erat, 2017/03/06 21:18:26]

shouldn't you be checking the return value here to make sure it was parsed
correctly before using it?

[igorcov, 2017/03/09 12:22:57]

Done.

+
+  std::unique_ptr<em::ChromeDeviceSettingsProto> payload(
+      new em::ChromeDeviceSettingsProto());
+  payload->ParseFromString(policy_data->policy_value());
+  if (payload->has_system_settings()) {
+    const em::SystemSettingsProto& container(payload->system_settings());
+    if (container.has_block_devmode()) {
+      block_devmode = container.block_devmode();
+    }
+  }
+
+  return block_devmode;
+}
+
 // GaiaOAuthClient::Delegate
 void EnrollmentHandlerChromeOS::OnRefreshTokenResponse(
     const std::string& access_token,