In-application Keystone ticket promotion

In-application Keystone ticket promotion.

The concept of "ticket promotion" is added to the application when Keystone is
in use.  Ticket promotion is used to turn a user Keystone ticket, which Chrome
normally establishes when it launches, into a system Keystone ticket, after
successful user authentication and authorization.  Having a system Keystone
with a system ticket means that updates are applied with root privileges
instead of user privileges, essentially eliminating the possibility that a
user will fall off of the auto-update train because they can read and execute
but not write the application.

Two principles of promotion apply:

 - An application on a user ticket NEEDS promotion if it determines that it
   doesn't have permission to write to itself.  Being on a user ticket, an
   update attempt would fail.
 - An application on a user ticket WANTS promotion if it already NEEDS
   promotion.  Additionally, if it is installed in a system-wide location
   such as /Applications, it will WANT promotion, even if it does not NEED it.

If promotion is needed, an info bar will show up on launch requesting it.
This info bar works similarly to the default browser info bar: it has a "don't
bother me again" button, it will only show up after the first launch, it won't
disappear on navigation if the navigation happens very quickly, and it won't
show itself if another info bar is up.  This means that if both the default
browser info bar and the promotion info bar have a shot at showing, only one
will win.  In my experience, each wins about half of the time.

If promotion is needed, the update UI in the About window will be hidden.
Checking for updates and offering to apply them doesn't make much sense when
the update won't be able to install successfully.  All of the auto-update
machinery is still working in the background, but the About window UI is
hidden.

If promotion is wanted, the About window will contain a new button allowing
the user to enter promotion.  This gives access to the same promotion routine
as the promotion info bar.  It can be used even from an administrative account
that is able to update the application without promotion.  It's intended to be
used by the system administrator of the family without requiring them to
switch to one of the kids' accounts.

BUG=16360
TEST=Exhaustively, please.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=33241

[Mark Mentovai, 2009-11-27 06:33:07]

I'm probably 80% done with this.  I still need to write the info bar nag part. 
This is in pretty good shape driving it from the about box.

I'm sending it out now so you've got a little more time to be upset with me for
ruining your extended weekend, and so that I don't wind up dropping it on you
when you're celebrating a soccer victory.

[TVL, 2009-11-28 01:23:53]

> I'm sending it out now so you've got a little more time to be upset with me
for
> ruining your extended weekend, and so that I don't wind up dropping it on you
> when you're celebrating a soccer victory.

No worries, both have finish soccer for the fall (and indoor doesn't start until
January).  :)

[TVL, 2009-11-28 02:43:57]

http://codereview.chromium.org/437053/diff/22/9001
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/437053/diff/22/9001#newcode3322
chrome/app/generated_resources.grd:3322: <ph name="PRODUCT_NAME">$1<ex>Google
Chrome</ex></ph> will set up automatic updates for all users of this computer.
do you want to pull all these in a darwin check since they are mac only?

http://codereview.chromium.org/437053/diff/22/9003
File chrome/app/keystone_glue.mm (left):

http://codereview.chromium.org/437053/diff/22/9003#oldcode388
chrome/app/keystone_glue.mm:388: 
why is this and the header empty?  just delete them?

http://codereview.chromium.org/437053/diff/22/9010
File chrome/browser/cocoa/about_window_controller.mm (right):

http://codereview.chromium.org/437053/diff/22/9010#newcode242
chrome/browser/cocoa/about_window_controller.mm:242: // quickly.
bug files for async?

http://codereview.chromium.org/437053/diff/22/9010#newcode372
chrome/browser/cocoa/about_window_controller.mm:372: [window setFrame:newFrame
display:YES animate:YES];
so these final call seem like they would resize the subviews.  Is that ok or
does the window have that turned off?  if it is turned off, why not just DCHECK
it in awakeFromNib, and skip the GTM helper that does sizing with it turned of?

http://codereview.chromium.org/437053/diff/22/9010#newcode451
chrome/browser/cocoa/about_window_controller.mm:451: bool enablePromoteButton =
true;
is it worth any comment about button enabling has nothing to do with if the
promotion ui is/isn't visible?  ie-we might enable the button but the the view
is hidden.

http://codereview.chromium.org/437053/diff/22/9014
File chrome/browser/cocoa/keystone_glue.h (right):

http://codereview.chromium.org/437053/diff/22/9014#newcode116
chrome/browser/cocoa/keystone_glue.h:116: // perform the update.
so it is only true when wantsPromption is true and there is an update available
(and it can't be installed by the current user)?

http://codereview.chromium.org/437053/diff/22/9013
File chrome/browser/cocoa/keystone_glue.mm (right):

http://codereview.chromium.org/437053/diff/22/9013#newcode72
chrome/browser/cocoa/keystone_glue.mm:72:
authorization:(AuthorizationRef)authorization;
mixed style, function before uses no space before *, this has a space before *

http://codereview.chromium.org/437053/diff/22/9013#newcode262
chrome/browser/cocoa/keystone_glue.mm:262: NSLog(@"registerWithKeystone");
oops

http://codereview.chromium.org/437053/diff/22/9013#newcode526
chrome/browser/cocoa/keystone_glue.mm:526: if (![self isUserTicket] || [self
isOnReadOnlyFilesystem]) {
isn't this the same as what needPromotion will check next?  why check is twice?

http://codereview.chromium.org/437053/diff/22/9013#newcode610
chrome/browser/cocoa/keystone_glue.mm:610: // handle this.
Why is this needed?  i.e.-how would the permissions on this be wrong?

http://codereview.chromium.org/437053/diff/22/9013#newcode634
chrome/browser/cocoa/keystone_glue.mm:634: // mac_util::MainAppBundle().
given how many places related to keystone have this comment, should we go ahead
and have mac_util::KeystoneBundle() so the comment goes in one place, and
if/when we do app shortcuts and it needs to change, it's already in one bottle
neck?

http://codereview.chromium.org/437053/diff/22/9013#newcode700
chrome/browser/cocoa/keystone_glue.mm:700: std::vector<const char*> arguments(2,
static_cast<const char*>(NULL));
why no ctype since it's only 2 entries?

http://codereview.chromium.org/437053/diff/22/9013#newcode710
chrome/browser/cocoa/keystone_glue.mm:710: DLOG(ERROR) <<
"AuthorizationExecuteWithPrivileges: " << status;
do you care about messaging this to the user?

http://codereview.chromium.org/437053/diff/22/9015
File chrome/browser/cocoa/keystone_promotion_infobar.h (right):

http://codereview.chromium.org/437053/diff/22/9015#newcode6
chrome/browser/cocoa/keystone_promotion_infobar.h:6: #define
CHROME_BROWSER_COCOA_KEYSTONE_PROMOTION_INFOBAR_H_
should this just be keystone_infobars.h so it could also do one for update
notifications?

http://codereview.chromium.org/437053/diff/22/9016
File chrome/browser/cocoa/keystone_promotion_infobar.mm (right):

http://codereview.chromium.org/437053/diff/22/9016#newcode186
chrome/browser/cocoa/keystone_promotion_infobar.mm:186: if (tabContents &&
tabContents->infobar_delegate_count() == 0) {
why check the count?  this means a password prompt for default browser could
block his from showing, right?  meaning hey might never get it if they always
ignore the default browser one?

http://codereview.chromium.org/437053/diff/22/9023
File chrome/tools/build/mac/promote_preflight.sh (right):

http://codereview.chromium.org/437053/diff/22/9023#newcode35
chrome/tools/build/mac/promote_preflight.sh:35: find "${LIB_GOOG_GSU}" -type l
-exec chmod -h "${CHMOD_MODE}" {} + >& \
standard comment about why it's needed?

[Mark Mentovai, 2009-11-28 04:56:03]

Comments addressed (anything not specifically responded to here has been
addressed, and most of the things that I'm responding to here are addressed
too.)

Ready for another look.

http://codereview.chromium.org/437053/diff/22/9003
File chrome/app/keystone_glue.mm (left):

http://codereview.chromium.org/437053/diff/22/9003#oldcode388
chrome/app/keystone_glue.mm:388: 
TVL wrote:
> why is this and the header empty?  just delete them?

It is deleted, but for some reason, Rietveld is showing it as "M" instead of
"D".  Maybe because it was "M" in an earlier version of the change before I
moved the file.

http://codereview.chromium.org/437053/diff/22/9010
File chrome/browser/cocoa/about_window_controller.mm (right):

http://codereview.chromium.org/437053/diff/22/9010#newcode242
chrome/browser/cocoa/about_window_controller.mm:242: // quickly.
TVL wrote:
> bug files for async?

Yeah, and I marked the three spots where it's significant.

http://codereview.chromium.org/437053/diff/22/9010#newcode372
chrome/browser/cocoa/about_window_controller.mm:372: [window setFrame:newFrame
display:YES animate:YES];
TVL wrote:
> so these final call seem like they would resize the subviews.  Is that ok or
> does the window have that turned off?  if it is turned off, why not just
DCHECK
> it in awakeFromNib, and skip the GTM helper that does sizing with it turned
of?

We discussed this one.  Doing the resizes without GTM, even with autoresizing
subviews turned off, doesn't work.  I think it has to be on for the legal block
to get laid out properly.  Doing the resizes this way does work.

http://codereview.chromium.org/437053/diff/22/9010#newcode451
chrome/browser/cocoa/about_window_controller.mm:451: bool enablePromoteButton =
true;
TVL wrote:
> is it worth any comment about button enabling has nothing to do with if the
> promotion ui is/isn't visible?  ie-we might enable the button but the the view
> is hidden.

Yeah, but the comment's at the bottom where I call -[NSButton setEnabled:] on
these guys.  I can move it up here if you think that would be better?

http://codereview.chromium.org/437053/diff/22/9014
File chrome/browser/cocoa/keystone_glue.h (right):

http://codereview.chromium.org/437053/diff/22/9014#newcode116
chrome/browser/cocoa/keystone_glue.h:116: // perform the update.
TVL wrote:
> so it is only true when wantsPromption is true and there is an update
available
> (and it can't be installed by the current user)?

needs implies wants, but we can want promotion without needing it.

We'll want promotion if we're on a user ticket and living in /Applications,
regardless of whether we actually need it.

This is intended to allow admin users to set up auto-update from their own
account.  We'll never nag them with an info bar (because we never NEED
promotion), but we'd like to be promoted.

I'll put something like this in the comment.

http://codereview.chromium.org/437053/diff/22/9013
File chrome/browser/cocoa/keystone_glue.mm (right):

http://codereview.chromium.org/437053/diff/22/9013#newcode634
chrome/browser/cocoa/keystone_glue.mm:634: // mac_util::MainAppBundle().
TVL wrote:
> given how many places related to keystone have this comment, should we go
ahead
> and have mac_util::KeystoneBundle() so the comment goes in one place, and
> if/when we do app shortcuts and it needs to change, it's already in one bottle
> neck?

I didn't really want a function for this, but I've changed it into an instance
variable which eliminates the bottleneck.

http://codereview.chromium.org/437053/diff/22/9013#newcode710
chrome/browser/cocoa/keystone_glue.mm:710: DLOG(ERROR) <<
"AuthorizationExecuteWithPrivileges: " << status;
TVL wrote:
> do you care about messaging this to the user?

Not really in the application, but I've changed them into LOGs to aid with
troubleshooting.  Except for the one that would LOG when the user cancels
authentication.  I've made that one silent.

http://codereview.chromium.org/437053/diff/22/9016
File chrome/browser/cocoa/keystone_promotion_infobar.mm (right):

http://codereview.chromium.org/437053/diff/22/9016#newcode186
chrome/browser/cocoa/keystone_promotion_infobar.mm:186: if (tabContents &&
tabContents->infobar_delegate_count() == 0) {
TVL wrote:
> why check the count?  this means a password prompt for default browser could
> block his from showing, right?  meaning hey might never get it if they always
> ignore the default browser one?

I added a comment, but the idea here was to make this work the same way as the
default browser info bar.

For that matter, this and the default browser info bar can "fight" for who gets
to show up first.  In my experience testing this, it's about 50:50.  Both need
to do some stuff on another thread (or wait for other stuff to happen) and then
come back to the main thread and ask for the bar to be shown, and both are fired
off at the same time.

Remember, if promotion is needed, the button will always show up in the about
box.

[TVL, 2009-11-28 17:04:30]

lg™

http://codereview.chromium.org/437053/diff/7004/8016
File chrome/browser/cocoa/keystone_glue.mm (right):

http://codereview.chromium.org/437053/diff/7004/8016#newcode510
chrome/browser/cocoa/keystone_glue.mm:510: NSString* executablePath = [[NSBundle
mainBundle] executablePath];
s/[NSBundle mainBundle]/appPath_/ ? wantsPromotion uses appPath_

[Mark Mentovai, 2009-11-28 17:08:14]

http://codereview.chromium.org/437053/diff/7004/8016
File chrome/browser/cocoa/keystone_glue.mm (right):

http://codereview.chromium.org/437053/diff/7004/8016#newcode510
chrome/browser/cocoa/keystone_glue.mm:510: NSString* executablePath = [[NSBundle
mainBundle] executablePath];
TVL wrote:
> s/[NSBundle mainBundle]/appPath_/ ? wantsPromotion uses appPath_

I use appPath_ here also.

appPath_ is GC.app.

executablePath is GC.app/Contents/MacOS/GC.

I want to check that both are writable.