Implemented BluetoothServiceRecordMac with unittest.

device/bluetooth/bluetooth_service_record_mac.mm

+// Copyright 2013 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 "device/bluetooth/bluetooth_service_record_mac.h"
+
+#include <IOBluetooth/Bluetooth.h>
+#include <IOBluetooth/IOBluetoothUtilities.h>

[Mark Mentovai, 2013/03/12 16:02:54]

If you look at this header, you’ll see that this one actually is an Objective-C
header when included by __OBJC__, so #import’s good here.

[youngki, 2013/03/12 19:45:47]

Done.

+#import <IOBluetooth/objc/IOBluetoothDevice.h>
+#import <IOBluetooth/objc/IOBluetoothSDPDataElement.h>
+#import <IOBluetooth/objc/IOBluetoothSDPServiceRecord.h>
+#import <IOBluetooth/objc/IOBluetoothSDPUUID.h>
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/stringprintf.h"
+#include "base/strings/sys_string_conversions.h"
+
+namespace {
+
+const BluetoothSDPServiceAttributeID kServiceClassId = 1;

[Mark Mentovai, 2013/03/12 16:02:54]

This doesn’t need to be file-scoped, you can move it down to the function where
it’s used.

[youngki, 2013/03/12 19:45:47]

Done.

+
+void ExtractUuid(IOBluetoothSDPDataElement* service_class_data,
+                 std::string* uuid) {
+  NSArray* inner_elements = [service_class_data getArrayValue];
+  IOBluetoothSDPUUID* sdp_uuid = nil;
+  for (IOBluetoothSDPDataElement* inner_element in inner_elements) {

[Mark Mentovai, 2013/03/12 16:02:54]

Assuming -getArrayValue did return an array (as opposed to nil), might the
inner_elements array contain objects of other types than
IOBluetoothSDPDataElement?

[youngki, 2013/03/12 19:45:47]

I think it's safe to assume the type is only IOBluetoothSDPDataElement according
to the specification:
http://developer.apple.com/library/mac/#documentation/IOBluetooth/Reference/I...

Basically I am checking if |service_class_data| type descriptor is a sequence
type before calling ExtractUuid(), which means that getArrayValue() will return
an array of IOBluetoothSDPDataElement's.

+    if ([inner_element getTypeDescriptor] == kBluetoothSDPDataElementTypeUUID) {
+      sdp_uuid = [[inner_element getUUIDValue] getUUIDWithLength:16];
+      break;
+    }
+  }
+  if (sdp_uuid != nil) {
+    const uint8* uuid_bytes = reinterpret_cast<const uint8*>([sdp_uuid bytes]);
+    *uuid = base::StringPrintf(
+        "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
+        uuid_bytes[0], uuid_bytes[1], uuid_bytes[2], uuid_bytes[3],
+        uuid_bytes[4], uuid_bytes[5], uuid_bytes[6], uuid_bytes[7],
+        uuid_bytes[8], uuid_bytes[9], uuid_bytes[10], uuid_bytes[11],
+        uuid_bytes[12], uuid_bytes[13], uuid_bytes[14], uuid_bytes[15]);
+  }
+}
+
+}  // namespace
+
+namespace device {
+
+BluetoothServiceRecordMac::BluetoothServiceRecordMac(
+    IOBluetoothSDPServiceRecord* record) : device_([record device]) {

[Mark Mentovai, 2013/03/12 16:02:54]

Since you already had to break the line for the first argument, you should put
the initializer list on a new line too, like:

BSRM::BSRM(
    IOBSSR* record)
    : device_([record device]) {

[youngki, 2013/03/12 19:45:47]

Done.

+  name_ = base::SysNSStringToUTF8([record getServiceName]);

[Mark Mentovai, 2013/03/12 16:02:54]

There are fields in BluetoothServiceRecord that aren’t initialized by this
constructor or the BluetoothServiceRecord constructor. Is it OK to leave those
values undefined?

[Mark Mentovai, 2013/03/12 16:02:54]

Why don’t you initialize the rest of the fields in the initializer list too?
That’s what I had in mind when I suggested it…

BSRM::BSRM(
    IOBSSR* record)
    : device_([record device]),
      name_(base::SysNSStringToUTF8(…)),
      address_(…),
      supports_rfcomm_(…) {

OK, I see that this won’t work because some of the fields are protected members
of the superclass. I got it. Since you can’t initialize some fields in this
manner, then it’s probably better to be consistent and not initialize any of
them. Sorry for the runaround.

However, to be explicit about the inheritance relationship, you should call the
superclass’ constructor from the initializer list:

BSRM::BSRM(
    IOBSSR* record)
    : BluetoothServiceRecord() {

[youngki, 2013/03/12 19:45:47]

Done.

[youngki, 2013/03/12 19:45:47]

Done.

+  address_ = base::SysNSStringToUTF8(IOBluetoothNSStringFromDeviceAddress(
+      [device_ getAddress]));
+
+  supports_rfcomm_ =
+      [record getRFCOMMChannelID:&rfcomm_channel_] == kIOReturnSuccess;
+
+  IOBluetoothSDPDataElement* service_class_data =
+      [record getAttributeDataElement:kServiceClassId];
+  if ([service_class_data getTypeDescriptor] ==
+          kBluetoothSDPDataElementTypeDataElementSequence) {
+    ExtractUuid(service_class_data, &uuid_);
+  }
+}
+
+}  // namespace device