[Chromoting] Update uninstaller to facilitate testing automation.

[Chromoting] Update uninstaller to facilitate testing automation.

With this change, if launched from shell as root, the uninstaller will:
* Not show any confirmation UX
* Not prompt for admin elevation

Also in this cl, cleanup some of the shared Mac constants by pushing
more into into constants_mac and fixing up the naming style. This
required minor updates to the prefpane and daemon controller.

BUG=None
TEST=None
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=148442

[Lambros, 2012-07-23 20:42:35]

http://codereview.chromium.org/10807061/diff/1/remoting/host/installer/mac/un...
File remoting/host/installer/mac/uninstaller/remoting_uninstaller.mm (right):

http://codereview.chromium.org/10807061/diff/1/remoting/host/installer/mac/un...
remoting/host/installer/mac/uninstaller/remoting_uninstaller.mm:86: // We're
running as root, so just run the command directly.
This feels wrong to me - are you absolutely sure you need to special-case NULL
here?  I'd expect that, if you are already running as root, you could still call
exactly the same code, and AuthorizationCopyRights would be smart enough to not
bother prompting for a password (because the process is already root, and the
AuthService should grant the rights automatically).  It would return success,
and you should be able to pass the resulting token to ExecuteWithPrivileges().

[Lambros, 2012-07-23 22:06:46]

LGTM with some optional nits.  I'm still curious to know why the special-casing
of NULL auth ref is needed here?

http://codereview.chromium.org/10807061/diff/1/remoting/host/installer/mac/un...
File remoting/host/installer/mac/uninstaller/remoting_uninstaller.mm (right):

http://codereview.chromium.org/10807061/diff/1/remoting/host/installer/mac/un...
remoting/host/installer/mac/uninstaller/remoting_uninstaller.mm:31: -
(NSArray*)convertToNSArray:(const char**)array {
Is it normal ObjC++ style to add these kinds of private utility functions as
member methods and not put them in the interface declaration?  In this case, it
doesn't use "self" anywhere, so maybe it makes more sense to put this as a plain
function in an anonymous namespace?

http://codereview.chromium.org/10807061/diff/1/remoting/host/installer/mac/un...
File remoting/host/installer/mac/uninstaller/remoting_uninstaller_app.mm
(right):

http://codereview.chromium.org/10807061/diff/1/remoting/host/installer/mac/un...
remoting/host/installer/mac/uninstaller/remoting_uninstaller_app.mm:96: int euid
= geteuid();
nit: s/int/uid_t, but I think it's OK to get rid of the temporary in this case.

http://codereview.chromium.org/10807061/diff/1/remoting/host/plugin/daemon_co...
File remoting/host/plugin/daemon_controller_mac.cc (right):

http://codereview.chromium.org/10807061/diff/1/remoting/host/plugin/daemon_co...
remoting/host/plugin/daemon_controller_mac.cc:304: CFRelease(service_name);
Won't the scoper call CFRelease() anyway?

[garykac, 2012-07-23 22:41:09]

ptal

http://codereview.chromium.org/10807061/diff/1/remoting/host/installer/mac/un...
File remoting/host/installer/mac/uninstaller/remoting_uninstaller.mm (right):

http://codereview.chromium.org/10807061/diff/1/remoting/host/installer/mac/un...
remoting/host/installer/mac/uninstaller/remoting_uninstaller.mm:31: -
(NSArray*)convertToNSArray:(const char**)array {
On 2012/07/23 22:06:47, Lambros wrote:
> Is it normal ObjC++ style to add these kinds of private utility functions as
> member methods and not put them in the interface declaration?  In this case,
it
> doesn't use "self" anywhere, so maybe it makes more sense to put this as a
plain
> function in an anonymous namespace?

Not sure about style. I moved it out as a plain function.

http://codereview.chromium.org/10807061/diff/1/remoting/host/installer/mac/un...
remoting/host/installer/mac/uninstaller/remoting_uninstaller.mm:86: // We're
running as root, so just run the command directly.
On 2012/07/23 20:42:35, Lambros wrote:
> This feels wrong to me - are you absolutely sure you need to special-case NULL
> here?  I'd expect that, if you are already running as root, you could still
call
> exactly the same code, and AuthorizationCopyRights would be smart enough to
not
> bother prompting for a password (because the process is already root, and the
> AuthService should grant the rights automatically).  It would return success,
> and you should be able to pass the resulting token to ExecuteWithPrivileges().

Hmm.. Previous tests made me think this was required to avoid an elevation
prompt but I must have been doing them wrong.

I tried it now and using sudo skipped the prompt.

I updated the code to remove the nil check.

http://codereview.chromium.org/10807061/diff/1/remoting/host/installer/mac/un...
File remoting/host/installer/mac/uninstaller/remoting_uninstaller_app.mm
(right):

http://codereview.chromium.org/10807061/diff/1/remoting/host/installer/mac/un...
remoting/host/installer/mac/uninstaller/remoting_uninstaller_app.mm:96: int euid
= geteuid();
On 2012/07/23 22:06:47, Lambros wrote:
> nit: s/int/uid_t, but I think it's OK to get rid of the temporary in this
case.

No longer applicable

http://codereview.chromium.org/10807061/diff/1/remoting/host/plugin/daemon_co...
File remoting/host/plugin/daemon_controller_mac.cc (right):

http://codereview.chromium.org/10807061/diff/1/remoting/host/plugin/daemon_co...
remoting/host/plugin/daemon_controller_mac.cc:304: CFRelease(service_name);
On 2012/07/23 22:06:47, Lambros wrote:
> Won't the scoper call CFRelease() anyway?

Done.

[dcaiafa, 2012-07-23 23:16:01]

LGTM after comment is addressed.

http://codereview.chromium.org/10807061/diff/2002/remoting/host/installer/mac...
File remoting/host/installer/mac/uninstaller/remoting_uninstaller_app.mm
(right):

http://codereview.chromium.org/10807061/diff/2002/remoting/host/installer/mac...
remoting/host/installer/mac/uninstaller/remoting_uninstaller_app.mm:26:
CFUserNotificationDisplayAlert(0, kCFUserNotificationNoteAlertLevel,
CF23Alert should only be used when the application is mostly incapable of doing
UI. Otherwise, NSAlert is the Cocoa friendly way to do dialog boxes.

[Lambros, 2012-07-23 23:48:54]

Just a typo, otherwise lgtm

http://codereview.chromium.org/10807061/diff/2002/remoting/host/installer/mac...
File remoting/host/installer/mac/uninstaller/remoting_uninstaller_app.mm
(right):

http://codereview.chromium.org/10807061/diff/2002/remoting/host/installer/mac...
remoting/host/installer/mac/uninstaller/remoting_uninstaller_app.mm:80: // The
no-ui option skips the UI confirmatino dialogs. This is provided as
typo: confirmatino

[garykac, 2012-07-24 00:33:10]

On 2012/07/23 23:48:54, Lambros wrote:
> Just a typo, otherwise lgtm
> 
>
http://codereview.chromium.org/10807061/diff/2002/remoting/host/installer/mac...
> File remoting/host/installer/mac/uninstaller/remoting_uninstaller_app.mm
> (right):
> 
>
http://codereview.chromium.org/10807061/diff/2002/remoting/host/installer/mac...
> remoting/host/installer/mac/uninstaller/remoting_uninstaller_app.mm:80: // The
> no-ui option skips the UI confirmatino dialogs. This is provided as
> typo: confirmatino

Oops. I slipped into Italian for a bit there.

[garykac, 2012-07-24 00:33:33]

On 2012/07/23 23:16:01, dcaiafa wrote:
> LGTM after comment is addressed.
> 
>
http://codereview.chromium.org/10807061/diff/2002/remoting/host/installer/mac...
> File remoting/host/installer/mac/uninstaller/remoting_uninstaller_app.mm
> (right):
> 
>
http://codereview.chromium.org/10807061/diff/2002/remoting/host/installer/mac...
> remoting/host/installer/mac/uninstaller/remoting_uninstaller_app.mm:26:
> CFUserNotificationDisplayAlert(0, kCFUserNotificationNoteAlertLevel,
> CF23Alert should only be used when the application is mostly incapable of
doing
> UI. Otherwise, NSAlert is the Cocoa friendly way to do dialog boxes.

Done.

[commit-bot: I haz the power, 2012-07-25 16:44:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/garykac@chromium.org/10807061/11002

[commit-bot: I haz the power, 2012-07-25 16:44:50]

Can't process patch for file
remoting/host/installer/mac/uninstaller/remoting_uninstaller_app.mm.
Unsupported svn property format.