[Mac] Stop using deprecated IOBluetoothSDPServiceRecordRef APIs.

[Mac] Stop using deprecated IOBluetoothSDPServiceRecordRef APIs.

When compiling against the 10.9 SDK, IOBluetoothAddServiceDict() and
IOBluetoothRemoveServiceWithRecordHandle() generate deprecation errors.
This cl removes the instances of this old API.

R=isherman@chromium.org
BUG=650836
Committed: https://crrev.com/07aa6dca25c20da6a14d96eca46b8b4b639c5117
Cr-Commit-Position: refs/heads/master@{#427184}

[shrike, 2016-10-04 23:12:22]

PTAL

Also, are there (unit/browser)_tests set up to test this class? It wasn't
immediately obvious.

[Ilya Sherman, 2016-10-05 00:49:56]

Sorry, it has been a long enough time since I've worked on Bluetooth code to
where I don't feel comfortable reviewing this CL.  Could you please loop in one
of the //device/bluetooth OWNERS?

[shrike, 2016-10-05 17:52:47]

Description was changed from

==========
[Mac] Stop using depcreated IOBluetoothSDPServiceRecordRef APIs.

When compiling against the 10.9 SDK, IOBluetoothAddServiceDict() and
IOBluetoothRemoveServiceWithRecordHandle() generate deprecation errors.
This cl removes the instances of this old API.

R=isherman@chromium.org
BUG=650836
==========

to

==========
[Mac] Stop using depcreated IOBluetoothSDPServiceRecordRef APIs.

When compiling against the 10.9 SDK, IOBluetoothAddServiceDict() and
IOBluetoothRemoveServiceWithRecordHandle() generate deprecation errors.
This cl removes the instances of this old API.

R= ortuno@chromium.org
BUG=650836
==========

[shrike, 2016-10-05 17:52:47]

shrike@chromium.org changed reviewers:
+ ortuno@chromium.org
- isherman@chromium.org

[shrike, 2016-10-05 17:53:50]

Hello ortuno@,

PTAL at this change (or let me know who would be a good person to do so). I'm
also trying to understand what kind of test coverage exists for this class I'm
touching.

[shrike, 2016-10-05 17:54:12]

On 2016/10/05 00:49:56, Ilya Sherman wrote:
> Sorry, it has been a long enough time since I've worked on Bluetooth code to
> where I don't feel comfortable reviewing this CL.  Could you please loop in
one
> of the //device/bluetooth OWNERS?

OK, thank you.

[shrike, 2016-10-05 18:28:36]

Description was changed from

==========
[Mac] Stop using depcreated IOBluetoothSDPServiceRecordRef APIs.

When compiling against the 10.9 SDK, IOBluetoothAddServiceDict() and
IOBluetoothRemoveServiceWithRecordHandle() generate deprecation errors.
This cl removes the instances of this old API.

R= ortuno@chromium.org
BUG=650836
==========

to

==========
[Mac] Stop using deprecated IOBluetoothSDPServiceRecordRef APIs.

When compiling against the 10.9 SDK, IOBluetoothAddServiceDict() and
IOBluetoothRemoveServiceWithRecordHandle() generate deprecation errors.
This cl removes the instances of this old API.

R= ortuno@chromium.org
BUG=650836
==========

[ortuno, 2016-10-06 02:27:22]

isherman: Neither of the current OWNERS has worked with that part of the API. I
can take a look but I would appreciate it if you could do a first pass since you
are probably the person that knows the most about that code.

[shrike, 2016-10-10 23:51:53]

Description was changed from

==========
[Mac] Stop using deprecated IOBluetoothSDPServiceRecordRef APIs.

When compiling against the 10.9 SDK, IOBluetoothAddServiceDict() and
IOBluetoothRemoveServiceWithRecordHandle() generate deprecation errors.
This cl removes the instances of this old API.

R= ortuno@chromium.org
BUG=650836
==========

to

==========
[Mac] Stop using deprecated IOBluetoothSDPServiceRecordRef APIs.

When compiling against the 10.9 SDK, IOBluetoothAddServiceDict() and
IOBluetoothRemoveServiceWithRecordHandle() generate deprecation errors.
This cl removes the instances of this old API.

R=isherman@chromium.org
BUG=650836
==========

[shrike, 2016-10-10 23:51:53]

shrike@chromium.org changed reviewers:
+ isherman@chromium.org
- ortuno@chromium.org

[shrike, 2016-10-10 23:52:10]

Hello Ilya, PTAL

[Ilya Sherman, 2016-10-10 23:57:11]

On 2016/10/06 02:27:22, ortuno wrote:
> isherman: Neither of the current OWNERS has worked with that part of the API.
I
> can take a look but I would appreciate it if you could do a first pass since
you
> are probably the person that knows the most about that code.

Oof, okay.  At a glance, the code changes seem okay.  I'll need to read up on
the Mac Bluetooth APIs to remind myself of the deeper context... I can do that,
but it'll probably take me a couple of days to get around to it -- sorry!

[erikchen, 2016-10-14 16:51:01]

On 2016/10/10 23:57:11, Ilya Sherman wrote:
> On 2016/10/06 02:27:22, ortuno wrote:
> > isherman: Neither of the current OWNERS has worked with that part of the
API.
> I
> > can take a look but I would appreciate it if you could do a first pass since
> you
> > are probably the person that knows the most about that code.
> 
> Oof, okay.  At a glance, the code changes seem okay.  I'll need to read up on
> the Mac Bluetooth APIs to remind myself of the deeper context... I can do
that,
> but it'll probably take me a couple of days to get around to it -- sorry!

isherman: Ping?

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

Okay, I think this LGTM % two comments.  I'm still not super certain that I'm
not missing any subtle differences between the previous API and the new one, but
I think it's probably the right migration.  Sorry for the delay, and thanks for
your patience!

https://codereview.chromium.org/2393723002/diff/1/device/bluetooth/bluetooth_...
File device/bluetooth/bluetooth_socket_mac.mm (right):

https://codereview.chromium.org/2393723002/diff/1/device/bluetooth/bluetooth_...
device/bluetooth/bluetooth_socket_mac.mm:344: IOBluetoothSDPServiceRecord*
RegisterService(
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.

https://codereview.chromium.org/2393723002/diff/1/device/bluetooth/bluetooth_...
device/bluetooth/bluetooth_socket_mac.mm:350:
publishedServiceRecordWithDictionary:service_definition];
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:19]

https://codereview.chromium.org/2393723002/diff/1/device/bluetooth/bluetooth_...
File device/bluetooth/bluetooth_socket_mac.mm (right):

https://codereview.chromium.org/2393723002/diff/1/device/bluetooth/bluetooth_...
device/bluetooth/bluetooth_socket_mac.mm:344: IOBluetoothSDPServiceRecord*
RegisterService(
On 2016/10/20 01:11:05, Ilya Sherman wrote:
> 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.

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.

https://codereview.chromium.org/2393723002/diff/1/device/bluetooth/bluetooth_...
device/bluetooth/bluetooth_socket_mac.mm:350:
publishedServiceRecordWithDictionary:service_definition];
On 2016/10/20 01:11:05, Ilya Sherman wrote:
> 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! =)

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]

https://codereview.chromium.org/2393723002/diff/1/device/bluetooth/bluetooth_...
File device/bluetooth/bluetooth_socket_mac.mm (right):

https://codereview.chromium.org/2393723002/diff/1/device/bluetooth/bluetooth_...
device/bluetooth/bluetooth_socket_mac.mm:344: IOBluetoothSDPServiceRecord*
RegisterService(
On 2016/10/20 19:26:18, shrike wrote:
> On 2016/10/20 01:11:05, Ilya Sherman wrote:
> > 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.
> 
> 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.

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.

https://codereview.chromium.org/2393723002/diff/1/device/bluetooth/bluetooth_...
device/bluetooth/bluetooth_socket_mac.mm:350:
publishedServiceRecordWithDictionary:service_definition];
On 2016/10/20 19:26:18, shrike wrote:
> On 2016/10/20 01:11:05, Ilya Sherman wrote:
> > 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! =)
> 
> 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.

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 =)

[erikchen, 2016-10-21 01:17:59]

erikchen@chromium.org changed reviewers:
+ erikchen@chromium.org

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

https://codereview.chromium.org/2393723002/diff/1/device/bluetooth/bluetooth_...
File device/bluetooth/bluetooth_socket_mac.mm (right):

https://codereview.chromium.org/2393723002/diff/1/device/bluetooth/bluetooth_...
device/bluetooth/bluetooth_socket_mac.mm:344: IOBluetoothSDPServiceRecord*
RegisterService(
On 2016/10/20 19:37:35, Ilya Sherman wrote:
> On 2016/10/20 19:26:18, shrike wrote:
> > On 2016/10/20 01:11:05, Ilya Sherman wrote:
> > > 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.
> > 
> > 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.
> 
> 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.

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.

[erikchen, 2016-10-21 01:18:06]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[erikchen, 2016-10-21 01:18:14]

The CQ bit was unchecked by erikchen@chromium.org

[erikchen, 2016-10-21 01:18:30]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-21 01:18:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-21 03:55:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-21 03:55:26]

Dry run: This issue passed the CQ dry run.

[shrike, 2016-10-21 23:07:00]

The CQ bit was checked by shrike@chromium.org

[commit-bot: I haz the power, 2016-10-21 23:07:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-22 00:07:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-22 00:08:00]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[erikchen, 2016-10-22 00:11:43]

erikchen@chromium.org changed reviewers:
+ scheib@chromium.org

[erikchen, 2016-10-22 00:11:43]

scheib: Please review.

[scheib, 2016-10-24 21:08:19]

LGTM

[shrike, 2016-10-24 21:50:15]

The CQ bit was checked by shrike@chromium.org

[commit-bot: I haz the power, 2016-10-24 21:50:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-25 00:09:51]

Description was changed from

==========
[Mac] Stop using deprecated IOBluetoothSDPServiceRecordRef APIs.

When compiling against the 10.9 SDK, IOBluetoothAddServiceDict() and
IOBluetoothRemoveServiceWithRecordHandle() generate deprecation errors.
This cl removes the instances of this old API.

R=isherman@chromium.org
BUG=650836
==========

to

==========
[Mac] Stop using deprecated IOBluetoothSDPServiceRecordRef APIs.

When compiling against the 10.9 SDK, IOBluetoothAddServiceDict() and
IOBluetoothRemoveServiceWithRecordHandle() generate deprecation errors.
This cl removes the instances of this old API.

R=isherman@chromium.org
BUG=650836
==========

[commit-bot: I haz the power, 2016-10-25 00:09:54]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-10-25 00:12:39]

Description was changed from

==========
[Mac] Stop using deprecated IOBluetoothSDPServiceRecordRef APIs.

When compiling against the 10.9 SDK, IOBluetoothAddServiceDict() and
IOBluetoothRemoveServiceWithRecordHandle() generate deprecation errors.
This cl removes the instances of this old API.

R=isherman@chromium.org
BUG=650836
==========

to

==========
[Mac] Stop using deprecated IOBluetoothSDPServiceRecordRef APIs.

When compiling against the 10.9 SDK, IOBluetoothAddServiceDict() and
IOBluetoothRemoveServiceWithRecordHandle() generate deprecation errors.
This cl removes the instances of this old API.

R=isherman@chromium.org
BUG=650836

Committed: https://crrev.com/07aa6dca25c20da6a14d96eca46b8b4b639c5117
Cr-Commit-Position: refs/heads/master@{#427184}
==========

[commit-bot: I haz the power, 2016-10-25 00:12:40]

Patchset 1 (id:??) landed as
https://crrev.com/07aa6dca25c20da6a14d96eca46b8b4b639c5117
Cr-Commit-Position: refs/heads/master@{#427184}