bluetooth: Use WebBluetoothDeviceId instead of string

content/common/bluetooth/bluetooth_device_id_unittest.cc

Index: content/common/bluetooth/bluetooth_device_id_unittest.cc
diff --git a/content/common/bluetooth/bluetooth_device_id_unittest.cc b/content/common/bluetooth/bluetooth_device_id_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..0dcf397267c87ff3a3fec0ea65a51ec949e2a0cf
--- /dev/null
+++ b/content/common/bluetooth/bluetooth_device_id_unittest.cc
@@ -0,0 +1,77 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/common/bluetooth/bluetooth_device_id.h"
+
+#include "base/base64.h"
+#include "base/strings/string_util.h"
+#include "crypto/random.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using content::BluetoothDeviceId;
+
+namespace {
+
+const char kValidDeviceId1[] = "1234567890123456789012==";
+const char kValidDeviceId2[] = "AbCdEfGhIjKlMnOpQrSt+/==";
+const char kInvalidLongDeviceId[] = "12345678901234567890123=";
+const char kInvalidShortDeviceId[] = "12345678901234567890";
+const char kInvalidCharacterDeviceId[] = "123456789012345678901*==";
+// A base64 string should have a length of a multiple of 4.
+const char kInvalidLengthDeviceId[] = "123456789012345678901";
+
+}  // namespace
+
+TEST(BluetoothDeviceIdTest, DefaulConstructor) {
+  BluetoothDeviceId default_id1;
+  BluetoothDeviceId default_id2;
+  BluetoothDeviceId valid_id(kValidDeviceId1);
+
+  ASSERT_TRUE(BluetoothDeviceId::IsValid(valid_id.str()));

[Jeffrey Yasskin, 2016/06/03 17:22:05]

Also EXPECT_FALSE(BluetoothDeviceId::IsValid(default_id1));

[ortuno, 2016/06/06 22:23:00]

IsValid takes a string and we can't get a string because str DCHECKs. But I did
add an ASSERT_DEATH(default_id1.str(), ""), which makes sense and I should have
included to begin with.

[Jeffrey Yasskin, 2016/06/06 23:44:02]

Oh, right. What you've done looks good.

+
+  EXPECT_DEATH_IF_SUPPORTED([&]() { return default_id1 == default_id2; }(), "");
+  EXPECT_DEATH_IF_SUPPORTED([&]() { return default_id1 != default_id2; }(), "");
+
+  EXPECT_DEATH_IF_SUPPORTED([&]() { return default_id1 == valid_id; }(), "");
+  EXPECT_DEATH_IF_SUPPORTED([&]() { return valid_id == default_id1; }(), "");
+
+  EXPECT_DEATH_IF_SUPPORTED([&]() { return default_id1 != valid_id; }(), "");
+  EXPECT_DEATH_IF_SUPPORTED([&]() { return valid_id != default_id1; }(), "");
+}
+
+TEST(BluetoothDeviceIdTest, StrConstructor) {

[Jeffrey Yasskin, 2016/06/03 17:22:05]

Assert on == and != too.

[ortuno, 2016/06/06 22:23:00]

Do you mean add statements that compare the objects returned by this
constructor? If so, done.

[Jeffrey Yasskin, 2016/06/06 23:44:02]

Yep, that. Thanks.

+  BluetoothDeviceId(kValidDeviceId1);
+  BluetoothDeviceId(kValidDeviceId2);
+  // Above call should not crash.
+  EXPECT_TRUE(true);

[Jeffrey Yasskin, 2016/06/03 17:22:05]

This doesn't add anything: the test will fail if it crashes and won't run the
EXPECT_TRUE anyway.

[ortuno, 2016/06/06 22:23:00]

Done.

+
+  EXPECT_DEATH_IF_SUPPORTED(BluetoothDeviceId(""), "");
+  EXPECT_DEATH_IF_SUPPORTED(
+      [&]() { return BluetoothDeviceId(kInvalidLongDeviceId); }(), "");
+  EXPECT_DEATH_IF_SUPPORTED(
+      [&]() { return BluetoothDeviceId(kInvalidShortDeviceId); }(), "");
+  EXPECT_DEATH_IF_SUPPORTED(
+      [&]() { return BluetoothDeviceId(kInvalidCharacterDeviceId); }(), "");
+  EXPECT_DEATH_IF_SUPPORTED(
+      [&]() { return BluetoothDeviceId(kInvalidLengthDeviceId); }(), "");
+}
+
+TEST(BluetoothDeviceIdTest, IsValid_Valid) {
+  EXPECT_TRUE(BluetoothDeviceId::IsValid(kValidDeviceId1));
+  EXPECT_TRUE(BluetoothDeviceId::IsValid(kValidDeviceId2));
+}
+
+TEST(BluetoothDeviceIdTest, IsValid_Invalid) {
+  EXPECT_FALSE(BluetoothDeviceId::IsValid(""));
+  EXPECT_FALSE(BluetoothDeviceId::IsValid(kInvalidLongDeviceId));
+  EXPECT_FALSE(BluetoothDeviceId::IsValid(kInvalidShortDeviceId));
+  EXPECT_FALSE(BluetoothDeviceId::IsValid(kInvalidCharacterDeviceId));
+  EXPECT_FALSE(BluetoothDeviceId::IsValid(kInvalidLengthDeviceId));
+}
+
+TEST(BluetoothDeviceIdTest, Create) {
+  // Tests that Create generates a valid Device Id.
+  EXPECT_TRUE(BluetoothDeviceId::IsValid(BluetoothDeviceId::Create().str()))
+      << "This should never fail.";

[Jeffrey Yasskin, 2016/06/03 17:22:05]

That's implied by it being in a test. :) Do you mean that if it's broken it
might fail flakily, and folks should ignore that less than usual?

[ortuno, 2016/06/06 22:23:00]

Hmm actually the opposite haha. I'm afraid that since the string is random it
might find a bug in Base64Encode. So if this test ever fails we should be
notified.

[Jeffrey Yasskin, 2016/06/06 23:44:02]

Say something like that in the failure message.

[ortuno, 2016/06/24 17:39:43]

After some more thinking I decided to remove the message. I think it does not
make sense to test for a bug in Base64 in our code. A fuzzer should be used to
find any bugs in it.

+}