bluetooth: Create an alternate set of Bluetooth DBus clients for Bluetooth System |
|||||
Issue descriptionBluetoothSystem[1] and BluetoothAdapter[2] can’t share the same DBus clients since they could interfere with each other. For example, if BluetoothSystem calls SetDiscoveryFilters after BluetoothAdapter, it will override BluetoothAdapter’s filters. To avoid this, BluetoothSystem needs to use a separate set of DBus Clients. BlueZ differentiates between clients based on their DBus address, so in order to have two separate clients, we need two different addresses. We will need to create a separate DBus connection in order to get a new address. DBusThreadManagerLinux[3] creates a connection to the system bus on Linux. We will rename this class to "AlternateDBusThreadManager" and use it to create a new connection to BlueZ on Chrome OS. AlternateDBusThreadManager will be initialized and shutdown together with BluezDBusManager[4]. We will have two connections until we completely deprecate BluetoothAdapter at which point we’ll change BluetoothSystem to use the original Bus. [1] https://cs.chromium.org/chromium/src/services/device/public/mojom/bluetooth_system.mojom [2] https://cs.chromium.org/chromium/src/device/bluetooth/bluez/bluetooth_adapter_bluez.h [3] https://cs.chromium.org/chromium/src/device/bluetooth/dbus/dbus_thread_manager_linux.h [4] https://cs.chromium.org/chromium/src/device/bluetooth/dbus/bluez_dbus_manager.h
,
Sep 11
,
Sep 11
,
Sep 11
I'm not sure why you need two sets of clients. Is it caused by Bluez's implementation, or Chrome's D-Bus code? AlternateDBusThreadManager sounds a bit cryptic, could you come up with a more descriptive name? Why do you need to rename DBusThreadManagerLinux to use it on Chrome OS?
,
Sep 11
BlueZ's implementation. BlueZ has some functions that change the state for example SetDiscoveryFilters. If the same client is shared between BluetoothSystem and BluetoothAdapter, then calling SetDiscoveryFilters from BluetoothSystem will override the filters set by BluetoothAdapter. If we have separate clients, then BlueZ will merge the filters of the two clients. From BlueZ docs[1]: When multiple clients call SetDiscoveryFilter, their filters are internally merged... When SetDiscoveryFilter is called multiple times by the same client, last filter passed will be active for given client. [1] https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/adapter-api.txt#n128 re AlternateDBusThreadManager. What about BlueZDBusThreadManager? It's just that DBusThreadManagerLinux sounds very Linux specific.
,
Sep 11
I'd recommend just keeping it named DBusThreadManagerLinux and adding a comment documenting why it's being instantiated. Keeping this isn't the long-term plan. If it feels odd, that's because it *is* an odd temporary setup. :-P
,
Sep 11
DBusThreadManagerLinux seems like a confusing name for something that lives in device/bluetooth/dbus/ (at least, it confused me for a bit until I noticed that). Maybe rename it DBusThreadManagerBluez since that appears to be all that it's used for, or even just integrate it with BluezDBusManager? (If we were planning to use it elsewhere it should probably live in /dbus, renamed to DBusThreadManager, and chromeos::DBusThreadManager should become DBusThreadManagerChromeOS and derive from it, but I'm not suggesting we do that).
,
Sep 11
Sure, makes sense. I'm supportive of Bluez being added to the name for clarity.
,
Sep 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/506d933cd9c0ce348b97449615acbb575718ada1 commit 506d933cd9c0ce348b97449615acbb575718ada1 Author: Giovanni Ortuño Urquidi <ortuno@chromium.org> Date: Mon Sep 24 22:56:38 2018 bluetooth: Always use DBusThreadManagerLinux or DBusThreadManager We always end up using DBusThreadManagerLinux on Linux and DBusThreadManager on Chrome OS. Rather than having clients pass these when initializing BluezDBusThreadManager, have BluezDBusThreadManager choose which one to use based on the OS. This puts the logic for choosing which one to use in a single place. Bug: 882771 Change-Id: I9c36eb9de8419fd5ac4bc6fbab3ded2738b33d91 Reviewed-on: https://chromium-review.googlesource.com/1237694 Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org> Cr-Commit-Position: refs/heads/master@{#593730} [modify] https://crrev.com/506d933cd9c0ce348b97449615acbb575718ada1/ash/ash_service.cc [modify] https://crrev.com/506d933cd9c0ce348b97449615acbb575718ada1/ash/shell/content/client/shell_browser_main_parts.cc [modify] https://crrev.com/506d933cd9c0ce348b97449615acbb575718ada1/ash/test/ash_test_helper.cc [modify] https://crrev.com/506d933cd9c0ce348b97449615acbb575718ada1/chrome/browser/chrome_browser_main_linux.cc [modify] https://crrev.com/506d933cd9c0ce348b97449615acbb575718ada1/chrome/browser/chromeos/chrome_browser_main_chromeos.cc [modify] https://crrev.com/506d933cd9c0ce348b97449615acbb575718ada1/chrome/test/base/view_event_test_platform_part_chromeos.cc [modify] https://crrev.com/506d933cd9c0ce348b97449615acbb575718ada1/content/shell/browser/shell_browser_main_parts.cc [modify] https://crrev.com/506d933cd9c0ce348b97449615acbb575718ada1/device/bluetooth/bluez/bluetooth_advertisement_bluez_unittest.cc [modify] https://crrev.com/506d933cd9c0ce348b97449615acbb575718ada1/device/bluetooth/dbus/bluez_dbus_manager.cc [modify] https://crrev.com/506d933cd9c0ce348b97449615acbb575718ada1/device/bluetooth/dbus/bluez_dbus_manager.h [modify] https://crrev.com/506d933cd9c0ce348b97449615acbb575718ada1/device/bluetooth/dbus/dbus_bluez_manager_wrapper_linux.cc [modify] https://crrev.com/506d933cd9c0ce348b97449615acbb575718ada1/extensions/shell/browser/shell_browser_main_parts.cc
,
Sep 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3e60c04292e92cc617937515f0c9a9df826bcc0f commit 3e60c04292e92cc617937515f0c9a9df826bcc0f Author: Giovanni Ortuño Urquidi <ortuno@chromium.org> Date: Tue Sep 25 00:08:28 2018 bluetooth: Rename DBusThreadManagerLinux to BluezDBusThreadManager DBusThreadManagerLinux will be used on Chrome OS to create an alternate set of DBus clients. Renames the file in preparation of this. TBR=avi@chromium.org, michaelpg@chromium.org Bug: 882771 Change-Id: I953fb4bfcc0b3faf0b68fba5f0c18b248f6072f6 Reviewed-on: https://chromium-review.googlesource.com/1238097 Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org> Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org> Cr-Commit-Position: refs/heads/master@{#593762} [modify] https://crrev.com/3e60c04292e92cc617937515f0c9a9df826bcc0f/chrome/browser/chrome_browser_main_linux.cc [modify] https://crrev.com/3e60c04292e92cc617937515f0c9a9df826bcc0f/device/bluetooth/BUILD.gn [modify] https://crrev.com/3e60c04292e92cc617937515f0c9a9df826bcc0f/device/bluetooth/dbus/bluez_dbus_manager.cc [add] https://crrev.com/3e60c04292e92cc617937515f0c9a9df826bcc0f/device/bluetooth/dbus/bluez_dbus_thread_manager.cc [add] https://crrev.com/3e60c04292e92cc617937515f0c9a9df826bcc0f/device/bluetooth/dbus/bluez_dbus_thread_manager.h [modify] https://crrev.com/3e60c04292e92cc617937515f0c9a9df826bcc0f/device/bluetooth/dbus/dbus_bluez_manager_wrapper_linux.cc [modify] https://crrev.com/3e60c04292e92cc617937515f0c9a9df826bcc0f/device/bluetooth/dbus/dbus_bluez_manager_wrapper_linux.h [delete] https://crrev.com/93cf6f918c86a39381c4712120323867e1c92eb9/device/bluetooth/dbus/dbus_thread_manager_linux.cc [delete] https://crrev.com/93cf6f918c86a39381c4712120323867e1c92eb9/device/bluetooth/dbus/dbus_thread_manager_linux.h [modify] https://crrev.com/3e60c04292e92cc617937515f0c9a9df826bcc0f/extensions/shell/browser/shell_browser_main_parts.cc
,
Oct 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/837b3c2074fca77f66fda4c08d7c65a2b5b762ef commit 837b3c2074fca77f66fda4c08d7c65a2b5b762ef Author: Giovanni Ortuño Urquidi <ortuno@chromium.org> Date: Thu Oct 04 06:01:19 2018 bluetooth: Add a new BluetoothAdapterClient instance and use it for GetState There are two parts to this change: 1. Create a new BluetoothAdapterClient instance. For this we add "alternate_bluetooth_adapter_client()" to the bundle of DBus clients. This new instance uses a separate DBus Connection through BluezDBusThreadManager. This ensures actions on one client won't affect the other client. 2. Implement BluetoothSystem::GetState using the new client. Bug: 870192, 882771 Change-Id: I9faa92e8234b14dd374a04b4c9e9acbcfd7e6201 Reviewed-on: https://chromium-review.googlesource.com/c/1215427 Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Dan Erat <derat@chromium.org> Reviewed-by: Sam McNally <sammc@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/master@{#596520} [modify] https://crrev.com/837b3c2074fca77f66fda4c08d7c65a2b5b762ef/device/bluetooth/dbus/bluetooth_dbus_client_bundle.cc [modify] https://crrev.com/837b3c2074fca77f66fda4c08d7c65a2b5b762ef/device/bluetooth/dbus/bluetooth_dbus_client_bundle.h [modify] https://crrev.com/837b3c2074fca77f66fda4c08d7c65a2b5b762ef/device/bluetooth/dbus/bluez_dbus_manager.cc [modify] https://crrev.com/837b3c2074fca77f66fda4c08d7c65a2b5b762ef/device/bluetooth/dbus/bluez_dbus_manager.h [modify] https://crrev.com/837b3c2074fca77f66fda4c08d7c65a2b5b762ef/services/device/bluetooth/BUILD.gn [add] https://crrev.com/837b3c2074fca77f66fda4c08d7c65a2b5b762ef/services/device/bluetooth/DEPS [modify] https://crrev.com/837b3c2074fca77f66fda4c08d7c65a2b5b762ef/services/device/bluetooth/bluetooth_system.cc [modify] https://crrev.com/837b3c2074fca77f66fda4c08d7c65a2b5b762ef/services/device/bluetooth/bluetooth_system.h [modify] https://crrev.com/837b3c2074fca77f66fda4c08d7c65a2b5b762ef/services/device/bluetooth/bluetooth_system_unittest.cc [modify] https://crrev.com/837b3c2074fca77f66fda4c08d7c65a2b5b762ef/services/device/public/mojom/bluetooth_system.mojom
,
Oct 4
,
Oct 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/467e19a62cd2826f933b18930e71d56d97ca7cd3 commit 467e19a62cd2826f933b18930e71d56d97ca7cd3 Author: kylechar <kylechar@chromium.org> Date: Thu Oct 04 15:44:41 2018 Revert "bluetooth: Add a new BluetoothAdapterClient instance and use it for GetState" This reverts commit 837b3c2074fca77f66fda4c08d7c65a2b5b762ef. Reason for revert: Crash on startup. https://crbug.com/892182 Original change's description: > bluetooth: Add a new BluetoothAdapterClient instance and use it for GetState > > There are two parts to this change: > > 1. Create a new BluetoothAdapterClient instance. For this we add > "alternate_bluetooth_adapter_client()" to the bundle of DBus clients. > This new instance uses a separate DBus Connection through > BluezDBusThreadManager. This ensures actions on one client won't > affect the other client. > > 2. Implement BluetoothSystem::GetState using the new client. > > Bug: 870192, 882771 > Change-Id: I9faa92e8234b14dd374a04b4c9e9acbcfd7e6201 > Reviewed-on: https://chromium-review.googlesource.com/c/1215427 > Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org> > Reviewed-by: Steven Bennetts <stevenjb@chromium.org> > Reviewed-by: Dan Erat <derat@chromium.org> > Reviewed-by: Sam McNally <sammc@chromium.org> > Reviewed-by: Reilly Grant <reillyg@chromium.org> > Cr-Commit-Position: refs/heads/master@{#596520} TBR=derat@chromium.org,stevenjb@chromium.org,reillyg@chromium.org,sammc@chromium.org,ortuno@chromium.org,sonnysasaka@chromium.org Change-Id: Icf5a244f163152ccc41d84f7ea43220afebae791 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 870192, 882771 , 892182 Reviewed-on: https://chromium-review.googlesource.com/c/1261878 Reviewed-by: kylechar <kylechar@chromium.org> Commit-Queue: kylechar <kylechar@chromium.org> Cr-Commit-Position: refs/heads/master@{#596685} [modify] https://crrev.com/467e19a62cd2826f933b18930e71d56d97ca7cd3/device/bluetooth/dbus/bluetooth_dbus_client_bundle.cc [modify] https://crrev.com/467e19a62cd2826f933b18930e71d56d97ca7cd3/device/bluetooth/dbus/bluetooth_dbus_client_bundle.h [modify] https://crrev.com/467e19a62cd2826f933b18930e71d56d97ca7cd3/device/bluetooth/dbus/bluez_dbus_manager.cc [modify] https://crrev.com/467e19a62cd2826f933b18930e71d56d97ca7cd3/device/bluetooth/dbus/bluez_dbus_manager.h [modify] https://crrev.com/467e19a62cd2826f933b18930e71d56d97ca7cd3/services/device/bluetooth/BUILD.gn [delete] https://crrev.com/40ceb08df809a852a986f97c4e5d23c475fdb060/services/device/bluetooth/DEPS [modify] https://crrev.com/467e19a62cd2826f933b18930e71d56d97ca7cd3/services/device/bluetooth/bluetooth_system.cc [modify] https://crrev.com/467e19a62cd2826f933b18930e71d56d97ca7cd3/services/device/bluetooth/bluetooth_system.h [modify] https://crrev.com/467e19a62cd2826f933b18930e71d56d97ca7cd3/services/device/bluetooth/bluetooth_system_unittest.cc [modify] https://crrev.com/467e19a62cd2826f933b18930e71d56d97ca7cd3/services/device/public/mojom/bluetooth_system.mojom
,
Oct 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8aa00e1b59237b66917fc8be06324c97958d3b88 commit 8aa00e1b59237b66917fc8be06324c97958d3b88 Author: Giovanni Ortuño Urquidi <ortuno@chromium.org> Date: Fri Oct 05 03:09:15 2018 Reland "bluetooth: Add a new BluetoothAdapterClient instance and use it for GetState" This is a reland of 837b3c2074fca77f66fda4c08d7c65a2b5b762ef The original patch attempted to init the alternate dbus client on Linux where there is no alternate bus connection. The latest patchset fixes this by returning early if there is no alternate bus connection. TBRing since it's a minor addition to the original patch. Original change's description: > bluetooth: Add a new BluetoothAdapterClient instance and use it for GetState > > There are two parts to this change: > > 1. Create a new BluetoothAdapterClient instance. For this we add > "alternate_bluetooth_adapter_client()" to the bundle of DBus clients. > This new instance uses a separate DBus Connection through > BluezDBusThreadManager. This ensures actions on one client won't > affect the other client. > > 2. Implement BluetoothSystem::GetState using the new client. > > Bug: 870192, 882771 > Change-Id: I9faa92e8234b14dd374a04b4c9e9acbcfd7e6201 > Reviewed-on: https://chromium-review.googlesource.com/c/1215427 > Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org> > Reviewed-by: Steven Bennetts <stevenjb@chromium.org> > Reviewed-by: Dan Erat <derat@chromium.org> > Reviewed-by: Sam McNally <sammc@chromium.org> > Reviewed-by: Reilly Grant <reillyg@chromium.org> > Cr-Commit-Position: refs/heads/master@{#596520} TBR=stevenjb@chromium.org, derat@chromium.org, sammc@chromium.org, reillyg@chromium.org Bug: 870192, 882771 Change-Id: I0e92cc90f095c6049b2b967e46249e8488f71fcb Reviewed-on: https://chromium-review.googlesource.com/c/1263217 Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org> Cr-Commit-Position: refs/heads/master@{#596975} [modify] https://crrev.com/8aa00e1b59237b66917fc8be06324c97958d3b88/device/bluetooth/dbus/bluetooth_dbus_client_bundle.cc [modify] https://crrev.com/8aa00e1b59237b66917fc8be06324c97958d3b88/device/bluetooth/dbus/bluetooth_dbus_client_bundle.h [modify] https://crrev.com/8aa00e1b59237b66917fc8be06324c97958d3b88/device/bluetooth/dbus/bluez_dbus_manager.cc [modify] https://crrev.com/8aa00e1b59237b66917fc8be06324c97958d3b88/device/bluetooth/dbus/bluez_dbus_manager.h [modify] https://crrev.com/8aa00e1b59237b66917fc8be06324c97958d3b88/services/device/bluetooth/BUILD.gn [add] https://crrev.com/8aa00e1b59237b66917fc8be06324c97958d3b88/services/device/bluetooth/DEPS [modify] https://crrev.com/8aa00e1b59237b66917fc8be06324c97958d3b88/services/device/bluetooth/bluetooth_system.cc [modify] https://crrev.com/8aa00e1b59237b66917fc8be06324c97958d3b88/services/device/bluetooth/bluetooth_system.h [modify] https://crrev.com/8aa00e1b59237b66917fc8be06324c97958d3b88/services/device/bluetooth/bluetooth_system_unittest.cc [modify] https://crrev.com/8aa00e1b59237b66917fc8be06324c97958d3b88/services/device/public/mojom/bluetooth_system.mojom
,
Oct 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9525c97843d5cfa932646b472cca18ec6494e011 commit 9525c97843d5cfa932646b472cca18ec6494e011 Author: Giovanni Ortuño Urquidi <ortuno@chromium.org> Date: Fri Oct 05 03:32:57 2018 Revert "Reland "bluetooth: Add a new BluetoothAdapterClient instance and use it for GetState"" This reverts commit 8aa00e1b59237b66917fc8be06324c97958d3b88. Reason for revert: Causes crash on shutdown: Received signal 11 SEGV_MAPERR 000000000088 #0 0x7f228f7b5a3f base::debug::StackTrace::StackTrace() #1 0x7f228f7b5521 base::debug::(anonymous namespace)::StackDumpSignalHandler() #2 0x7f22832c80c0 <unknown> #3 0x7f228a01add1 std::__1::__tree<>::find<>() #4 0x7f228a0294dc dbus::ObjectManager::UnregisterInterface() #5 0x7f228a1978b3 bluez::BluetoothAdapterClientImpl::~BluetoothAdapterClientImpl() #6 0x7f228a1a05b0 bluez::BluetoothDBusClientBundle::~BluetoothDBusClientBundle() #7 0x7f228a1c808e bluez::BluezDBusManager::Shutdown() #8 0x559074b4094e ChromeBrowserMainPartsLinux::PostDestroyThreads() Original change's description: > Reland "bluetooth: Add a new BluetoothAdapterClient instance and use it for GetState" > > This is a reland of 837b3c2074fca77f66fda4c08d7c65a2b5b762ef > > The original patch attempted to init the alternate dbus client on Linux where > there is no alternate bus connection. The latest patchset fixes this by > returning early if there is no alternate bus connection. > > TBRing since it's a minor addition to the original patch. > > Original change's description: > > bluetooth: Add a new BluetoothAdapterClient instance and use it for GetState > > > > There are two parts to this change: > > > > 1. Create a new BluetoothAdapterClient instance. For this we add > > "alternate_bluetooth_adapter_client()" to the bundle of DBus clients. > > This new instance uses a separate DBus Connection through > > BluezDBusThreadManager. This ensures actions on one client won't > > affect the other client. > > > > 2. Implement BluetoothSystem::GetState using the new client. > > > > Bug: 870192, 882771 > > Change-Id: I9faa92e8234b14dd374a04b4c9e9acbcfd7e6201 > > Reviewed-on: https://chromium-review.googlesource.com/c/1215427 > > Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org> > > Reviewed-by: Steven Bennetts <stevenjb@chromium.org> > > Reviewed-by: Dan Erat <derat@chromium.org> > > Reviewed-by: Sam McNally <sammc@chromium.org> > > Reviewed-by: Reilly Grant <reillyg@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#596520} > > TBR=stevenjb@chromium.org, derat@chromium.org, sammc@chromium.org, reillyg@chromium.org > > Bug: 870192, 882771 > Change-Id: I0e92cc90f095c6049b2b967e46249e8488f71fcb > Reviewed-on: https://chromium-review.googlesource.com/c/1263217 > Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> > Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org> > Cr-Commit-Position: refs/heads/master@{#596975} TBR=derat@chromium.org,stevenjb@chromium.org,reillyg@chromium.org,sammc@chromium.org,ortuno@chromium.org,sonnysasaka@chromium.org Change-Id: I92bd726f551e48e0b2013d9e58f76e10b90196ec No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 870192, 882771 Reviewed-on: https://chromium-review.googlesource.com/c/1263639 Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org> Cr-Commit-Position: refs/heads/master@{#596981} [modify] https://crrev.com/9525c97843d5cfa932646b472cca18ec6494e011/device/bluetooth/dbus/bluetooth_dbus_client_bundle.cc [modify] https://crrev.com/9525c97843d5cfa932646b472cca18ec6494e011/device/bluetooth/dbus/bluetooth_dbus_client_bundle.h [modify] https://crrev.com/9525c97843d5cfa932646b472cca18ec6494e011/device/bluetooth/dbus/bluez_dbus_manager.cc [modify] https://crrev.com/9525c97843d5cfa932646b472cca18ec6494e011/device/bluetooth/dbus/bluez_dbus_manager.h [modify] https://crrev.com/9525c97843d5cfa932646b472cca18ec6494e011/services/device/bluetooth/BUILD.gn [delete] https://crrev.com/0fadde63ade9d82026102e27a1b7256a98fa9455/services/device/bluetooth/DEPS [modify] https://crrev.com/9525c97843d5cfa932646b472cca18ec6494e011/services/device/bluetooth/bluetooth_system.cc [modify] https://crrev.com/9525c97843d5cfa932646b472cca18ec6494e011/services/device/bluetooth/bluetooth_system.h [modify] https://crrev.com/9525c97843d5cfa932646b472cca18ec6494e011/services/device/bluetooth/bluetooth_system_unittest.cc [modify] https://crrev.com/9525c97843d5cfa932646b472cca18ec6494e011/services/device/public/mojom/bluetooth_system.mojom
,
Oct 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/40f128ac00336e9166a9ce26727a6ddb3a172c3c commit 40f128ac00336e9166a9ce26727a6ddb3a172c3c Author: Giovanni Ortuño Urquidi <ortuno@chromium.org> Date: Fri Oct 05 05:42:44 2018 Reland "bluetooth: Add a new BluetoothAdapterClient instance and use it for GetState" This is a reland of 837b3c2074fca77f66fda4c08d7c65a2b5b762ef The original patch attempted to init the alternate dbus client on Linux where there is no alternate bus connection. The latest patchset fixes this by returning early if there is no alternate bus connection. Additionally, BluetoothAdapterClientImpl assumed it was always initialized. This caused a crash on Linux during shutdown. This latest patchset fixes this by only cleaning up if the client has been initialized. TBR'ing since it's a small change from the original CL. Original change's description: > bluetooth: Add a new BluetoothAdapterClient instance and use it for GetState > > There are two parts to this change: > > 1. Create a new BluetoothAdapterClient instance. For this we add > "alternate_bluetooth_adapter_client()" to the bundle of DBus clients. > This new instance uses a separate DBus Connection through > BluezDBusThreadManager. This ensures actions on one client won't > affect the other client. > > 2. Implement BluetoothSystem::GetState using the new client. > > Bug: 870192, 882771 > Change-Id: I9faa92e8234b14dd374a04b4c9e9acbcfd7e6201 > Reviewed-on: https://chromium-review.googlesource.com/c/1215427 > Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org> > Reviewed-by: Steven Bennetts <stevenjb@chromium.org> > Reviewed-by: Dan Erat <derat@chromium.org> > Reviewed-by: Sam McNally <sammc@chromium.org> > Reviewed-by: Reilly Grant <reillyg@chromium.org> > Cr-Commit-Position: refs/heads/master@{#596520} TBR=derat@chromium.org,stevenjb@chromium.org,reillyg@chromium.org,sammc@chromium.org Bug: 870192, 882771 Change-Id: Ie089e30d297e8afd9ceb70c0e9a4af7600e633de Reviewed-on: https://chromium-review.googlesource.com/c/1263640 Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org> Cr-Commit-Position: refs/heads/master@{#597006} [modify] https://crrev.com/40f128ac00336e9166a9ce26727a6ddb3a172c3c/device/bluetooth/dbus/bluetooth_adapter_client.cc [modify] https://crrev.com/40f128ac00336e9166a9ce26727a6ddb3a172c3c/device/bluetooth/dbus/bluetooth_dbus_client_bundle.cc [modify] https://crrev.com/40f128ac00336e9166a9ce26727a6ddb3a172c3c/device/bluetooth/dbus/bluetooth_dbus_client_bundle.h [modify] https://crrev.com/40f128ac00336e9166a9ce26727a6ddb3a172c3c/device/bluetooth/dbus/bluez_dbus_manager.cc [modify] https://crrev.com/40f128ac00336e9166a9ce26727a6ddb3a172c3c/device/bluetooth/dbus/bluez_dbus_manager.h [modify] https://crrev.com/40f128ac00336e9166a9ce26727a6ddb3a172c3c/services/device/bluetooth/BUILD.gn [add] https://crrev.com/40f128ac00336e9166a9ce26727a6ddb3a172c3c/services/device/bluetooth/DEPS [modify] https://crrev.com/40f128ac00336e9166a9ce26727a6ddb3a172c3c/services/device/bluetooth/bluetooth_system.cc [modify] https://crrev.com/40f128ac00336e9166a9ce26727a6ddb3a172c3c/services/device/bluetooth/bluetooth_system.h [modify] https://crrev.com/40f128ac00336e9166a9ce26727a6ddb3a172c3c/services/device/bluetooth/bluetooth_system_unittest.cc [modify] https://crrev.com/40f128ac00336e9166a9ce26727a6ddb3a172c3c/services/device/public/mojom/bluetooth_system.mojom |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by ortuno@chromium.org
, Sep 11