[bt][l2cap] Use unique_ptr<L2cap> instead of RefPtr<L2cap>
Replace instances of fbl::RefPtr<L2cap> with std::unique_ptr<L2cap>
(in Adapter) or L2cap*. Now that L2cap is not shared across threads,
ownership is better modeled with a unique_ptr<L2cap> owned by Adapter
and weak pointers passed to other classes.
Bug: 98304
Test: fx test //src/connectivity/bluetooth/core/bt-host
Change-Id: I0e70502b7063b3b5aa368488a4fad790842b228e
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/670689
Reviewed-by: Lucas Jenkins <lucasjenkins@google.com>
Reviewed-by: Faraaz Sareshwala <fsareshwala@google.com>
Commit-Queue: Ben Lawson <benlawson@google.com>
diff --git a/src/connectivity/bluetooth/core/bt-host/fidl/adapter_test_fixture.cc b/src/connectivity/bluetooth/core/bt-host/fidl/adapter_test_fixture.cc
index 4cd090b..17b451d 100644
--- a/src/connectivity/bluetooth/core/bt-host/fidl/adapter_test_fixture.cc
+++ b/src/connectivity/bluetooth/core/bt-host/fidl/adapter_test_fixture.cc
@@ -12,9 +12,10 @@
void AdapterTestFixture::SetUp() {
TestingBase::SetUp();
- data_plane_ = bt::l2cap::testing::FakeL2cap::Create();
+ auto l2cap = std::make_unique<bt::l2cap::testing::FakeL2cap>();
+ l2cap_ = l2cap.get();
gatt_ = std::make_unique<bt::gatt::testing::FakeLayer>();
- adapter_ = bt::gap::Adapter::Create(transport()->WeakPtr(), gatt_->AsWeakPtr(), data_plane_);
+ adapter_ = bt::gap::Adapter::Create(transport()->WeakPtr(), gatt_->AsWeakPtr(), std::move(l2cap));
FakeController::Settings settings;
settings.ApplyDualModeDefaults();
@@ -34,11 +35,11 @@
RunLoopUntilIdle();
// Cleanly shut down the stack.
+ l2cap_ = nullptr;
adapter_ = nullptr;
RunLoopUntilIdle();
gatt_ = nullptr;
- data_plane_ = nullptr;
TestingBase::TearDown();
}
diff --git a/src/connectivity/bluetooth/core/bt-host/fidl/adapter_test_fixture.h b/src/connectivity/bluetooth/core/bt-host/fidl/adapter_test_fixture.h
index c87f345..52d3fbb 100644
--- a/src/connectivity/bluetooth/core/bt-host/fidl/adapter_test_fixture.h
+++ b/src/connectivity/bluetooth/core/bt-host/fidl/adapter_test_fixture.h
@@ -30,11 +30,11 @@
fxl::WeakPtr<bt::gap::Adapter> adapter() const { return adapter_->AsWeakPtr(); }
bt::gatt::testing::FakeLayer* gatt() const { return gatt_.get(); }
std::unique_ptr<bt::gatt::testing::FakeLayer> take_gatt() { return std::move(gatt_); }
- fbl::RefPtr<bt::l2cap::testing::FakeL2cap> l2cap() const { return data_plane_; }
+ bt::l2cap::testing::FakeL2cap* l2cap() const { return l2cap_; }
private:
std::unique_ptr<bt::gap::Adapter> adapter_;
- fbl::RefPtr<bt::l2cap::testing::FakeL2cap> data_plane_;
+ bt::l2cap::testing::FakeL2cap* l2cap_;
std::unique_ptr<bt::gatt::testing::FakeLayer> gatt_;
DISALLOW_COPY_ASSIGN_AND_MOVE(AdapterTestFixture);
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/adapter.cc b/src/connectivity/bluetooth/core/bt-host/gap/adapter.cc
index 843266c..25ac37d 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/adapter.cc
+++ b/src/connectivity/bluetooth/core/bt-host/gap/adapter.cc
@@ -49,7 +49,7 @@
// instance is created. The Adapter instance will use it for all of its
// asynchronous tasks.
explicit AdapterImpl(fxl::WeakPtr<hci::Transport> hci, fxl::WeakPtr<gatt::GATT> gatt,
- std::optional<fbl::RefPtr<l2cap::L2cap>> l2cap);
+ std::unique_ptr<l2cap::L2cap> l2cap);
~AdapterImpl() override;
AdapterId identifier() const override { return identifier_; }
@@ -396,8 +396,9 @@
// devices.
PeerCache peer_cache_;
- // The data domain used by GAP to interact with L2CAP and RFCOMM layers.
- fbl::RefPtr<l2cap::L2cap> l2cap_;
+ // L2CAP layer used by GAP. This must be destroyed after the following members because they raw
+ // pointers to this member.
+ std::unique_ptr<l2cap::L2cap> l2cap_;
// The GATT profile. We use this reference to add and remove data bearers and
// for service discovery.
@@ -435,13 +436,14 @@
};
AdapterImpl::AdapterImpl(fxl::WeakPtr<hci::Transport> hci, fxl::WeakPtr<gatt::GATT> gatt,
- std::optional<fbl::RefPtr<l2cap::L2cap>> l2cap)
+ std::unique_ptr<l2cap::L2cap> l2cap)
: identifier_(Random<AdapterId>()),
dispatcher_(async_get_default_dispatcher()),
hci_(std::move(hci)),
init_state_(State::kNotInitialized),
max_lmp_feature_page_index_(0),
peer_cache_(),
+ l2cap_(std::move(l2cap)),
gatt_(gatt),
weak_ptr_factory_(this) {
ZX_DEBUG_ASSERT(hci_);
@@ -450,10 +452,6 @@
init_seq_runner_ = std::make_unique<hci::SequentialCommandRunner>(dispatcher_, hci_);
- if (l2cap.has_value()) {
- l2cap_ = *l2cap;
- }
-
auto self = weak_ptr_factory_.GetWeakPtr();
hci_->SetTransportClosedCallback([self] {
if (self) {
@@ -896,21 +894,14 @@
hci_->AttachInspect(adapter_node_);
- // Create L2cap, if we haven't been provided one for testing. Doing so here lets us guarantee that
- // AclDataChannel's lifetime is a superset of L2cap's lifetime.
+ // Create ChannelManager, if we haven't been provided one for testing. Doing so here lets us
+ // guarantee that AclDataChannel's lifetime is a superset of ChannelManager's lifetime.
if (!l2cap_) {
- // Initialize L2cap to make L2cap available for the next initialization step. The AclDataChannel
- // must be initialized before creating L2cap.
- auto l2cap =
- AdoptRef(new l2cap::ChannelManager(hci_->acl_data_channel(), /*random_channel_ids=*/true));
- if (!l2cap) {
- bt_log(ERROR, "gap", "Failed to initialize Data L2cap");
- CleanUp();
- callback(false);
- return;
- }
- l2cap->AttachInspect(adapter_node_, l2cap::L2cap::kInspectNodeName);
- l2cap_ = l2cap;
+ // Initialize ChannelManager to make it available for the next initialization step. The
+ // AclDataChannel must be initialized before creating ChannelManager.
+ l2cap_ = std::make_unique<l2cap::ChannelManager>(hci_->acl_data_channel(),
+ /*random_channel_ids=*/true);
+ l2cap_->AttachInspect(adapter_node_, l2cap::L2cap::kInspectNodeName);
}
// HCI_Set_Event_Mask
@@ -1014,7 +1005,7 @@
fit::bind_member<&AdapterImpl::OnLeAutoConnectRequest>(this));
le_connection_manager_ = std::make_unique<LowEnergyConnectionManager>(
- hci_, le_address_manager_.get(), hci_le_connector_.get(), &peer_cache_, l2cap_, gatt_,
+ hci_, le_address_manager_.get(), hci_le_connector_.get(), &peer_cache_, l2cap_.get(), gatt_,
le_discovery_manager_->GetWeakPtr(), sm::SecurityManager::Create);
le_connection_manager_->AttachInspect(adapter_node_, kInspectLowEnergyConnectionManagerNodeName);
@@ -1027,7 +1018,7 @@
DeviceAddress local_bredr_address(DeviceAddress::Type::kBREDR, state_.controller_address());
bredr_connection_manager_ = std::make_unique<BrEdrConnectionManager>(
- hci_, &peer_cache_, local_bredr_address, l2cap_,
+ hci_, &peer_cache_, local_bredr_address, l2cap_.get(),
state_.features().HasBit(0, hci_spec::LMPFeature::kInterlacedPageScan));
bredr_connection_manager_->AttachInspect(adapter_node_, kInspectBrEdrConnectionManagerNodeName);
@@ -1041,7 +1032,7 @@
bredr_discovery_manager_ = std::make_unique<BrEdrDiscoveryManager>(hci_, mode, &peer_cache_);
bredr_discovery_manager_->AttachInspect(adapter_node_, kInspectBrEdrDiscoveryManagerNodeName);
- sdp_server_ = std::make_unique<sdp::Server>(l2cap_);
+ sdp_server_ = std::make_unique<sdp::Server>(l2cap_.get());
sdp_server_->AttachInspect(adapter_node_);
bredr_ = std::make_unique<BrEdrImpl>(this);
@@ -1191,8 +1182,8 @@
std::unique_ptr<Adapter> Adapter::Create(fxl::WeakPtr<hci::Transport> hci,
fxl::WeakPtr<gatt::GATT> gatt,
- std::optional<fbl::RefPtr<l2cap::L2cap>> l2cap) {
- return std::make_unique<AdapterImpl>(hci, gatt, l2cap);
+ std::unique_ptr<l2cap::L2cap> l2cap) {
+ return std::make_unique<AdapterImpl>(hci, gatt, std::move(l2cap));
}
} // namespace bt::gap
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/adapter.h b/src/connectivity/bluetooth/core/bt-host/gap/adapter.h
index df8928d..9aa1e63 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/adapter.h
+++ b/src/connectivity/bluetooth/core/bt-host/gap/adapter.h
@@ -67,11 +67,11 @@
public:
static constexpr const char* kMetricsInspectNodeName = "metrics";
- // Optionally, a FakeL2cap may be passed for testing purposes as |l2cap|. If nullopt is
+ // Optionally, a FakeL2cap may be passed for testing purposes as |l2cap|. If nullptr is
// passed, then the Adapter will create and initialize its own L2cap.
static std::unique_ptr<Adapter> Create(fxl::WeakPtr<hci::Transport> hci,
fxl::WeakPtr<gatt::GATT> gatt,
- std::optional<fbl::RefPtr<l2cap::L2cap>> l2cap);
+ std::unique_ptr<l2cap::L2cap> l2cap = nullptr);
virtual ~Adapter() = default;
// Returns a uniquely identifier for this adapter on the current system.
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/adapter_unittest.cc b/src/connectivity/bluetooth/core/bt-host/gap/adapter_unittest.cc
index 0fae0b3..2ccb0b7 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/adapter_unittest.cc
+++ b/src/connectivity/bluetooth/core/bt-host/gap/adapter_unittest.cc
@@ -50,10 +50,9 @@
transport_closed_called_ = false;
- auto l2cap = l2cap::testing::FakeL2cap::Create();
+ auto l2cap = std::make_unique<l2cap::testing::FakeL2cap>();
gatt_ = std::make_unique<gatt::testing::FakeLayer>();
- adapter_ = Adapter::Create(transport()->WeakPtr(), gatt_->AsWeakPtr(),
- std::optional(std::move(l2cap)));
+ adapter_ = Adapter::Create(transport()->WeakPtr(), gatt_->AsWeakPtr(), std::move(l2cap));
StartTestDevice();
}
@@ -1088,7 +1087,7 @@
void SetUp() override {
TestingBase::SetUp();
- l2cap_ = l2cap::testing::FakeL2cap::Create();
+ l2cap_ = std::make_unique<l2cap::testing::FakeL2cap>();
gatt_ = std::make_unique<gatt::testing::FakeLayer>();
}
@@ -1099,7 +1098,7 @@
}
protected:
- fbl::RefPtr<l2cap::testing::FakeL2cap> l2cap_;
+ std::unique_ptr<l2cap::testing::FakeL2cap> l2cap_;
std::unique_ptr<gatt::testing::FakeLayer> gatt_;
};
@@ -1123,8 +1122,7 @@
EXPECT_EQ(set_persist_cb_count, 0);
EXPECT_EQ(set_retrieve_cb_count, 0);
- auto adapter =
- Adapter::Create(transport()->WeakPtr(), gatt_->AsWeakPtr(), std::optional(std::move(l2cap_)));
+ auto adapter = Adapter::Create(transport()->WeakPtr(), gatt_->AsWeakPtr(), std::move(l2cap_));
EXPECT_EQ(set_persist_cb_count, 1);
EXPECT_EQ(set_retrieve_cb_count, 1);
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection.cc b/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection.cc
index da10353..3b5403d 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection.cc
+++ b/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection.cc
@@ -14,11 +14,13 @@
}
-BrEdrConnection::BrEdrConnection(
- fxl::WeakPtr<Peer> peer, std::unique_ptr<hci::BrEdrConnection> link,
- fit::closure send_auth_request_cb, fit::callback<void()> disconnect_cb,
- fit::closure on_peer_disconnect_cb, fbl::RefPtr<l2cap::L2cap> l2cap,
- fxl::WeakPtr<hci::Transport> transport, std::optional<Request> request)
+BrEdrConnection::BrEdrConnection(fxl::WeakPtr<Peer> peer,
+ std::unique_ptr<hci::BrEdrConnection> link,
+ fit::closure send_auth_request_cb,
+ fit::callback<void()> disconnect_cb,
+ fit::closure on_peer_disconnect_cb, l2cap::L2cap* l2cap,
+ fxl::WeakPtr<hci::Transport> transport,
+ std::optional<Request> request)
: peer_id_(peer->identifier()),
peer_(std::move(peer)),
link_(std::move(link)),
@@ -27,7 +29,7 @@
peer_, link_.get(), request_ && request_->AwaitingOutgoing(),
std::move(send_auth_request_cb),
fit::bind_member<&BrEdrConnection::OnPairingStateStatus>(this))),
- domain_(std::move(l2cap)),
+ l2cap_(l2cap),
sco_manager_(std::make_unique<sco::ScoConnectionManager>(
peer_id_, link_->handle(), link_->peer_address(), link_->local_address(),
std::move(transport))),
@@ -81,7 +83,7 @@
}
bt_log(INFO, "gap-bredr", "opening l2cap channel on psm %#.4x", psm);
- domain_->OpenL2capChannel(link().handle(), psm, params, std::move(cb));
+ l2cap_->OpenL2capChannel(link().handle(), psm, params, std::move(cb));
}
BrEdrConnection::ScoRequestHandle BrEdrConnection::OpenScoConnection(
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection.h b/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection.h
index 933d199..3989ee1 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection.h
+++ b/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection.h
@@ -36,7 +36,7 @@
using Request = BrEdrConnectionRequest;
BrEdrConnection(fxl::WeakPtr<Peer> peer, std::unique_ptr<hci::BrEdrConnection> link,
fit::closure send_auth_request_cb, fit::callback<void()> disconnect_cb,
- fit::closure on_peer_disconnect_cb, fbl::RefPtr<l2cap::L2cap> l2cap,
+ fit::closure on_peer_disconnect_cb, l2cap::L2cap* l2cap,
fxl::WeakPtr<hci::Transport> transport, std::optional<Request> request);
~BrEdrConnection();
@@ -93,7 +93,7 @@
std::unique_ptr<hci::BrEdrConnection> link_;
std::optional<Request> request_;
std::unique_ptr<PairingState> pairing_state_;
- fbl::RefPtr<l2cap::L2cap> domain_;
+ l2cap::L2cap* l2cap_;
std::unique_ptr<sco::ScoConnectionManager> sco_manager_;
// Time this object was constructed.
zx::time create_time_;
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection_manager.cc b/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection_manager.cc
index 67cca64..66292b6 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection_manager.cc
+++ b/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection_manager.cc
@@ -135,8 +135,7 @@
BrEdrConnectionManager::BrEdrConnectionManager(fxl::WeakPtr<hci::Transport> hci,
PeerCache* peer_cache, DeviceAddress local_address,
- fbl::RefPtr<l2cap::L2cap> l2cap,
- bool use_interlaced_scan)
+ l2cap::L2cap* l2cap, bool use_interlaced_scan)
: hci_(std::move(hci)),
cache_(peer_cache),
local_address_(local_address),
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection_manager.h b/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection_manager.h
index 801210c..e6380b1 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection_manager.h
+++ b/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection_manager.h
@@ -67,7 +67,7 @@
class BrEdrConnectionManager final {
public:
BrEdrConnectionManager(fxl::WeakPtr<hci::Transport> hci, PeerCache* peer_cache,
- DeviceAddress local_address, fbl::RefPtr<l2cap::L2cap> l2cap,
+ DeviceAddress local_address, l2cap::L2cap* l2cap,
bool use_interlaced_scan);
~BrEdrConnectionManager();
@@ -329,7 +329,7 @@
const DeviceAddress local_address_;
- fbl::RefPtr<l2cap::L2cap> l2cap_;
+ l2cap::L2cap* l2cap_;
// Interregator for new connections to pass.
BrEdrInterrogator interrogator_;
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection_manager_unittest.cc b/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection_manager_unittest.cc
index 8d413c9..42a8da3 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection_manager_unittest.cc
+++ b/src/connectivity/bluetooth/core/bt-host/gap/bredr_connection_manager_unittest.cc
@@ -582,7 +582,7 @@
InitializeACLDataChannel(kBrEdrBufferInfo, kLeBufferInfo);
peer_cache_ = std::make_unique<PeerCache>();
- l2cap_ = l2cap::testing::FakeL2cap::Create();
+ l2cap_ = std::make_unique<l2cap::testing::FakeL2cap>();
// Respond to BrEdrConnectionManager controller setup with success.
EXPECT_CMD_PACKET_OUT(
@@ -591,7 +591,7 @@
&kWritePageTimeoutRsp);
connection_manager_ = std::make_unique<BrEdrConnectionManager>(
- transport()->WeakPtr(), peer_cache_.get(), kLocalDevAddr, l2cap_, true);
+ transport()->WeakPtr(), peer_cache_.get(), kLocalDevAddr, l2cap_.get(), true);
StartTestDevice();
RunLoopUntilIdle();
@@ -801,7 +801,7 @@
private:
std::unique_ptr<BrEdrConnectionManager> connection_manager_;
std::unique_ptr<PeerCache> peer_cache_;
- fbl::RefPtr<l2cap::testing::FakeL2cap> l2cap_;
+ std::unique_ptr<l2cap::testing::FakeL2cap> l2cap_;
int transaction_count_ = 0;
DISALLOW_COPY_AND_ASSIGN_ALLOW_MOVE(BrEdrConnectionManagerTest);
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection.cc b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection.cc
index 8f7090c..5932485 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection.cc
+++ b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection.cc
@@ -28,14 +28,13 @@
LowEnergyConnection::LowEnergyConnection(
fxl::WeakPtr<Peer> peer, std::unique_ptr<hci::LowEnergyConnection> link,
LowEnergyConnectionOptions connection_options, PeerDisconnectCallback peer_disconnect_cb,
- ErrorCallback error_cb, fxl::WeakPtr<LowEnergyConnectionManager> conn_mgr,
- fbl::RefPtr<l2cap::L2cap> l2cap, fxl::WeakPtr<gatt::GATT> gatt,
- fxl::WeakPtr<hci::Transport> transport)
+ ErrorCallback error_cb, fxl::WeakPtr<LowEnergyConnectionManager> conn_mgr, l2cap::L2cap* l2cap,
+ fxl::WeakPtr<gatt::GATT> gatt, fxl::WeakPtr<hci::Transport> transport)
: peer_(std::move(peer)),
link_(std::move(link)),
connection_options_(connection_options),
conn_mgr_(std::move(conn_mgr)),
- l2cap_(std::move(l2cap)),
+ l2cap_(l2cap),
gatt_(std::move(gatt)),
transport_(std::move(transport)),
peer_disconnect_callback_(std::move(peer_disconnect_cb)),
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection.h b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection.h
index 14248d8..15e21c6 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection.h
+++ b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection.h
@@ -52,9 +52,8 @@
LowEnergyConnection(fxl::WeakPtr<Peer> peer, std::unique_ptr<hci::LowEnergyConnection> link,
LowEnergyConnectionOptions connection_options,
PeerDisconnectCallback peer_disconnect_cb, ErrorCallback error_cb,
- fxl::WeakPtr<LowEnergyConnectionManager> conn_mgr,
- fbl::RefPtr<l2cap::L2cap> l2cap, fxl::WeakPtr<gatt::GATT> gatt,
- fxl::WeakPtr<hci::Transport> transport);
+ fxl::WeakPtr<LowEnergyConnectionManager> conn_mgr, l2cap::L2cap* l2cap,
+ fxl::WeakPtr<gatt::GATT> gatt, fxl::WeakPtr<hci::Transport> transport);
// Notifies request callbacks and connection refs of the disconnection.
~LowEnergyConnection() override;
@@ -268,9 +267,8 @@
InspectProperties inspect_properties_;
inspect::Node inspect_node_;
- // Reference to the data plane is used to update the L2CAP layer to
- // reflect the correct link security level.
- fbl::RefPtr<l2cap::L2cap> l2cap_;
+ // Used to update the L2CAP layer to reflect the correct link security level.
+ l2cap::L2cap* l2cap_;
// Reference to the GATT profile layer is used to initiate service discovery
// and register the link.
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager.cc b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager.cc
index e26d10e..7e7a9d8 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager.cc
+++ b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager.cc
@@ -89,7 +89,7 @@
LowEnergyConnectionManager::LowEnergyConnectionManager(
fxl::WeakPtr<hci::Transport> hci, hci::LocalAddressDelegate* addr_delegate,
- hci::LowEnergyConnector* connector, PeerCache* peer_cache, fbl::RefPtr<l2cap::L2cap> l2cap,
+ hci::LowEnergyConnector* connector, PeerCache* peer_cache, l2cap::L2cap* l2cap,
fxl::WeakPtr<gatt::GATT> gatt, fxl::WeakPtr<LowEnergyDiscoveryManager> discovery_manager,
sm::SecurityManagerFactory sm_creator)
: hci_(std::move(hci)),
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager.h b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager.h
index d826735..b349a51 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager.h
+++ b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager.h
@@ -84,7 +84,7 @@
LowEnergyConnectionManager(fxl::WeakPtr<hci::Transport> hci,
hci::LocalAddressDelegate* addr_delegate,
hci::LowEnergyConnector* connector, PeerCache* peer_cache,
- fbl::RefPtr<l2cap::L2cap> l2cap, fxl::WeakPtr<gatt::GATT> gatt,
+ l2cap::L2cap* l2cap, fxl::WeakPtr<gatt::GATT> gatt,
fxl::WeakPtr<LowEnergyDiscoveryManager> discovery_manager,
sm::SecurityManagerFactory sm_creator);
~LowEnergyConnectionManager();
@@ -280,10 +280,10 @@
// connection parameters, etc). Expected to outlive this instance.
PeerCache* peer_cache_; // weak
- // The reference to the data domain, used to interact with the L2CAP layer to
+ // The reference to L2CAP, used to interact with the L2CAP layer to
// manage LE logical links, fixed channels, and LE-specific L2CAP signaling
// events (e.g. connection parameter update).
- fbl::RefPtr<l2cap::L2cap> l2cap_;
+ l2cap::L2cap* l2cap_;
// The GATT layer reference, used to add and remove ATT data bearers and
// service discovery.
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager_unittest.cc b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager_unittest.cc
index 102ce4f..1cf37b3 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager_unittest.cc
+++ b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager_unittest.cc
@@ -94,7 +94,7 @@
test_device()->set_settings(settings);
peer_cache_ = std::make_unique<PeerCache>();
- l2cap_ = l2cap::testing::FakeL2cap::Create();
+ l2cap_ = std::make_unique<l2cap::testing::FakeL2cap>();
connector_ = std::make_unique<hci::LowEnergyConnector>(
transport()->WeakPtr(), &addr_delegate_, dispatcher(),
@@ -110,7 +110,7 @@
discovery_manager_ = std::make_unique<LowEnergyDiscoveryManager>(
transport()->WeakPtr(), scanner_.get(), peer_cache_.get());
conn_mgr_ = std::make_unique<LowEnergyConnectionManager>(
- transport()->WeakPtr(), &addr_delegate_, connector_.get(), peer_cache_.get(), l2cap_,
+ transport()->WeakPtr(), &addr_delegate_, connector_.get(), peer_cache_.get(), l2cap_.get(),
gatt_->AsWeakPtr(), discovery_manager_->GetWeakPtr(),
fit::bind_member<&TestSmFactory::CreateSm>(sm_factory_.get()));
@@ -190,7 +190,7 @@
}
}
- fbl::RefPtr<l2cap::testing::FakeL2cap> l2cap_;
+ std::unique_ptr<l2cap::testing::FakeL2cap> l2cap_;
hci::FakeLocalAddressDelegate addr_delegate_;
std::unique_ptr<PeerCache> peer_cache_;
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connector.cc b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connector.cc
index 221f350..75f5b5b 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connector.cc
+++ b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connector.cc
@@ -36,7 +36,7 @@
PeerId peer_id, LowEnergyConnectionOptions options, hci::LowEnergyConnector* connector,
zx::duration request_timeout, fxl::WeakPtr<hci::Transport> transport, PeerCache* peer_cache,
fxl::WeakPtr<LowEnergyDiscoveryManager> discovery_manager,
- fxl::WeakPtr<LowEnergyConnectionManager> conn_mgr, fbl::RefPtr<l2cap::L2cap> l2cap,
+ fxl::WeakPtr<LowEnergyConnectionManager> conn_mgr, l2cap::L2cap* l2cap,
fxl::WeakPtr<gatt::GATT> gatt, ResultCallback cb) {
return std::unique_ptr<LowEnergyConnector>(new LowEnergyConnector(
/*outbound=*/true, peer_id, /*connection=*/nullptr, options, connector, request_timeout,
@@ -46,8 +46,8 @@
std::unique_ptr<LowEnergyConnector> LowEnergyConnector::CreateInboundConnector(
PeerId peer_id, std::unique_ptr<hci::LowEnergyConnection> connection,
LowEnergyConnectionOptions options, fxl::WeakPtr<hci::Transport> transport,
- PeerCache* peer_cache, fxl::WeakPtr<LowEnergyConnectionManager> conn_mgr,
- fbl::RefPtr<l2cap::L2cap> l2cap, fxl::WeakPtr<gatt::GATT> gatt, ResultCallback cb) {
+ PeerCache* peer_cache, fxl::WeakPtr<LowEnergyConnectionManager> conn_mgr, l2cap::L2cap* l2cap,
+ fxl::WeakPtr<gatt::GATT> gatt, ResultCallback cb) {
return std::unique_ptr<LowEnergyConnector>(new LowEnergyConnector(
/*outbound=*/false, peer_id, std::move(connection), options, /*connector=*/nullptr,
/*request_timeout=*/zx::duration(0), transport, peer_cache, conn_mgr,
@@ -59,7 +59,7 @@
LowEnergyConnectionOptions options, hci::LowEnergyConnector* connector,
zx::duration request_timeout, fxl::WeakPtr<hci::Transport> transport, PeerCache* peer_cache,
fxl::WeakPtr<LowEnergyConnectionManager> conn_mgr,
- fxl::WeakPtr<LowEnergyDiscoveryManager> discovery_manager, fbl::RefPtr<l2cap::L2cap> l2cap,
+ fxl::WeakPtr<LowEnergyDiscoveryManager> discovery_manager, l2cap::L2cap* l2cap,
fxl::WeakPtr<gatt::GATT> gatt, ResultCallback cb)
: state_(State::kIdle, /*convert=*/[](auto s) { return StateToString(s); }),
peer_id_(peer_id),
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connector.h b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connector.h
index 2914322..4b8d7d7 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connector.h
+++ b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connector.h
@@ -25,7 +25,7 @@
PeerId peer_id, LowEnergyConnectionOptions options, hci::LowEnergyConnector* connector,
zx::duration request_timeout, fxl::WeakPtr<hci::Transport> transport, PeerCache* peer_cache,
fxl::WeakPtr<LowEnergyDiscoveryManager> discovery_manager,
- fxl::WeakPtr<LowEnergyConnectionManager> conn_mgr, fbl::RefPtr<l2cap::L2cap> l2cap,
+ fxl::WeakPtr<LowEnergyConnectionManager> conn_mgr, l2cap::L2cap* l2cap,
fxl::WeakPtr<gatt::GATT> gatt, ResultCallback cb);
// Start interrogating peer using an already established |connection|. |cb| will be called with
@@ -33,8 +33,8 @@
static std::unique_ptr<LowEnergyConnector> CreateInboundConnector(
PeerId peer_id, std::unique_ptr<hci::LowEnergyConnection> connection,
LowEnergyConnectionOptions options, fxl::WeakPtr<hci::Transport> transport,
- PeerCache* peer_cache, fxl::WeakPtr<LowEnergyConnectionManager> conn_mgr,
- fbl::RefPtr<l2cap::L2cap> l2cap, fxl::WeakPtr<gatt::GATT> gatt, ResultCallback cb);
+ PeerCache* peer_cache, fxl::WeakPtr<LowEnergyConnectionManager> conn_mgr, l2cap::L2cap* l2cap,
+ fxl::WeakPtr<gatt::GATT> gatt, ResultCallback cb);
// Instances should only be destroyed after the result callback is called (except for stack tear
// down). Due to the asynchronous nature of cancelling the connection process, it is NOT safe to
@@ -67,9 +67,8 @@
LowEnergyConnectionOptions options, hci::LowEnergyConnector* connector,
zx::duration request_timeout, fxl::WeakPtr<hci::Transport> transport,
PeerCache* peer_cache, fxl::WeakPtr<LowEnergyConnectionManager> conn_mgr,
- fxl::WeakPtr<LowEnergyDiscoveryManager> discovery_manager,
- fbl::RefPtr<l2cap::L2cap> l2cap, fxl::WeakPtr<gatt::GATT> gatt,
- ResultCallback cb);
+ fxl::WeakPtr<LowEnergyDiscoveryManager> discovery_manager, l2cap::L2cap* l2cap,
+ fxl::WeakPtr<gatt::GATT> gatt, ResultCallback cb);
static const char* StateToString(State);
@@ -115,7 +114,7 @@
PeerCache* peer_cache_;
// Layer pointers to be passed to LowEnergyConnection.
- fbl::RefPtr<l2cap::L2cap> l2cap_;
+ l2cap::L2cap* l2cap_;
fxl::WeakPtr<gatt::GATT> gatt_;
// True if this connector is connecting an outbound connection, false if it is connecting an
diff --git a/src/connectivity/bluetooth/core/bt-host/host.cc b/src/connectivity/bluetooth/core/bt-host/host.cc
index 784a036..20c30f7 100644
--- a/src/connectivity/bluetooth/core/bt-host/host.cc
+++ b/src/connectivity/bluetooth/core/bt-host/host.cc
@@ -38,7 +38,7 @@
gatt_ = gatt::GATT::Create();
- gap_ = gap::Adapter::Create(hci_->WeakPtr(), gatt_->AsWeakPtr(), std::nullopt);
+ gap_ = gap::Adapter::Create(hci_->WeakPtr(), gatt_->AsWeakPtr());
if (!gap_)
return false;
diff --git a/src/connectivity/bluetooth/core/bt-host/l2cap/fake_l2cap.h b/src/connectivity/bluetooth/core/bt-host/l2cap/fake_l2cap.h
index 4c22dbb..126eb04 100644
--- a/src/connectivity/bluetooth/core/bt-host/l2cap/fake_l2cap.h
+++ b/src/connectivity/bluetooth/core/bt-host/l2cap/fake_l2cap.h
@@ -17,7 +17,8 @@
// layers for unit testing.
class FakeL2cap final : public L2cap {
public:
- inline static fbl::RefPtr<FakeL2cap> Create() { return fbl::AdoptRef(new FakeL2cap()); }
+ FakeL2cap() = default;
+ ~FakeL2cap() override;
void AttachInspect(inspect::Node& parent, std::string name) override {}
@@ -85,8 +86,6 @@
}
private:
- friend class fbl::RefPtr<FakeL2cap>;
-
// TODO(armansito): Consider moving the following logic into an internal fake
// that is L2CAP-specific.
struct ChannelData {
@@ -111,9 +110,6 @@
LEConnectionParameterUpdateCallback le_conn_param_cb;
};
- FakeL2cap() = default;
- ~FakeL2cap() override;
-
LinkData* RegisterInternal(hci_spec::ConnectionHandle handle, hci_spec::ConnectionRole role,
bt::LinkType link_type, LinkErrorCallback link_error_callback);
diff --git a/src/connectivity/bluetooth/core/bt-host/l2cap/l2cap.h b/src/connectivity/bluetooth/core/bt-host/l2cap/l2cap.h
index 1b11ef5..98638fe 100644
--- a/src/connectivity/bluetooth/core/bt-host/l2cap/l2cap.h
+++ b/src/connectivity/bluetooth/core/bt-host/l2cap/l2cap.h
@@ -23,8 +23,11 @@
// Represents the task domain that implements the host subsystem's data plane.
// This domain runs on the thread it is created on, and is not thread-safe.
// Protocols implemented here are: L2CAP.
-class L2cap : public fbl::RefCounted<L2cap> {
+class L2cap {
public:
+ L2cap() = default;
+ virtual ~L2cap() = default;
+
// Attach L2cap's inspect node as a child of |parent| with the given |name|
static constexpr const char* kInspectNodeName = "l2cap";
virtual void AttachInspect(inspect::Node& parent, std::string name) = 0;
@@ -136,14 +139,6 @@
//
// Has no effect if this L2cap is uninitialized or shut down.
virtual void UnregisterService(l2cap::PSM psm) = 0;
-
- protected:
- friend class fbl::RefPtr<L2cap>;
- L2cap() = default;
- virtual ~L2cap() = default;
-
- private:
- DISALLOW_COPY_AND_ASSIGN_ALLOW_MOVE(L2cap);
};
} // namespace bt::l2cap
diff --git a/src/connectivity/bluetooth/core/bt-host/l2cap/l2cap_fuzztest.cc b/src/connectivity/bluetooth/core/bt-host/l2cap/l2cap_fuzztest.cc
index 9cd77fd..fd88383b 100644
--- a/src/connectivity/bluetooth/core/bt-host/l2cap/l2cap_fuzztest.cc
+++ b/src/connectivity/bluetooth/core/bt-host/l2cap/l2cap_fuzztest.cc
@@ -48,14 +48,14 @@
const auto bredr_buffer_info = hci::DataBufferInfo(kMaxDataPacketLength, kMaxPacketCount);
InitializeACLDataChannel(bredr_buffer_info);
- domain_ = AdoptRef(
- new l2cap::ChannelManager(transport()->acl_data_channel(), /*random_channel_ids=*/true));
+ channel_manager_ = std::make_unique<l2cap::ChannelManager>(transport()->acl_data_channel(),
+ /*random_channel_ids=*/true);
StartTestDevice();
}
~DataFuzzTest() override {
- domain_ = nullptr;
+ channel_manager_ = nullptr;
TestingBase::TearDown();
}
@@ -114,27 +114,27 @@
}
void RegisterService() {
- domain_->RegisterService(l2cap::kAVDTP, l2cap::ChannelParameters(),
- [this](fbl::RefPtr<l2cap::Channel> chan) {
- if (!chan) {
- return;
- }
- chan->Activate(/*rx_callback=*/[](auto) {}, /*closed_callback=*/
- [this, id = chan->id()] { channels_.erase(id); });
- channels_.emplace(chan->id(), std::move(chan));
- });
+ channel_manager_->RegisterService(
+ l2cap::kAVDTP, l2cap::ChannelParameters(), [this](fbl::RefPtr<l2cap::Channel> chan) {
+ if (!chan) {
+ return;
+ }
+ chan->Activate(/*rx_callback=*/[](auto) {}, /*closed_callback=*/
+ [this, id = chan->id()] { channels_.erase(id); });
+ channels_.emplace(chan->id(), std::move(chan));
+ });
}
void ToggleConnection() {
if (connection_) {
acl_data_channel()->UnregisterLink(kHandle);
- domain_->RemoveConnection(kHandle);
+ channel_manager_->RemoveConnection(kHandle);
connection_ = false;
return;
}
acl_data_channel()->RegisterLink(kHandle, bt::LinkType::kACL);
- domain_->AddACLConnection(
+ channel_manager_->AddACLConnection(
kHandle, hci_spec::ConnectionRole::kCentral, /*link_error_callback=*/[] {},
/*security_callback=*/[](auto, auto, auto) {});
connection_ = true;
@@ -142,7 +142,7 @@
private:
FuzzedDataProvider data_;
- fbl::RefPtr<l2cap::L2cap> domain_;
+ std::unique_ptr<l2cap::ChannelManager> channel_manager_;
bool connection_;
std::unordered_map<l2cap::ChannelId, fbl::RefPtr<l2cap::Channel>> channels_;
};
diff --git a/src/connectivity/bluetooth/core/bt-host/l2cap/l2cap_integration_unittest.cc b/src/connectivity/bluetooth/core/bt-host/l2cap/l2cap_integration_unittest.cc
index 605e015..d66ff68 100644
--- a/src/connectivity/bluetooth/core/bt-host/l2cap/l2cap_integration_unittest.cc
+++ b/src/connectivity/bluetooth/core/bt-host/l2cap/l2cap_integration_unittest.cc
@@ -60,8 +60,8 @@
InitializeACLDataChannel(bredr_buffer_info);
// TODO(63074): Remove assumptions about channel ordering so we can turn random ids on.
- l2cap_ =
- AdoptRef(new ChannelManager(transport()->acl_data_channel(), /*random_channel_ids=*/false));
+ l2cap_ = std::make_unique<ChannelManager>(transport()->acl_data_channel(),
+ /*random_channel_ids=*/false);
StartTestDevice();
@@ -175,7 +175,7 @@
L2cap* l2cap() const { return l2cap_.get(); }
private:
- fbl::RefPtr<L2cap> l2cap_;
+ std::unique_ptr<l2cap::L2cap> l2cap_;
l2cap::CommandId next_command_id_;
std::unique_ptr<socket::SocketFactory<l2cap::Channel>> socket_factory_;
diff --git a/src/connectivity/bluetooth/core/bt-host/sdp/server.cc b/src/connectivity/bluetooth/core/bt-host/sdp/server.cc
index 58b0e7e..82a0581 100644
--- a/src/connectivity/bluetooth/core/bt-host/sdp/server.cc
+++ b/src/connectivity/bluetooth/core/bt-host/sdp/server.cc
@@ -127,7 +127,7 @@
return sdp;
}
-Server::Server(fbl::RefPtr<l2cap::L2cap> l2cap)
+Server::Server(l2cap::L2cap* l2cap)
: l2cap_(l2cap), next_handle_(kFirstUnreservedHandle), db_state_(0), weak_ptr_factory_(this) {
ZX_ASSERT(l2cap_);
diff --git a/src/connectivity/bluetooth/core/bt-host/sdp/server.h b/src/connectivity/bluetooth/core/bt-host/sdp/server.h
index f88b301..8f881e7 100644
--- a/src/connectivity/bluetooth/core/bt-host/sdp/server.h
+++ b/src/connectivity/bluetooth/core/bt-host/sdp/server.h
@@ -34,7 +34,7 @@
// A new SDP server, which starts with just a ServiceDiscoveryService record.
// Registers itself with |l2cap| when created.
- explicit Server(fbl::RefPtr<l2cap::L2cap> l2cap);
+ explicit Server(l2cap::L2cap* l2cap);
~Server();
// Attach SDP server inspect node as a child node of |parent|.
@@ -121,9 +121,8 @@
// |conn|. Logs an error if channel not found.
void Send(l2cap::Channel::UniqueId channel_id, ByteBufferPtr bytes);
- // The data domain that owns the L2CAP layer. Used to register callbacks for
- // the channels of services registered.
- fbl::RefPtr<l2cap::L2cap> l2cap_;
+ // Used to register callbacks for the channels of services registered.
+ l2cap::L2cap* l2cap_;
struct InspectProperties {
// Inspect hierarchy node representing the sdp server.
diff --git a/src/connectivity/bluetooth/core/bt-host/sdp/server_unittest.cc b/src/connectivity/bluetooth/core/bt-host/sdp/server_unittest.cc
index 97498d0..0ee120f 100644
--- a/src/connectivity/bluetooth/core/bt-host/sdp/server_unittest.cc
+++ b/src/connectivity/bluetooth/core/bt-host/sdp/server_unittest.cc
@@ -40,14 +40,14 @@
protected:
void SetUp() override {
- l2cap_ = l2cap::testing::FakeL2cap::Create();
+ l2cap_ = std::make_unique<l2cap::testing::FakeL2cap>();
l2cap_->set_channel_callback([this](auto fake_chan) {
channel_ = std::move(fake_chan);
set_fake_chan(channel_->AsWeakPtr());
});
l2cap_->AddACLConnection(kTestHandle1, hci_spec::ConnectionRole::kPeripheral, nullptr, nullptr);
l2cap_->AddACLConnection(kTestHandle2, hci_spec::ConnectionRole::kPeripheral, nullptr, nullptr);
- server_ = std::make_unique<Server>(l2cap_);
+ server_ = std::make_unique<Server>(l2cap_.get());
}
void TearDown() override {
@@ -58,7 +58,7 @@
Server* server() const { return server_.get(); }
- fbl::RefPtr<l2cap::testing::FakeL2cap> l2cap() const { return l2cap_; }
+ l2cap::testing::FakeL2cap* l2cap() const { return l2cap_.get(); }
RegistrationHandle AddSPP(sdp::Server::ConnectCallback cb = NopConnectCallback) {
ServiceRecord record;
@@ -116,7 +116,7 @@
private:
fbl::RefPtr<l2cap::testing::FakeChannel> channel_;
- fbl::RefPtr<l2cap::testing::FakeL2cap> l2cap_;
+ std::unique_ptr<l2cap::testing::FakeL2cap> l2cap_;
std::unique_ptr<Server> server_;
};
diff --git a/src/connectivity/bluetooth/core/bt-host/testing/fake_sdp_server.cc b/src/connectivity/bluetooth/core/bt-host/testing/fake_sdp_server.cc
index 0bf8f36..07deb9b1 100644
--- a/src/connectivity/bluetooth/core/bt-host/testing/fake_sdp_server.cc
+++ b/src/connectivity/bluetooth/core/bt-host/testing/fake_sdp_server.cc
@@ -10,7 +10,8 @@
namespace bt::testing {
-FakeSdpServer::FakeSdpServer() : server_(sdp::Server(l2cap::testing::FakeL2cap::Create())) {}
+FakeSdpServer::FakeSdpServer()
+ : l2cap_(std::make_unique<l2cap::testing::FakeL2cap>()), server_(l2cap_.get()) {}
void FakeSdpServer::RegisterWithL2cap(FakeL2cap* l2cap_) {
auto channel_cb = [this](fxl::WeakPtr<FakeDynamicChannel> channel) {
diff --git a/src/connectivity/bluetooth/core/bt-host/testing/fake_sdp_server.h b/src/connectivity/bluetooth/core/bt-host/testing/fake_sdp_server.h
index 4aba9fe..a945a51 100644
--- a/src/connectivity/bluetooth/core/bt-host/testing/fake_sdp_server.h
+++ b/src/connectivity/bluetooth/core/bt-host/testing/fake_sdp_server.h
@@ -37,6 +37,8 @@
sdp::Server* server() { return &server_; }
private:
+ std::unique_ptr<l2cap::testing::FakeL2cap> l2cap_;
+
// The production SDP server associated with this FakeSdpServer,
sdp::Server server_;
