mac: Stop using deprecated FSRef functions in service_process_util_mac.mm

mac: Stop using deprecated FSRef functions in service_process_util_mac.mm

Original CL by thakis at: https://codereview.chromium.org/2122283002/.

> Instead of an FSRef, use a fileReferenceURL, the replacement.
>
> The replacement function to check if a file is in the trash
> only exists in 10.10+, but the old way was deprecated in 10.8,
> so call the new code on 10.10+ and call the old code on 10.9
> (we can delete that code path once we stop supporting 10.9).
>
> No behavior change intended.

BUG=74983, 622481
TEST= https://bugs.chromium.org/p/chromium/issues/detail?id=74983#c16

Committed: https://crrev.com/29c978360378e82108b2f1d336228995cd8dedb6
Cr-Commit-Position: refs/heads/master@{#424289}

[erikchen, 2016-10-06 20:33:17]

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

[commit-bot: I haz the power, 2016-10-06 20:33:51]

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

[erikchen, 2016-10-06 20:35:32]

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

[erikchen, 2016-10-06 20:35:32]

thakis: Please review.

This logic is meant to run when the service is still registered in launchctl,
but Chrome has exited. I wasn't able to reproduce this in normal Chrome
behavior.  

I modified ForceServiceProcessShutdown() to return true but do nothing, and was
able to reproduce the desired condition. I tested that even with the new logic,
moving Chrome to the trash would immediately deregister the service.

[commit-bot: I haz the power, 2016-10-06 21:36:15]

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

[commit-bot: I haz the power, 2016-10-06 21:36:16]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[erikchen, 2016-10-07 00:18:44]

https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
File chrome/common/service_process_util_mac.mm (right):

https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
chrome/common/service_process_util_mac.mm:385: // This is ok because it only
happens on 10.9 and won't be needed once
clang format. *shrug*

https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
File chrome/common/service_process_util_mac_unittest.mm (right):

https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
chrome/common/service_process_util_mac_unittest.mm:38: void TrashFunc(const
base::FilePath& src) {
Most of this file was ripped out of service_process_util_unittest. This function
is the only change.

[erikchen, 2016-10-07 00:18:47]

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

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

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-07 02:21:22]

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

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

Dry run: This issue passed the CQ dry run.

[erikchen, 2016-10-10 19:07:04]

On 2016/10/07 00:18:44, erikchen wrote:
>
https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
> File chrome/common/service_process_util_mac.mm (right):
> 
>
https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
> chrome/common/service_process_util_mac.mm:385: // This is ok because it only
> happens on 10.9 and won't be needed once
> clang format. *shrug*
> 
>
https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
> File chrome/common/service_process_util_mac_unittest.mm (right):
> 
>
https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
> chrome/common/service_process_util_mac_unittest.mm:38: void TrashFunc(const
> base::FilePath& src) {
> Most of this file was ripped out of service_process_util_unittest. This
function
> is the only change.

thakis: Ping.

[Nico, 2016-10-10 19:14:15]

lgtm

https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
File chrome/common/service_process_util_mac.mm (right):

https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
chrome/common/service_process_util_mac.mm:364: // 10.8, but didn't ad
getRelationship:... until 10.10.  So fall back to
s/ad/add/

(who wrote this comment?! :-P)

https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
chrome/common/service_process_util_mac.mm:385: // This is ok because it only
happens on 10.9 and won't be needed once
On 2016/10/07 00:18:44, erikchen wrote:
> clang format. *shrug*

If you move the comment after the pragma. But as-is is fine too.

https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
File chrome/common/service_process_util_mac_unittest.mm (right):

https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
chrome/common/service_process_util_mac_unittest.mm:38: void TrashFunc(const
base::FilePath& src) {
On 2016/10/07 00:18:44, erikchen wrote:
> Most of this file was ripped out of service_process_util_unittest. This
function
> is the only change.

(fwiw I think ripping out as-is in one cl and then changing in another would've
been much clearer)

https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
chrome/common/service_process_util_mac_unittest.mm:45: error:nil];
When I looked at using trashItemAtURL: a while ago I found some reports of it
not actually working very well (eg
https://github.com/sindresorhus/macos-trash/issues/4). I guess this is just a
test and this might be fine for the test, but in general this might cause
issues.

[erikchen, 2016-10-10 22:49:31]

https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
File chrome/common/service_process_util_mac.mm (right):

https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
chrome/common/service_process_util_mac.mm:364: // 10.8, but didn't ad
getRelationship:... until 10.10.  So fall back to
On 2016/10/10 19:14:14, Nico wrote:
> s/ad/add/
> 
> (who wrote this comment?! :-P)

Done.

https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
chrome/common/service_process_util_mac.mm:385: // This is ok because it only
happens on 10.9 and won't be needed once
On 2016/10/10 19:14:14, Nico wrote:
> On 2016/10/07 00:18:44, erikchen wrote:
> > clang format. *shrug*
> 
> If you move the comment after the pragma. But as-is is fine too.

Done.

https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
File chrome/common/service_process_util_mac_unittest.mm (right):

https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
chrome/common/service_process_util_mac_unittest.mm:38: void TrashFunc(const
base::FilePath& src) {
On 2016/10/10 19:14:15, Nico wrote:
> On 2016/10/07 00:18:44, erikchen wrote:
> > Most of this file was ripped out of service_process_util_unittest. This
> function
> > is the only change.
> 
> (fwiw I think ripping out as-is in one cl and then changing in another
would've
> been much clearer)

Yeah, guess so.

https://codereview.chromium.org/2398243002/diff/40001/chrome/common/service_p...
chrome/common/service_process_util_mac_unittest.mm:45: error:nil];
On 2016/10/10 19:14:14, Nico wrote:
> When I looked at using trashItemAtURL: a while ago I found some reports of it
> not actually working very well (eg
> https://github.com/sindresorhus/macos-trash/issues/4). I guess this is just a
> test and this might be fine for the test, but in general this might cause
> issues.

Noted. I guess we can revisit if there are test issues.

[erikchen, 2016-10-10 22:49:37]

The CQ bit was checked by erikchen@chromium.org

[erikchen, 2016-10-10 22:49:38]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/2398243002/#ps60001
(title: "Comments from thakis.")

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

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-10 23:44:24]

Committed patchset #4 (id:60001)

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

Description was changed from

==========
mac: Stop using deprecated FSRef functions in service_process_util_mac.mm

Original CL by thakis at: https://codereview.chromium.org/2122283002/.

> Instead of an FSRef, use a fileReferenceURL, the replacement.
>
> The replacement function to check if a file is in the trash
> only exists in 10.10+, but the old way was deprecated in 10.8,
> so call the new code on 10.10+ and call the old code on 10.9
> (we can delete that code path once we stop supporting 10.9).
>
> No behavior change intended.

BUG=74983,622481
TEST= https://bugs.chromium.org/p/chromium/issues/detail?id=74983#c16
==========

to

==========
mac: Stop using deprecated FSRef functions in service_process_util_mac.mm

Original CL by thakis at: https://codereview.chromium.org/2122283002/.

> Instead of an FSRef, use a fileReferenceURL, the replacement.
>
> The replacement function to check if a file is in the trash
> only exists in 10.10+, but the old way was deprecated in 10.8,
> so call the new code on 10.10+ and call the old code on 10.9
> (we can delete that code path once we stop supporting 10.9).
>
> No behavior change intended.

BUG=74983,622481
TEST= https://bugs.chromium.org/p/chromium/issues/detail?id=74983#c16

Committed: https://crrev.com/29c978360378e82108b2f1d336228995cd8dedb6
Cr-Commit-Position: refs/heads/master@{#424289}
==========

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

Patchset 4 (id:??) landed as
https://crrev.com/29c978360378e82108b2f1d336228995cd8dedb6
Cr-Commit-Position: refs/heads/master@{#424289}