arc-bridge: Add the ARC Bridge Service

chromeos/arc/bridge/arc_bridge_service.cc

+// 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 "chromeos/arc/bridge/arc_bridge_service.h"
+
+#include "base/files/file_util.h"
+#include "chrome/browser/profiles/profile.h"

[oshima, 2015/10/23 20:29:45]

ditto

[Luis Héctor Chávez, 2015/10/27 00:37:47]

Done.

+#include "chromeos/dbus/arc_instance_client.h"
+#include "chromeos/dbus/dbus_method_call_status.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+
+namespace {
+
+const char kArcBridgeSocketPath[] = "/home/chronos/ArcBridge/bridge.sock";

[satorux1, 2015/10/23 06:10:58]

should this be shared between here and the chromeos side? Then you can put the
constant in third_party/cros_system_api.

That's a bit tedious so maybe you can just add a TODO here.

[Luis Héctor Chávez, 2015/10/27 00:37:47]

There will be no sharing of this constant. It will be passed as an explicit
parameter to StartInstance.

+
+}  // namespace
+
+namespace arc {
+
+ArcBridgeService::ArcBridgeService(Profile* profile)
+    : BridgeHostEndpoint(),
+      enabled_(true),
+      running_(false),
+      profile_(profile) {}
+
+ArcBridgeService::~ArcBridgeService() {}
+
+void ArcBridgeService::HandleStartup() {
+  if (!IsEnabled())
+    return;
+  if (!SocketConnect(kArcBridgeSocketPath)) {
+    LOG(ERROR) << "ARC++ unable to connect to the socket";
+    return;
+  }
+  connected_ = true;
+  // This will fail if the ArcInstanceService is not running on Chrome OS.
+  chromeos::DBusThreadManager::Get()->GetArcInstanceClient()->StartInstance(

[satorux1, 2015/10/23 06:10:58]

Is the D-Bus client code checked-in? I'm reviewing from a laptop without source
code, so I cannot check.

Anyway, I was curious what instance was started here.

[Luis Héctor Chávez, 2015/10/27 00:37:47]

Not yet, but here's its review: https://codereview.chromium.org/1421023002/

+      kArcBridgeSocketPath,
+      base::Bind(&ArcBridgeService::OnInstanceStarted, base::Unretained(this)));

[satorux1, 2015/10/23 06:10:58]

Is it safe to base::Unretained() here? Please document. 

In general, base::Unretained() shouldn't be used, because it's tricky to
guarantee that |this| is alive when the callback is run.

However, this seems safe because this is a keyed service instance.

[Luis Héctor Chávez, 2015/10/27 00:37:47]

Done.

+}
+
+void ArcBridgeService::Shutdown() {
+  if (!connected_)
+    return;
+  channel_->Close();

[satorux1, 2015/10/23 06:10:58]

channel_ is defined in the parent class right? Maybe give it a more descriptive
name like ipc_channel_?

[Luis Héctor Chávez, 2015/10/27 00:37:47]

Done.

+  base::DeleteFile(base::FilePath(kArcBridgeSocketPath), false);
+  if (!running_)
+    return;
+  chromeos::DBusThreadManager::Get()->GetArcInstanceClient()->StopInstance(
+      base::Bind(&ArcBridgeService::OnInstanceStopped, base::Unretained(this)));

[satorux1, 2015/10/23 06:10:58]

ditto. add a comment about base::Unretained()?

[Luis Héctor Chávez, 2015/10/27 00:37:47]

Done.

+}
+
+void ArcBridgeService::AddObserver(Observer* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void ArcBridgeService::RemoveObserver(Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
+void ArcBridgeService::OnInstanceStarted(chromeos::DBusMethodCallStatus status,
+                                         bool success) {
+  running_ = success;
+  if (!running_) {
+    LOG(ERROR) << "ARC++ instance unable to start. Shutting down the bridge";
+    Shutdown();
+  }
+  FOR_EACH_OBSERVER(Observer, observer_list_, OnInstanceStarted(success));
+}
+
+void ArcBridgeService::OnInstanceStopped(
+    chromeos::DBusMethodCallStatus status) {
+  if (running_)
+    FOR_EACH_OBSERVER(Observer, observer_list_, OnInstanceStopped());
+  running_ = false;
+}
+
+void ArcBridgeService::OnInstanceReady() {
+  FOR_EACH_OBSERVER(Observer, observer_list_, OnInstanceReady());
+}
+
+void ArcBridgeService::OnPong() {}
+
+}  // namespace arc