Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(237)

Unified Diff: components/arc/arc_bridge_service.cc

Issue 1412863004: arc-bridge: Add the ARC Bridge Service (Closed) Base URL: https://chromium.googlesource.com/a/chromium/src.git@master
Patch Set: Addressed feedback. Also extracted the enabled_ bit since it is orthogonal to state. Created 5 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « components/arc/arc_bridge_service.h ('k') | components/arc/arc_bridge_service_factory.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..73aebb63e814f163470d5d4b04d4a46b54c74a03
--- /dev/null
+++ b/components/arc/arc_bridge_service.cc
@@ -0,0 +1,235 @@
+// 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");
+
+// Ensures the parent directory of |socket_path| is present. Returns true if it
+// was created or if it was already present.
+bool EnsureParentDirectoryPresent(const base::FilePath& socket_path) {
+ base::FilePath path(socket_path);
+ return base::CreateDirectory(path.DirName());
+}
+
+// Changes the permissions on the socket so that the ARC instance can use it.
+bool SetSocketPermissions(const base::FilePath& socket_path) {
+ // TODO(lhchavez): Tighten the security around the socket by tying it to the
Shuhei Takahashi 2015/10/28 21:13:57 Could you file a bug so we don't forget this?
Luis Héctor Chávez 2015/10/28 23:12:20 It's part of the same bug.
+ // user the instance will run as.
+ return HANDLE_EINTR(chmod(socket_path.value().c_str(), 0777)) == 0;
+}
+
+} // namespace
+
+namespace arc {
+
+ArcBridgeService::ArcBridgeService(
+ const scoped_refptr<base::SequencedTaskRunner>& file_task_runner)
+ : ipc_thread_("ARC bridge listener"),
+ origin_task_runner_(base::ThreadTaskRunnerHandle::Get()),
+ file_task_runner_(file_task_runner),
+ available_(false),
Shuhei Takahashi 2015/10/28 21:13:57 Hmm,I don't see any code setting available_ to tru
Luis Héctor Chávez 2015/10/28 23:12:20 It's not there yet, but I want to have it availabl
+ 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_ || state_ != ArcBridgeService::STOPPED)
+ return;
+ base::PostTaskAndReplyWithResult(
+ file_task_runner_.get(),
+ FROM_HERE,
+ base::Bind(&EnsureParentDirectoryPresent,
+ base::FilePath(kArcBridgeSocketPath)),
+ base::Bind(&ArcBridgeService::SocketConnect,
+ weak_factory_.GetWeakPtr(),
+ base::FilePath(kArcBridgeSocketPath)));
+}
+
+void ArcBridgeService::Shutdown() {
+ DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+ if (state_ != ArcBridgeService::CONNECTED &&
+ state_ != ArcBridgeService::STARTING &&
+ state_ != ArcBridgeService::READY)
+ return;
+ ipc_channel_->Close();
Shuhei Takahashi 2015/10/28 21:13:57 Could you set ipc_channel_ to null?
Luis Héctor Chávez 2015/10/28 23:12:20 Done.
+ base::PostTaskAndReplyWithResult(
+ file_task_runner_.get(),
+ FROM_HERE,
+ base::Bind(&base::DeleteFile, base::FilePath(kArcBridgeSocketPath),
+ false),
+ base::Bind(&ArcBridgeService::FinishShutdownAfterSocketDeleted,
+ weak_factory_.GetWeakPtr()));
+}
+
+void ArcBridgeService::AddObserver(Observer* observer) {
+ observer_list_.AddObserver(observer);
Shuhei Takahashi 2015/10/28 21:13:57 ObserverList is not thread-safe. Could you insert
Luis Héctor Chávez 2015/10/28 23:12:20 Done.
+}
+
+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_)
+ Shutdown();
Shuhei Takahashi 2015/10/28 21:13:57 Shutdown() will fail when state_ is either STOPPED
Luis Héctor Chávez 2015/10/28 23:12:20 Good catch. Done. The lifetime of ArcBridgeServic
+ FOR_EACH_OBSERVER(Observer, observer_list_, OnEnabledChanged(enabled_));
+}
+
+bool ArcBridgeService::RegisterInputDevice(
+ const std::string& name, const std::string& device_type,
+ base::ScopedFD fd) {
+ return ipc_channel_->Send(new ArcInstanceMsg_RegisterInputDevice(
Shuhei Takahashi 2015/10/28 21:13:57 Could you check state_? In some cases ipc_channel_
Luis Héctor Chávez 2015/10/28 23:12:20 Done.
+ name, device_type, base::FileDescriptor(fd.Pass())));
+}
+
+void ArcBridgeService::SocketConnect(const base::FilePath& socket_path,
+ bool directory_creation_success) {
+ DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+
+ if (!directory_creation_success) {
+ LOG(ERROR) << "Error creating directory for " << socket_path.value();
+ NotifyStateChanged(ArcBridgeService::ERROR);
+ return;
+ }
+
+ if (!Connect(IPC::ChannelHandle(socket_path.value()),
+ IPC::Channel::MODE_OPEN_NAMED_SERVER)) {
+ LOG(ERROR) << "Error connecting to " << socket_path.value();
+ NotifyStateChanged(ArcBridgeService::ERROR);
+ return;
+ }
+
+ base::PostTaskAndReplyWithResult(
+ file_task_runner_.get(),
+ FROM_HERE,
+ base::Bind(&SetSocketPermissions, socket_path),
+ base::Bind(&ArcBridgeService::FinishConnectAfterSetSocketPermissions,
+ weak_factory_.GetWeakPtr(), socket_path));
+}
+
+void ArcBridgeService::FinishConnectAfterSetSocketPermissions(
+ const base::FilePath& socket_path, bool socket_permissions_success) {
+ DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+
+ if (!socket_permissions_success) {
+ LOG(ERROR) << "Error setting socket permissions for "
+ << socket_path.value();
+ NotifyStateChanged(ArcBridgeService::ERROR);
+ return;
+ }
+
+ // This will fail if the ArcInstanceService is not running on Chrome OS.
+ // Use base::Unretained since this is a KeyedService.
Shuhei Takahashi 2015/10/28 21:13:57 Unretained seems not used here?
Luis Héctor Chávez 2015/10/28 23:12:20 Missed that one. Done.
+ 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());
+
+ ipc_channel_ = IPC::ChannelProxy::Create(handle, mode, this,
+ ipc_thread_.task_runner().get());
+ if (!ipc_channel_)
+ return false;
+ NotifyStateChanged(ArcBridgeService::CONNECTED);
Shuhei Takahashi 2015/10/28 21:13:57 It is confusing that state is changed only when IP
Luis Héctor Chávez 2015/10/28 23:12:20 Connect is also used from tests, so it needs to st
+ return true;
+}
+
+void ArcBridgeService::OnInstanceReady() {
+ DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+ NotifyStateChanged(ArcBridgeService::READY);
+}
+
+void ArcBridgeService::NotifyStateChanged(State state) {
Shuhei Takahashi 2015/10/28 21:13:57 This function does set the current state, not only
Luis Héctor Chávez 2015/10/28 23:12:20 Done.
+ DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+ if (state_ == state)
+ return;
+ state_ = state;
+ FOR_EACH_OBSERVER(Observer, observer_list_, OnStateChanged(state_));
+}
+
+void ArcBridgeService::FinishShutdownAfterSocketDeleted(bool socket_deleted) {
+ if (state_ != ArcBridgeService::STARTING && state_ != ArcBridgeService::READY)
+ return;
Shuhei Takahashi 2015/10/28 21:13:57 In this code path, state_ is not reset to STOPPED.
Luis Héctor Chávez 2015/10/28 23:12:20 It still needs to wait for the DBus message to ret
+ chromeos::DBusThreadManager::Get()->GetArcInstanceClient()->StopInstance(
+ base::Bind(&ArcBridgeService::OnInstanceStopped,
+ weak_factory_.GetWeakPtr()));
+}
+
+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(
Shuhei Takahashi 2015/10/28 21:13:57 Could you swap the position of OnInstanceStarted()
Luis Héctor Chávez 2015/10/28 23:12:20 I don't want to reorder any further until it's rea
+ chromeos::DBusMethodCallStatus status) {
+ DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+ if (status != chromeos::DBUS_METHOD_CALL_SUCCESS) {
+ LOG(ERROR) << "ARC instance unable to start. Shutting down the bridge";
+ Shutdown();
+ return;
+ }
+ NotifyStateChanged(ArcBridgeService::STARTING);
+}
+
+void ArcBridgeService::OnInstanceStopped(
+ chromeos::DBusMethodCallStatus status) {
+ DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
+ NotifyStateChanged(ArcBridgeService::STOPPED);
+}
+
+} // namespace arc
« no previous file with comments | « components/arc/arc_bridge_service.h ('k') | components/arc/arc_bridge_service_factory.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698