New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 133173 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
OOO
Closed: Jul 2012
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 122756

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment

Fix Chrome Registration

Project Member Reported by gab@chromium.org, Jun 16 2012

Issue description

This bug is simply to keep track of the fixes going in for a set of CLs to fix the way we register Chrome.

Design doc: https://docs.google.com/a/chromium.org/document/d/1qmcV3uYBh3JwvXhYkI7asg0nN7KfVMWVOzND4p0jQ3E/edit#

We want this to go in M21 at all costs as it will prevent divergence of registrations on Win8 (M21 is the first Win8-Metro-enabled release).
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 17 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=142634

------------------------------------------------------------------------
r142634 | gab@chromium.org | Sun Jun 17 00:54:24 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/shell_util.h?r1=142634&r2=142633&pathrev=142634
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/install_util.cc?r1=142634&r2=142633&pathrev=142634
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/setup/uninstall.h?r1=142634&r2=142633&pathrev=142634
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/shell_integration_win.cc?r1=142634&r2=142633&pathrev=142634
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/setup/uninstall.cc?r1=142634&r2=142633&pathrev=142634
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/shell_util.cc?r1=142634&r2=142633&pathrev=142634
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/setup/setup_main.cc?r1=142634&r2=142633&pathrev=142634

Always suffix ChromeHTML entries on Windows for user-level installs.

This also adds the same suffixing to Chrome's appname for Default Programs registration (this suffix is not user-facing though as we don't suffix the actual string representing Chrome in the UI... obviously!)

Design doc: https://docs.google.com/a/chromium.org/document/d/1qmcV3uYBh3JwvXhYkI7asg0nN7KfVMWVOzND4p0jQ3E/edit

BUG=125362,124013, 133173 
TEST=http://goo.gl/ZZ7gE

Review URL: https://chromiumcodereview.appspot.com/10451074
------------------------------------------------------------------------
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 17 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=142641

------------------------------------------------------------------------
r142641 | gab@chromium.org | Sun Jun 17 08:23:37 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/install_util.cc?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/google_chrome_distribution.h?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/setup/uninstall.cc?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/google_chrome_distribution.cc?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/chromium_binaries_distribution.cc?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/browser_distribution.cc?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/chrome_frame_distribution.h?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/shell_util.h?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/shell_integration_win.cc?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/google_chrome_sxs_distribution.cc?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/google_chrome_sxs_distribution.h?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/shell_util.cc?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/chrome_frame_distribution.cc?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/installer_state.cc?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/browser_distribution_unittest.cc?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/setup/setup_main.cc?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/chromium_binaries_distribution.h?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/browser_distribution.h?r1=142641&r2=142640&pathrev=142641
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/google_chrome_distribution_dummy.cc?r1=142641&r2=142640&pathrev=142641

Abstract suffixing logic away from GetApplicationName


BUG=124013, 125362,  133173 
TEST=http://goo.gl/ZZ7gE

Review URL: https://chromiumcodereview.appspot.com/10446111
------------------------------------------------------------------------
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 17 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=142648

------------------------------------------------------------------------
r142648 | gab@chromium.org | Sun Jun 17 11:28:29 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/shell_util.h?r1=142648&r2=142647&pathrev=142648
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/setup/uninstall.cc?r1=142648&r2=142647&pathrev=142648
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/shell_util.cc?r1=142648&r2=142647&pathrev=142648

Make all registrations in HKCU for user-level installs on Win8


BUG=124013,125362,  133173 
TEST=No UAC on first run for user-level installs on Win8.
http://goo.gl/ZZ7gE

Review URL: https://chromiumcodereview.appspot.com/10399054
------------------------------------------------------------------------
Labels: ReleaseBlock-Beta
Marking this as beta blocker based on Gab's comment#0: 
We want this to go in M21 at all costs as it will prevent divergence of registrations on Win8 (M21 is the first Win8-Metro-enabled release).
Thanks, Anantha. Gab, if you could update this bug as things progress today with your commits, that would be great!
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 19 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=142909

------------------------------------------------------------------------
r142909 | grt@chromium.org | Mon Jun 18 21:32:10 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/google_chrome_distribution.h?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/web_applications/web_app_ui.cc?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/setup/uninstall.cc?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/google_chrome_distribution.cc?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/chromium_binaries_distribution.cc?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/browser_distribution.cc?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/shell_util_unittest.cc?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/shell_util.h?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/util_constants.h?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/shell_integration_win.cc?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/google_chrome_sxs_distribution.cc?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/google_chrome_sxs_distribution.h?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/web_applications/web_app_win.cc?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/util_constants.cc?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/shell_util.cc?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/shell_integration_unittest.cc?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/shell_integration.h?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/extensions/shell_window_views.cc?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/browser.cc?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/chromium_binaries_distribution.h?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/browser_distribution.h?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/google_chrome_distribution_dummy.cc?r1=142909&r2=142908&pathrev=142909
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/jumplist_win.cc?r1=142909&r2=142908&pathrev=142909

Suffix Chrome's appid on user-level installs

ShellIntegration::GetAppId() --> ShellIntegration::GetProfileAppId() for clarity (i.e. having two functions called "GetAppId" is confusing imo).

Patch From Gabriel Charette <gab@chromium.org>
BUG=125362,  133173 
TEST=Make sure 125362 doesn't repro.
http://goo.gl/ZZ7gE
ShellIntegrationTest.GetAppModelIdForProfileTest
ShellUtilTest.BuildAppModelId*
SessionRestoreTest.RestoreAfterClosingTabbedBrowserWithAppAndLaunching
AppModeTest.EnableAppModeTest

Review URL: https://chromiumcodereview.appspot.com/10542031
Patch from Gabriel Charette <gab@chromium.org>.
------------------------------------------------------------------------

Comment 7 by gab@chromium.org, Jun 19 2012

And delegate_execute change: "New suffixed appid logic duplicated in delegate_execute"

https://chromereviews.googleplex.com/4541041/

Comment 8 by gab@chromium.org, Jun 19 2012

Status: Fixed
Epic!

Comment 10 by gab@chromium.org, Jun 29 2012

Status: Started
Now pending a fix for  bug 133810 .

Comment 11 by kareng@google.com, Jun 29 2012

don't mark beta blocker pls. stable blocker :)

Comment 12 by gab@chromium.org, Jun 29 2012

Labels: -ReleaseBlock-Beta

Comment 13 by gab@chromium.org, Jun 29 2012

My bad didn't realize blocker marker was still on it :)

Comment 14 by gab@chromium.org, Jun 29 2012

Labels: ReleaseBlock-Stable
Project Member

Comment 15 by bugdroid1@chromium.org, Jul 6 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=145596

------------------------------------------------------------------------
r145596 | gab@chromium.org | Fri Jul 06 08:53:20 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/shell_util_unittest.cc?r1=145596&r2=145595&pathrev=145596
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/shell_util.h?r1=145596&r2=145595&pathrev=145596
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/setup/uninstall.cc?r1=145596&r2=145595&pathrev=145596
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/shell_util.cc?r1=145596&r2=145595&pathrev=145596

Use a better registration suffix that will always be unique while respecting the MSDN rules (2nd try). 

The suffix will now be the base32 encoding of the md5 hash of the user's username. This is always unique and can be determined many times deterministically without I/O access (as opposed to other methods we considered). 

It respects all MSDN constraints for properties onto which it is appended. 

This replaces the prior-style username suffixes for new installs. 

Old-style suffixes (i.e. unsuffixed and username suffixed) will however be kept as is through future updates. 

Design doc: https://docs.google.com/a/chromium.org/document/d/1qmcV3uYBh3JwvXhYkI7asg0nN7KfVMWVOzND4p0jQ3E/edit 

Note: This is the continuation of http://codereview.chromium.org/10617002/ which was reverted in https://chromiumcodereview.appspot.com/10667006/

BUG= 133810 ,  133173 
TEST= http://goo.gl/ZZ7gE 
installer_util_unittests.exe --gtest_filter=ShellUtilTest.*


Review URL: https://chromiumcodereview.appspot.com/10662052
------------------------------------------------------------------------

Comment 16 by gab@chromium.org, Jul 6 2012

Along with https://chromereviews.googleplex.com/4620013/. Hopefully this is all :)!

Comment 17 by gab@chromium.org, Jul 6 2012

And https://chromereviews.googleplex.com/4647013/ for deps roll for delegate_execute.

Comment 18 by gab@chromium.org, Jul 6 2012

Status: Fixed
Fixed, to be verified in merged as part of  bug 133810 .
Awesome! Thanks so much for your work on this, Gab. Great to have it resolved.
Project Member

Comment 20 by bugdroid1@chromium.org, Jul 12 2012

Labels: merge-merged-1180
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=146243

------------------------------------------------------------------------
r146243 | gab@google.com | Wed Jul 11 17:01:52 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/installer/setup/uninstall.cc?r1=146243&r2=146242&pathrev=146243
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/installer/util/shell_util.cc?r1=146243&r2=146242&pathrev=146243
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/installer/util/shell_util_unittest.cc?r1=146243&r2=146242&pathrev=146243
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/installer/util/shell_util.h?r1=146243&r2=146242&pathrev=146243

Merge 145596 - Use a better registration suffix that will always be unique while respecting the MSDN rules (2nd try). 

The suffix will now be the base32 encoding of the md5 hash of the user's SID. This is always unique and can be determined many times deterministically without I/O access (as opposed to other methods we considered). 

It respects all MSDN constraints for properties onto which it is appended. 

This replaces the prior-style username suffixes for new installs. 

Old-style suffixes (i.e. unsuffixed and username suffixed) will however be kept as is through future updates. 

Design doc: https://docs.google.com/a/chromium.org/document/d/1qmcV3uYBh3JwvXhYkI7asg0nN7KfVMWVOzND4p0jQ3E/edit 

Note: This is the continuation of http://codereview.chromium.org/10617002/ which was reverted in https://chromiumcodereview.appspot.com/10667006/

BUG= 133810 ,  133173 
TEST= http://goo.gl/ZZ7gE 
installer_util_unittests.exe --gtest_filter=ShellUtilTest.*


Review URL: https://chromiumcodereview.appspot.com/10662052

TBR=gab@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10703155
------------------------------------------------------------------------
Blocking: chromium:122756
Project Member

Comment 22 by bugdroid1@chromium.org, Oct 13 2012

Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member

Comment 23 by bugdroid1@chromium.org, Nov 14 2012

The following revision refers to this bug:
    http://goto.ext.google.com/viewvc/chrome-internal?view=rev&revision=25974

------------------------------------------------------------------------
r25974 | grt@google.com | 2012-06-19T04:33:48.857160Z

------------------------------------------------------------------------
Project Member

Comment 24 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Area-Internals -Internals-Install -Mstone-21 M-21 Cr-Internals Cr-Internals-Install
Project Member

Comment 25 by bugdroid1@chromium.org, Mar 14 2013

Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Project Member

Comment 26 by bugdroid1@chromium.org, Apr 5 2013

Labels: -Cr-Internals-Install Cr-Internals-Installer

Sign in to add a comment