arc-bridge: Add the ARC Bridge Service

chromeos/arc/bridge/arc_bridge_service.h

+// Copyright (c) 2012 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.
+
+#ifndef CHROMEOS_ARC_BRIDGE_ARC_BRIDGE_SERVICE_H_
+#define CHROMEOS_ARC_BRIDGE_ARC_BRIDGE_SERVICE_H_
+
+#include "base/macros.h"
+#include "base/observer_list.h"
+#include "chromeos/arc/libarcbridge/arc_bridge.h"
+#include "chromeos/dbus/dbus_method_call_status.h"
+#include "components/keyed_service/core/keyed_service.h"
+
+class Profile;

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

chromeos/ should not depend on Profile

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

Done.

+
+namespace arc {
+
+class ArcBridgeService : public KeyedService, public BridgeHostEndpoint {

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

class comment is missing. please describe what this class does and what is for,
etc.

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

Done.

+ public:
+  class Observer {
+   public:
+    // Called whenever an instance start request has finished.
+    virtual void OnInstanceStarted(bool success) {}
+
+    // Called when an instance is ready for interaction.
+    virtual void OnInstanceReady() {}
+
+    // Called when an instance has stopped.  This is not called if
+    // OnInstanceStarted() was called with |success| = false.
+    virtual void OnInstanceStopped() {}
+
+   protected:
+    virtual ~Observer() {}
+  };
+
+  explicit ArcBridgeService(Profile* profile);
+  virtual ~ArcBridgeService();
+
+  // HandleStartup() should be called upon profile startup.  This will only
+  // launch an instance if the instance service is available and it is enabled.
+  void HandleStartup();
+
+  // Shuts down the running instance, if any.  This is safe to call even if
+  // there is no instance running.
+  void Shutdown();
+
+  // Add or remove observers.

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

nit: Adds or removes

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

Done.

+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
+  // Methods to access preferences.
+  bool IsEnabled() { return enabled_; }
+
+  // TODO(lhchavez): Setup a persistent Prefs object to store the setting.
+  void SetEnabled(bool enabled) { enabled_ = enabled; }
+
+  bool IsRunning() const { return running_; }

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

function comment is missing.

// Returns true if the service is running

would be a typical function comment, but would be nice to explain what it means
to be "running".

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

Done.

+
+ private:
+  // DBus callbacks.
+  void OnInstanceStarted(chromeos::DBusMethodCallStatus status, bool success);
+  void OnInstanceStopped(chromeos::DBusMethodCallStatus status);
+
+  // arc::BridgeHostEndpoint overrides:
+  void OnInstanceReady() override;
+  void OnPong() override;
+
+  // List of observers to notify of state changes.
+  // Makes sure list is empty on destruction.
+  base::ObserverList<Observer, true> observer_list_;
+
+  // TODO(lhchavez): Move to a real preference.
+  bool enabled_;
+
+  bool connected_;
+  bool running_;

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

enabled_, connected_, running_ sound similar to me. Could you add comments and
explain the differences?

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

Changed to an enum.

+  Profile* profile_;
+
+  DISALLOW_COPY_AND_ASSIGN(ArcBridgeService);
+};
+
+}  // namespace arc
+
+#endif  // CHROMEOS_ARC_BRIDGE_ARC_BRIDGE_SERVICE_H_