Skip to content

Commit

Permalink
Replace std::unordered_map with std::map for dbus::Property
Browse files Browse the repository at this point in the history
std::map is more preferable than std::unordered_map:
https://chromium.googlesource.com/chromium/src/+/master/base/containers/README.md.
For newblue's dispatcher std::map for dbus::Property is required to
match with libbrillo ExportedProperty.

BUG=chromium:812468
TEST=Unit test still passes, manual test on device exercising Bluetooth
device discovery.

Change-Id: I378c383a612fd40997b44191a1ba6df33aa6899d
Reviewed-on: https://chromium-review.googlesource.com/959362
Reviewed-by: Rahul Chaturvedi <rkc@chromium.org>
Reviewed-by: Satoru Takabayashi <satorux@chromium.org>
Commit-Queue: Sonny Sasaka <sonnysasaka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542677}
  • Loading branch information
Sonny Sasaka authored and Commit Bot committed Mar 13, 2018
1 parent d65550b commit bd3098c
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 51 deletions.
22 changes: 11 additions & 11 deletions dbus/property.cc
Original file line number Diff line number Diff line change
Expand Up @@ -661,13 +661,13 @@ void Property<std::vector<std::pair<std::vector<uint8_t>, uint16_t>>>::
}

//
// Property<std::unordered_map<std::string, std::vector<uint8_t>>>
// Property<std::map<std::string, std::vector<uint8_t>>>
// specialization.
//

template <>
bool Property<std::unordered_map<std::string, std::vector<uint8_t>>>::
PopValueFromReader(MessageReader* reader) {
bool Property<std::map<std::string, std::vector<uint8_t>>>::PopValueFromReader(
MessageReader* reader) {
MessageReader variant_reader(nullptr);
MessageReader dict_reader(nullptr);
if (!reader->PopVariant(&variant_reader) ||
Expand Down Expand Up @@ -697,7 +697,7 @@ bool Property<std::unordered_map<std::string, std::vector<uint8_t>>>::
}

template <>
void Property<std::unordered_map<std::string, std::vector<uint8_t>>>::
void Property<std::map<std::string, std::vector<uint8_t>>>::
AppendSetValueToWriter(MessageWriter* writer) {
MessageWriter variant_writer(nullptr);
MessageWriter dict_writer(nullptr);
Expand Down Expand Up @@ -725,13 +725,13 @@ void Property<std::unordered_map<std::string, std::vector<uint8_t>>>::
}

//
// Property<std::unordered_map<uint16_t, std::vector<uint8_t>>>
// Property<std::map<uint16_t, std::vector<uint8_t>>>
// specialization.
//

template <>
bool Property<std::unordered_map<uint16_t, std::vector<uint8_t>>>::
PopValueFromReader(MessageReader* reader) {
bool Property<std::map<uint16_t, std::vector<uint8_t>>>::PopValueFromReader(
MessageReader* reader) {
MessageReader variant_reader(nullptr);
MessageReader dict_reader(nullptr);
if (!reader->PopVariant(&variant_reader) ||
Expand Down Expand Up @@ -761,8 +761,8 @@ bool Property<std::unordered_map<uint16_t, std::vector<uint8_t>>>::
}

template <>
void Property<std::unordered_map<uint16_t, std::vector<uint8_t>>>::
AppendSetValueToWriter(MessageWriter* writer) {
void Property<std::map<uint16_t, std::vector<uint8_t>>>::AppendSetValueToWriter(
MessageWriter* writer) {
MessageWriter variant_writer(nullptr);
MessageWriter dict_writer(nullptr);

Expand Down Expand Up @@ -804,7 +804,7 @@ template class Property<std::vector<ObjectPath>>;
template class Property<std::vector<uint8_t>>;
template class Property<std::map<std::string, std::string>>;
template class Property<std::vector<std::pair<std::vector<uint8_t>, uint16_t>>>;
template class Property<std::unordered_map<std::string, std::vector<uint8_t>>>;
template class Property<std::unordered_map<uint16_t, std::vector<uint8_t>>>;
template class Property<std::map<std::string, std::vector<uint8_t>>>;
template class Property<std::map<uint16_t, std::vector<uint8_t>>>;

} // namespace dbus
21 changes: 10 additions & 11 deletions dbus/property.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

#include <map>
#include <string>
#include <unordered_map>
#include <utility>
#include <vector>

Expand Down Expand Up @@ -613,25 +612,25 @@ extern template class CHROME_DBUS_EXPORT

template <>
CHROME_DBUS_EXPORT bool
Property<std::unordered_map<std::string, std::vector<uint8_t>>>::
PopValueFromReader(MessageReader* reader);
Property<std::map<std::string, std::vector<uint8_t>>>::PopValueFromReader(
MessageReader* reader);
template <>
CHROME_DBUS_EXPORT void
Property<std::unordered_map<std::string, std::vector<uint8_t>>>::
AppendSetValueToWriter(MessageWriter* writer);
Property<std::map<std::string, std::vector<uint8_t>>>::AppendSetValueToWriter(
MessageWriter* writer);
extern template class CHROME_DBUS_EXPORT
Property<std::unordered_map<std::string, std::vector<uint8_t>>>;
Property<std::map<std::string, std::vector<uint8_t>>>;

template <>
CHROME_DBUS_EXPORT bool
Property<std::unordered_map<uint16_t, std::vector<uint8_t>>>::
PopValueFromReader(MessageReader* reader);
Property<std::map<uint16_t, std::vector<uint8_t>>>::PopValueFromReader(
MessageReader* reader);
template <>
CHROME_DBUS_EXPORT void
Property<std::unordered_map<uint16_t, std::vector<uint8_t>>>::
AppendSetValueToWriter(MessageWriter* writer);
Property<std::map<uint16_t, std::vector<uint8_t>>>::AppendSetValueToWriter(
MessageWriter* writer);
extern template class CHROME_DBUS_EXPORT
Property<std::unordered_map<uint16_t, std::vector<uint8_t>>>;
Property<std::map<uint16_t, std::vector<uint8_t>>>;

#pragma GCC diagnostic pop

Expand Down
12 changes: 6 additions & 6 deletions dbus/property_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ TEST(PropertyTestStatic, ReadWriteStringToByteVectorMap) {
writer.CloseContainer(&variant_writer);

MessageReader reader(message.get());
Property<std::unordered_map<std::string, std::vector<uint8_t>>> test_property;
Property<std::map<std::string, std::vector<uint8_t>>> test_property;
EXPECT_TRUE(test_property.PopValueFromReader(&reader));

ASSERT_EQ(arraysize(keys), test_property.value().size());
Expand All @@ -470,15 +470,15 @@ TEST(PropertyTestStatic, ReadWriteStringToByteVectorMap) {
}

TEST(PropertyTestStatic, SerializeStringToByteVectorMap) {
std::unordered_map<std::string, std::vector<uint8_t>> test_map;
std::map<std::string, std::vector<uint8_t>> test_map;
test_map["Hi"] = {1, 2, 3};
test_map["Map"] = {0xab, 0xcd};
test_map["Random"] = {0x0};

std::unique_ptr<Response> message(Response::CreateEmpty());
MessageWriter writer(message.get());

Property<std::unordered_map<std::string, std::vector<uint8_t>>> test_property;
Property<std::map<std::string, std::vector<uint8_t>>> test_property;
test_property.ReplaceSetValueForTesting(test_map);
test_property.AppendSetValueToWriter(&writer);

Expand Down Expand Up @@ -516,7 +516,7 @@ TEST(PropertyTestStatic, ReadWriteUInt16ToByteVectorMap) {
writer.CloseContainer(&variant_writer);

MessageReader reader(message.get());
Property<std::unordered_map<uint16_t, std::vector<uint8_t>>> test_property;
Property<std::map<uint16_t, std::vector<uint8_t>>> test_property;
EXPECT_TRUE(test_property.PopValueFromReader(&reader));

ASSERT_EQ(arraysize(keys), test_property.value().size());
Expand All @@ -525,15 +525,15 @@ TEST(PropertyTestStatic, ReadWriteUInt16ToByteVectorMap) {
}

TEST(PropertyTestStatic, SerializeUInt16ToByteVectorMap) {
std::unordered_map<uint16_t, std::vector<uint8_t>> test_map;
std::map<uint16_t, std::vector<uint8_t>> test_map;
test_map[11] = {1, 2, 3};
test_map[12] = {0xab, 0xcd};
test_map[13] = {0x0};

std::unique_ptr<Response> message(Response::CreateEmpty());
MessageWriter writer(message.get());

Property<std::unordered_map<uint16_t, std::vector<uint8_t>>> test_property;
Property<std::map<uint16_t, std::vector<uint8_t>>> test_property;
test_property.ReplaceSetValueForTesting(test_map);
test_property.AppendSetValueToWriter(&writer);

Expand Down
8 changes: 3 additions & 5 deletions device/bluetooth/dbus/bluetooth_device_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@

#include <stdint.h>

#include <map>
#include <string>
#include <unordered_map>
#include <vector>

#include "base/callback.h"
Expand Down Expand Up @@ -111,13 +111,11 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothDeviceClient : public BluezDBusClient {

// Manufacturer specific advertisement data. Keys are 16 bits Manufacturer
// ID followed by its byte array value. Read-only.
dbus::Property<std::unordered_map<uint16_t, std::vector<uint8_t>>>
manufacturer_data;
dbus::Property<std::map<uint16_t, std::vector<uint8_t>>> manufacturer_data;

// Service advertisement data. Keys are the UUIDs in string format followed
// by its byte array value. Read-only.
dbus::Property<std::unordered_map<std::string, std::vector<uint8_t>>>
service_data;
dbus::Property<std::map<std::string, std::vector<uint8_t>>> service_data;

// Indicate whether or not service discovery has been resolved. Read-only.
dbus::Property<bool> services_resolved;
Expand Down
14 changes: 6 additions & 8 deletions device/bluetooth/dbus/fake_bluetooth_device_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1562,9 +1562,8 @@ void FakeBluetoothDeviceClient::UpdateDeviceRSSI(
void FakeBluetoothDeviceClient::UpdateServiceAndManufacturerData(
const dbus::ObjectPath& object_path,
const std::vector<std::string>& service_uuids,
const std::unordered_map<std::string, std::vector<uint8_t>>& service_data,
const std::unordered_map<uint16_t, std::vector<uint8_t>>&
manufacturer_data) {
const std::map<std::string, std::vector<uint8_t>>& service_data,
const std::map<uint16_t, std::vector<uint8_t>>& manufacturer_data) {
PropertiesMap::const_iterator iter = properties_map_.find(object_path);
if (iter == properties_map_.end()) {
VLOG(2) << "Fake device does not exist: " << object_path.value();
Expand All @@ -1586,13 +1585,13 @@ void FakeBluetoothDeviceClient::UpdateServiceAndManufacturerData(
properties->uuids.value().end());
properties->uuids.ReplaceValue(merged_uuids);

std::unordered_map<std::string, std::vector<uint8_t>> merged_service_data =
std::map<std::string, std::vector<uint8_t>> merged_service_data =
service_data;
merged_service_data.insert(properties->service_data.value().begin(),
properties->service_data.value().end());
properties->service_data.ReplaceValue(merged_service_data);

std::unordered_map<uint16_t, std::vector<uint8_t>> merged_manufacturer_data =
std::map<uint16_t, std::vector<uint8_t>> merged_manufacturer_data =
manufacturer_data;
merged_manufacturer_data.insert(properties->manufacturer_data.value().begin(),
properties->manufacturer_data.value().end());
Expand Down Expand Up @@ -1825,9 +1824,8 @@ void FakeBluetoothDeviceClient::CreateTestDevice(
const std::string device_address,
const std::vector<std::string>& service_uuids,
device::BluetoothTransport type,
const std::unordered_map<std::string, std::vector<uint8_t>>& service_data,
const std::unordered_map<uint16_t, std::vector<uint8_t>>&
manufacturer_data) {
const std::map<std::string, std::vector<uint8_t>>& service_data,
const std::map<uint16_t, std::vector<uint8_t>>& manufacturer_data) {
// Create a random device path.
dbus::ObjectPath device_path;
std::string id;
Expand Down
10 changes: 4 additions & 6 deletions device/bluetooth/dbus/fake_bluetooth_device_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,8 @@ class DEVICE_BLUETOOTH_EXPORT FakeBluetoothDeviceClient
const std::string device_address,
const std::vector<std::string>& service_uuids,
device::BluetoothTransport type,
const std::unordered_map<std::string, std::vector<uint8_t>>& service_data,
const std::unordered_map<uint16_t, std::vector<uint8_t>>&
manufacturer_data);
const std::map<std::string, std::vector<uint8_t>>& service_data,
const std::map<uint16_t, std::vector<uint8_t>>& manufacturer_data);

void set_delay_start_discovery(bool value) { delay_start_discovery_ = value; }

Expand All @@ -184,9 +183,8 @@ class DEVICE_BLUETOOTH_EXPORT FakeBluetoothDeviceClient
void UpdateServiceAndManufacturerData(
const dbus::ObjectPath& object_path,
const std::vector<std::string>& service_uuids,
const std::unordered_map<std::string, std::vector<uint8_t>>& service_data,
const std::unordered_map<uint16_t, std::vector<uint8_t>>&
manufacturer_data);
const std::map<std::string, std::vector<uint8_t>>& service_data,
const std::map<uint16_t, std::vector<uint8_t>>& manufacturer_data);

static const char kTestPinCode[];
static const int kTestPassKey;
Expand Down
8 changes: 4 additions & 4 deletions device/bluetooth/test/bluetooth_test_bluez.cc
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ BluetoothDevice* BluetoothTestBlueZ::SimulateLowEnergyDevice(
std::string device_address = kTestDeviceAddress1;
std::vector<std::string> service_uuids;
BluetoothTransport device_type = BLUETOOTH_TRANSPORT_LE;
std::unordered_map<std::string, std::vector<uint8_t>> service_data;
std::unordered_map<uint16_t, std::vector<uint8_t>> manufacturer_data;
std::map<std::string, std::vector<uint8_t>> service_data;
std::map<uint16_t, std::vector<uint8_t>> manufacturer_data;

switch (device_ordinal) {
case 1:
Expand Down Expand Up @@ -180,8 +180,8 @@ BluetoothDevice* BluetoothTestBlueZ::SimulateClassicDevice() {
std::string device_name = kTestDeviceName;
std::string device_address = kTestDeviceAddress3;
std::vector<std::string> service_uuids;
std::unordered_map<std::string, std::vector<uint8_t>> service_data;
std::unordered_map<uint16_t, std::vector<uint8_t>> manufacturer_data;
std::map<std::string, std::vector<uint8_t>> service_data;
std::map<uint16_t, std::vector<uint8_t>> manufacturer_data;

if (!adapter_->GetDevice(device_address)) {
fake_bluetooth_device_client_->CreateTestDevice(
Expand Down

0 comments on commit bd3098c

Please sign in to comment.