Usb-Enrollment: Use actual DBus service

Bug: 854101
Change-Id: I8c08f755f36f6b8ddddac6018aae8c9c022ee66a
Reviewed-on: https://chromium-review.googlesource.com/1228202
Commit-Queue: Denis Kuznetsov <antrim@chromium.org>
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594668}

chrome/browser/chromeos/BUILD.gn

@@ -84,6 +84,7 @@ source_set("chromeos") {
     "//chromeos:cryptohome_signkey_proto",
     "//chromeos:login_manager_proto",
     "//chromeos:metrics_event_proto",
+    "//chromeos:oobe_config_proto",
     "//chromeos/assistant:buildflags",
     "//chromeos/components/drivefs",
     "//chromeos/components/drivefs/mojom",

chromeos/BUILD.gn

@@ -26,6 +26,7 @@ component("chromeos") {
     ":login_manager_proto",
     ":media_perception_proto",
     ":metrics_event_proto",
+    ":oobe_config_proto",
     ":power_manager_proto",
     ":seneschal_proto",
     ":smbprovider_proto",
@@ -633,6 +634,7 @@ test("chromeos_unittests") {
     ":authpolicy_proto",
     ":biod_proto",
     ":cryptohome_proto",
+    ":oobe_config_proto",
     ":power_manager_proto",
     ":test_support",
     ":test_support_without_gmock",
@@ -679,6 +681,7 @@ test("chromeos_unittests") {
     "dbus/fake_power_manager_client_unittest.cc",
     "dbus/gsm_sms_client_unittest.cc",
     "dbus/modem_messaging_client_unittest.cc",
+    "dbus/oobe_configuration_client_unittest.cc",
     "dbus/pipe_reader_unittest.cc",
     "dbus/power_manager_client_unittest.cc",
     "dbus/power_policy_controller_unittest.cc",
@@ -910,6 +913,14 @@ proto_library("media_perception_proto") {
   proto_out_dir = "chromeos/dbus/media_perception"
 }
 
+proto_library("oobe_config_proto") {
+  sources = [
+    "//third_party/cros_system_api/dbus/oobe_config/oobe_config.proto",
+  ]
+
+  proto_out_dir = "chromeos/dbus/oobe_config"
+}
+
 proto_library("seneschal_proto") {
   sources = [
     "//third_party/cros_system_api/dbus/seneschal/seneschal_service.proto",

chromeos/dbus/oobe_configuration_client.cc

@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/macros.h"
+#include "chromeos/dbus/oobe_config/oobe_config.pb.h"
 #include "dbus/bus.h"
 #include "dbus/message.h"
 #include "dbus/object_path.h"
@@ -27,13 +28,20 @@ class OobeConfigurationClientImpl : public OobeConfigurationClient {
 
   // OobeConfigurationClient override:
   void CheckForOobeConfiguration(ConfigurationCallback callback) override {
-    // TODO (antrim): do a method call once https://crbug.com/869209 is fixed.
-    OnData(std::move(callback), nullptr);
+    dbus::MethodCall method_call(
+        oobe_config::kOobeConfigRestoreInterface,
+        oobe_config::kProcessAndGetOobeAutoConfigMethod);
+    proxy_->CallMethod(
+        &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::BindOnce(&OobeConfigurationClientImpl::OnData,
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
   }
 
  protected:
   void Init(dbus::Bus* bus) override {
-    // TODO (antrim): get proxy object once https://crbug.com/869209 is fixed.
+    proxy_ = bus->GetObjectProxy(
+        oobe_config::kOobeConfigRestoreServiceName,
+        dbus::ObjectPath(oobe_config::kOobeConfigRestoreServicePath));
   }
 
  private:
@@ -42,11 +50,35 @@ class OobeConfigurationClientImpl : public OobeConfigurationClient {
       std::move(callback).Run(false, std::string());
       return;
     }
-
-    // TODO (antrim): read response once https://crbug.com/869209 is fixed.
-    NOTREACHED();
+    oobe_config::OobeRestoreData response_proto;
+    dbus::MessageReader reader(response);
+    int error_code;
+    if (!reader.PopInt32(&error_code)) {
+      LOG(ERROR) << "Failed to parse error code from DBus Response.";
+      std::move(callback).Run(false, std::string());
+      return;
+    }
+    if (error_code) {
+      LOG(ERROR) << "Error during DBus call " << error_code;
+      std::move(callback).Run(false, std::string());
+      return;
+    }
+    if (!reader.PopArrayOfBytesAsProto(&response_proto)) {
+      LOG(ERROR) << "Failed to parse proto from DBus Response.";
+      std::move(callback).Run(false, std::string());
+      return;
+    }
+    VLOG(0) << "Got oobe config, size = "
+            << response_proto.chrome_config_json().size();
+    if (response_proto.chrome_config_json().empty()) {
+      std::move(callback).Run(false, std::string());
+      return;
+    }
+    std::move(callback).Run(true, response_proto.chrome_config_json());
   }
 
+  dbus::ObjectProxy* proxy_ = nullptr;
+
   // Note: This should remain the last member so it'll be destroyed and
   // invalidate its weak pointers before any other members are destroyed.
   base::WeakPtrFactory<OobeConfigurationClientImpl> weak_ptr_factory_;

chromeos/dbus/oobe_configuration_client.h

@@ -34,6 +34,8 @@ class CHROMEOS_EXPORT OobeConfigurationClient : public DBusClient {
   virtual void CheckForOobeConfiguration(ConfigurationCallback callback) = 0;
 
  protected:
+  friend class OobeConfigurationClientTest;
+
   // Create() should be used instead.
   OobeConfigurationClient() = default;
 

chromeos/dbus/oobe_configuration_client_unittest.cc

+// Copyright 2016 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 "chromeos/dbus/oobe_configuration_client.h"
+
+#include <memory>
+#include <string>
+
+#include "base/bind.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "chromeos/dbus/oobe_config/oobe_config.pb.h"
+#include "dbus/message.h"
+#include "dbus/mock_bus.h"
+#include "dbus/mock_object_proxy.h"
+#include "dbus/object_path.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
+
+using ::testing::_;
+using ::testing::Invoke;
+using ::testing::Return;
+
+namespace chromeos {
+
+namespace {
+
+// A mock ConfigurationCallback
+class MockConfigurationCallback {
+ public:
+  MOCK_METHOD2(Run, void(bool, const std::string&));
+};
+
+// Expect the reader to be empty.
+void ExpectNoArgument(dbus::MessageReader* reader) {
+  EXPECT_FALSE(reader->HasMoreData());
+}
+
+}  // namespace
+
+class OobeConfigurationClientTest : public testing::Test {
+ public:
+  OobeConfigurationClientTest()
+      : interface_name_(oobe_config::kOobeConfigRestoreInterface),
+        response_(nullptr) {}
+
+  void SetUp() override {
+    // Create a mock bus.
+    dbus::Bus::Options options;
+    options.bus_type = dbus::Bus::SYSTEM;
+    mock_bus_ = new dbus::MockBus(options);
+
+    // Create a mock oobe config proxy.
+    mock_proxy_ = new dbus::MockObjectProxy(
+        mock_bus_.get(), oobe_config::kOobeConfigRestoreServiceName,
+        dbus::ObjectPath(oobe_config::kOobeConfigRestoreServicePath));
+
+    // Set an expectation so mock_proxy's CallMethod() will use
+    // OnCallMethod() to return responses.
+    EXPECT_CALL(*mock_proxy_.get(), DoCallMethod(_, _, _))
+        .WillRepeatedly(
+            Invoke(this, &OobeConfigurationClientTest::OnCallMethod));
+
+    // Set an expectation so mock_bus's GetObjectProxy() for the given
+    // service name and the object path will return mock_proxy_.
+    EXPECT_CALL(*mock_bus_.get(),
+                GetObjectProxy(oobe_config::kOobeConfigRestoreServiceName,
+                               dbus::ObjectPath(
+                                   oobe_config::kOobeConfigRestoreServicePath)))
+        .WillOnce(Return(mock_proxy_.get()));
+
+    // ShutdownAndBlock() will be called in TearDown().
+    EXPECT_CALL(*mock_bus_.get(), ShutdownAndBlock()).WillOnce(Return());
+
+    // Create a client with the mock bus.
+    client_ = OobeConfigurationClient::Create();
+    client_->Init(mock_bus_.get());
+    // Run the message loop to run the signal connection result callback.
+    base::RunLoop().RunUntilIdle();
+  }
+
+  void TearDown() override { mock_bus_->ShutdownAndBlock(); }
+
+ protected:
+  // A callback to intercept and check the method call arguments.
+  typedef base::RepeatingCallback<void(dbus::MessageReader* reader)>
+      ArgumentCheckCallback;
+
+  // Sets expectations for called method name and arguments, and sets response.
+  void PrepareForMethodCall(const std::string& method_name,
+                            const ArgumentCheckCallback& argument_checker,
+                            dbus::Response* response) {
+    expected_method_name_ = method_name;
+    argument_checker_ = argument_checker;
+    response_ = response;
+  }
+
+  // The interface name.
+  const std::string interface_name_;
+  // The client to be tested.
+  std::unique_ptr<OobeConfigurationClient> client_;
+  // A message loop to emulate asynchronous behavior.
+  base::MessageLoop message_loop_;
+  // The mock bus.
+  scoped_refptr<dbus::MockBus> mock_bus_;
+  // The mock object proxy.
+  scoped_refptr<dbus::MockObjectProxy> mock_proxy_;
+
+  // The name of the method which is expected to be called.
+  std::string expected_method_name_;
+  // The response which the mock proxy returns.
+  dbus::Response* response_;
+  // A callback to intercept and check the method call arguments.
+  ArgumentCheckCallback argument_checker_;
+
+ private:
+  // Checks the content of the method call and returns the response.
+  // Used to implement the mock oobe config proxy.
+  void OnCallMethod(dbus::MethodCall* method_call,
+                    int timeout_ms,
+                    dbus::ObjectProxy::ResponseCallback* response) {
+    EXPECT_EQ(interface_name_, method_call->GetInterface());
+    EXPECT_EQ(expected_method_name_, method_call->GetMember());
+    dbus::MessageReader reader(method_call);
+    argument_checker_.Run(&reader);
+    message_loop_.task_runner()->PostTask(
+        FROM_HERE, base::BindOnce(std::move(*response), response_));
+  }
+};
+
+TEST_F(OobeConfigurationClientTest, CheckEmptyConfiguration) {
+  // Create response.
+  std::unique_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
+
+  oobe_config::OobeRestoreData proto;
+  proto.set_chrome_config_json(std::string());
+  dbus::MessageWriter writer(response.get());
+  writer.AppendInt32(0);
+  writer.AppendProtoAsArrayOfBytes(proto);
+
+  // Set expectations.
+  PrepareForMethodCall(oobe_config::kProcessAndGetOobeAutoConfigMethod,
+                       base::BindRepeating(&ExpectNoArgument), response.get());
+
+  MockConfigurationCallback callback;
+  EXPECT_CALL(callback, Run(false, _)).Times(1);
+
+  client_->CheckForOobeConfiguration(base::BindOnce(
+      &MockConfigurationCallback::Run, base::Unretained(&callback)));
+
+  base::RunLoop().RunUntilIdle();
+}
+
+TEST_F(OobeConfigurationClientTest, CheckServiceError) {
+  // Create response.
+  std::unique_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
+
+  oobe_config::OobeRestoreData proto;
+  proto.set_chrome_config_json("{key:true}");
+  dbus::MessageWriter writer(response.get());
+  writer.AppendInt32(1);
+  writer.AppendProtoAsArrayOfBytes(proto);
+
+  // Set expectations.
+  PrepareForMethodCall(oobe_config::kProcessAndGetOobeAutoConfigMethod,
+                       base::BindRepeating(&ExpectNoArgument), response.get());
+
+  MockConfigurationCallback callback;
+  EXPECT_CALL(callback, Run(false, _)).Times(1);
+
+  client_->CheckForOobeConfiguration(base::BindOnce(
+      &MockConfigurationCallback::Run, base::Unretained(&callback)));
+
+  base::RunLoop().RunUntilIdle();
+}
+
+TEST_F(OobeConfigurationClientTest, CheckConfigurationExists) {
+  // Create response.
+  std::unique_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
+
+  oobe_config::OobeRestoreData proto;
+  proto.set_chrome_config_json("{key:true}");
+  dbus::MessageWriter writer(response.get());
+  writer.AppendInt32(0);
+  writer.AppendProtoAsArrayOfBytes(proto);
+
+  // Set expectations.
+  PrepareForMethodCall(oobe_config::kProcessAndGetOobeAutoConfigMethod,
+                       base::BindRepeating(&ExpectNoArgument), response.get());
+
+  MockConfigurationCallback callback;
+  EXPECT_CALL(callback, Run(true, "{key:true}")).Times(1);
+
+  client_->CheckForOobeConfiguration(base::BindOnce(
+      &MockConfigurationCallback::Run, base::Unretained(&callback)));
+
+  base::RunLoop().RunUntilIdle();
+}
+
+}  // namespace chromeos