Added ConsumerManagementService class to handle enroll state and device owner info in boot lockbox.

chrome/browser/chromeos/policy/consumer_management_service_unittest.cc

+// Copyright 2014 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/consumer_management_service.h"
+
+#include <string>

[bartfab (slow), 2014/08/04 18:44:53]

Nit: Already included by header.

[davidyu, 2014/08/05 07:26:59]

Done.

+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/prefs/pref_registry_simple.h"
+#include "base/prefs/testing_pref_service.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/test/base/scoped_testing_local_state.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chromeos/dbus/cryptohome/rpc.pb.h"
+#include "chromeos/dbus/cryptohome_client.h"
+#include "chromeos/dbus/dbus_method_call_status.h"

[bartfab (slow), 2014/08/04 18:44:53]

Nit: Already included by header.

[davidyu, 2014/08/05 07:26:59]

Done.

+#include "chromeos/dbus/mock_cryptohome_client.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using testing::Invoke;
+using testing::NiceMock;
+using testing::_;
+
+namespace policy {
+namespace {

[bartfab (slow), 2014/08/04 18:44:55]

Nit: No need to wrap the entire test in an anonymous namespace.

[davidyu, 2014/08/05 07:27:00]

Done.

+
+class ConsumerManagementServiceTest : public testing::Test {
+ public:
+  ConsumerManagementServiceTest() :

[bartfab (slow), 2014/08/04 18:44:54]

Nit: Per style guide, move the : to the next line.

[davidyu, 2014/08/05 07:27:00]

Done.

+      testing_local_state_(TestingBrowserProcess::GetGlobal()),
+      service_(new ConsumerManagementService(&mock_cryptohome_client_)) {
+    ON_CALL(mock_cryptohome_client_, GetBootAttribute(_, _))
+        .WillByDefault(
+            Invoke(this, &ConsumerManagementServiceTest::MockGetBootAttribute));
+    ON_CALL(mock_cryptohome_client_, SetBootAttribute(_, _))
+        .WillByDefault(
+            Invoke(this, &ConsumerManagementServiceTest::MockSetBootAttribute));
+    ON_CALL(mock_cryptohome_client_, FlushAndSignBootAttributes(_, _))
+        .WillByDefault(
+            Invoke(this,
+                   &ConsumerManagementServiceTest::
+                       MockFlushAndSignBootAttributes));
+  }
+
+  void MockGetBootAttribute(
+      const cryptohome::GetBootAttributeRequest& request,
+      const chromeos::CryptohomeClient::ProtobufMethodCallback& callback) {
+    get_boot_attribute_request_ = request;
+    callback.Run(cryptohome_status_, cryptohome_result_, cryptohome_reply_);
+  }
+
+  void MockSetBootAttribute(
+      const cryptohome::SetBootAttributeRequest& request,
+      const chromeos::CryptohomeClient::ProtobufMethodCallback& callback) {
+    set_boot_attribute_request_ = request;
+    callback.Run(cryptohome_status_, cryptohome_result_, cryptohome_reply_);
+  }
+
+  void MockFlushAndSignBootAttributes(
+      const cryptohome::FlushAndSignBootAttributesRequest& request,
+      const chromeos::CryptohomeClient::ProtobufMethodCallback& callback) {
+    callback.Run(cryptohome_status_, cryptohome_result_, cryptohome_reply_);
+  }
+
+  void OnGetOwnerDone(const std::string& owner) {
+    owner_ = owner;
+  }
+
+  void OnSetOwnerDone(bool status) {
+    set_owner_status_ = status;
+  }
+
+  ScopedTestingLocalState testing_local_state_;
+  NiceMock<chromeos::MockCryptohomeClient> mock_cryptohome_client_;
+  scoped_ptr<ConsumerManagementService> service_;
+
+  chromeos::DBusMethodCallStatus cryptohome_status_;
+  bool cryptohome_result_;

[bartfab (slow), 2014/08/04 18:44:54]

Nit: Initialize this in the constructor to guarantee you will not be using an
uninitialized variable.

[davidyu, 2014/08/05 07:27:00]

Done.

+  cryptohome::BaseReply cryptohome_reply_;
+  cryptohome::GetBootAttributeRequest get_boot_attribute_request_;
+  cryptohome::SetBootAttributeRequest set_boot_attribute_request_;
+
+  std::string owner_;
+  bool set_owner_status_;

[bartfab (slow), 2014/08/04 18:44:54]

Nit: Initialize this in the constructor to guarantee you will not be using an
uninitialized variable.

[davidyu, 2014/08/05 07:26:59]

Done.

+};
+
+TEST_F(ConsumerManagementServiceTest, CanGetEnrollState) {

[bartfab (slow), 2014/08/04 18:44:55]

Nit: s/Enroll/Enrollment/

[davidyu, 2014/08/05 07:27:00]

Done.

+  testing_local_state_.Get()->SetInteger(
+      prefs::kConsumerManagementEnrollState,
+      ConsumerManagementService::ENROLL_ENROLLING);
+
+  EXPECT_EQ(ConsumerManagementService::ENROLL_ENROLLING,

[bartfab (slow), 2014/08/04 18:44:54]

Nit: Check first that before the set call, GetEnrollState() returns something
different.

[davidyu, 2014/08/05 07:26:59]

Done.

+            service_->GetEnrollState());
+}
+
+TEST_F(ConsumerManagementServiceTest, CanSetEnrollState) {

[bartfab (slow), 2014/08/04 18:44:54]

Nit: s/Enroll/Enrollment/

[davidyu, 2014/08/05 07:26:59]

Done.

+  service_->SetEnrollState(ConsumerManagementService::ENROLL_ENROLLING);
+
+  EXPECT_EQ(ConsumerManagementService::ENROLL_ENROLLING,

[bartfab (slow), 2014/08/04 18:44:54]

Nit: Check first that before the set call, GetInteger() returns something
different.

[davidyu, 2014/08/05 07:27:00]

Done.

+            testing_local_state_.Get()->GetInteger(
+                prefs::kConsumerManagementEnrollState));
+}
+
+TEST_F(ConsumerManagementServiceTest, CanGetOwner) {
+  cryptohome_status_ = chromeos::DBUS_METHOD_CALL_SUCCESS;
+  cryptohome_result_ = true;
+  cryptohome_reply_.MutableExtension(cryptohome::GetBootAttributeReply::reply)
+      ->set_value("test@chromium.org");

[bartfab (slow), 2014/08/04 18:44:54]

Nit 1: Move the -> operator to the previous line.
Nit 2: Extract "test@chromium.org" to a constant at the top of the file.
Nit 3: Use a domain ending in one of the reserved extensions, like ".example" or
".test," not a real domain.

[davidyu, 2014/08/05 07:26:59]

Done.

+
+  service_->GetOwner(base::Bind(&ConsumerManagementServiceTest::OnGetOwnerDone,
+                                base::Unretained(this)));
+
+  EXPECT_EQ("consumer_management.owner", get_boot_attribute_request_.name());

[bartfab (slow), 2014/08/04 18:44:54]

Nit: Extract "consumer_management.owner" to a constant at the top of the file.

[davidyu, 2014/08/05 07:26:59]

Done.

+  EXPECT_EQ("test@chromium.org", owner_);
+}
+
+TEST_F(ConsumerManagementServiceTest, GetOwnerReturnsAnEmptyStringWhenFails) {

[bartfab (slow), 2014/08/04 18:44:53]

Nit: s/When/WhenIt/

[davidyu, 2014/08/05 07:26:59]

Done.

+  cryptohome_status_ = chromeos::DBUS_METHOD_CALL_FAILURE;
+  cryptohome_result_ = false;
+  cryptohome_reply_.MutableExtension(cryptohome::GetBootAttributeReply::reply)
+      ->set_value("test@chromium.org");

[bartfab (slow), 2014/08/04 18:44:55]

Nit: Move the -> operator to the previous line.

[davidyu, 2014/08/05 07:27:00]

Done.

+
+  service_->GetOwner(base::Bind(&ConsumerManagementServiceTest::OnGetOwnerDone,
+                                base::Unretained(this)));
+
+  EXPECT_EQ("", owner_);
+}
+
+TEST_F(ConsumerManagementServiceTest, CanSetOwner) {
+  cryptohome_status_ = chromeos::DBUS_METHOD_CALL_SUCCESS;
+  cryptohome_result_ = true;
+
+  service_->SetOwner("test@chromium.org",
+                     base::Bind(&ConsumerManagementServiceTest::OnSetOwnerDone,
+                                base::Unretained(this)));
+
+  EXPECT_EQ("consumer_management.owner", set_boot_attribute_request_.name());
+  EXPECT_EQ("test@chromium.org", set_boot_attribute_request_.value());
+  EXPECT_TRUE(set_owner_status_);
+}
+
+TEST_F(ConsumerManagementServiceTest, SetOwnerReturnsFalseWhenFails) {

[bartfab (slow), 2014/08/04 18:44:54]

Nit: s/When/WhenIt/

[davidyu, 2014/08/05 07:27:00]

Done.

+  cryptohome_status_ = chromeos::DBUS_METHOD_CALL_FAILURE;
+  cryptohome_result_ = false;
+
+  service_->SetOwner("test@chromium.org",
+                     base::Bind(&ConsumerManagementServiceTest::OnSetOwnerDone,
+                                base::Unretained(this)));
+
+  EXPECT_FALSE(set_owner_status_);
+}
+
+}  // namespace
+}  // namespace policy