mac: Fix launchd test on 10.10

mac: Fix launchd test on 10.10

ServiceProcessStateFileManipulationTest.VerifyLaunchD tests that the user owned
launchd process is functioning correctly (it will stop working if
/tmp/launchd*/sock is deleted). This is no longer relevant in 10.10. There is
no user owned launchd process, and there is no file at /tmp/launchd*/sock.

Previously, the test checked that "launchctl list com.apple.launchctl.Aqua" had
exit code 0. This doesn't work on 10.10, since that particular launchctl
listing is for the user-owned instance of launchd, which no longer exists. I
changed the test to check the exit code for "launchctl limit". This fails on
10.9 if /tmp/launchd*/sock is deleted, which is the intended behavior of the
test, and it still serves as a useful sanity check on 10.10.

I also added a new test to explicitly check that GTMSMJobSubmit and
GTMSMJobRemove work correctly.

BUG=390276
Committed: https://crrev.com/51645a45bf568d614b6cd44d9cfe065fabd488c8
Cr-Commit-Position: refs/heads/master@{#296467}

[erikchen, 2014-09-20 00:42:17]

Patchset #1 (id:1) has been deleted

[erikchen, 2014-09-20 00:42:59]

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

[erikchen, 2014-09-20 00:43:00]

mark: Please review.

[Mark Mentovai, 2014-09-20 02:43:53]

I think that this is testing launchctl because some other code (probably still
GTMSMJobSubmit()) uses it and we want to make sure it works. That’s still a
valid thing to test, even if launchd no longer has a named socket-based
interface.

It would be better to test the interface that it’s actually using, but absent
that, if it’s still using GTMSMJobSubmit() and GTMSMJobSubmit() is still running
launchctl, it’s not unreasonable to make sure that launchctl works.
com.apple.launchctl.Aqua may no longer exist, but I’m sure you can find another
way to sanity-check that we know how to talk to launchctl.

[erikchen, 2014-09-23 02:55:57]

mark: PTAL.

Your comment has several references to launchctl where I think you meant
launchd. Nevertheless, your rationale that we should sanity check launch(d) on
10.10 is still reasonable, and I've updated the test to be 10.9 and 10.10
compatible.

[Mark Mentovai, 2014-09-23 03:20:47]

No, I meant launchctl. Take a look at the implementation of GTMSMJobSubmit(),
third_party/google_toolbox_for_mac/src/Foundation/GTMServiceManagement.c.

[Mark Mentovai, 2014-09-23 03:24:20]

LGTM.

I’d prefer a test that verifies that GTMSMJobSubmit() and GTMSMJobRemove()
actually work since those are the interfaces that we are using in Chrome, but
since they’re known to use launchctl under the hood, making sure that launchctl
operates is a second-best thing.

Crashpad/bpoop has an example of the sort of test I’m talking about:
https://chromium.googlesource.com/crashpad/crashpad/+/75988925f9161e4c339f3a0....

[erikchen, 2014-09-23 19:13:07]

Patchset #3 (id:60001) has been deleted

[erikchen, 2014-09-23 19:15:54]

On 2014/09/23 03:24:20, Mark Mentovai wrote:
> LGTM.
> 
> I’d prefer a test that verifies that GTMSMJobSubmit() and GTMSMJobRemove()
> actually work since those are the interfaces that we are using in Chrome, but
> since they’re known to use launchctl under the hood, making sure that
launchctl
> operates is a second-best thing.
> 
> Crashpad/bpoop has an example of the sort of test I’m talking about:
>
https://chromium.googlesource.com/crashpad/crashpad/+/75988925f9161e4c339f3a0....

mark: PTAL

I added a test for GTMSMJobSubmit and GTMSMJobRemove, in the style of the one
from crashpad. I kept the launchd test, since it's not unreasonable to sanity
check that launchd is working.

[Mark Mentovai, 2014-09-23 19:34:31]

LGTM

[erikchen, 2014-09-23 19:58:35]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2014-09-23 20:31:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/588833002/80001

[commit-bot: I haz the power, 2014-09-23 20:53:55]

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

[commit-bot: I haz the power, 2014-09-23 20:53:57]

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

[erikchen, 2014-09-23 20:56:19]

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

[erikchen, 2014-09-23 20:56:20]

thestig: Looking for an OWNER review.

[Lei Zhang, 2014-09-23 21:59:07]

lgtm stamp with a nit.

https://codereview.chromium.org/588833002/diff/80001/chrome/chrome_tests_unit...
File chrome/chrome_tests_unit.gypi (right):

https://codereview.chromium.org/588833002/diff/80001/chrome/chrome_tests_unit...
chrome/chrome_tests_unit.gypi:2393:
'browser/service_process/service_process_control_mac_unittest.mm',
You can put this in the general list and build/filename_rules.gypi is smart
enough to figure out this is Mac-only.

[erikchen, 2014-09-24 17:19:03]

https://codereview.chromium.org/588833002/diff/80001/chrome/chrome_tests_unit...
File chrome/chrome_tests_unit.gypi (right):

https://codereview.chromium.org/588833002/diff/80001/chrome/chrome_tests_unit...
chrome/chrome_tests_unit.gypi:2393:
'browser/service_process/service_process_control_mac_unittest.mm',
On 2014/09/23 21:59:06, Lei Zhang wrote:
> You can put this in the general list and build/filename_rules.gypi is smart
> enough to figure out this is Mac-only.

Done.

[erikchen, 2014-09-24 17:19:06]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2014-09-24 17:20:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/588833002/100001

[commit-bot: I haz the power, 2014-09-24 18:19:14]

Committed patchset #4 (id:100001) as 9994853be8007506b741221cc7b890165545a4db

[commit-bot: I haz the power, 2014-09-24 18:20:08]

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