Skip to content

Commit

Permalink
[Floss] Delay calling powered callback until HCI enabled
Browse files Browse the repository at this point in the history
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 <laikatherine@chromium.org>
Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@google.com>
Cr-Commit-Position: refs/heads/main@{#1010161}
  • Loading branch information
Katherine Lai authored and Chromium LUCI CQ committed Jun 2, 2022
1 parent 2f833ad commit f37dfb6
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 2 deletions.
54 changes: 52 additions & 2 deletions device/bluetooth/floss/floss_manager_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -63,6 +64,35 @@ void HandleExported(const std::string& method_name,

} // namespace

FlossManagerClient::PoweredCallback::PoweredCallback(ResponseCallback<Void> cb,
int timeout_ms) {
cb_ = std::move(cb);
timeout_ms_ = timeout_ms;
}

FlossManagerClient::PoweredCallback::~PoweredCallback() = default;

// static
std::unique_ptr<FlossManagerClient::PoweredCallback>
FlossManagerClient::PoweredCallback::CreateWithTimeout(
ResponseCallback<Void> cb,
int timeout_ms) {
std::unique_ptr<FlossManagerClient::PoweredCallback> self =
std::make_unique<FlossManagerClient::PoweredCallback>(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";
Expand Down Expand Up @@ -140,6 +170,10 @@ void FlossManagerClient::SetFlossEnabled(bool enabled) {
void FlossManagerClient::SetAdapterEnabled(int adapter,
bool enabled,
ResponseCallback<Void> callback) {
if (adapter != GetDefaultAdapter()) {
return;
}

dbus::ObjectProxy* object_proxy =
bus_->GetObjectProxy(service_name_, dbus::ObjectPath(kManagerObject));
if (!object_proxy) {
Expand All @@ -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<Void>,
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.
Expand Down Expand Up @@ -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_) {
Expand Down
34 changes: 34 additions & 0 deletions device/bluetooth/floss/floss_manager_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,33 @@ class DEVICE_BLUETOOTH_EXPORT FlossManagerClient
virtual void AdapterEnabledChanged(int adapter, bool enabled) {}
};

class PoweredCallback {
public:
explicit PoweredCallback(ResponseCallback<Void> cb, int timeout_ms);
~PoweredCallback();

static std::unique_ptr<FlossManagerClient::PoweredCallback>
CreateWithTimeout(ResponseCallback<Void> 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<Void> cb_;
int timeout_ms_;
base::WeakPtrFactory<PoweredCallback> weak_ptr_factory_{this};
};

// Convert adapter number to object path.
static dbus::ObjectPath GenerateAdapterPath(int adapter);

Expand Down Expand Up @@ -155,12 +182,19 @@ class DEVICE_BLUETOOTH_EXPORT FlossManagerClient
base::ObserverList<Observer> 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<PoweredCallback> powered_callback_;

base::WeakPtrFactory<FlossManagerClient> weak_ptr_factory_{this};
};

Expand Down

0 comments on commit f37dfb6

Please sign in to comment.