arc/bluetooth: Fix the powered state synchronization between Chrome and Android

Since the observation of the local powered state change and the receiver of
enable/disable requests from Android is in arc_bluetooth_service are in
arc_setting_service and arc_bluetooth_bridge separately, the power state can
get out of sync or restore to the wrong state due to the lack of coordination
between these two pieces of information.

Issues:
- The powered state can get out-of-sync between Android and Chrome if the power
  state of adapter is repeatedly toggled from Chrome setting.
- If the powered state is ON before deep suspend/resume, the state will
  turn to OFF after resume and turn immediately back to ON on Chrome. However
  without waiting for the previous disable intent to finish, arc_setting_service
  sends the enable intent right after while Android Bluetooth statck is still
  processing the disable intent. Also there is a 3~4 seconds delay for bringing
  down to complete on Android Bluetooth stack. Besides, the enable intent
  is treated as a no op if the ongoing intent is disable. So after 3~4 seconds
  Android sends a disable request to Chrome while the powered state is ON on
  Chrome, so the powered state fall to the wrong one, disable, instead.

The changes includes:
 - moving the observation of Bluetooth adapter from arc_settings_service
   to arc_bluetooth_bridge,
 - changing the cycle of observing Bluetooth adapter from living along with ARC
   Bluetooth instance to living along with ARC Bluetooth bridge,
 - adding IntentHelperObserver to listen to the OnInstanceReady event and fire
   the initial powered state of Bluetooth to Android via intent,
 - adding two queues and a timer to track the powered state changes initiated by
   either Chrome or Android and to track the completion of the power
   changes initiated by Chrome,
 - and adding helper functions to enqueue/dequeue the power change intents and
   to compress the toggling of state to reduce the intents sent to Android.

BUG=b:62578573
TEST=(1) On Android start-up, verify that the powered state is synchronized
     between Android and Chrome.
     (2) After Repeatedly toggling Bluetooth powered state, the state
     remains synchronized between Android and Chrome.
     (3) Put device into deep suspend and resume, the state restores to
     the previous state.

Change-Id: I753839e2bb9228703492ebd23703cf8b67494998
Reviewed-on: https://chromium-review.googlesource.com/580314
Commit-Queue: Miao-chen Chou <mcchou@chromium.org>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Reviewed-by: Rahul Chaturvedi <rkc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491556}

chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc

@@ -34,8 +34,6 @@
 #include "components/proxy_config/pref_proxy_config_tracker_impl.h"
 #include "components/proxy_config/proxy_config_dictionary.h"
 #include "components/proxy_config/proxy_config_pref_names.h"
-#include "device/bluetooth/bluetooth_adapter.h"
-#include "device/bluetooth/bluetooth_adapter_factory.h"
 #include "net/proxy/proxy_config.h"
 
 using ::chromeos::CrosSettings;
@@ -99,7 +97,6 @@ class ArcSettingsServiceFactory
 // about and sends the new values to Android to keep the state in sync.
 class ArcSettingsServiceImpl
     : public chromeos::system::TimezoneSettings::Observer,
-      public device::BluetoothAdapter::Observer,
       public ArcSessionManager::Observer,
       public chromeos::NetworkStateHandlerObserver {
  public:
@@ -114,10 +111,6 @@ class ArcSettingsServiceImpl
   // TimezoneSettings::Observer:
   void TimezoneChanged(const icu::TimeZone& timezone) override;
 
-  // BluetoothAdapter::Observer:
-  void AdapterPoweredChanged(device::BluetoothAdapter* adapter,
-                             bool powered) override;
-
   // ArcSessionManager::Observer:
   void OnArcInitialStart() override;
 
@@ -164,9 +157,6 @@ class ArcSettingsServiceImpl
   void SyncTimeZoneByGeolocation() const;
   void SyncUse24HourClock() const;
 
-  void OnBluetoothAdapterInitialized(
-      scoped_refptr<device::BluetoothAdapter> adapter);
-
   // Registers to listen to a particular perf.
   void AddPrefToObserve(const std::string& pref_name);
 
@@ -198,9 +188,6 @@ class ArcSettingsServiceImpl
   std::unique_ptr<chromeos::CrosSettings::ObserverSubscription>
       reporting_consent_subscription_;
 
-  scoped_refptr<device::BluetoothAdapter> bluetooth_adapter_;
-
-  // WeakPtrFactory to use for callback for getting the bluetooth adapter.
   base::WeakPtrFactory<ArcSettingsServiceImpl> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ArcSettingsServiceImpl);
@@ -224,9 +211,6 @@ ArcSettingsServiceImpl::~ArcSettingsServiceImpl() {
   ArcSessionManager* arc_session_manager = ArcSessionManager::Get();
   if (arc_session_manager)
     arc_session_manager->RemoveObserver(this);
-
-  if (bluetooth_adapter_)
-    bluetooth_adapter_->RemoveObserver(this);
 }
 
 void ArcSettingsServiceImpl::OnPrefChanged(const std::string& pref_name) const {
@@ -274,15 +258,6 @@ void ArcSettingsServiceImpl::TimezoneChanged(const icu::TimeZone& timezone) {
   SyncTimeZone();
 }
 
-void ArcSettingsServiceImpl::AdapterPoweredChanged(
-    device::BluetoothAdapter* adapter,
-    bool powered) {
-  base::DictionaryValue extras;
-  extras.SetBoolean("enable", powered);
-  SendSettingsBroadcast("org.chromium.arc.intent_helper.SET_BLUETOOTH_STATE",
-                        extras);
-}
-
 void ArcSettingsServiceImpl::OnArcInitialStart() {
   SyncInitialSettings();
 }
@@ -328,12 +303,6 @@ void ArcSettingsServiceImpl::StartObservingSettingsChanges() {
 
   TimezoneSettings::GetInstance()->AddObserver(this);
 
-  if (device::BluetoothAdapterFactory::IsBluetoothSupported()) {
-    device::BluetoothAdapterFactory::GetAdapter(
-        base::Bind(&ArcSettingsServiceImpl::OnBluetoothAdapterInitialized,
-                   weak_factory_.GetWeakPtr()));
-  }
-
   chromeos::NetworkHandler::Get()->network_state_handler()->AddObserver(
       this, FROM_HERE);
 }
@@ -569,15 +538,6 @@ void ArcSettingsServiceImpl::SyncUse24HourClock() const {
                         extras);
 }
 
-void ArcSettingsServiceImpl::OnBluetoothAdapterInitialized(
-    scoped_refptr<device::BluetoothAdapter> adapter) {
-  DCHECK(adapter);
-  bluetooth_adapter_ = adapter;
-  bluetooth_adapter_->AddObserver(this);
-
-  AdapterPoweredChanged(adapter.get(), adapter->IsPowered());
-}
-
 void ArcSettingsServiceImpl::AddPrefToObserve(const std::string& pref_name) {
   registrar_.Add(pref_name, base::Bind(&ArcSettingsServiceImpl::OnPrefChanged,
                                        base::Unretained(this)));

components/arc/bluetooth/arc_bluetooth_bridge.h

@@ -9,6 +9,7 @@
 
 #include <map>
 #include <memory>
+#include <queue>
 #include <string>
 #include <unordered_map>
 #include <unordered_set>
@@ -17,6 +18,7 @@
 #include "base/callback.h"
 #include "base/timer/timer.h"
 #include "components/arc/common/bluetooth.mojom.h"
+#include "components/arc/common/intent_helper.mojom.h"
 #include "components/arc/instance_holder.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "device/bluetooth/bluetooth_adapter.h"
@@ -62,7 +64,10 @@ class ArcBluetoothBridge
 
   void OnAdapterInitialized(scoped_refptr<device::BluetoothAdapter> adapter);
 
-  // Overridden from device::BluetoothAdadpter::Observer
+  // Overridden from device::BluetoothAdapter::Observer
+  void AdapterPoweredChanged(device::BluetoothAdapter* adapter,
+                             bool powered) override;
+
   void DeviceAdded(device::BluetoothAdapter* adapter,
                    device::BluetoothDevice* device) override;
 
@@ -341,6 +346,49 @@ class ArcBluetoothBridge
       std::unique_ptr<device::BluetoothGattNotifySession> notify_session);
 
  private:
+  // IntentHelperObserver listens to the OnInstanceReady call on the intent
+  // helper which indicated the IntentHelperService has been brought up and the
+  // initial powered state of Bluetooth adapter can be sent to Android.
+  class IntentHelperObserver
+      : public InstanceHolder<mojom::IntentHelperInstance>::Observer {
+   public:
+    explicit IntentHelperObserver(ArcBluetoothBridge* bluetooth_bridge);
+    ~IntentHelperObserver() override;
+
+   private:
+    // InstanceHolder<mojom::IntentHelperInstance>::Observer overrides
+    void OnInstanceReady() override;
+
+    // ArcBluetoothBridge owns IntentHelperObserver, and ArcBluetoothBridge will
+    // always outlive it.
+    ArcBluetoothBridge* const bluetooth_bridge_;
+
+    DISALLOW_COPY_AND_ASSIGN(IntentHelperObserver);
+  };
+
+  // Power state change on Bluetooth adapter.
+  enum class AdapterPowerState { TURN_OFF, TURN_ON };
+
+  bool IsInstanceUp() const { return is_bluetooth_instance_up_; }
+
+  // Indicates if a power change is initiated by Chrome / Android.
+  bool IsPowerChangeInitiatedByRemote(
+      ArcBluetoothBridge::AdapterPowerState powered) const;
+  bool IsPowerChangeInitiatedByLocal(
+      ArcBluetoothBridge::AdapterPowerState powered) const;
+
+  // Called by IntentHelperObserver to send the initial power state.
+  void SendInitialPowerChange();
+
+  // Manages the powered change intents sent to Android.
+  void EnqueueLocalPowerChange(AdapterPowerState powered);
+  void DequeueLocalPowerChange(AdapterPowerState powered);
+
+  // Manages the powered change requests received from Android.
+  void EnqueueRemotePowerChange(AdapterPowerState powered,
+                                const EnableAdapterCallback& callback);
+  void DequeueRemotePowerChange(AdapterPowerState powered);
+
   std::vector<mojom::BluetoothPropertyPtr> GetDeviceProperties(
       mojom::BluetoothPropertyType type,
       const device::BluetoothDevice* device) const;
@@ -362,6 +410,9 @@ class ArcBluetoothBridge
       mojom::BluetoothGattIDPtr char_id,
       mojom::BluetoothGattIDPtr desc_id) const;
 
+  // Send the power status change to Android via an intent.
+  void SendBluetoothPoweredStateBroadcast(AdapterPowerState powered) const;
+
   // Propagates the list of paired device to Android.
   void SendCachedPairedDevices() const;
 
@@ -471,6 +522,19 @@ class ArcBluetoothBridge
   // Timer to turn adapter discoverable off.
   base::OneShotTimer discoverable_off_timer_;
 
+  // This indicates whether the remote Bluetooth ARC instance is ready to
+  // receive events.
+  bool is_bluetooth_instance_up_;
+
+  // Observer to listen the start-up of Intent Helper.
+  IntentHelperObserver intent_helper_observer_;
+  // Queue to track the powered state changes initiated by Android.
+  std::queue<AdapterPowerState> remote_power_changes_;
+  // Queue to track the powered state changes initiated by Chrome.
+  std::queue<AdapterPowerState> local_power_changes_;
+  // Timer to track the completion of power-changed intent sent to Android.
+  base::OneShotTimer power_intent_timer_;
+
   // Holds advertising data registered by the instance.
   //
   // When a handle is reserved, an entry is placed into the advertisements_