Configure system-level Chrome settings (i.e. install its shortcuts) immediately following self-dest…

Configure system-level Chrome settings (i.e. install its shortcuts) immediately following self-destruction of a user-level Chrome.

Also refactor RemoveShortcut() to use base::win::ResolveShortcut() instead of coming up with its own method (which I'm scared could still hold handles to the shortcut when trying to delete it; doesn't seem to have been a problem, but I'd rather be safe).

BUG=158524
TEST=Install user-level Chrome on user A, install system-level Chrome on user B (while A is still logged in), go back to user A and run Chrome, user-level Chrome self-destructs and new shortcuts are installed pointing to system-level Chrome.


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

[gab, 2012-11-15 00:44:20]

Sir, PTAL. Fixes a ReleaseBlocker-Stable.

Testing this I feel that we should also launch system-level Chrome at the end of
a user-level self-destruct... this was already the user's intention, the
shortcut he just clicked now points to a new target (system-level Chrome), but
there is no reason he should have to click it again (worst case Chrome was not
invoked via a shortcut, but via http/htmlfile and we launch Chrome on NTP when
the intention was to open something else... I think that's ok for this one time
thing though)... what do you think?

Thanks,
Gab

[robertshield, 2012-11-15 20:24:09]

lgtm

[commit-bot: I haz the power, 2012-11-15 20:26:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/11359219/1

[commit-bot: I haz the power, 2012-11-15 23:45:51]

Change committed as 168080

[grt (UTC plus 2), 2012-11-16 04:32:32]

https://codereview.chromium.org/11359219/diff/1/chrome/installer/setup/uninst...
File chrome/installer/setup/uninstall.cc (right):

https://codereview.chromium.org/11359219/diff/1/chrome/installer/setup/uninst...
chrome/installer/setup/uninstall.cc:1281:
HandleActiveSetupForBrowser(system_chrome_path, product, true);
this will do active setup for whatever version of user-level chrome is being
uninstalled rather than the version of chrome installed at system-level.  this
seems fragile.  is this intended?

[gab, 2012-11-16 04:51:20]

https://codereview.chromium.org/11359219/diff/1/chrome/installer/setup/uninst...
File chrome/installer/setup/uninstall.cc (right):

https://codereview.chromium.org/11359219/diff/1/chrome/installer/setup/uninst...
chrome/installer/setup/uninstall.cc:1281:
HandleActiveSetupForBrowser(system_chrome_path, product, true);
On 2012/11/16 04:32:32, grt wrote:
> this will do active setup for whatever version of user-level chrome is being
> uninstalled rather than the version of chrome installed at system-level.  this
> seems fragile.  is this intended?

True, the other option is to trigger the system-level's active setup command;
this is what I was going to do originally, but somehow the other setup wasn't
dying early and I couldn't figure out why, so I went the simple route instead in
this CL; now that I think about it and have played a little bit more with
LaunchProcess, I think it's because I wasn't launching the other setup out of
job and this setup would terminate almost immediately after and bring the other
one down with it (does that even make sense?).

For now this only installs shortcuts so it's fine, but I agree that it could
diverge over time.

[grt (UTC plus 2), 2012-11-16 05:07:21]

On 2012/11/16 04:51:20, gab wrote:
> For now this only installs shortcuts so it's fine

"it's fine" -> "it works today".  it won't work reliably in the future unless
the active setup tasks are carved in stone.

[gab, 2012-11-16 20:38:51]

On 2012/11/16 05:07:21, grt wrote:
> On 2012/11/16 04:51:20, gab wrote:
> > For now this only installs shortcuts so it's fine
> 
> "it's fine" -> "it works today".  it won't work reliably in the future unless
> the active setup tasks are carved in stone.

Agreed, restarting a new CL at https://codereview.chromium.org/11420045 to call
the system-level's Active Setup command instead of doing it as part of the
user-level's uninstall flow.