arc: Revamp the ArcBridgeService interface

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 <string>
 #include <vector>
 
@@ skipping 57 matching lines @@
    public:
     // Called whenever the state of the bridge has changed.
     // TODO(lchavez): Rename to OnStateChangedForTest
     virtual void OnStateChanged(State state) {}
     virtual void OnBridgeReady() {}
     virtual void OnBridgeStopped() {}
 
     // Called whenever ARC's availability has changed for this system.
     virtual void OnAvailableChanged(bool available) {}
 
-    // Called whenever the ARC app interface state changes.
-    virtual void OnAppInstanceReady() {}
-    virtual void OnAppInstanceClosed() {}
-
-    // Called whenever the ARC audio interface state changes.
-    virtual void OnAudioInstanceReady() {}
-    virtual void OnAudioInstanceClosed() {}
-
-    // Called whenever the ARC auth interface state changes.
-    virtual void OnAuthInstanceReady() {}
-    virtual void OnAuthInstanceClosed() {}
-
-    // Called whenever ARC Bluetooth instance is ready.
-    virtual void OnBluetoothInstanceReady() {}
-    virtual void OnBluetoothInstanceClosed() {}
-
-    // Called whenever the ARC clipboard interface state changes.
-    virtual void OnClipboardInstanceReady() {}
-    virtual void OnClipboardInstanceClosed() {}
-
-    // Called whenever the ARC crash collector interface state changes.
-    virtual void OnCrashCollectorInstanceReady() {}
-    virtual void OnCrashCollectorInstanceClosed() {}
-
-    // Called whenever the ARC file system interface state changes.
-    virtual void OnFileSystemInstanceReady() {}
-    virtual void OnFileSystemInstanceClosed() {}
-
-    // Called whenever the ARC IME interface state changes.
-    virtual void OnImeInstanceReady() {}
-    virtual void OnImeInstanceClosed() {}
-
-    // Called whenever the ARC intent helper interface state changes.
-    virtual void OnIntentHelperInstanceReady() {}
-    virtual void OnIntentHelperInstanceClosed() {}
-
-    // Called whenever the ARC metrics interface state changes.
-    virtual void OnMetricsInstanceReady() {}
-    virtual void OnMetricsInstanceClosed() {}
-
-    // Called whenever the ARC notification interface state changes.
-    virtual void OnNotificationsInstanceReady() {}
-    virtual void OnNotificationsInstanceClosed() {}
-
-    // Called whenever the ARC net interface state changes.
-    virtual void OnNetInstanceReady() {}
-    virtual void OnNetInstanceClosed() {}
-
-    // Called whenever the ARC OBB mounter interface state changes.
-    virtual void OnObbMounterInstanceReady() {}
-    virtual void OnObbMounterInstanceClosed() {}
-
-    // Called whenever the ARC policy interface state changes.
-    virtual void OnPolicyInstanceReady() {}
-    virtual void OnPolicyInstanceClosed() {}
-
-    // Called whenever the ARC power interface state changes.
-    virtual void OnPowerInstanceReady() {}
-    virtual void OnPowerInstanceClosed() {}
-
-    // Called whenever the ARC process interface state changes.
-    virtual void OnProcessInstanceReady() {}
-    virtual void OnProcessInstanceClosed() {}
-
-    // Called whenever the ARC storage manager interface state changes.
-    virtual void OnStorageManagerInstanceReady() {}
-    virtual void OnStorageManagerInstanceClosed() {}
-
-    // Called whenever the ARC video interface state changes.
-    virtual void OnVideoInstanceReady() {}
-    virtual void OnVideoInstanceClosed() {}
-
-    // Called whenever the ARC window manager interface state changes.
-    virtual void OnWindowManagerInstanceReady() {}
-    virtual void OnWindowManagerInstanceClosed() {}
-
    protected:
     virtual ~Observer() {}
   };
 
+  // Notifies about connection events for individual instances.
+  template <typename T>

[hidehiko, 2016/07/11 05:24:53]

Could you comment that T needs to be a mojo interface?

[Luis Héctor Chávez, 2016/07/11 17:13:34]

Done.

+  class InstanceObserver {

[hidehiko, 2016/07/11 05:24:53]

InstanceObserver and InstanceHolder looks enough big. So, could you move this
out from this class?

Optional: Introducing a new file may be another option?

[Luis Héctor Chávez, 2016/07/11 17:13:34]

Done.

+   public:
+    // Called once the instance is ready. It is guaranteed that |instance| will
+    // not be null.
+    virtual void OnInstanceReady(T* instance, uint32_t version) {}
+
+    // The parameter is always nullptr, but is required to be able to
+    // distinguish between implementations of InstanceObserver for different
+    // types.
+    virtual void OnInstanceClosed(T*) {}

[hidehiko, 2016/07/11 05:24:53]

Could you get rid of T*?

I'm guessing that it is supposed to inherit two ore more InstanceObserver
interface for a class, e.g.

class A : public InstanceObserver<T1>, public InstanceObserver<T2> ... {
};

etc. Though, in that case, I think it looks better to split the class into two
or more.

[Luis Héctor Chávez, 2016/07/11 17:13:34]

I can't :/ The metrics service tries needs to wait for both the metrics and
process instances.

+
+   protected:
+    virtual ~InstanceObserver() {}

[hidehiko, 2016/07/11 05:24:53]

nit: s/{}/= default/

[Luis Héctor Chávez, 2016/07/11 17:13:34]

Done.

+  };
+
+  template <typename T>
+  class InstanceHolder {
+   public:
+    InstanceHolder() : weak_factory_(this) {}
+
+    // Gets the Mojo interface for all the instance services. This will return
+    // nullptr if that particular service is not ready yet. Use an
+    // InstanceObserver if you want to be notified when this is ready. This can
+    // only be called on the thread that this class was created on.
+    T* instance() const { return raw_ptr_; }

[hidehiko, 2016/07/11 05:24:53]

To access the raw pointer of T, there are two ways:

accessing InstanceHolder::instance(), or kee pthe ptr passed via
InstanceObserver::OnInstanceReady().
Actually, currently those are mixed in use. Which is the preferable way?

[Luis Héctor Chávez, 2016/07/11 17:13:34]

I'd prefer to not keep the ptr since it is more error-prone (since the user
needs to be diligent to reset the ptr to nullptr in the OnInstanceClosed()).
Having said that, keeping the pointer does save some boilerplate :/

Let's discuss this offline to see if it is possible to add this to the best
practices doc.

+    uint32_t version() const { return version_; }
+
+    // Adds or removes observers. This can only be called on the thread that
+    // this class was created on. RemoveObserver does nothing if |observer| is
+    // not in the list.
+    void AddObserver(InstanceObserver<T>* observer) {
+      DCHECK(thread_checker_.CalledOnValidThread());
+      observer_list_.AddObserver(observer);
+
+      if (instance())
+        observer->OnInstanceReady(instance(), version());
+    }
+
+    void RemoveObserver(InstanceObserver<T>* observer) {
+      DCHECK(thread_checker_.CalledOnValidThread());
+      observer_list_.RemoveObserver(observer);
+    }
+
+    // Called when the channel is closed.
+    void CloseChannel() {
+      if (!ptr_)
+        return;
+
+      ptr_.reset();
+      raw_ptr_ = nullptr;
+      version_ = 0;
+      if (observer_list_.might_have_observers()) {

[hidehiko, 2016/07/11 05:24:53]

FOR_EACH_OBSERVER(InstanceObserver<T>, observer_list_, OnInstanceClosed());

should work?

[Luis Héctor Chávez, 2016/07/11 17:13:34]

It doesn't since it needs the 'typename' since InstanceObserver<T> cannot be
resolved to a type otherwise. I can try to fix the FOR_EACH_OBSERVER, but that
should be done in a different change.

[Luis Héctor Chávez, 2016/07/11 20:10:44]

Filed https://bugs.chromium.org/p/chromium/issues/detail?id=627209 and will
follow-up separately. Will amend this change if the fix lands first.

+        typename base::ObserverList<InstanceObserver<T>>::Iterator it(
+            &observer_list_);
+        InstanceObserver<T>* obs;
+        while ((obs = it.GetNext()) != nullptr)
+          obs->OnInstanceClosed(instance());
+      }
+    }
+
+    // Sets the interface pointer to |ptr|, once the version is determined. This
+    // will eventually invoke SetInstance(), which will notify the observers.
+    void OnInstanceReady(mojo::InterfacePtr<T> ptr) {
+      temporary_ptr_ = std::move(ptr);
+      temporary_ptr_.QueryVersion(base::Bind(&InstanceHolder<T>::OnVersionReady,
+                                             weak_factory_.GetWeakPtr()));
+    }
+
+    // This method is not intended to be called directly. Normally it is called
+    // by OnInstanceReady once the version of the instance is determined, but it
+    // is also exposed so that tests can directly inject a raw pointer+version
+    // combination.
+    void SetInstance(T* raw_ptr, uint32_t raw_version = T::Version_) {
+      raw_ptr_ = raw_ptr;
+      version_ = raw_version;
+      if (observer_list_.might_have_observers()) {

[hidehiko, 2016/07/11 05:24:53]

Ditto for FOR_EACH_OBSERVER.

[Luis Héctor Chávez, 2016/07/11 17:13:34]

Acknowledged.

+        typename base::ObserverList<InstanceObserver<T>>::Iterator it(
+            &observer_list_);
+        InstanceObserver<T>* obs;
+        while ((obs = it.GetNext()) != nullptr)
+          obs->OnInstanceReady(instance(), version());
+      }
+    }
+
+   private:
+    void OnVersionReady(uint32_t version) {
+      ptr_ = std::move(temporary_ptr_);
+      ptr_.set_connection_error_handler(base::Bind(
+          &InstanceHolder<T>::CloseChannel, weak_factory_.GetWeakPtr()));
+      SetInstance(ptr_.get(), version);
+    }
+
+    // These two are copies of the contents of ptr_. They are provided here just
+    // so that tests can provide non-mojo implementations.
+    T* raw_ptr_ = nullptr;
+    uint32_t version_ = 0;
+
+    mojo::InterfacePtr<T> ptr_;
+
+    // Temporary Mojo interfaces.  After a Mojo interface pointer has been
+    // received from the other endpoint, we still need to asynchronously query
+    // its version.  While that is going on, we should still return nullptr on
+    // the instance() function.
+    // To keep the instance() functions being trivial, store the instance
+    // pointer in a temporary variable to avoid losing its reference.
+    mojo::InterfacePtr<T> temporary_ptr_;
+
+    base::ThreadChecker thread_checker_;
+    base::ObserverList<InstanceObserver<T>> observer_list_;
+    base::WeakPtrFactory<InstanceHolder<T>> weak_factory_;

[hidehiko, 2016/07/11 05:24:53]

Could you comment that this needs to be the last member, in order to cancel all
inflight callback with weak_ptr generated by this factory before destructing any
of other members?

[Luis Héctor Chávez, 2016/07/11 17:13:34]

Sure

+
+    DISALLOW_COPY_AND_ASSIGN(InstanceHolder<T>);
+  };
+
   ~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);
 
@@ skipping 10 matching lines @@
   // Shutdown() should be called when the browser is shutting down. This can
   // only be called on the thread that this class was created on.
   virtual void Shutdown() = 0;
 
   // Adds or removes observers. This can only be called on the thread that this
   // class was created on. RemoveObserver does nothing if |observer| is not in
   // the list.
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
-  // Gets the Mojo interface for all the instance services. This will return
-  // nullptr if that particular service is not ready yet. Use an Observer if
-  // you want to be notified when this is ready. This can only be called on the
-  // thread that this class was created on.
-  mojom::AppInstance* app_instance() { return app_ptr_.get(); }
-  mojom::AudioInstance* audio_instance() { return audio_ptr_.get(); }
-  mojom::AuthInstance* auth_instance() { return auth_ptr_.get(); }
-  mojom::BluetoothInstance* bluetooth_instance() {
-    return bluetooth_ptr_.get();
+  InstanceHolder<mojom::AppInstance>* app() { return &app_; }
+  InstanceHolder<mojom::AudioInstance>* audio() { return &audio_; }
+  InstanceHolder<mojom::AuthInstance>* auth() { return &auth_; }
+  InstanceHolder<mojom::BluetoothInstance>* bluetooth() { return &bluetooth_; }
+  InstanceHolder<mojom::ClipboardInstance>* clipboard() { return &clipboard_; }
+  InstanceHolder<mojom::CrashCollectorInstance>* crash_collector() {
+    return &crash_collector_;
   }
-  mojom::ClipboardInstance* clipboard_instance() {
-    return clipboard_ptr_.get();
+  InstanceHolder<mojom::FileSystemInstance>* file_system() {
+    return &file_system_;
   }
-  mojom::CrashCollectorInstance* crash_collector_instance() {
-    return crash_collector_ptr_.get();
+  InstanceHolder<mojom::ImeInstance>* ime() { return &ime_; }
+  InstanceHolder<mojom::IntentHelperInstance>* intent_helper() {
+    return &intent_helper_;
   }
-  mojom::FileSystemInstance* file_system_instance() {
-    return file_system_ptr_.get();
+  InstanceHolder<mojom::MetricsInstance>* metrics() { return &metrics_; }
+  InstanceHolder<mojom::NetInstance>* net() { return &net_; }
+  InstanceHolder<mojom::NotificationsInstance>* notifications() {
+    return &notifications_;
   }
-  mojom::ImeInstance* ime_instance() { return ime_ptr_.get(); }
-  mojom::IntentHelperInstance* intent_helper_instance() {
-    return intent_helper_ptr_.get();
+  InstanceHolder<mojom::ObbMounterInstance>* obb_mounter() {
+    return &obb_mounter_;
   }
-  mojom::MetricsInstance* metrics_instance() { return metrics_ptr_.get(); }
-  mojom::NetInstance* net_instance() { return net_ptr_.get(); }
-  mojom::NotificationsInstance* notifications_instance() {
-    return notifications_ptr_.get();
+  InstanceHolder<mojom::PolicyInstance>* policy() { return &policy_; }
+  InstanceHolder<mojom::PowerInstance>* power() { return &power_; }
+  InstanceHolder<mojom::ProcessInstance>* process() { return &process_; }
+  InstanceHolder<mojom::StorageManagerInstance>* storage_manager() {
+    return &storage_manager_;
   }
-  mojom::ObbMounterInstance* obb_mounter_instance() {
-    return obb_mounter_ptr_.get();
-  }
-  mojom::PolicyInstance* policy_instance() { return policy_ptr_.get(); }
-  mojom::PowerInstance* power_instance() { return power_ptr_.get(); }
-  mojom::ProcessInstance* process_instance() { return process_ptr_.get(); }
-  mojom::StorageManagerInstance* storage_manager_instance() {
-    return storage_manager_ptr_.get();
-  }
-  mojom::VideoInstance* video_instance() { return video_ptr_.get(); }
-  mojom::WindowManagerInstance* window_manager_instance() {
-    return window_manager_ptr_.get();
-  }
-
-  int32_t app_version() const { return app_ptr_.version(); }
-  int32_t audio_version() const { return audio_ptr_.version(); }
-  int32_t bluetooth_version() const { return bluetooth_ptr_.version(); }
-  int32_t auth_version() const { return auth_ptr_.version(); }
-  int32_t clipboard_version() const { return clipboard_ptr_.version(); }
-  int32_t crash_collector_version() const {
-    return crash_collector_ptr_.version();
-  }
-  int32_t file_system_version() const { return file_system_ptr_.version(); }
-  int32_t ime_version() const { return ime_ptr_.version(); }
-  int32_t intent_helper_version() const { return intent_helper_ptr_.version(); }
-  int32_t metrics_version() const { return metrics_ptr_.version(); }
-  int32_t net_version() const { return net_ptr_.version(); }
-  int32_t notifications_version() const { return notifications_ptr_.version(); }
-  int32_t obb_mounter_version() const { return obb_mounter_ptr_.version(); }
-  int32_t policy_version() const { return policy_ptr_.version(); }
-  int32_t power_version() const { return power_ptr_.version(); }
-  int32_t process_version() const { return process_ptr_.version(); }
-  int32_t storage_manager_version() const {
-    return storage_manager_ptr_.version();
-  }
-  int32_t video_version() const { return video_ptr_.version(); }
-  int32_t window_manager_version() const {
-    return window_manager_ptr_.version();
+  InstanceHolder<mojom::VideoInstance>* video() { return &video_; }
+  InstanceHolder<mojom::WindowManagerInstance>* window_manager() {
+    return &window_manager_;
   }
 
   // ArcHost:
   void OnAppInstanceReady(mojom::AppInstancePtr app_ptr) override;

[hidehiko, 2016/07/11 05:24:53]

Clarification: Not necessary to do in this CL (moreover I think it is better to
do it in a separate CL,... but), WDYT about moving these implementation to the
ABSImpl?

If you agree, could you move some part of InstanceHolder implementation to
another new (maybe template) class in ABSImpl file, specifically,
CloseChannel(), OnInstanceReady(), and OnVersionReady()?

[hidehiko, 2016/07/11 06:09:32]

Oh, your CL following this one actually did the ArcHost moving. So, splitting
InstanceHost is just my request.

[Luis Héctor Chávez, 2016/07/11 17:13:34]

Acknowledged.

   void OnAudioInstanceReady(mojom::AudioInstancePtr audio_ptr) override;
   void OnAuthInstanceReady(mojom::AuthInstancePtr auth_ptr) override;
   void OnBluetoothInstanceReady(
       mojom::BluetoothInstancePtr bluetooth_ptr) override;
   void OnClipboardInstanceReady(
       mojom::ClipboardInstancePtr clipboard_ptr) override;
   void OnCrashCollectorInstanceReady(
       mojom::CrashCollectorInstancePtr crash_collector_ptr) override;
   void OnFileSystemInstanceReady(
       mojom::FileSystemInstancePtr file_system_ptr) override;
@@ skipping 37 matching lines @@
   // Closes all Mojo channels.
   void CloseAllChannels();
 
  private:
   friend class ArcBridgeTest;
   FRIEND_TEST_ALL_PREFIXES(ArcBridgeTest, Basic);
   FRIEND_TEST_ALL_PREFIXES(ArcBridgeTest, Prerequisites);
   FRIEND_TEST_ALL_PREFIXES(ArcBridgeTest, ShutdownMidStartup);
   FRIEND_TEST_ALL_PREFIXES(ArcBridgeTest, Restart);
 
-  // Called when one of the individual channels is closed.
-  void CloseAppChannel();
-  void CloseAudioChannel();
-  void CloseAuthChannel();
-  void CloseBluetoothChannel();
-  void CloseClipboardChannel();
-  void CloseCrashCollectorChannel();
-  void CloseFileSystemChannel();
-  void CloseImeChannel();
-  void CloseIntentHelperChannel();
-  void CloseMetricsChannel();
-  void CloseNetChannel();
-  void CloseNotificationsChannel();
-  void CloseObbMounterChannel();
-  void ClosePolicyChannel();
-  void ClosePowerChannel();
-  void CloseProcessChannel();
-  void CloseStorageManagerChannel();
-  void CloseVideoChannel();
-  void CloseWindowManagerChannel();
-
-  // Callbacks for QueryVersion.
-  void OnAppVersionReady(uint32_t version);
-  void OnAudioVersionReady(uint32_t version);
-  void OnAuthVersionReady(uint32_t version);
-  void OnBluetoothVersionReady(uint32_t version);
-  void OnClipboardVersionReady(uint32_t version);
-  void OnCrashCollectorVersionReady(uint32_t version);
-  void OnFileSystemVersionReady(uint32_t version);
-  void OnImeVersionReady(uint32_t version);
-  void OnIntentHelperVersionReady(uint32_t version);
-  void OnMetricsVersionReady(uint32_t version);
-  void OnNetVersionReady(uint32_t version);
-  void OnNotificationsVersionReady(uint32_t version);
-  void OnObbMounterVersionReady(uint32_t version);
-  void OnPolicyVersionReady(uint32_t version);
-  void OnPowerVersionReady(uint32_t version);
-  void OnProcessVersionReady(uint32_t version);
-  void OnStorageManagerVersionReady(uint32_t version);
-  void OnVideoVersionReady(uint32_t version);
-  void OnWindowManagerVersionReady(uint32_t version);
-
-  // Mojo interfaces.
-  mojom::AppInstancePtr app_ptr_;
-  mojom::AudioInstancePtr audio_ptr_;
-  mojom::AuthInstancePtr auth_ptr_;
-  mojom::BluetoothInstancePtr bluetooth_ptr_;
-  mojom::ClipboardInstancePtr clipboard_ptr_;
-  mojom::CrashCollectorInstancePtr crash_collector_ptr_;
-  mojom::FileSystemInstancePtr file_system_ptr_;
-  mojom::ImeInstancePtr ime_ptr_;
-  mojom::IntentHelperInstancePtr intent_helper_ptr_;
-  mojom::MetricsInstancePtr metrics_ptr_;
-  mojom::NetInstancePtr net_ptr_;
-  mojom::NotificationsInstancePtr notifications_ptr_;
-  mojom::ObbMounterInstancePtr obb_mounter_ptr_;
-  mojom::PolicyInstancePtr policy_ptr_;
-  mojom::PowerInstancePtr power_ptr_;
-  mojom::ProcessInstancePtr process_ptr_;
-  mojom::StorageManagerInstancePtr storage_manager_ptr_;
-  mojom::VideoInstancePtr video_ptr_;
-  mojom::WindowManagerInstancePtr window_manager_ptr_;
-
-  // Temporary Mojo interfaces.  After a Mojo interface pointer has been
-  // received from the other endpoint, we still need to asynchronously query
-  // its version.  While that is going on, we should still return nullptr on
-  // the xxx_instance() functions.
-  // To keep the xxx_instance() functions being trivial, store the instance
-  // pointer in a temporary variable to avoid losing its reference.
-  mojom::AppInstancePtr temporary_app_ptr_;
-  mojom::AudioInstancePtr temporary_audio_ptr_;
-  mojom::AuthInstancePtr temporary_auth_ptr_;
-  mojom::BluetoothInstancePtr temporary_bluetooth_ptr_;
-  mojom::ClipboardInstancePtr temporary_clipboard_ptr_;
-  mojom::CrashCollectorInstancePtr temporary_crash_collector_ptr_;
-  mojom::FileSystemInstancePtr temporary_file_system_ptr_;
-  mojom::ImeInstancePtr temporary_ime_ptr_;
-  mojom::IntentHelperInstancePtr temporary_intent_helper_ptr_;
-  mojom::MetricsInstancePtr temporary_metrics_ptr_;
-  mojom::NetInstancePtr temporary_net_ptr_;
-  mojom::NotificationsInstancePtr temporary_notifications_ptr_;
-  mojom::ObbMounterInstancePtr temporary_obb_mounter_ptr_;
-  mojom::PolicyInstancePtr temporary_policy_ptr_;
-  mojom::PowerInstancePtr temporary_power_ptr_;
-  mojom::ProcessInstancePtr temporary_process_ptr_;
-  mojom::StorageManagerInstancePtr temporary_storage_manager_ptr_;
-  mojom::VideoInstancePtr temporary_video_ptr_;
-  mojom::WindowManagerInstancePtr temporary_window_manager_ptr_;
+  // Instance holders.
+  InstanceHolder<mojom::AppInstance> app_;
+  InstanceHolder<mojom::AudioInstance> audio_;
+  InstanceHolder<mojom::AuthInstance> auth_;
+  InstanceHolder<mojom::BluetoothInstance> bluetooth_;
+  InstanceHolder<mojom::ClipboardInstance> clipboard_;
+  InstanceHolder<mojom::CrashCollectorInstance> crash_collector_;
+  InstanceHolder<mojom::FileSystemInstance> file_system_;
+  InstanceHolder<mojom::ImeInstance> ime_;
+  InstanceHolder<mojom::IntentHelperInstance> intent_helper_;
+  InstanceHolder<mojom::MetricsInstance> metrics_;
+  InstanceHolder<mojom::NetInstance> net_;
+  InstanceHolder<mojom::NotificationsInstance> notifications_;
+  InstanceHolder<mojom::ObbMounterInstance> obb_mounter_;
+  InstanceHolder<mojom::PolicyInstance> policy_;
+  InstanceHolder<mojom::PowerInstance> power_;
+  InstanceHolder<mojom::ProcessInstance> process_;
+  InstanceHolder<mojom::StorageManagerInstance> storage_manager_;
+  InstanceHolder<mojom::VideoInstance> video_;
+  InstanceHolder<mojom::WindowManagerInstance> window_manager_;
 
   base::ObserverList<Observer> observer_list_;
 
   base::ThreadChecker thread_checker_;
 
   // 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);
 };
 
 }  // namespace arc
 
 #endif  // COMPONENTS_ARC_ARC_BRIDGE_SERVICE_H_