Do not elevate to register Chrome for system-level Win7 installs.

Do not elevate to register Chrome for system-level Win7 installs.

On Windows 7, browsers must be registered with the OS in HKLM. Chrome's
installer takes care of this during install/update for system-level
installs. For user-level installs, however, Chrome must pop a UAC prompt
to have setup.exe run at high integrity for this registration.

This change suppresses the elevate-to-register codepath for system-level
installs on Windows 7 since this is the installer's responsibility. This
prevents over-prompting some users (see the bug for details) at the risk
of not prompting users for whom Chrome truly isn't registered. If those
users are running a properly updating Chrome, its registration will be
repaired on the next update. If they are not running an updating Chrome,
they have bigger problems.

BUG=717913
R=gab@chromium.org

Review-Url: https://codereview.chromium.org/2878953003
Cr-Commit-Position: refs/heads/master@{#472750}
Committed: https://chromium.googlesource.com/chromium/src/+/31546fb25253afc574bf366d92ef20790c7d418c

[grt (UTC plus 2), 2017-05-12 21:14:12]

PTAL, thanks.

[gab, 2017-05-15 16:50:17]

https://codereview.chromium.org/2878953003/diff/1/chrome/installer/util/shell...
File chrome/installer/util/shell_util.cc (right):

https://codereview.chromium.org/2878953003/diff/1/chrome/installer/util/shell...
chrome/installer/util/shell_util.cc:2106: (!user_level ||
ElevateAndRegisterChrome(dist, chrome_exe, suffix,
Would it perhaps be cleaner to tweak |elevate_if_not_admin| instead of adding an
extra condition?

[grt (UTC plus 2), 2017-05-15 21:13:59]

Back at ya

https://codereview.chromium.org/2878953003/diff/1/chrome/installer/util/shell...
File chrome/installer/util/shell_util.cc (right):

https://codereview.chromium.org/2878953003/diff/1/chrome/installer/util/shell...
chrome/installer/util/shell_util.cc:2106: (!user_level ||
ElevateAndRegisterChrome(dist, chrome_exe, suffix,
On 2017/05/15 16:50:17, gab wrote:
> Would it perhaps be cleaner to tweak |elevate_if_not_admin| instead of adding
an
> extra condition?

I think we want the quick "result = true;" here as if the elevation succeeded
rather than going to the else clause below.

...or did I misunderstand the question?

[gab, 2017-05-16 15:08:11]

https://codereview.chromium.org/2878953003/diff/1/chrome/installer/util/shell...
File chrome/installer/util/shell_util.cc (right):

https://codereview.chromium.org/2878953003/diff/1/chrome/installer/util/shell...
chrome/installer/util/shell_util.cc:2106: (!user_level ||
ElevateAndRegisterChrome(dist, chrome_exe, suffix,
On 2017/05/15 21:13:59, grt (UTC plus 2) wrote:
> On 2017/05/15 16:50:17, gab wrote:
> > Would it perhaps be cleaner to tweak |elevate_if_not_admin| instead of
adding
> an
> > extra condition?
> 
> I think we want the quick "result = true;" here as if the elevation succeeded
> rather than going to the else clause below.
> 
> ...or did I misunderstand the question?

I think we want |elevate_if_not_admin| to be false for system-level installs.
That makes us proceed to the next block (which should s/else/else if
(user_level)/) and we can then have a last else with result = true that explains
when we can get there? It feels weird to have an inline condition here for
!user_level that makes us pretend it worked..?

[grt (UTC plus 2), 2017-05-17 09:24:03]

What do you think of this?

[grt (UTC plus 2), 2017-05-17 09:24:11]

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

[commit-bot: I haz the power, 2017-05-17 09:24:42]

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, 2017-05-17 10:13:00]

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

[commit-bot: I haz the power, 2017-05-17 10:13:00]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[gab, 2017-05-17 20:16:25]

Neat, LGTM :)

[grt (UTC plus 2), 2017-05-17 20:33:10]

The CQ bit was checked by grt@chromium.org

[commit-bot: I haz the power, 2017-05-17 20:34:50]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[grt (UTC plus 2), 2017-05-17 20:35:13]

The CQ bit was unchecked by grt@chromium.org

[grt (UTC plus 2), 2017-05-17 20:41:40]

The CQ bit was checked by grt@chromium.org

[grt (UTC plus 2), 2017-05-17 20:41:41]

The patchset sent to the CQ was uploaded after l-g-t-m from gab@chromium.org
Link to the patchset: https://codereview.chromium.org/2878953003/#ps40001
(title: "fix")

[commit-bot: I haz the power, 2017-05-17 20:43:02]

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, 2017-05-17 22:35:50]

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

[commit-bot: I haz the power, 2017-05-17 22:35:51]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[grt (UTC plus 2), 2017-05-18 06:28:14]

The CQ bit was checked by grt@chromium.org

[commit-bot: I haz the power, 2017-05-18 06:29:02]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[grt (UTC plus 2), 2017-05-18 07:24:03]

The CQ bit was checked by grt@chromium.org

[grt (UTC plus 2), 2017-05-18 07:24:03]

The patchset sent to the CQ was uploaded after l-g-t-m from gab@chromium.org
Link to the patchset: https://codereview.chromium.org/2878953003/#ps60001
(title: "reset the released callback")

[commit-bot: I haz the power, 2017-05-18 07:24:23]

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, 2017-05-18 08:35:33]

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

[commit-bot: I haz the power, 2017-05-18 08:35:34]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[grt (UTC plus 2), 2017-05-18 08:36:11]

The CQ bit was checked by grt@chromium.org

[commit-bot: I haz the power, 2017-05-18 08:36:45]

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, 2017-05-18 09:20:06]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1495096571382570,
"parent_rev": "5cfb0199ae5af577ca239feccd2d3c3f976ddac6", "commit_rev":
"31546fb25253afc574bf366d92ef20790c7d418c"}

[commit-bot: I haz the power, 2017-05-18 09:20:22]

Description was changed from

==========
Do not elevate to register Chrome for system-level Win7 installs.

On Windows 7, browsers must be registered with the OS in HKLM. Chrome's
installer takes care of this during install/update for system-level
installs. For user-level installs, however, Chrome must pop a UAC prompt
to have setup.exe run at high integrity for this registration.

This change suppresses the elevate-to-register codepath for system-level
installs on Windows 7 since this is the installer's responsibility. This
prevents over-prompting some users (see the bug for details) at the risk
of not prompting users for whom Chrome truly isn't registered. If those
users are running a properly updating Chrome, its registration will be
repaired on the next update. If they are not running an updating Chrome,
they have bigger problems.

BUG=717913
R=gab@chromium.org
==========

to

==========
Do not elevate to register Chrome for system-level Win7 installs.

On Windows 7, browsers must be registered with the OS in HKLM. Chrome's
installer takes care of this during install/update for system-level
installs. For user-level installs, however, Chrome must pop a UAC prompt
to have setup.exe run at high integrity for this registration.

This change suppresses the elevate-to-register codepath for system-level
installs on Windows 7 since this is the installer's responsibility. This
prevents over-prompting some users (see the bug for details) at the risk
of not prompting users for whom Chrome truly isn't registered. If those
users are running a properly updating Chrome, its registration will be
repaired on the next update. If they are not running an updating Chrome,
they have bigger problems.

BUG=717913
R=gab@chromium.org

Review-Url: https://codereview.chromium.org/2878953003
Cr-Commit-Position: refs/heads/master@{#472750}
Committed:
https://chromium.googlesource.com/chromium/src/+/31546fb25253afc574bf366d92ef...
==========

[commit-bot: I haz the power, 2017-05-18 09:20:25]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/31546fb25253afc574bf366d92ef...