[Mac] Stop using deprecated IOBluetoothSDPServiceRecordRef APIs.

device/bluetooth/bluetooth_socket_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_socket_mac.h"
 
 #import <IOBluetooth/IOBluetooth.h>
 #include <stdint.h>
 
 #include <limits>
@@ skipping 199 matching lines @@
 
   socket_->OnChannelOpened(std::unique_ptr<device::BluetoothChannelMac>(
       new device::BluetoothL2capChannelMac(NULL, [l2capChannel retain])));
 }
 
 @end
 
 namespace device {
 namespace {
 
-// It's safe to use 0 to represent an unregistered service, as implied by the
-// documentation at [ http://goo.gl/YRtCkF ].
-const BluetoothSDPServiceRecordHandle kInvalidServiceRecordHandle = 0;
-
-// Likewise, it's safe to use 0 to represent invalid channel or PSM port
-// numbers, as both are required to be non-zero for valid services.
+// It's safe to use 0 to represent invalid channel or PSM port numbers, as both
+// are required to be non-zero for valid services.
 const BluetoothRFCOMMChannelID kInvalidRfcommChannelId = 0;
 const BluetoothL2CAPPSM kInvalidL2capPsm = 0;
 
 const char kInvalidOrUsedChannel[] = "Invalid channel or already in use";
 const char kInvalidOrUsedPsm[] = "Invalid PSM or already in use";
 const char kProfileNotFound[] = "Profile not found";
 const char kSDPQueryFailed[] = "SDP query failed";
 const char kSocketConnecting[] = "The socket is currently connecting";
 const char kSocketAlreadyConnected[] = "The socket is already connected";
 const char kSocketNotConnected[] = "The socket is not connected";
@@ skipping 99 matching lines @@
         @"DataElementSize" : @2,  // 2 bytes.
         @"DataElementValue" : @(psm),
       },
     ],
   ];
   return BuildServiceDefinition(
       uuid, options.name.get(), l2cap_protocol_definition);
 }
 
 // Registers a Bluetooth service with the specified |service_definition| in the
-// system SDP server. Returns a handle to the registered service on success. If
-// the service could not be registered, or if |verify_service_callback|
-// indicates that the to-be-registered service is not configured correctly,
-// returns |kInvalidServiceRecordHandle|.
-BluetoothSDPServiceRecordHandle RegisterService(
+// system SDP server. Returns the registered service on success. If the service
+// could not be registered, or if |verify_service_callback| indicates that the
+// to-be-registered service was not configured correctly, returns nil.
+IOBluetoothSDPServiceRecord* RegisterService(

[Ilya Sherman, 2016/10/20 01:11:05]

In regular C++, I'd ask you to change this to be a smart pointer rather than a
raw pointer.  I'm not sure whether Objective-C++ smart pointers make this
problematic somehow; but if they don't, I think it would be good to return a
scoped object here.

[shrike, 2016/10/20 19:26:18]

The object returned by +publishedServiceRecordWithDictionary: is already
auto-released, and this method simply passes that object along. Downstream
callers of this method place it into a scoped ptr.

[Ilya Sherman, 2016/10/20 19:37:35]

That's fair, but it's hard  to tell from the method signature that this object
is autoreleased.  Then again, my intuitions are honed from C++ programming, not
Objective-C.  So, I defer to folks more familiar with Objective-C best
practices.

[erikchen, 2016/10/21 01:18:00]

In ObjC, any function/method that doesn't have the key phrases: copy, init, or
create is assumed to return an auto-released object. We generally prefer to
return scoped_nsobjects to avoid the extra autorelease, but in this case, that's
out of our control. 

I think the signature is fine as is.

     NSDictionary* service_definition,
     const base::Callback<bool(IOBluetoothSDPServiceRecord*)>&
         verify_service_callback) {
   // Attempt to register the service.
-  IOBluetoothSDPServiceRecordRef service_record_ref;
-  IOReturn result =
-      IOBluetoothAddServiceDict((CFDictionaryRef)service_definition,
-                                &service_record_ref);
-  if (result != kIOReturnSuccess)
-    return kInvalidServiceRecordHandle;
-  // Transfer ownership to a scoped object, to simplify memory management.
-  base::ScopedCFTypeRef<IOBluetoothSDPServiceRecordRef>
-      scoped_service_record_ref(service_record_ref);
-
-  // Extract the service record handle.
-  BluetoothSDPServiceRecordHandle service_record_handle;
-  IOBluetoothSDPServiceRecord* service_record =
-      [IOBluetoothSDPServiceRecord withSDPServiceRecordRef:service_record_ref];
-  result = [service_record getServiceRecordHandle:&service_record_handle];
-  if (result != kIOReturnSuccess)
-    return kInvalidServiceRecordHandle;
+  IOBluetoothSDPServiceRecord* service_record = [IOBluetoothSDPServiceRecord
+      publishedServiceRecordWithDictionary:service_definition];

[Ilya Sherman, 2016/10/20 01:11:05]

Okay, it looks like this method is only available in the 10.9+ SDK.  Are all of
our builds targeting that SDK these days?  It's certainly nice to have a simpler
code flow here! =)

[shrike, 2016/10/20 19:26:18]

We only support Chrome running on OS X 10.9 and up. This change (and others)
will allow us to start building against the 10.9 SDK without deprecation
warnings.

[Ilya Sherman, 2016/10/20 19:37:35]

Oh, wow, I hadn't realized we dropped support for 10.7 and 10.8 already!  Okay,
this CL makes a lot more sense to me in that context =)

 
   // Verify that the registered service was configured correctly. If not,
   // withdraw the service.
-  if (!verify_service_callback.Run(service_record)) {
-    IOBluetoothRemoveServiceWithRecordHandle(service_record_handle);
-    return kInvalidServiceRecordHandle;
+  if (!service_record || !verify_service_callback.Run(service_record)) {
+    [service_record removeServiceRecord];
+    service_record = nil;
   }
 
-  return service_record_handle;
+  return service_record;
 }
 
 // Returns true iff the |requested_channel_id| was registered in the RFCOMM
 // |service_record|. If it was, also updates |registered_channel_id| with the
 // registered value, as the requested id may have been left unspecified.
 bool VerifyRfcommService(const int* requested_channel_id,
                          BluetoothRFCOMMChannelID* registered_channel_id,
                          IOBluetoothSDPServiceRecord* service_record) {
   // Test whether the requested channel id was available.
   // TODO(isherman): The OS doesn't seem to actually pick a random channel if we
   // pass in |kInvalidRfcommChannelId|.
   BluetoothRFCOMMChannelID rfcomm_channel_id;
   IOReturn result = [service_record getRFCOMMChannelID:&rfcomm_channel_id];
   if (result != kIOReturnSuccess ||
       (requested_channel_id && rfcomm_channel_id != *requested_channel_id)) {
     return false;
   }
 
   *registered_channel_id = rfcomm_channel_id;
   return true;
 }
 
 // Registers an RFCOMM service with the specified |uuid|, |options.channel_id|,
 // and |options.name| in the system SDP server. Automatically allocates a
 // channel if |options.channel_id| is null. Does not specify a name if
 // |options.name| is null. Returns a handle to the registered service and
-// updates |registered_channel_id| to the actual channel id, or returns
-// |kInvalidServiceRecordHandle| if the service could not be registered.
-BluetoothSDPServiceRecordHandle RegisterRfcommService(
+// updates |registered_channel_id| to the actual channel id, or returns nil if
+// the service could not be registered.
+IOBluetoothSDPServiceRecord* RegisterRfcommService(
     const BluetoothUUID& uuid,
     const BluetoothAdapter::ServiceOptions& options,
     BluetoothRFCOMMChannelID* registered_channel_id) {
   return RegisterService(
       BuildRfcommServiceDefinition(uuid, options),
       base::Bind(
           &VerifyRfcommService, options.channel.get(), registered_channel_id));
 }
 
 // Returns true iff the |requested_psm| was registered in the L2CAP
@@ skipping 13 matching lines @@
   }
 
   *registered_psm = l2cap_psm;
   return true;
 }
 
 // Registers an L2CAP service with the specified |uuid|, |options.psm|, and
 // |options.name| in the system SDP server. Automatically allocates a PSM if
 // |options.psm| is null. Does not register a name if |options.name| is null.
 // Returns a handle to the registered service and updates |registered_psm| to
-// the actual PSM, or returns |kInvalidServiceRecordHandle| if the service could
-// not be registered.
-BluetoothSDPServiceRecordHandle RegisterL2capService(
+// the actual PSM, or returns nil if the service could not be registered.
+IOBluetoothSDPServiceRecord* RegisterL2capService(
     const BluetoothUUID& uuid,
     const BluetoothAdapter::ServiceOptions& options,
     BluetoothL2CAPPSM* registered_psm) {
   return RegisterService(
       BuildL2capServiceDefinition(uuid, options),
       base::Bind(&VerifyL2capService, options.psm.get(), registered_psm));
 }
 
 }  // namespace
 
@@ skipping 31 matching lines @@
     const BluetoothAdapter::ServiceOptions& options,
     const base::Closure& success_callback,
     const ErrorCompletionCallback& error_callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   adapter_ = adapter;
   uuid_ = uuid;
 
   DVLOG(1) << uuid_.canonical_value() << ": Registering RFCOMM service.";
   BluetoothRFCOMMChannelID registered_channel_id;
-  service_record_handle_ =
-      RegisterRfcommService(uuid, options, &registered_channel_id);
-  if (service_record_handle_ == kInvalidServiceRecordHandle) {
+  service_record_.reset(
+      RegisterRfcommService(uuid, options, &registered_channel_id));
+  if (!service_record_.get()) {
     error_callback.Run(kInvalidOrUsedChannel);
     return;
   }
 
   rfcomm_connection_listener_.reset(
       [[BluetoothRfcommConnectionListener alloc]
           initWithSocket:this
                channelID:registered_channel_id]);
 
   success_callback.Run();
 }
 
 void BluetoothSocketMac::ListenUsingL2cap(
     scoped_refptr<BluetoothAdapterMac> adapter,
     const BluetoothUUID& uuid,
     const BluetoothAdapter::ServiceOptions& options,
     const base::Closure& success_callback,
     const ErrorCompletionCallback& error_callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   adapter_ = adapter;
   uuid_ = uuid;
 
   DVLOG(1) << uuid_.canonical_value() << ": Registering L2CAP service.";
   BluetoothL2CAPPSM registered_psm;
-  service_record_handle_ = RegisterL2capService(uuid, options, &registered_psm);
-  if (service_record_handle_ == kInvalidServiceRecordHandle) {
+  service_record_.reset(RegisterL2capService(uuid, options, &registered_psm));
+  if (!service_record_.get()) {
     error_callback.Run(kInvalidOrUsedPsm);
     return;
   }
 
   l2cap_connection_listener_.reset(
       [[BluetoothL2capConnectionListener alloc] initWithSocket:this
                                                            psm:registered_psm]);
 
   success_callback.Run();
 }
@@ skipping 121 matching lines @@
   }
 
   temp->success_callback.Run();
 }
 
 void BluetoothSocketMac::Close() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (channel_)
     ReleaseChannel();
-  else if (service_record_handle_ != kInvalidServiceRecordHandle)
+  else if (service_record_.get())
     ReleaseListener();
 }
 
 void BluetoothSocketMac::Disconnect(const base::Closure& callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   Close();
   callback.Run();
 }
 
@@ skipping 224 matching lines @@
 BluetoothSocketMac::SendRequest::~SendRequest() {}
 
 BluetoothSocketMac::ReceiveCallbacks::ReceiveCallbacks() {}
 
 BluetoothSocketMac::ReceiveCallbacks::~ReceiveCallbacks() {}
 
 BluetoothSocketMac::ConnectCallbacks::ConnectCallbacks() {}
 
 BluetoothSocketMac::ConnectCallbacks::~ConnectCallbacks() {}
 
-BluetoothSocketMac::BluetoothSocketMac()
-    : service_record_handle_(kInvalidServiceRecordHandle) {
-}
+BluetoothSocketMac::BluetoothSocketMac() {}
 
 BluetoothSocketMac::~BluetoothSocketMac() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!channel_);
   DCHECK(!rfcomm_connection_listener_);
 }
 
 void BluetoothSocketMac::ReleaseChannel() {
   DCHECK(thread_checker_.CalledOnValidThread());
   channel_.reset();
 
   // Closing the channel above prevents the callback delegate from being called
   // so it is now safe to release all callback state.
   connect_callbacks_.reset();
   receive_callbacks_.reset();
   empty_queue(receive_queue_);
   empty_queue(send_queue_);
 }
 
 void BluetoothSocketMac::ReleaseListener() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK_NE(service_record_handle_, kInvalidServiceRecordHandle);
+  DCHECK(service_record_.get());
 
-  IOBluetoothRemoveServiceWithRecordHandle(service_record_handle_);
+  [service_record_ removeServiceRecord];
+  service_record_.reset();
   rfcomm_connection_listener_.reset();
   l2cap_connection_listener_.reset();
 
   // Destroying the listener above prevents the callback delegate from being
   // called so it is now safe to release all callback state.
   accept_request_.reset();
   empty_queue(accept_queue_);
 }
 
 }  // namespace device