Remove the ARC Bridge Service

chromeos/dbus/arc_instance_client.cc

+// Copyright (c) 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/dbus/arc_instance_client.h"
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/memory/weak_ptr.h"
+#include "dbus/bus.h"
+#include "dbus/message.h"
+#include "dbus/object_path.h"
+#include "dbus/object_proxy.h"
+
+namespace chromeos {
+
+namespace {
+
+// todo(denniskempin): Move constants to the chromiumos platform
+// service_constants.h
+const char kArcInstanceServicePath[] = "/org/chromium/arc/instanceservice";
+const char kArcInstanceServiceName[] = "org.chromium.arc.InstanceService";
+const char kArcInstanceServiceInterface[] = "org.chromium.arc.InstanceService";
+
+const char kStartInstanceMethod[] = "StartInstance";
+const char kStopInstanceMethod[] = "StopInstance";
+
+void OnVoidDBusMethod(const VoidDBusMethodCallback& callback,
+                      dbus::Response* response) {
+  callback.Run(response ? DBUS_METHOD_CALL_SUCCESS : DBUS_METHOD_CALL_FAILURE);
+}
+
+void OnBoolDBusMethod(const BoolDBusMethodCallback& callback,

[denniskempin (chromium), 2015/10/23 12:22:10]

dbus already has a way of passing back errors, isn't the additional boolean
unnecessary then? The method could just return DBUS_METHOD_CALL_FAILURE.
See:
https://chrome-internal-review.googlesource.com/#/c/234260/10/chromeos-base/a...

[Luis Héctor Chávez, 2015/10/23 17:33:14]

What I originally wanted was to have an extra flag to indicate availablity (so
the call could be successful, but the instance would not be available), but now
that I think about it, it's simpler to just go with the failure.

+                      dbus::Response* response) {
+  if (!response) {
+    callback.Run(DBUS_METHOD_CALL_FAILURE, false);
+    return;
+  }
+  dbus::MessageReader reader(response);
+  bool success = false;
+  if (!reader.PopBool(&success)) {
+    LOG(ERROR) << "Error reading response from ArcInstanceService";
+    callback.Run(DBUS_METHOD_CALL_FAILURE, false);
+    return;
+  }
+  callback.Run(DBUS_METHOD_CALL_SUCCESS, success);
+}
+
+class ArcInstanceClientImpl : public ArcInstanceClient {
+ public:
+  ArcInstanceClientImpl() : proxy_(nullptr), weak_ptr_factory_(this) {}
+
+  ~ArcInstanceClientImpl() override {}
+
+  // Start an instance of ARC.  |callback| is called after the method call
+  // succeeds.
+  void StartInstance(const std::string& socket_path,
+                     const BoolDBusMethodCallback& callback) override {
+    DCHECK(proxy_);
+
+    dbus::MethodCall method_call(kArcInstanceServiceInterface,
+                                 kStartInstanceMethod);
+    dbus::MessageWriter builder(&method_call);
+    builder.AppendString(socket_path);
+    proxy_->CallMethod(&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+                       base::Bind(&OnBoolDBusMethod, callback));

[stevenjb, 2015/10/23 16:34:45]

To elaborate the above comment, typically we would use
proxy_->CallMethodWithErrorCallback() which passes in an error callback handler
which receives a dbus::ErrorResponse containing the error code.

Note: it is fine having this method just taking a single callback that receives
true or false (although a return code might be more useful in the future), but
we should use DBus error codes between processes.

[Luis Héctor Chávez, 2015/10/23 17:33:14]

I went with Dennis' suggestion and just return the DBus status to the callback.

+  }
+
+  // Stop a currently running instance of ARC.  |callback| is called after the
+  // method call succeeds.
+  void StopInstance(const VoidDBusMethodCallback& callback) override {
+    DCHECK(proxy_);
+
+    dbus::MethodCall method_call(kArcInstanceServiceInterface,
+                                 kStopInstanceMethod);
+    proxy_->CallMethod(&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+                       base::Bind(&OnVoidDBusMethod, callback));
+  }
+
+ protected:
+  void Init(dbus::Bus* bus) override {
+    proxy_ = bus->GetObjectProxy(kArcInstanceServiceName,
+                                 dbus::ObjectPath(kArcInstanceServicePath));
+    DCHECK(proxy_);
+  }
+
+ private:
+  dbus::ObjectProxy* proxy_;
+
+  // 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<ArcInstanceClientImpl> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(ArcInstanceClientImpl);
+};
+
+}  // namespace
+
+////////////////////////////////////////////////////////////////////////////////
+// ArcInstanceClient
+
+ArcInstanceClient::ArcInstanceClient() {}
+
+ArcInstanceClient::~ArcInstanceClient() {}
+
+// static
+ArcInstanceClient* ArcInstanceClient::Create() {
+  return new ArcInstanceClientImpl();
+}
+
+}  // namespace chromeos