Index: components/arc/arc_bridge_service.h |
diff --git a/components/arc/arc_bridge_service.h b/components/arc/arc_bridge_service.h |
index 3d719694314ee0219463b873144255e23136cbde..40ea1f4921ccdbd5452420b3040029c6e7bf24a8 100644 |
--- a/components/arc/arc_bridge_service.h |
+++ b/components/arc/arc_bridge_service.h |
@@ -75,84 +75,124 @@ class ArcBridgeService : public mojom::ArcBridgeHost { |
// 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() {} |
+ protected: |
+ virtual ~Observer() {} |
+ }; |
- // Called whenever the ARC video interface state changes. |
- virtual void OnVideoInstanceReady() {} |
- virtual void OnVideoInstanceClosed() {} |
+ // 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 interf
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 b
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) {} |
- // Called whenever the ARC window manager interface state changes. |
- virtual void OnWindowManagerInstanceReady() {} |
- virtual void OnWindowManagerInstanceClosed() {} |
+ // 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
Luis Héctor Chávez
2016/07/11 17:13:34
I can't :/ The metrics service tries needs to wait
|
protected: |
- virtual ~Observer() {} |
+ 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
Luis Héctor Chávez
2016/07/11 17:13:34
I'd prefer to not keep the ptr since it is more er
|
+ 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_li
Luis Héctor Chávez
2016/07/11 17:13:34
It doesn't since it needs the 'typename' since Ins
Luis Héctor Chávez
2016/07/11 20:10:44
Filed https://bugs.chromium.org/p/chromium/issues/
|
+ 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 m
Luis Héctor Chávez
2016/07/11 17:13:34
Sure
|
+ |
+ DISALLOW_COPY_AND_ASSIGN(InstanceHolder<T>); |
}; |
~ArcBridgeService() override; |
@@ -185,72 +225,38 @@ class ArcBridgeService : public mojom::ArcBridgeHost { |
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(); |
- } |
- mojom::ClipboardInstance* clipboard_instance() { |
- return clipboard_ptr_.get(); |
- } |
- mojom::CrashCollectorInstance* crash_collector_instance() { |
- return crash_collector_ptr_.get(); |
- } |
- mojom::FileSystemInstance* file_system_instance() { |
- return file_system_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::ImeInstance* ime_instance() { return ime_ptr_.get(); } |
- mojom::IntentHelperInstance* intent_helper_instance() { |
- return intent_helper_ptr_.get(); |
+ InstanceHolder<mojom::FileSystemInstance>* file_system() { |
+ return &file_system_; |
} |
- 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::ImeInstance>* ime() { return &ime_; } |
+ InstanceHolder<mojom::IntentHelperInstance>* intent_helper() { |
+ return &intent_helper_; |
} |
- mojom::ObbMounterInstance* obb_mounter_instance() { |
- return obb_mounter_ptr_.get(); |
+ InstanceHolder<mojom::MetricsInstance>* metrics() { return &metrics_; } |
+ InstanceHolder<mojom::NetInstance>* net() { return &net_; } |
+ InstanceHolder<mojom::NotificationsInstance>* notifications() { |
+ return ¬ifications_; |
} |
- 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(); |
+ InstanceHolder<mojom::ObbMounterInstance>* obb_mounter() { |
+ return &obb_mounter_; |
} |
- 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(); |
+ 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_; |
} |
- 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: |
@@ -312,94 +318,26 @@ class ArcBridgeService : public mojom::ArcBridgeHost { |
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_; |