arc-bridge: Add the ARC Bridge Service

components/arc/arc_bridge_service.cc

Index: components/arc/arc_bridge_service.cc
diff --git a/components/arc/arc_bridge_service.cc b/components/arc/arc_bridge_service.cc
new file mode 100644
index 0000000000000000000000000000000000000000..8b67c85e901afd7415b58c186c72b670767a7511
--- /dev/null
+++ b/components/arc/arc_bridge_service.cc
@@ -0,0 +1,276 @@
+// Copyright 2015 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 "components/arc/arc_bridge_service.h"
+
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/prefs/pref_registry_simple.h"
+#include "base/prefs/pref_service.h"
+#include "base/task_runner_util.h"
+#include "base/thread_task_runner_handle.h"
+#include "chromeos/arc/bridge/common/arc_host_messages.h"
+#include "chromeos/arc/bridge/common/arc_instance_messages.h"
+#include "chromeos/dbus/arc_instance_client.h"
+#include "chromeos/dbus/dbus_method_call_status.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "components/arc/arc_pref_names.h"
+#include "ipc/ipc_channel.h"
+
+namespace {
+
+const base::FilePath::CharType kArcBridgeSocketPath[] =
+    FILE_PATH_LITERAL("/home/chronos/ArcBridge/bridge.sock");
+
+}  // namespace
+
+namespace arc {
+
+ArcBridgeService::ArcBridgeService(
+      const scoped_refptr<base::SequencedTaskRunner>& file_task_runner)

[Shuhei Takahashi, 2015/11/02 17:58:02]

nit: excess indent (4 spaces here)

In general I recommend using auto indentation.

[Luis Héctor Chávez, 2015/11/04 17:49:18]

I messed up with my Vim setup recently and lost the auto-indent settings :(
Done.

[Luis Héctor Chávez, 2015/11/09 16:41:04]

Turns out this was the correct indentation :/

+    : ipc_thread_("ARC bridge listener"),
+      origin_task_runner_(base::ThreadTaskRunnerHandle::Get()),
+      file_task_runner_(file_task_runner),
+      observer_list_(new base::ObserverListThreadSafe<Observer>()),
+      available_(false),
+      enabled_(false),
+      state_(ArcBridgeService::STOPPED),
+      weak_factory_(this) {
+  ipc_thread_.StartWithOptions(
+      base::Thread::Options(base::MessageLoop::TYPE_IO, 0));
+}
+
+ArcBridgeService::~ArcBridgeService() {}
+
+// static
+void ArcBridgeService::RegisterPrefs(PrefRegistrySimple* registry) {
+  registry->RegisterBooleanPref(prefs::kArcEnabled, false);
+}
+
+// static
+bool ArcBridgeService::GetEnabledPref(PrefService* pref_service) {
+  // TODO(lhchavez): Once this is user-configurable, use the real pref.
+  return true;
+}
+
+void ArcBridgeService::HandleStartup() {
+  DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+  if (!enabled_)
+    return;
+  SocketConnect(base::FilePath(kArcBridgeSocketPath));
+}
+
+void ArcBridgeService::Shutdown() {
+  DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+  if (state_ == ArcBridgeService::STOPPED ||
+      state_ == ArcBridgeService::STOPPING) {
+    LOG(ERROR) << "Shutdown() called when ARC is not running";

[oshima, 2015/11/02 19:13:49]

This file is using too many LOG(ERROR). Consider using

1) DCHECK, or CHECK if you want to catch an issue that can be caused by coding
error.
2) VLOG and enable using vmodule if it's not severer and won't affect the
functionality
3) Keep LOG(ERROR) to catch unexpected condition on the device that leads to bad
state.

[Luis Héctor Chávez, 2015/11/04 17:49:18]

Done.

+    return;
+  }
+  if (state_ == ArcBridgeService::CONNECTED ||
+      state_ == ArcBridgeService::CONNECTING) {
+    // This was stopped before the D-Bus command to start the instance. Just
+    // close the socket (if it was opened).
+    if (state_ == ArcBridgeService::CONNECTED) {
+      base::AutoLock scoped_lock(ipc_channel_lock_);
+      ipc_channel_->Close();
+      ipc_channel_.reset();
+    }
+    SetState(ArcBridgeService::STOPPED);
+    return;
+  }
+
+  SetState(ArcBridgeService::STOPPING);
+  chromeos::DBusThreadManager::Get()->GetArcInstanceClient()->StopInstance(
+      base::Bind(&ArcBridgeService::OnInstanceStopped,
+        weak_factory_.GetWeakPtr()));

[Shuhei Takahashi, 2015/11/02 17:58:02]

nit: wrong indent

[Luis Héctor Chávez, 2015/11/04 17:49:18]

Done.

+}
+
+void ArcBridgeService::AddObserver(Observer* observer) {
+  observer_list_->AddObserver(observer);
+}
+
+void ArcBridgeService::RemoveObserver(Observer* observer) {
+  observer_list_->RemoveObserver(observer);
+}
+
+void ArcBridgeService::SetEnabled(bool enabled) {
+  DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+
+  if (enabled_ == enabled)
+    return;
+  enabled_ = enabled;
+  if (!enabled_ && state_ != ArcBridgeService::STOPPED &&
+      state_ != ArcBridgeService::STOPPING)
+    Shutdown();
+  observer_list_->Notify(FROM_HERE,
+                         &Observer::OnEnabledChanged,
+                         enabled_);
+}
+
+bool ArcBridgeService::RegisterInputDevice(
+    const std::string& name, const std::string& device_type,
+    base::ScopedFD fd) {
+  // ipc_channel_->Send() is thread-safe, so there is no thread check.

[Shuhei Takahashi, 2015/11/02 17:58:02]

This comment is obsolete.

[Luis Héctor Chávez, 2015/11/04 17:49:18]

Done.

+  {
+    base::AutoLock scoped_lock(ipc_channel_lock_);
+    if (!ipc_channel_) {
+      LOG(ERROR) << "Called RegisterInputDevice when the service is not ready";
+      return false;
+    }
+    return ipc_channel_->Send(new ArcInstanceMsg_RegisterInputDevice(
+          name, device_type, base::FileDescriptor(fd.Pass())));
+  }
+}
+
+void ArcBridgeService::SocketConnect(const base::FilePath& socket_path) {
+  DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+  if (state_ != ArcBridgeService::STOPPED) {
+    LOG(ERROR) << "SocketConnect() called when instance is not stopped";
+    return;
+  }
+  SetState(ArcBridgeService::CONNECTING);
+  base::PostTaskAndReplyWithResult(
+      file_task_runner_.get(),
+      FROM_HERE,
+      base::Bind(&base::CreateDirectory, socket_path.DirName()),
+      base::Bind(&ArcBridgeService::SocketConnectAfterEnsureParentDirectory,
+                 weak_factory_.GetWeakPtr(),
+                 socket_path));
+}
+
+void ArcBridgeService::SocketConnectAfterEnsureParentDirectory(
+    const base::FilePath& socket_path, bool directory_present) {
+  DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+  if (state_ != ArcBridgeService::CONNECTING) {
+    LOG(ERROR) << "Shutdown requested while connecting";
+    return;
+  }
+  if (!directory_present) {
+    LOG(ERROR) << "Error creating directory for " << socket_path.value();
+    Shutdown();
+    return;
+  }
+
+  if (!Connect(IPC::ChannelHandle(socket_path.value()),
+               IPC::Channel::MODE_OPEN_NAMED_SERVER)) {
+    LOG(ERROR) << "Error connecting to " << socket_path.value();
+    Shutdown();
+    return;
+  }
+
+  base::PostTaskAndReplyWithResult(
+      file_task_runner_.get(),
+      FROM_HERE,
+      // TODO(lhchavez): Tighten the security around the socket by tying it to
+      // the user the instance will run as.
+      base::Bind(&base::SetPosixFilePermissions, socket_path, 0777),
+      base::Bind(&ArcBridgeService::SocketConnectAfterSetSocketPermissions,
+                 weak_factory_.GetWeakPtr(), socket_path));
+}
+
+void ArcBridgeService::SocketConnectAfterSetSocketPermissions(
+    const base::FilePath& socket_path, bool socket_permissions_success) {
+  DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+  if (state_ != ArcBridgeService::CONNECTED) {
+    LOG(ERROR) << "Shutdown requested while connecting";
+    return;
+  }
+
+  if (!socket_permissions_success) {
+    LOG(ERROR) << "Error setting socket permissions for "
+               << socket_path.value();
+    Shutdown();
+    return;
+  }
+
+  SetState(ArcBridgeService::STARTING);
+  // This will fail if the ArcInstanceService is not running on Chrome OS.
+  chromeos::DBusThreadManager::Get()->GetArcInstanceClient()->StartInstance(
+      socket_path.value(),
+      base::Bind(&ArcBridgeService::OnInstanceStarted,
+                 weak_factory_.GetWeakPtr()));
+}
+
+bool ArcBridgeService::Connect(const IPC::ChannelHandle& handle,
+                               IPC::Channel::Mode mode) {
+  DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+  // Testing code can have the state in STOPPED since it does not go through
+  // the whole connection flow.
+  if (state_ != ArcBridgeService::CONNECTING &&
+      state_ != ArcBridgeService::STOPPED) {
+    LOG(ERROR) << "Shutdown requested while connecting";
+    return false;
+  }
+
+  base::AutoLock scoped_lock(ipc_channel_lock_);
+  ipc_channel_ = IPC::ChannelProxy::Create(handle, mode, this,
+                                           ipc_thread_.task_runner().get());
+  if (!ipc_channel_)
+    return false;
+  SetState(ArcBridgeService::CONNECTED);
+  return true;
+}
+
+void ArcBridgeService::OnInstanceReady() {
+  DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+  if (state_ != ArcBridgeService::STARTING) {
+    LOG(ERROR) << "Shutdown requested while connecting";
+    return;
+  }
+  SetState(ArcBridgeService::READY);
+}
+
+void ArcBridgeService::SetState(State state) {
+  DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+  if (state_ == state)

[Shuhei Takahashi, 2015/11/02 17:58:02]

I think we should rather ensure this condition:

DCHECK(state != state_);

Otherwise something should have went wrong (e.g. a callback called twice).

[Luis Héctor Chávez, 2015/11/04 17:49:18]

Done.

+    return;
+  state_ = state;
+  observer_list_->Notify(FROM_HERE,
+                         &Observer::OnStateChanged,
+                         state_);
+}
+
+bool ArcBridgeService::OnMessageReceived(const IPC::Message& message) {
+  DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+  bool handled = true;
+
+  IPC_BEGIN_MESSAGE_MAP(ArcBridgeService, message)
+    IPC_MESSAGE_HANDLER(ArcInstanceHostMsg_InstanceReady, OnInstanceReady)
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+
+  if (!handled)
+    LOG(ERROR) << "Invalid message with type = " << message.type();
+  return handled;
+}
+
+void ArcBridgeService::OnInstanceStarted(
+    chromeos::DBusMethodCallStatus status) {
+  DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+  if (state_ != ArcBridgeService::STARTING) {
+    LOG(ERROR) << "Shutdown requested while connecting";
+    return;
+  }
+  if (status != chromeos::DBUS_METHOD_CALL_SUCCESS) {
+    LOG(ERROR) << "ARC instance unable to start. Shutting down the bridge";
+    Shutdown();
+    return;
+  }
+}
+
+void ArcBridgeService::OnInstanceStopped(
+    chromeos::DBusMethodCallStatus status) {
+  DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+  // STOPPING is the only valid state for this function.
+  CHECK(state_ == ArcBridgeService::STOPPING);

[Shuhei Takahashi, 2015/11/02 17:58:02]

Maybe DCHECK? I remember Chrome should avoid CHECK().

[Luis Héctor Chávez, 2015/11/04 17:49:18]

Done.

+  {
+    base::AutoLock scoped_lock(ipc_channel_lock_);
+    ipc_channel_->Close();
+    ipc_channel_.reset();
+  }
+  SetState(ArcBridgeService::STOPPED);
+}
+
+}  // namespace arc