Update FWMP in TPM

chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/policy/enrollment_handler_chromeos.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 375 matching lines @@
 
 // GaiaOAuthClient::Delegate callback for OAuth2 refresh token fetched.
 void EnrollmentHandlerChromeOS::OnGetTokensResponse(
     const std::string& refresh_token,
     const std::string& access_token,
     int expires_in_seconds) {
   CHECK_EQ(STEP_ROBOT_AUTH_REFRESH, enrollment_step_);
 
   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,
     int expires_in_seconds) {
   // We never use the code that should trigger this callback.
   LOG(FATAL) << "Unexpected callback invoked.";
 }
 
 // GaiaOAuthClient::Delegate OAuth2 error when fetching refresh token request.
 void EnrollmentHandlerChromeOS::OnOAuthError() {
@@ skipping 189 matching lines @@
     callback.Run(status);
 }
 
 void EnrollmentHandlerChromeOS::SetStep(EnrollmentStep step) {
   DCHECK_LE(enrollment_step_, step);
   VLOG(1) << "Step: " << step;
   enrollment_step_ = step;
 }
 
 }  // namespace policy