Allow two user level installs of Chrome to have default browser settings.

Allow two user level installs of Chrome to have default browser settings.

This change will allow two Windows users to install thier own copy of Chrome
and set it as default browser. There is a bunch of refactoring in this change.
- Divided the registry entries into three categories:
  * Entries that are only read from HKLM
  * Entries that actually tie file/protocol associations to Chrome. This will
    almost always be in HKCU (except when it is a OEM install).
  * Entries that we can create in HKCU if we fail to create them in HKLM.
- For single user machine nothing should change.
- For two users on the same machine (each with a different user level Chrome
installed), the second user's default browser will get renamed to
Chromium.<user> and the required registry entries will also be renamed so that
they do not overwrite first users's registry entries.

BUG=3641
TEST=Install and set Chromium as default browser from two different logins on the same machine for various combinations of XP/Vista, admin users/non admin user, user-level install/system-level install.

Committed: http://src.chromium.org/viewvc/21428

[huanr, 2009-07-22 22:04:14]

In addition to the scenarios you described in TEST section, do we need to also
test how the new way works with the old system? It seems the new system works
fine with old build. But would there be some interesting cases? For example,
assuming user A and B set default browser in old build and only one gets set, if
they try again with the new build we should work fine?

http://codereview.chromium.org/159172/diff/11/17
File chrome/installer/util/shell_util.cc (left):

http://codereview.chromium.org/159172/diff/11/17#oldcode209
Line 209: }; // class RegistryEntry
Based on the usage, this class needs to be copyable and we are the compiler
generated copy constructor. The style guide asks to clearly document that.

http://codereview.chromium.org/159172/diff/11/17
File chrome/installer/util/shell_util.cc (right):

http://codereview.chromium.org/159172/diff/11/17#newcode300
Line 300: registered = (*itr).ExistsInHKLM();
should it be registered = registered && (*itr).ExistsInHKLM();

http://codereview.chromium.org/159172/diff/11/17#newcode357
Line 357: // not meant to detect all cases. It just tries handle the most common
case.
Question: if both users registered chrome to the system list, will both chrome
entries show up in the list for each user?

http://codereview.chromium.org/159172/diff/11/17#newcode364
Line 364: //   of the above key should differ from |chrome_exe| only in user
name.
During uninstall, is it a best effort to delete keys in HKLM? If so, then this
could be a left over of an uninstalled chrome?

What if we always append user name when setting default browser moving forward?
Will it cause more problems?

http://codereview.chromium.org/159172/diff/11/17#newcode454
Line 454: for (std::list<RegistryEntry>::const_iterator itr = entries.begin();
Why we always try to clean up both places? My understanding is we should not
remove the entry without suffix if a user specific one exists. on the other
hand, would both places set for one user under following situation:
- User A set default
- User B set default
- User A uninstall
- User B set default again

http://codereview.chromium.org/159172/diff/11/18
File chrome/installer/util/shell_util.h (right):

http://codereview.chromium.org/159172/diff/11/18#newcode146
Line 146: static bool GetUserSpecificDefaultBrowserSuffix(std::wstring* entry);
I'd change 'returns' to 'gets', and add in the end 'The suffix is returned by
appending to the entry string passed in.

[kuchhal, 2009-07-23 00:10:31]

See below...
On 2009/07/22 22:04:14, huanr wrote:
> In addition to the scenarios you described in TEST section, do we need to also
> test how the new way works with the old system? It seems the new system works
> fine with old build. But would there be some interesting cases? For example,
> assuming user A and B set default browser in old build and only one gets set,
if
> they try again with the new build we should work fine?

Yes i think the upgraded builds should also work fine, thats why I wanted to
retain the existing behavior of not appending user name if there is only one
user. If there are two users who get the new build, whoever sets Chrome default
first gets to use Google Chrome as identifier. the second one setting it as
default will get Google Chrome.<user>

> Based on the usage, this class needs to be copyable and we are the compiler
> generated copy constructor. The style guide asks to clearly document that.

I am not sure why I changed the original behavior of keeping pointers to
RegistryEntry. I have changed it back and disallowed copy constructor.

> should it be registered = registered && (*itr).ExistsInHKLM();
Added comment to explain why this isn't needed (we exit as soon as registered is
false).

> Question: if both users registered chrome to the system list, will both chrome
> entries show up in the list for each user?

Yes both will show up for both users. There is just no way to out of this
(afaik) since the registration entries are only read from HKLM and they are
visible to all the users.

> During uninstall, is it a best effort to delete keys in HKLM? If so, then this
> could be a left over of an uninstalled chrome?
> What if we always append user name when setting default browser moving
forward?
> Will it cause more problems?

Like I explained above, for the most common case of only one user on a Windows
machine I was trying to retain the existing behavior.

> Why we always try to clean up both places? My understanding is we should not
> remove the entry without suffix if a user specific one exists. on the other
> hand, would both places set for one user under following situation:
> - User A set default
> - User B set default
> - User A uninstall
> - User B set default again
I think in your example user B will still use Google Chrome.B entry since it
would already exist but there are other scenarios that cause issues during
uninstall. For example user A sets Chrome as default, user B installs Chrome but
*doesn't* set it default. Now if user B tries to uninstall we should not delete
browser registration but to determine that we need to read the actual path to
chrome.exe and compare it. This I will add in future change list. For now this
change will keep the existing uninstall behavior so the problem wont be new.

> Line 146: static bool GetUserSpecificDefaultBrowserSuffix(std::wstring*
entry);
> I'd change 'returns' to 'gets', and add in the end 'The suffix is returned by
> appending to the entry string passed in.
Done.

[huanr, 2009-07-23 02:28:52]

LGTM. Thanks for the explanations.