Support transition between chrome and user mode console

chromeos/dbus/console_service_client.cc

+// Copyright (c) 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 "base/bind.h"
+#include "chromeos/dbus/console_service_client.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "dbus/bus.h"
+#include "dbus/exported_object.h"
+#include "dbus/message.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
+
+namespace chromeos {
+
+ConsoleServiceClient* ConsoleServiceClient::instance = NULL;

[stevenjb, 2014/10/31 17:00:46]

File local in anonymous namespace, 's_console_service_client = NULL';

[dsodman, 2014/10/31 17:54:34]

Done.

+
+class ConsoleServiceClientImpl : public ConsoleServiceClient {

[stevenjb, 2014/10/31 17:00:46]

The implementation can also be in an anonymous namespace.

[dsodman, 2014/10/31 17:54:34]

Done.

+ public:
+  ConsoleServiceClientImpl();

[stevenjb, 2014/10/31 17:00:46]

This needs a destructor override.

[dsodman, 2014/10/31 17:54:34]

Done.

+
+  void AddObserver(Observer* observer) override {
+    observers_.AddObserver(observer);
+  }

[stevenjb, 2014/10/31 17:00:46]

With the exception of trivial accessors, it is better to either not inline any
methods (preferred) or inline all of them (acceptable for small classes with
only short implementations).

[dsodman, 2014/10/31 17:54:33]

Done.

+
+  void RemoveObserver(Observer* observer) override {
+    observers_.RemoveObserver(observer);
+  }
+
+  bool HasObserver(Observer* observer) override {
+    return observers_.HasObserver(observer);
+  }
+
+  void OnOwnership(const std::string& service_name, bool success);

[stevenjb, 2014/10/31 17:00:46]

Include a brief comment for each of these methods.

[dsodman, 2014/10/31 17:54:34]

Done.

+
+  void OnExported(const std::string& interface_name,
+                  const std::string& method_name,
+                  bool success);
+
+  void ActivateConsole(dbus::MethodCall* method_call,
+                       dbus::ExportedObject::ResponseSender response_sender);
+
+ protected:
+  virtual void Init(dbus::Bus* bus) override;

[stevenjb, 2014/10/31 17:00:46]

No 'virtual' for overrides.

[dsodman, 2014/10/31 17:54:34]

Done.

+
+ private:
+  dbus::Bus* bus_;
+  ObserverList<Observer> observers_;
+  scoped_refptr<dbus::ExportedObject> exported_object_;
+  base::WeakPtrFactory<ConsoleServiceClientImpl> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(ConsoleServiceClientImpl);
+};
+
+void ConsoleServiceClientImpl::OnOwnership(const std::string& service_name,

[stevenjb, 2014/10/31 17:00:46]

Ordering of method implementations should match declaration order.

[dsodman, 2014/10/31 17:54:34]

Done.

+                                           bool success) {
+  LOG_IF(FATAL, !success) << "failed to own: " << service_name;
+}
+
+void ConsoleServiceClientImpl::OnExported(const std::string& interface_name,
+                                          const std::string& method_name,
+                                          bool success) {
+  if (!success) {
+    LOG(ERROR) << "failed to export " << interface_name << "." << method_name;
+  }
+}
+
+ConsoleServiceClientImpl::ConsoleServiceClientImpl() : weak_ptr_factory_(this) {

[stevenjb, 2014/10/31 17:00:46]

bus_ needs to be initialized.

[dsodman, 2014/10/31 17:54:34]

Done.

+}
+
+ConsoleServiceClient::ConsoleServiceClient() {
+  instance = this;

[stevenjb, 2014/10/31 17:00:46]

CHECK_EQ(NULL, s_chonsole_service_client);
ALso, better to set this in Create() instead.

[dsodman, 2014/10/31 17:54:34]

Done.

+}
+
+ConsoleServiceClient* ConsoleServiceClient::Create() {
+  return new ConsoleServiceClientImpl();
+}
+
+ConsoleServiceClient* ConsoleServiceClient::GetInstance() {
+  return instance;
+}
+
+void ConsoleServiceClientImpl::ActivateConsole(
+    dbus::MethodCall* method_call,
+    dbus::ExportedObject::ResponseSender response_sender) {
+  dbus::MessageReader reader(method_call);
+  int console_id = 0;
+  if (reader.PopInt32(&console_id)) {
+    FOR_EACH_OBSERVER(Observer, observers_, OnActivateConsole(console_id));
+  } else {
+    LOG(ERROR) << "Unable to parse " << kActivateConsole << " request";
+  }
+  response_sender.Run(dbus::Response::FromMethodCall(method_call));
+}
+
+void ConsoleServiceClientImpl::Init(dbus::Bus* bus) {
+  LOG(ERROR) << "ConsoleServiceClientImpl::Init";

[stevenjb, 2014/10/31 17:00:46]

VLOG

[dsodman, 2014/10/31 17:54:34]

Done.

+  bus_ = DBusThreadManager::Get()->GetSystemBus();
+
+  if (!bus_) {
+    LOG(ERROR) << "Unable to get system d-bus for the ConsoleServiceClient";
+    return;
+  }
+
+  bus_->RequestOwnership(kConsoleServiceName,
+                         dbus::Bus::REQUIRE_PRIMARY_ALLOW_REPLACEMENT,
+                         base::Bind(&ConsoleServiceClientImpl::OnOwnership,
+                                    base::Unretained(this)));
+
+  exported_object_ = bus_->GetExportedObject(
+      dbus::ObjectPath(dbus::ObjectPath(kConsoleServicePath)));
+
+  exported_object_->ExportMethod(
+      kConsoleServiceInterface,
+      kActivateConsole,
+      base::Bind(&ConsoleServiceClientImpl::ActivateConsole,
+                 weak_ptr_factory_.GetWeakPtr()),
+      base::Bind(&ConsoleServiceClientImpl::OnExported,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+}  // namespace chromeos