bluetooth: Refactor UnifiedBluetoothDetailedViewController

Changes UnifiedBluetoothDetailedViewController to update the device list
every 500ms while a scan is active.

Also adds a new observer method specifically for when Bluetooth State
changes and changes UnifiedBluetoothDetailedViewController to use this
method to update the list of devices as well.

Bug: 882346
Change-Id: I13ef2555e6c4c451213afb5d405f707d5f4f19a7
Reviewed-on: https://chromium-review.googlesource.com/c/1314069Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605951}

ash/system/bluetooth/bluetooth_feature_pod_controller.cc

@@ -131,11 +131,15 @@ void BluetoothFeaturePodController::SetTooltipState(
       IDS_ASH_STATUS_TRAY_BLUETOOTH_SETTINGS_TOOLTIP, tooltip_state));
 }
 
-void BluetoothFeaturePodController::OnBluetoothRefresh() {
+void BluetoothFeaturePodController::OnBluetoothSystemStateChanged() {
   UpdateButton();
 }
 
-void BluetoothFeaturePodController::OnBluetoothDiscoveringChanged() {
+void BluetoothFeaturePodController::OnBluetoothScanStateChanged() {
+  UpdateButton();
+}
+
+void BluetoothFeaturePodController::OnBluetoothDeviceListChanged() {
   UpdateButton();
 }
 

ash/system/bluetooth/bluetooth_feature_pod_controller.h

@@ -32,8 +32,9 @@ class BluetoothFeaturePodController : public FeaturePodControllerBase,
   void SetTooltipState(const base::string16& tooltip_state);
 
   // BluetoothObserver:
-  void OnBluetoothRefresh() override;
-  void OnBluetoothDiscoveringChanged() override;
+  void OnBluetoothSystemStateChanged() override;
+  void OnBluetoothScanStateChanged() override;
+  void OnBluetoothDeviceListChanged() override;
 
   // Unowned.
   UnifiedSystemTrayController* const tray_controller_;

ash/system/bluetooth/bluetooth_observer.h

@@ -11,8 +11,14 @@ class BluetoothObserver {
  public:
   virtual ~BluetoothObserver() {}
 
-  virtual void OnBluetoothRefresh() = 0;
-  virtual void OnBluetoothDiscoveringChanged() = 0;
+  // Called when the state of Bluetooth in the system changes.
+  virtual void OnBluetoothSystemStateChanged() {}
+
+  // Called when a Bluetooth scan has started or stopped.
+  virtual void OnBluetoothScanStateChanged() {}
+
+  // Called when a device was added, removed, or changed.
+  virtual void OnBluetoothDeviceListChanged() {}
 };
 
 }  // namespace ash

ash/system/bluetooth/tray_bluetooth_helper_experimental.cc

@@ -84,13 +84,13 @@ bool TrayBluetoothHelperExperimental::HasBluetoothDiscoverySession() {
 void TrayBluetoothHelperExperimental::OnStateChanged(
     device::mojom::BluetoothSystem::State state) {
   cached_state_ = state;
-  Shell::Get()->system_tray_notifier()->NotifyRefreshBluetooth();
+  Shell::Get()->system_tray_notifier()->NotifyBluetoothSystemStateChanged();
 }
 
 void TrayBluetoothHelperExperimental::OnScanStateChanged(
     device::mojom::BluetoothSystem::ScanState state) {
   cached_scan_state_ = state;
-  Shell::Get()->system_tray_notifier()->NotifyBluetoothDiscoveringChanged();
+  Shell::Get()->system_tray_notifier()->NotifyBluetoothScanStateChanged();
 }
 
 }  // namespace ash

ash/system/bluetooth/tray_bluetooth_helper_legacy.cc

@@ -165,34 +165,34 @@ bool TrayBluetoothHelperLegacy::HasBluetoothDiscoverySession() {
 void TrayBluetoothHelperLegacy::AdapterPresentChanged(
     device::BluetoothAdapter* adapter,
     bool present) {
-  GetSystemTrayNotifier()->NotifyRefreshBluetooth();
+  GetSystemTrayNotifier()->NotifyBluetoothSystemStateChanged();
 }
 
 void TrayBluetoothHelperLegacy::AdapterPoweredChanged(
     device::BluetoothAdapter* adapter,
     bool powered) {
-  GetSystemTrayNotifier()->NotifyRefreshBluetooth();
+  GetSystemTrayNotifier()->NotifyBluetoothSystemStateChanged();
 }
 
 void TrayBluetoothHelperLegacy::AdapterDiscoveringChanged(
     device::BluetoothAdapter* adapter,
     bool discovering) {
-  GetSystemTrayNotifier()->NotifyBluetoothDiscoveringChanged();
+  GetSystemTrayNotifier()->NotifyBluetoothScanStateChanged();
 }
 
 void TrayBluetoothHelperLegacy::DeviceAdded(device::BluetoothAdapter* adapter,
                                             device::BluetoothDevice* device) {
-  GetSystemTrayNotifier()->NotifyRefreshBluetooth();
+  GetSystemTrayNotifier()->NotifyBluetoothDeviceListChanged();
 }
 
 void TrayBluetoothHelperLegacy::DeviceChanged(device::BluetoothAdapter* adapter,
                                               device::BluetoothDevice* device) {
-  GetSystemTrayNotifier()->NotifyRefreshBluetooth();
+  GetSystemTrayNotifier()->NotifyBluetoothDeviceListChanged();
 }
 
 void TrayBluetoothHelperLegacy::DeviceRemoved(device::BluetoothAdapter* adapter,
                                               device::BluetoothDevice* device) {
-  GetSystemTrayNotifier()->NotifyRefreshBluetooth();
+  GetSystemTrayNotifier()->NotifyBluetoothDeviceListChanged();
 }
 
 void TrayBluetoothHelperLegacy::OnStartDiscoverySession(
@@ -204,7 +204,7 @@ void TrayBluetoothHelperLegacy::OnStartDiscoverySession(
     return;
   VLOG(1) << "Claiming new Bluetooth device discovery session.";
   discovery_session_ = std::move(discovery_session);
-  GetSystemTrayNotifier()->NotifyBluetoothDiscoveringChanged();
+  GetSystemTrayNotifier()->NotifyBluetoothScanStateChanged();
 }
 
 }  // namespace ash

ash/system/bluetooth/unified_bluetooth_detailed_view_controller.cc

@@ -4,6 +4,9 @@
 
 #include "ash/system/bluetooth/unified_bluetooth_detailed_view_controller.h"
 
+#include <set>
+#include <string>
+
 #include "ash/session/session_controller.h"
 #include "ash/shell.h"
 #include "ash/system/bluetooth/bluetooth_detailed_view.h"
@@ -63,7 +66,6 @@ UnifiedBluetoothDetailedViewController::
   TrayBluetoothHelper* helper = Shell::Get()->tray_bluetooth_helper();
   if (helper && helper->HasBluetoothDiscoverySession()) {
     helper->StopBluetoothDiscovering();
-    view_->HideLoadingIndicator();
   }
 }
 
@@ -72,59 +74,69 @@ views::View* UnifiedBluetoothDetailedViewController::CreateView() {
   view_ = new tray::BluetoothDetailedView(
       detailed_view_delegate_.get(),
       Shell::Get()->session_controller()->login_status());
-  Update();
+  OnBluetoothSystemStateChanged();
   return view_;
 }
 
-void UnifiedBluetoothDetailedViewController::OnBluetoothRefresh() {
-  Update();
-}
-
-void UnifiedBluetoothDetailedViewController::OnBluetoothDiscoveringChanged() {
-  Update();
-}
+void UnifiedBluetoothDetailedViewController::OnBluetoothSystemStateChanged() {
+  auto* helper = Shell::Get()->tray_bluetooth_helper();
+  const BluetoothSystem::State bluetooth_state = helper->GetBluetoothState();
 
-void UnifiedBluetoothDetailedViewController::Update() {
-  // Update immediately for initial device list and
-  // when bluetooth is disabled.
-  if (view_->IsDeviceScrollListEmpty() ||
-      Shell::Get()->tray_bluetooth_helper()->GetBluetoothState() !=
-          BluetoothSystem::State::kPoweredOn) {
+  if (bluetooth_state == BluetoothSystem::State::kPoweredOn) {
+    // If Bluetooth was just turned on, start discovering.
+    Shell::Get()->tray_bluetooth_helper()->StartBluetoothDiscovering();
+  } else {
+    // Otherwise stop updating the list of devices.
     timer_.Stop();
-    DoUpdate();
-    return;
   }
 
-  // Return here since an update is already queued.
-  if (timer_.IsRunning())
-    return;
+  UpdateDeviceListAndUI();
+}
+
+void UnifiedBluetoothDetailedViewController::OnBluetoothScanStateChanged() {
+  // To avoid delaying showing devices, update the device list and UI
+  // immediately.
+  UpdateDeviceListAndUI();
 
-  // Update the detailed view after kUpdateFrequencyMs.
-  timer_.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(kUpdateFrequencyMs),
-               this, &UnifiedBluetoothDetailedViewController::DoUpdate);
+  if (Shell::Get()->tray_bluetooth_helper()->HasBluetoothDiscoverySession()) {
+    // Update the device list and UI every |kUpdateFrequencyMs|.
+    timer_.Start(
+        FROM_HERE, base::TimeDelta::FromMilliseconds(kUpdateFrequencyMs), this,
+        &UnifiedBluetoothDetailedViewController::UpdateDeviceListAndUI);
+    return;
+  }
+  timer_.Stop();
 }
 
-void UnifiedBluetoothDetailedViewController::DoUpdate() {
-  BluetoothStartDiscovering();
+void UnifiedBluetoothDetailedViewController::UpdateDeviceListAndUI() {
   UpdateBluetoothDeviceList();
 
-  // Update UI
-  view_->SetToggleIsOn(
-      Shell::Get()->tray_bluetooth_helper()->GetBluetoothState() ==
-      BluetoothSystem::State::kPoweredOn);
-  UpdateDeviceScrollList();
-}
+  auto* helper = Shell::Get()->tray_bluetooth_helper();
+  bool bluetooth_on =
+      helper->GetBluetoothState() == BluetoothSystem::State::kPoweredOn;
 
-void UnifiedBluetoothDetailedViewController::BluetoothStartDiscovering() {
-  TrayBluetoothHelper* helper = Shell::Get()->tray_bluetooth_helper();
-  if (helper->HasBluetoothDiscoverySession()) {
+  // Update toggle.
+  view_->SetToggleIsOn(bluetooth_on);
+
+  // Update loading indicator.
+  if (helper->HasBluetoothDiscoverySession())
     view_->ShowLoadingIndicator();
+  else
+    view_->HideLoadingIndicator();
+
+  // Update scroll list or show "BT disabled" panel
+  if (bluetooth_on) {
+    view_->HideBluetoothDisabledPanel();
+    view_->UpdateDeviceScrollList(connected_devices_, connecting_devices_,
+                                  paired_not_connected_devices_,
+                                  discovered_not_paired_devices_);
+
     return;
   }
 
-  view_->HideLoadingIndicator();
-  if (helper->GetBluetoothState() == BluetoothSystem::State::kPoweredOn)
-    helper->StartBluetoothDiscovering();
+  // If Bluetooth is disabled, show a panel which only indicates that it is
+  // disabled, instead of the scroller with Bluetooth devices.
+  view_->ShowBluetoothDisabledPanel();
 }
 
 void UnifiedBluetoothDetailedViewController::UpdateBluetoothDeviceList() {
@@ -160,30 +172,4 @@ void UnifiedBluetoothDetailedViewController::UpdateBluetoothDeviceList() {
                                          new_discovered_not_paired_devices);
 }
 
-void UnifiedBluetoothDetailedViewController::UpdateDeviceScrollList() {
-  const BluetoothSystem::State bluetooth_state =
-      Shell::Get()->tray_bluetooth_helper()->GetBluetoothState();
-
-  switch (bluetooth_state) {
-    case BluetoothSystem::State::kUnsupported:
-      // Bluetooth is always supported on Chrome OS.
-      NOTREACHED();
-      return;
-    case BluetoothSystem::State::kUnavailable:
-    case BluetoothSystem::State::kPoweredOff:
-    case BluetoothSystem::State::kTransitioning:
-      // If Bluetooth is disabled, show a panel which only indicates that it is
-      // disabled, instead of the scroller with Bluetooth devices.
-      view_->ShowBluetoothDisabledPanel();
-      return;
-    case BluetoothSystem::State::kPoweredOn:
-      break;
-  }
-
-  view_->HideBluetoothDisabledPanel();
-  view_->UpdateDeviceScrollList(connected_devices_, connecting_devices_,
-                                paired_not_connected_devices_,
-                                discovered_not_paired_devices_);
-}
-
 }  // namespace ash

ash/system/bluetooth/unified_bluetooth_detailed_view_controller.h

@@ -34,15 +34,12 @@ class UnifiedBluetoothDetailedViewController : public DetailedViewController,
   views::View* CreateView() override;
 
   // BluetoothObserver:
-  void OnBluetoothRefresh() override;
-  void OnBluetoothDiscoveringChanged() override;
+  void OnBluetoothSystemStateChanged() override;
+  void OnBluetoothScanStateChanged() override;
 
  private:
-  void Update();
-  void DoUpdate();
-  void BluetoothStartDiscovering();
+  void UpdateDeviceListAndUI();
   void UpdateBluetoothDeviceList();
-  void UpdateDeviceScrollList();
 
   const std::unique_ptr<DetailedViewDelegate> detailed_view_delegate_;
 
@@ -54,7 +51,7 @@ class UnifiedBluetoothDetailedViewController : public DetailedViewController,
   BluetoothDeviceList discovered_not_paired_devices_;
 
   // Timer used to limit the update frequency.
-  base::OneShotTimer timer_;
+  base::RepeatingTimer timer_;
 
   DISALLOW_COPY_AND_ASSIGN(UnifiedBluetoothDetailedViewController);
 };

ash/system/tray/system_tray_notifier.cc

@@ -26,14 +26,19 @@ void SystemTrayNotifier::RemoveBluetoothObserver(BluetoothObserver* observer) {
   bluetooth_observers_.RemoveObserver(observer);
 }
 
-void SystemTrayNotifier::NotifyRefreshBluetooth() {
+void SystemTrayNotifier::NotifyBluetoothSystemStateChanged() {
   for (auto& observer : bluetooth_observers_)
-    observer.OnBluetoothRefresh();
+    observer.OnBluetoothSystemStateChanged();
 }
 
-void SystemTrayNotifier::NotifyBluetoothDiscoveringChanged() {
+void SystemTrayNotifier::NotifyBluetoothScanStateChanged() {
   for (auto& observer : bluetooth_observers_)
-    observer.OnBluetoothDiscoveringChanged();
+    observer.OnBluetoothScanStateChanged();
+}
+
+void SystemTrayNotifier::NotifyBluetoothDeviceListChanged() {
+  for (auto& observer : bluetooth_observers_)
+    observer.OnBluetoothDeviceListChanged();
 }
 
 void SystemTrayNotifier::AddIMEObserver(IMEObserver* observer) {

ash/system/tray/system_tray_notifier.h

@@ -37,8 +37,9 @@ class ASH_EXPORT SystemTrayNotifier {
   // Bluetooth.
   void AddBluetoothObserver(BluetoothObserver* observer);
   void RemoveBluetoothObserver(BluetoothObserver* observer);
-  void NotifyRefreshBluetooth();
-  void NotifyBluetoothDiscoveringChanged();
+  void NotifyBluetoothSystemStateChanged();
+  void NotifyBluetoothScanStateChanged();
+  void NotifyBluetoothDeviceListChanged();
 
   // Input methods.
   void AddIMEObserver(IMEObserver* observer);