arc-bridge: Split ArcBridgeService to common part and IPC based impl.

components/arc/arc_bridge_service.h

 // 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.
 
 #ifndef COMPONENTS_ARC_ARC_BRIDGE_SERVICE_H_
 #define COMPONENTS_ARC_ARC_BRIDGE_SERVICE_H_
 
+#include "base/files/scoped_file.h"
 #include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/observer_list.h"
-#include "base/sequenced_task_runner.h"
 #include "components/arc/common/arc_message_types.h"
-#include "ipc/ipc_channel_proxy.h"
-#include "ipc/ipc_listener.h"
-#include "ipc/ipc_message.h"
+
+namespace base {

[hidehiko, 2015/11/26 02:21:51]

nit: please merge namespace to just below one.

[khmel1, 2015/11/26 05:46:43]

Done.

+class SequencedTaskRunner;
+class SingleThreadTaskRunner;
+}
 
 namespace base {
 class CommandLine;
 }
 
 namespace arc {
 
 // The Chrome-side service that handles ARC instances and ARC bridge creation.
 // This service handles the lifetime of ARC instances and sets up the
 // communication channel (the ARC bridge) used to send and receive messages.
-class ArcBridgeService : public IPC::Listener {
+class ArcBridgeService {
  public:
   // The possible states of the bridge.  In the normal flow, the state changes
   // in the following sequence:
   //
   // STOPPED
   //   SetAvailable(true) + HandleStartup() -> SocketConnect() ->
   // CONNECTING
   //   Connect() ->
   // CONNECTED
   //   SocketConnectAfterSetSocketPermissions() ->
@@ skipping 36 matching lines @@
     // Called when the instance has reached a boot phase
     virtual void OnInstanceBootPhase(InstanceBootPhase phase) {}
 
     // Called whenever ARC's availability has changed for this system.
     virtual void OnAvailableChanged(bool available) {}
 
    protected:
     virtual ~Observer() {}
   };
 
-  ArcBridgeService(
+  ArcBridgeService();

[hidehiko, 2015/11/26 02:21:51]

Please move this to protected.

[khmel1, 2015/11/26 05:46:43]

Done.

+  virtual ~ArcBridgeService();
+
+  // Create instance of |ArcBridgeService| for normal use.

[hidehiko, 2015/11/26 02:21:51]

nit: Create*s*. (For function or class comment, third-person singular present
form is preferred).

[khmel1, 2015/11/26 05:46:43]

Sorry, copied from legacy code as template and did not notice.

+  static scoped_ptr<ArcBridgeService> Create(
       const scoped_refptr<base::SingleThreadTaskRunner>& ipc_task_runner,
       const scoped_refptr<base::SequencedTaskRunner>& file_task_runner);
-  ~ArcBridgeService() override;
 
   // Gets the global instance of the ARC Bridge Service. This can only be
   // called on the thread that this class was created on.
   static ArcBridgeService* Get();
 
   // Return true if ARC has been enabled through a commandline
   // switch.
   static bool GetEnabled(const base::CommandLine* command_line);
 
   // DetectAvailability() should be called once D-Bus is available. It will
   // call CheckArcAvailability() on the session_manager. This can only be
   // called on the thread that this class was created on.
-  void DetectAvailability();
+  virtual void DetectAvailability() = 0;
 
   // HandleStartup() should be called upon profile startup.  This will only
   // launch an instance if the instance service is available and it is enabled.
   // This can only be called on the thread that this class was created on.
-  void HandleStartup();
+  virtual void HandleStartup() = 0;
 
   // Shutdown() should be called when the browser is shutting down. This can
   // only be called on the thread that this class was created on.
-  void Shutdown();
+  virtual void Shutdown() = 0;
 
   // Adds or removes observers. This can only be called on the thread that this
   // class was created on.
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
   // Gets the current state of the bridge service.
   State state() const { return state_; }
 
   // Gets if ARC is available in this system.
   bool available() const { return available_; }
 
   // Requests registration of an input device on the ARC instance.
   // TODO(denniskempin): Make this interface more typesafe.
   // |name| should be the displayable name of the emulated device (e.g. "Chrome
   // OS Keyboard"), |device_type| the name of the device type (e.g. "keyboard")
   // and |fd| a file descriptor that emulates the kernel events of the device.
   // This can only be called on the thread that this class was created on.
-  bool RegisterInputDevice(const std::string& name,
-                           const std::string& device_type,
-                           base::ScopedFD fd);
+  virtual bool RegisterInputDevice(const std::string& name,
+                                   const std::string& device_type,
+                                   base::ScopedFD fd) = 0;
 
- private:
-  friend class ArcBridgeTest;
-  FRIEND_TEST_ALL_PREFIXES(ArcBridgeTest, Basic);
-  FRIEND_TEST_ALL_PREFIXES(ArcBridgeTest, ShutdownMidStartup);
-
-  // If all pre-requisites are true (ARC is available, it has been enabled, and
-  // the session has started), and ARC is stopped, start ARC. If ARC is running
-  // and the pre-requisites stop being true, stop ARC.
-  void PrerequisitesChanged();
-
-  // Binds to the socket specified by |socket_path|.
-  void SocketConnect(const base::FilePath& socket_path);
-
-  // Binds to the socket specified by |socket_path| after creating its parent
-  // directory is present.
-  void SocketConnectAfterEnsureParentDirectory(
-      const base::FilePath& socket_path,
-      bool directory_present);
-
-  // Internal connection method. Separated to make testing easier.
-  bool Connect(const IPC::ChannelHandle& handle, IPC::Channel::Mode mode);
-
-  // Finishes connecting after setting socket permissions.
-  void SocketConnectAfterSetSocketPermissions(const base::FilePath& socket_path,
-                                              bool socket_permissions_success);
-
-  // Stops the running instance.
-  void StopInstance();
-
-  // Called when the instance has reached a boot phase
-  void OnInstanceBootPhase(InstanceBootPhase phase);
-
+ protected:
   // Changes the current state and notifies all observers.
   void SetState(State state);
 
-  // IPC::Listener:
-  bool OnMessageReceived(const IPC::Message& message) override;
-
-  // DBus callbacks.
-  void OnArcAvailable(bool available);
-  void OnInstanceStarted(bool success);
-  void OnInstanceStopped(bool success);
+  // Changes the current availability and notifies all observers.
+  void SetAvailable(bool availability);
 
   scoped_refptr<base::SequencedTaskRunner> origin_task_runner_;
-  scoped_refptr<base::SingleThreadTaskRunner> ipc_task_runner_;
-  scoped_refptr<base::SequencedTaskRunner> file_task_runner_;
-
-  scoped_ptr<IPC::ChannelProxy> ipc_channel_;
 
   base::ObserverList<Observer> observer_list_;

[hidehiko, 2015/11/26 02:21:51]

Following three fields can and should be private?

[khmel1, 2015/11/26 05:46:43]

Done

 
-  // If the user's session has started.
-  bool session_started_;
-
   // If the ARC instance service is available.
   bool available_;
 
   // The current state of the bridge.
   ArcBridgeService::State state_;
 
-  // WeakPtrFactory to use callbacks.
-  base::WeakPtrFactory<ArcBridgeService> weak_factory_;
-
   DISALLOW_COPY_AND_ASSIGN(ArcBridgeService);

[hidehiko, 2015/11/26 02:21:51]

private:

[khmel1, 2015/11/26 05:46:43]

Done.

 };
 
 }  // namespace arc
 
 #endif  // COMPONENTS_ARC_ARC_BRIDGE_SERVICE_H_