From f37dfb6cec6e2683f4da444adf34b4960f6274a2 Mon Sep 17 00:00:00 2001 From: Katherine Lai Date: Thu, 2 Jun 2022 17:19:34 +0000 Subject: [PATCH] [Floss] Delay calling powered callback until HCI enabled There have been inconsistencies in the UI where if btadapterd crashes, and we try to turn on bluetooth, the UI will show bluetooth as turned on successfully. This is because when the UI starts the bluetooth manager, there's no real indication of whether or not it succeeded, thus we should delay calling the success callback until we get HCI enabled. Bug: b/231249514, b/228905413 Change-Id: I4dd037ef23ad7b5567efb5458496d64e10abae29 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3656851 Commit-Queue: Katherine Lai Reviewed-by: Abhishek Pandit-Subedi Cr-Commit-Position: refs/heads/main@{#1010161} --- .../bluetooth/floss/floss_manager_client.cc | 54 ++++++++++++++++++- device/bluetooth/floss/floss_manager_client.h | 34 ++++++++++++ 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/device/bluetooth/floss/floss_manager_client.cc b/device/bluetooth/floss/floss_manager_client.cc index 328bce5c429dcb..91519c7fa97555 100644 --- a/device/bluetooth/floss/floss_manager_client.cc +++ b/device/bluetooth/floss/floss_manager_client.cc @@ -17,6 +17,7 @@ #include "base/logging.h" #include "base/observer_list.h" #include "base/strings/stringprintf.h" +#include "base/threading/thread_task_runner_handle.h" #include "dbus/bus.h" #include "dbus/exported_object.h" #include "dbus/message.h" @@ -63,6 +64,35 @@ void HandleExported(const std::string& method_name, } // namespace +FlossManagerClient::PoweredCallback::PoweredCallback(ResponseCallback cb, + int timeout_ms) { + cb_ = std::move(cb); + timeout_ms_ = timeout_ms; +} + +FlossManagerClient::PoweredCallback::~PoweredCallback() = default; + +// static +std::unique_ptr +FlossManagerClient::PoweredCallback::CreateWithTimeout( + ResponseCallback cb, + int timeout_ms) { + std::unique_ptr self = + std::make_unique(std::move(cb), + timeout_ms); + self->PostDelayedError(); + + return self; +} + +void FlossManagerClient::PoweredCallback::PostDelayedError() { + base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, + base::BindOnce(&PoweredCallback::RunError, + weak_ptr_factory_.GetWeakPtr()), + base::Milliseconds(timeout_ms_)); +} + // static const char FlossManagerClient::kExportedCallbacksPath[] = "/org/chromium/bluetooth/managerclient"; @@ -140,6 +170,10 @@ void FlossManagerClient::SetFlossEnabled(bool enabled) { void FlossManagerClient::SetAdapterEnabled(int adapter, bool enabled, ResponseCallback callback) { + if (adapter != GetDefaultAdapter()) { + return; + } + dbus::ObjectProxy* object_proxy = bus_->GetObjectProxy(service_name_, dbus::ObjectPath(kManagerObject)); if (!object_proxy) { @@ -155,10 +189,22 @@ void FlossManagerClient::SetAdapterEnabled(int adapter, dbus::MessageWriter writer(&method_call); writer.AppendInt32(adapter); + powered_callback_ = + PoweredCallback::CreateWithTimeout(std::move(callback), kDBusTimeoutMs); + object_proxy->CallMethodWithErrorResponse( &method_call, kDBusTimeoutMs, - base::BindOnce(&FlossManagerClient::DefaultResponseWithCallback, - weak_ptr_factory_.GetWeakPtr(), std::move(callback))); + base::BindOnce(&FlossManagerClient::OnSetAdapterEnabled, + weak_ptr_factory_.GetWeakPtr())); +} + +void FlossManagerClient::OnSetAdapterEnabled( + dbus::Response* response, + dbus::ErrorResponse* error_response) { + // Only handle error cases since non-error called in OnHciEnabledChange + if (powered_callback_ && (!response || error_response)) { + powered_callback_.release()->RunError(); + } } // Register manager client against manager. @@ -360,6 +406,10 @@ void FlossManagerClient::OnHciEnabledChange( return; } + if (adapter == GetDefaultAdapter() && powered_callback_) { + powered_callback_.release()->RunNoError(); + } + adapter_to_powered_[adapter] = enabled; for (auto& observer : observers_) { diff --git a/device/bluetooth/floss/floss_manager_client.h b/device/bluetooth/floss/floss_manager_client.h index 69a00dd5e95f35..5d695bb9052cc0 100644 --- a/device/bluetooth/floss/floss_manager_client.h +++ b/device/bluetooth/floss/floss_manager_client.h @@ -50,6 +50,33 @@ class DEVICE_BLUETOOTH_EXPORT FlossManagerClient virtual void AdapterEnabledChanged(int adapter, bool enabled) {} }; + class PoweredCallback { + public: + explicit PoweredCallback(ResponseCallback cb, int timeout_ms); + ~PoweredCallback(); + + static std::unique_ptr + CreateWithTimeout(ResponseCallback cb, int timeout_ms); + void RunError() { + if (cb_) { + std::move(cb_).Run( + /*ret=*/absl::nullopt, Error(kErrorNoResponse, std::string())); + } + }; + void RunNoError() { + if (cb_) { + std::move(cb_).Run(/*ret=*/absl::nullopt, /*err=*/absl::nullopt); + } + }; + + private: + void PostDelayedError(); + + ResponseCallback cb_; + int timeout_ms_; + base::WeakPtrFactory weak_ptr_factory_{this}; + }; + // Convert adapter number to object path. static dbus::ObjectPath GenerateAdapterPath(int adapter); @@ -155,12 +182,19 @@ class DEVICE_BLUETOOTH_EXPORT FlossManagerClient base::ObserverList observers_; private: + // Handle response to SetAdapterEnabled + void OnSetAdapterEnabled(dbus::Response* response, + dbus::ErrorResponse* error_response); + // Object path for exported callbacks registered against manager interface. static const char kExportedCallbacksPath[]; // Floss Manager registers ObjectManager at this path. static const char kObjectManagerPath[]; + // Powered callback called only when adapter actually powers on + std::unique_ptr powered_callback_; + base::WeakPtrFactory weak_ptr_factory_{this}; };