mash: Refactor Bluetooth methods out of SystemTrayDelegate / SystemTrayDelegateChromeos |
||||||||||
Issue descriptionThere are a bunch of Bluetooth methods in the ash SystemTrayDelegate interface. Eventually ash should speak directly to a mojo service:bluetooth. However, in the meantime those methods aren't really specific to the system tray and should probably live in their own delegate/interface. https://cs.chromium.org/chromium/src/ash/common/system/tray/system_tray_delegate.h?q=systemtraydelegate&sq=package:chromium&dr=CSs&l=168 // Returns a list of available bluetooth devices. virtual void GetAvailableBluetoothDevices(BluetoothDeviceList* devices); // Requests bluetooth start discovering devices. virtual void BluetoothStartDiscovering(); // Requests bluetooth stop discovering devices. virtual void BluetoothStopDiscovering(); // Connect to a specific bluetooth device. virtual void ConnectToBluetoothDevice(const std::string& address); // Returns true if bluetooth adapter is discovering bluetooth devices. virtual bool IsBluetoothDiscovering(); // Shows UI to manage bluetooth devices. virtual void ManageBluetoothDevices(); // Toggles bluetooth. virtual void ToggleBluetooth(); // Returns whether bluetooth capability is available. virtual bool GetBluetoothAvailable(); // Returns whether bluetooth is enabled. virtual bool GetBluetoothEnabled(); // Returns whether the delegate has initiated a bluetooth discovery session. virtual bool GetBluetoothDiscovering();
,
Dec 6 2016
steel, do you think this is worth doing? If so, should I do it now? My primary motivation is to make the SystemTrayDelegate interface go away completely, such that new code is all written against my new mojo-ified SystemTrayClient. It would also isolate the bluetooth pieces better in chrome (SystemTrayDelegateChromeOS is huge and unfocused. OTOH, if code in chrome is about to be converted to call into a mojo-ified bluetooth service then this could get ripped out and replaced by ash using mojo directly.
,
Dec 6 2016
The Bluetooth is going to be accessible via a system service, that is certain. At that time, ash code will not need to change at all - the current calls to //device/bluetooth would just work - over mojo. The timelines are less certain. Currently I am the only engineer working on it and I'm doing it part time. I expect the work to take over a quarter, optimistically. I am not sure if the WebBluetooth folks will be able to devote any cycles to it, but if they are, it may go quicker. If there isn't any real need for Bluetooth at the moment, I would hold off on this and work on other parts. If we want this for dogfood anytime before end of Q1 next year, then we can always write this interface - with the understanding that this would probably be throwaway code.
,
Mar 3 2017
,
Mar 3 2017
,
Mar 15 2017
rkc/scheib - what's the status on a mojo-ified interface to bluetooth? I see //device/bluetooth/public/interfaces with lots of mojo stuff, but it also seems like code in chrome browser routes directly down into a bluez wrapper. (For example, like SystemTrayDelegateChromeOS calling BluetoothAdapter::StartDiscoverySession()). Regardless it seems like the system tray functionality for bluetooth doesn't map exactly onto //device/bluetooth, so we're going to need some sort of lightweight glue between the UI widgets and the bluetooth API, so extracting these methods out from SystemTrayDelegate seems like a good start. Ideally the glue will move out of the chrome browser, into the ash process, and switch to using mojo.
,
Mar 15 2017
The Chrome OS BT focus is moving to a new BT stack at the Chrome OS layer at the moment (won't require any code change in Chrome). I don't expect any Bluetooth servicification work to start before end of Q2.
,
Mar 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7822c80225dbb6c6fe2a887c842294a74fec2962 commit 7822c80225dbb6c6fe2a887c842294a74fec2962 Author: jamescook <jamescook@chromium.org> Date: Fri Mar 17 05:09:44 2017 cros: Extract TrayBluetoothHelper from SystemTrayDelegateChromeOS This is first step toward moving system tray bluetooth glue code out of chrome browser into ash, which is needed for the eventual mojo interface conversion for mash. * Create TrayBluetoothHelper as part of SystemTrayDelegateChromeOS. * Forward method calls, don't change any functionality. * Add a browser test to make sure I'm forwarding calls correctly. * Add a unit test for TrayBluetoothHelper. * Clarify the name of HasDiscoverySession(), which is a weird API. TODO: * Eliminiate the two-stage initialization of SystemTrayDelegateChromeOS. I didn't want to mess with initialization timing in this CL. * Move TrayBluetoothHelper down into ash near TrayBluetooth. BUG= 660043 TEST=automated, added to chrome browser_tests and unit_tests Review-Url: https://codereview.chromium.org/2755643005 Cr-Commit-Position: refs/heads/master@{#457690} [modify] https://crrev.com/7822c80225dbb6c6fe2a887c842294a74fec2962/ash/common/system/tray/system_tray_delegate.h [modify] https://crrev.com/7822c80225dbb6c6fe2a887c842294a74fec2962/chrome/browser/ui/BUILD.gn [modify] https://crrev.com/7822c80225dbb6c6fe2a887c842294a74fec2962/chrome/browser/ui/ash/system_tray_delegate_chromeos.cc [modify] https://crrev.com/7822c80225dbb6c6fe2a887c842294a74fec2962/chrome/browser/ui/ash/system_tray_delegate_chromeos.h [modify] https://crrev.com/7822c80225dbb6c6fe2a887c842294a74fec2962/chrome/browser/ui/ash/system_tray_delegate_chromeos_browsertest_chromeos.cc [add] https://crrev.com/7822c80225dbb6c6fe2a887c842294a74fec2962/chrome/browser/ui/ash/tray_bluetooth_helper.cc [add] https://crrev.com/7822c80225dbb6c6fe2a887c842294a74fec2962/chrome/browser/ui/ash/tray_bluetooth_helper.h [add] https://crrev.com/7822c80225dbb6c6fe2a887c842294a74fec2962/chrome/browser/ui/ash/tray_bluetooth_helper_unittest.cc [modify] https://crrev.com/7822c80225dbb6c6fe2a887c842294a74fec2962/chrome/test/BUILD.gn
,
Mar 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/817c808f00fbc1d292d01d8fcfbf423e36c6c28d commit 817c808f00fbc1d292d01d8fcfbf423e36c6c28d Author: jamescook <jamescook@chromium.org> Date: Fri Mar 17 22:37:06 2017 cros: Init SystemTrayDelegate earlier to break bluetooth init dependency This is needed to allow TrayBluetoothHelper to move into //ash, which is required to get Bluetooth working in mutash. SystemTrayDelegateChromeOS used to have a two-step initialization process with some code being initialized after the Bluetooth adapter becomes available. I think that init doesn't need to wait on Bluetooth init. BUG= 660043 TEST=browser_tests, device starts normally Review-Url: https://codereview.chromium.org/2753313002 Cr-Commit-Position: refs/heads/master@{#457894} [modify] https://crrev.com/817c808f00fbc1d292d01d8fcfbf423e36c6c28d/ash/common/system/tray/system_tray_delegate.h [modify] https://crrev.com/817c808f00fbc1d292d01d8fcfbf423e36c6c28d/chrome/browser/ui/ash/system_tray_delegate_chromeos.cc [modify] https://crrev.com/817c808f00fbc1d292d01d8fcfbf423e36c6c28d/chrome/browser/ui/ash/system_tray_delegate_chromeos.h [modify] https://crrev.com/817c808f00fbc1d292d01d8fcfbf423e36c6c28d/chrome/browser/ui/ash/tray_bluetooth_helper.cc [modify] https://crrev.com/817c808f00fbc1d292d01d8fcfbf423e36c6c28d/chrome/browser/ui/ash/tray_bluetooth_helper.h [modify] https://crrev.com/817c808f00fbc1d292d01d8fcfbf423e36c6c28d/chrome/browser/ui/ash/tray_bluetooth_helper_unittest.cc
,
Mar 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/67435be443d0a9f45d85a0c5a013c7723114315e commit 67435be443d0a9f45d85a0c5a013c7723114315e Author: jamescook <jamescook@chromium.org> Date: Mon Mar 20 17:16:33 2017 cros: Convert SystemTrayDelegate::ManageBluetoothDevices to mojo for mash This is needed to eliminate the in-process ash-to-chrome calls via SystemTrayDelegate and to support Bluetooth directly from ash. * Convert ManageBluetoothDevices to SystemTrayClient::ShowBluetoothSettings * Introduce ShowBluetoothPairingDialog() which will be needed shortly The next step is to move TrayBluetoothHelper out of //chrome into //ash. BUG= 660043 TEST=Existing UI tests. The settings "gear" in the system tray bluetooth section still opens settings. Review-Url: https://codereview.chromium.org/2754143006 Cr-Commit-Position: refs/heads/master@{#458106} [modify] https://crrev.com/67435be443d0a9f45d85a0c5a013c7723114315e/ash/common/system/chromeos/bluetooth/tray_bluetooth.cc [modify] https://crrev.com/67435be443d0a9f45d85a0c5a013c7723114315e/ash/common/system/tray/system_tray_controller.cc [modify] https://crrev.com/67435be443d0a9f45d85a0c5a013c7723114315e/ash/common/system/tray/system_tray_controller.h [modify] https://crrev.com/67435be443d0a9f45d85a0c5a013c7723114315e/ash/common/system/tray/system_tray_delegate.cc [modify] https://crrev.com/67435be443d0a9f45d85a0c5a013c7723114315e/ash/common/system/tray/system_tray_delegate.h [modify] https://crrev.com/67435be443d0a9f45d85a0c5a013c7723114315e/ash/public/interfaces/system_tray.mojom [modify] https://crrev.com/67435be443d0a9f45d85a0c5a013c7723114315e/chrome/browser/chromeos/bluetooth/bluetooth_pairing_dialog.cc [modify] https://crrev.com/67435be443d0a9f45d85a0c5a013c7723114315e/chrome/browser/chromeos/bluetooth/bluetooth_pairing_dialog.h [modify] https://crrev.com/67435be443d0a9f45d85a0c5a013c7723114315e/chrome/browser/ui/ash/system_tray_client.cc [modify] https://crrev.com/67435be443d0a9f45d85a0c5a013c7723114315e/chrome/browser/ui/ash/system_tray_client.h [modify] https://crrev.com/67435be443d0a9f45d85a0c5a013c7723114315e/chrome/browser/ui/ash/system_tray_delegate_chromeos.cc [modify] https://crrev.com/67435be443d0a9f45d85a0c5a013c7723114315e/chrome/browser/ui/ash/system_tray_delegate_chromeos.h [modify] https://crrev.com/67435be443d0a9f45d85a0c5a013c7723114315e/chrome/browser/ui/ash/tray_bluetooth_helper.cc [modify] https://crrev.com/67435be443d0a9f45d85a0c5a013c7723114315e/chrome/browser/ui/webui/chromeos/bluetooth_pairing_ui_browsertest-inl.h
,
Mar 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ac672433b25e1fcf341b2d8a42bbc1ee520f5928 commit ac672433b25e1fcf341b2d8a42bbc1ee520f5928 Author: jamescook <jamescook@chromium.org> Date: Tue Mar 21 02:54:43 2017 cros: Move TrayBluetoothHelper out of chrome into ash This is another step toward moving bluetooth support into //ash, which is needed to make it work with mash. * Mechanically move the file and test * Change ownership from SystemTrayDelegateChromeOS to ash::Shell * Make ShowBluetoothPairingDialog use mojo to talk to chrome TODO: Change TrayBluetooth to use the helper directly and remove all the Bluetooth methods from SystemTrayDelegate BUG= 660043 TEST=ash_unittests, chrome unit_tests Review-Url: https://codereview.chromium.org/2764643003 Cr-Commit-Position: refs/heads/master@{#458287} [modify] https://crrev.com/ac672433b25e1fcf341b2d8a42bbc1ee520f5928/ash/BUILD.gn [rename] https://crrev.com/ac672433b25e1fcf341b2d8a42bbc1ee520f5928/ash/common/system/chromeos/bluetooth/tray_bluetooth_helper.cc [rename] https://crrev.com/ac672433b25e1fcf341b2d8a42bbc1ee520f5928/ash/common/system/chromeos/bluetooth/tray_bluetooth_helper.h [rename] https://crrev.com/ac672433b25e1fcf341b2d8a42bbc1ee520f5928/ash/common/system/chromeos/bluetooth/tray_bluetooth_helper_unittest.cc [modify] https://crrev.com/ac672433b25e1fcf341b2d8a42bbc1ee520f5928/ash/common/system/tray/system_tray_controller.cc [modify] https://crrev.com/ac672433b25e1fcf341b2d8a42bbc1ee520f5928/ash/common/system/tray/system_tray_controller.h [modify] https://crrev.com/ac672433b25e1fcf341b2d8a42bbc1ee520f5928/ash/shell.cc [modify] https://crrev.com/ac672433b25e1fcf341b2d8a42bbc1ee520f5928/ash/shell.h [modify] https://crrev.com/ac672433b25e1fcf341b2d8a42bbc1ee520f5928/chrome/browser/ui/BUILD.gn [modify] https://crrev.com/ac672433b25e1fcf341b2d8a42bbc1ee520f5928/chrome/browser/ui/ash/system_tray_delegate_chromeos.cc [modify] https://crrev.com/ac672433b25e1fcf341b2d8a42bbc1ee520f5928/chrome/browser/ui/ash/system_tray_delegate_chromeos.h [modify] https://crrev.com/ac672433b25e1fcf341b2d8a42bbc1ee520f5928/chrome/test/BUILD.gn
,
Mar 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b84d051d3c76ccb8e0e3b07d3c5e064f6069824c commit b84d051d3c76ccb8e0e3b07d3c5e064f6069824c Author: jamescook <jamescook@chromium.org> Date: Tue Mar 21 05:45:17 2017 cros: Eliminate bluetooth methods from SystemTrayDelegate This is another step toward making bluetooth work in mash. * Convert TrayBluetooth to use TrayBluetoothHelper directly * Rename helper methods to have "bluetooth" in them for clarity * Eliminate obsolete (and disabled) SystemTrayDelegateChromeOS unit test BUG= 660043 TEST=ash_unittests, chrome browser_tests Review-Url: https://codereview.chromium.org/2761993002 Cr-Commit-Position: refs/heads/master@{#458322} [modify] https://crrev.com/b84d051d3c76ccb8e0e3b07d3c5e064f6069824c/ash/common/system/chromeos/bluetooth/tray_bluetooth.cc [modify] https://crrev.com/b84d051d3c76ccb8e0e3b07d3c5e064f6069824c/ash/common/system/chromeos/bluetooth/tray_bluetooth_helper.cc [modify] https://crrev.com/b84d051d3c76ccb8e0e3b07d3c5e064f6069824c/ash/common/system/chromeos/bluetooth/tray_bluetooth_helper.h [modify] https://crrev.com/b84d051d3c76ccb8e0e3b07d3c5e064f6069824c/ash/common/system/chromeos/bluetooth/tray_bluetooth_helper_unittest.cc [modify] https://crrev.com/b84d051d3c76ccb8e0e3b07d3c5e064f6069824c/ash/common/system/tray/default_system_tray_delegate.cc [modify] https://crrev.com/b84d051d3c76ccb8e0e3b07d3c5e064f6069824c/ash/common/system/tray/default_system_tray_delegate.h [modify] https://crrev.com/b84d051d3c76ccb8e0e3b07d3c5e064f6069824c/ash/common/system/tray/system_tray_delegate.cc [modify] https://crrev.com/b84d051d3c76ccb8e0e3b07d3c5e064f6069824c/ash/common/system/tray/system_tray_delegate.h [modify] https://crrev.com/b84d051d3c76ccb8e0e3b07d3c5e064f6069824c/chrome/browser/ui/ash/system_tray_delegate_chromeos.cc [modify] https://crrev.com/b84d051d3c76ccb8e0e3b07d3c5e064f6069824c/chrome/browser/ui/ash/system_tray_delegate_chromeos.h [modify] https://crrev.com/b84d051d3c76ccb8e0e3b07d3c5e064f6069824c/chrome/browser/ui/ash/system_tray_delegate_chromeos_browsertest_chromeos.cc [delete] https://crrev.com/734e63436853247d8119b85d49c53053c957c34d/chrome/browser/ui/ash/system_tray_delegate_chromeos_unittest.cc [modify] https://crrev.com/b84d051d3c76ccb8e0e3b07d3c5e064f6069824c/chrome/browser/ui/webui/chromeos/emulator/device_emulator_message_handler.cc [modify] https://crrev.com/b84d051d3c76ccb8e0e3b07d3c5e064f6069824c/chrome/test/BUILD.gn
,
Mar 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ca8c398302a2c20ec014439d7d74ca1eeb6c9e5a commit ca8c398302a2c20ec014439d7d74ca1eeb6c9e5a Author: jamescook <jamescook@chromium.org> Date: Wed Mar 22 13:59:41 2017 cros: Clean up TrayBluetoothHelper todos * Make GetAvailableBluetoothDevices() directly return the list * Add comments BUG= 660043 TEST=ash_unittests Review-Url: https://codereview.chromium.org/2769553002 Cr-Commit-Position: refs/heads/master@{#458738} [modify] https://crrev.com/ca8c398302a2c20ec014439d7d74ca1eeb6c9e5a/ash/common/system/chromeos/bluetooth/tray_bluetooth.cc [modify] https://crrev.com/ca8c398302a2c20ec014439d7d74ca1eeb6c9e5a/ash/common/system/chromeos/bluetooth/tray_bluetooth.h [modify] https://crrev.com/ca8c398302a2c20ec014439d7d74ca1eeb6c9e5a/ash/common/system/chromeos/bluetooth/tray_bluetooth_helper.cc [modify] https://crrev.com/ca8c398302a2c20ec014439d7d74ca1eeb6c9e5a/ash/common/system/chromeos/bluetooth/tray_bluetooth_helper.h [modify] https://crrev.com/ca8c398302a2c20ec014439d7d74ca1eeb6c9e5a/ash/common/system/chromeos/bluetooth/tray_bluetooth_helper_unittest.cc
,
Mar 22 2017
This is done. Further Bluetooth work will require a mojo service for device/bluetooth.
,
May 30 2017
,
Aug 1 2017
,
Aug 2 2017
As per #14
,
Feb 26 2018
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by mcchou@chromium.org
, Oct 27 2016