Fix registerProtocolHandler OS registration on Windows 8

Fix registerProtocolHandler OS registration on Windows 8

On Windows 8 to register as the default handler for a protocol you need to go through a Windows shell UI flow, it cannot be done anymore via the IApplicationAssociationRegistration interface.

BUG=150850
TEST=Ensure gmail and other protocol handlers can be registered successfully on Windows 7 and 8.


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

[benwells, 2012-09-20 11:10:47]

sky for changes to chrome/browser/
grt for all else

http://codereview.chromium.org/10963004/diff/3002/chrome/installer/util/shell...
File chrome/installer/util/shell_util.cc (right):

http://codereview.chromium.org/10963004/diff/3002/chrome/installer/util/shell...
chrome/installer/util/shell_util.cc:914: bool
RegisterChromeAsDefaultProtocolClientForXP(BrowserDistribution* dist,
This code doesn't (and didn't) seem to deal with the system level install case.
If it's OK I'll deal with that in a seperate CL as it's orthogonal to the win 8
problem.

[grt (UTC plus 2), 2012-09-20 13:10:12]

http://codereview.chromium.org/10963004/diff/3002/chrome/installer/util/shell...
File chrome/installer/util/shell_util.cc (right):

http://codereview.chromium.org/10963004/diff/3002/chrome/installer/util/shell...
chrome/installer/util/shell_util.cc:914: bool
RegisterChromeAsDefaultProtocolClientForXP(BrowserDistribution* dist,
On 2012/09/20 11:10:47, benwells wrote:
> This code doesn't (and didn't) seem to deal with the system level install
case.
> If it's OK I'll deal with that in a seperate CL as it's orthogonal to the win
8
> problem.

Isn't this meant to be a per-user operation?  Otherwise, if user A chose Chrome
to handle mailto links, all other users on the system might be irritated that
pine stopped launching for them.

http://codereview.chromium.org/10963004/diff/12001/chrome/browser/shell_integ...
File chrome/browser/shell_integration_win.cc (right):

http://codereview.chromium.org/10963004/diff/12001/chrome/browser/shell_integ...
chrome/browser/shell_integration_win.cc:473: string16 wprotocol =
UTF8ToUTF16(protocol);
nit: string16 wprotocol(UTF8ToUTF16(protocol));

http://codereview.chromium.org/10963004/diff/12001/chrome/browser/shell_integ...
chrome/browser/shell_integration_win.cc:475: dist, chrome_exe.value(),
wprotocol)) {
nit: indent four additional spaces.

http://codereview.chromium.org/10963004/diff/12001/chrome/browser/shell_integ...
chrome/browser/shell_integration_win.cc:480: VLOG(1) << "Set-as-default Windows
UI triggered.";
I think it's more informative for "triggered" to be "completed" since
ShowMakeBlah is a blocking call.  If you agree, please make the change here and
on line 460.  Thanks.

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
File chrome/installer/util/shell_util.cc (right):

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
chrome/installer/util/shell_util.cc:914: bool
RegisterChromeAsDefaultProtocolClientForXP(BrowserDistribution* dist,
please add a comment for the function directing the reader to
http://msdn.microsoft.com/library/aa767914.aspx for reference.

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
chrome/installer/util/shell_util.cc:918: const string16
suffix(ShellUtil::GetCurrentInstallationSuffix(dist,
suffix is never used, please remove.

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
chrome/installer/util/shell_util.cc:926: LOG(ERROR) << "Could not make Chrome
default protocol client (XP).";
nit: de-indent two spaces

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
chrome/installer/util/shell_util.cc:1377: // links of this type (protocol)?"
dialog.  We choose the latter.
Looks like this is the text for line 1452 (and vice-versa).

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
chrome/installer/util/shell_util.cc:1397: if (!RegisterChromeForProtocol(dist,
chrome_exe, L"", protocol, true))
L"" -> string16()

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
chrome/installer/util/shell_util.cc:1444: if (!RegisterChromeForProtocol(dist,
chrome_exe, L"", protocol, true))
L"" -> string16()

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
chrome/installer/util/shell_util.cc:1445: return false;
de-indent two spaces

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
File chrome/installer/util/shell_util.h (right):

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
chrome/installer/util/shell_util.h:1: // Copyright (c) 2012 The Chromium
Authors. All rights reserved
Please put the '.' back at the end of this line.

[benwells, 2012-09-21 01:15:20]

http://codereview.chromium.org/10963004/diff/12001/chrome/browser/shell_integ...
File chrome/browser/shell_integration_win.cc (right):

http://codereview.chromium.org/10963004/diff/12001/chrome/browser/shell_integ...
chrome/browser/shell_integration_win.cc:473: string16 wprotocol =
UTF8ToUTF16(protocol);
On 2012/09/20 13:10:13, grt wrote:
> nit: string16 wprotocol(UTF8ToUTF16(protocol));

Done.

http://codereview.chromium.org/10963004/diff/12001/chrome/browser/shell_integ...
chrome/browser/shell_integration_win.cc:475: dist, chrome_exe.value(),
wprotocol)) {
On 2012/09/20 13:10:13, grt wrote:
> nit: indent four additional spaces.

Done.

http://codereview.chromium.org/10963004/diff/12001/chrome/browser/shell_integ...
chrome/browser/shell_integration_win.cc:480: VLOG(1) << "Set-as-default Windows
UI triggered.";
On 2012/09/20 13:10:13, grt wrote:
> I think it's more informative for "triggered" to be "completed" since
> ShowMakeBlah is a blocking call.  If you agree, please make the change here
and
> on line 460.  Thanks.

Done.

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
File chrome/installer/util/shell_util.cc (right):

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
chrome/installer/util/shell_util.cc:914: bool
RegisterChromeAsDefaultProtocolClientForXP(BrowserDistribution* dist,
On 2012/09/20 13:10:13, grt wrote:
> please add a comment for the function directing the reader to
> http://msdn.microsoft.com/library/aa767914.aspx for reference.

Done.

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
chrome/installer/util/shell_util.cc:918: const string16
suffix(ShellUtil::GetCurrentInstallationSuffix(dist,
On 2012/09/20 13:10:13, grt wrote:
> suffix is never used, please remove.

Done.

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
chrome/installer/util/shell_util.cc:926: LOG(ERROR) << "Could not make Chrome
default protocol client (XP).";
On 2012/09/20 13:10:13, grt wrote:
> nit: de-indent two spaces

Done.

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
chrome/installer/util/shell_util.cc:1377: // links of this type (protocol)?"
dialog.  We choose the latter.
On 2012/09/20 13:10:13, grt wrote:
> Looks like this is the text for line 1452 (and vice-versa).

Done.

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
chrome/installer/util/shell_util.cc:1397: if (!RegisterChromeForProtocol(dist,
chrome_exe, L"", protocol, true))
On 2012/09/20 13:10:13, grt wrote:
> L"" -> string16()

Done.

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
chrome/installer/util/shell_util.cc:1444: if (!RegisterChromeForProtocol(dist,
chrome_exe, L"", protocol, true))
On 2012/09/20 13:10:13, grt wrote:
> L"" -> string16()

Done.

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
chrome/installer/util/shell_util.cc:1445: return false;
On 2012/09/20 13:10:13, grt wrote:
> de-indent two spaces

Done.

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
File chrome/installer/util/shell_util.h (right):

http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
chrome/installer/util/shell_util.h:1: // Copyright (c) 2012 The Chromium
Authors. All rights reserved
On 2012/09/20 13:10:13, grt wrote:
> Please put the '.' back at the end of this line.

Done.

[benwells, 2012-09-24 07:05:55]

On 2012/09/21 01:15:20, benwells wrote:
>
http://codereview.chromium.org/10963004/diff/12001/chrome/browser/shell_integ...
> File chrome/browser/shell_integration_win.cc (right):
> 
>
http://codereview.chromium.org/10963004/diff/12001/chrome/browser/shell_integ...
> chrome/browser/shell_integration_win.cc:473: string16 wprotocol =
> UTF8ToUTF16(protocol);
> On 2012/09/20 13:10:13, grt wrote:
> > nit: string16 wprotocol(UTF8ToUTF16(protocol));
> 
> Done.
> 
>
http://codereview.chromium.org/10963004/diff/12001/chrome/browser/shell_integ...
> chrome/browser/shell_integration_win.cc:475: dist, chrome_exe.value(),
> wprotocol)) {
> On 2012/09/20 13:10:13, grt wrote:
> > nit: indent four additional spaces.
> 
> Done.
> 
>
http://codereview.chromium.org/10963004/diff/12001/chrome/browser/shell_integ...
> chrome/browser/shell_integration_win.cc:480: VLOG(1) << "Set-as-default
Windows
> UI triggered.";
> On 2012/09/20 13:10:13, grt wrote:
> > I think it's more informative for "triggered" to be "completed" since
> > ShowMakeBlah is a blocking call.  If you agree, please make the change here
> and
> > on line 460.  Thanks.
> 
> Done.
> 
>
http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
> File chrome/installer/util/shell_util.cc (right):
> 
>
http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
> chrome/installer/util/shell_util.cc:914: bool
> RegisterChromeAsDefaultProtocolClientForXP(BrowserDistribution* dist,
> On 2012/09/20 13:10:13, grt wrote:
> > please add a comment for the function directing the reader to
> > http://msdn.microsoft.com/library/aa767914.aspx for reference.
> 
> Done.
> 
>
http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
> chrome/installer/util/shell_util.cc:918: const string16
> suffix(ShellUtil::GetCurrentInstallationSuffix(dist,
> On 2012/09/20 13:10:13, grt wrote:
> > suffix is never used, please remove.
> 
> Done.
> 
>
http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
> chrome/installer/util/shell_util.cc:926: LOG(ERROR) << "Could not make Chrome
> default protocol client (XP).";
> On 2012/09/20 13:10:13, grt wrote:
> > nit: de-indent two spaces
> 
> Done.
> 
>
http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
> chrome/installer/util/shell_util.cc:1377: // links of this type (protocol)?"
> dialog.  We choose the latter.
> On 2012/09/20 13:10:13, grt wrote:
> > Looks like this is the text for line 1452 (and vice-versa).
> 
> Done.
> 
>
http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
> chrome/installer/util/shell_util.cc:1397: if (!RegisterChromeForProtocol(dist,
> chrome_exe, L"", protocol, true))
> On 2012/09/20 13:10:13, grt wrote:
> > L"" -> string16()
> 
> Done.
> 
>
http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
> chrome/installer/util/shell_util.cc:1444: if (!RegisterChromeForProtocol(dist,
> chrome_exe, L"", protocol, true))
> On 2012/09/20 13:10:13, grt wrote:
> > L"" -> string16()
> 
> Done.
> 
>
http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
> chrome/installer/util/shell_util.cc:1445: return false;
> On 2012/09/20 13:10:13, grt wrote:
> > de-indent two spaces
> 
> Done.
> 
>
http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
> File chrome/installer/util/shell_util.h (right):
> 
>
http://codereview.chromium.org/10963004/diff/12001/chrome/installer/util/shel...
> chrome/installer/util/shell_util.h:1: // Copyright (c) 2012 The Chromium
> Authors. All rights reserved
> On 2012/09/20 13:10:13, grt wrote:
> > Please put the '.' back at the end of this line.
> 
> Done.

Ping

[sky, 2012-09-24 14:18:21]

LGTM

[grt (UTC plus 2), 2012-09-24 14:46:20]

lgtm with one teeny tiny nit.

http://codereview.chromium.org/10963004/diff/5005/chrome/installer/util/shell...
File chrome/installer/util/shell_util.h (right):

http://codereview.chromium.org/10963004/diff/5005/chrome/installer/util/shell...
chrome/installer/util/shell_util.h:316: 
nit: only one blank line between functions.

[commit-bot: I haz the power, 2012-09-25 15:53:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/10963004/16001

[commit-bot: I haz the power, 2012-09-25 18:21:05]

Change committed as 158612

[gab, 2012-10-03 19:32:42]

late drive-by, just noticed this code

https://chromiumcodereview.appspot.com/10963004/diff/16001/chrome/installer/u...
File chrome/installer/util/shell_util.cc (right):

https://chromiumcodereview.appspot.com/10963004/diff/16001/chrome/installer/u...
chrome/installer/util/shell_util.cc:1438: bool
ShellUtil::ShowMakeChromeDefaultProtocolClientSystemUI(
I just saw this in the code, does this work from Chrome Metro? The similar
ShowMakeChromeDefaultSystemUI() call introduced for Win8 can never be called
from Metro as it's only ever called when Chrome is not the default browser. This
call however can happen to make other protocols default (and thus can happen
from Metro).

[benwells, 2012-10-08 03:23:58]

On 2012/10/03 19:32:42, gab wrote:
> late drive-by, just noticed this code
> 
>
https://chromiumcodereview.appspot.com/10963004/diff/16001/chrome/installer/u...
> File chrome/installer/util/shell_util.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10963004/diff/16001/chrome/installer/u...
> chrome/installer/util/shell_util.cc:1438: bool
> ShellUtil::ShowMakeChromeDefaultProtocolClientSystemUI(
> I just saw this in the code, does this work from Chrome Metro? The similar
> ShowMakeChromeDefaultSystemUI() call introduced for Win8 can never be called
> from Metro as it's only ever called when Chrome is not the default browser.
This
> call however can happen to make other protocols default (and thus can happen
> from Metro).

I just checked again, and yep, it works from Metro and from deskto pmode.