Make the metro viewer responsible for relaunching browser in desktop mode.

Make the metro viewer responsible for relaunching browser in desktop mode.

Browsers can force a metro -> desktop transition by calling ShellExecuteEx
with a special argument.  However, the process that calls ShellExecute must 
itself be a metro process.  This change implements that behavior.  Without
this change, triggering a relaunch in desktop mode while in ash would 
launch chrome in the desktop mode, but it would not automatically transition
from metro to desktop.

cpu@:*metro*
bauerb@:chrome\browser\plugins\*
sky@:ui\aura\*
jschuh@:security review for ipc messages in ui\metro_viewer

BUG=280377
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221075

[jschuh, 2013-08-30 03:25:02]

ipc security lgtm. privileged process -> privileged process

[Bernhard Bauer, 2013-08-30 07:50:33]

c/b/p LGTM.

https://codereview.chromium.org/23592024/diff/1/ui/aura/remote_root_window_ho...
File ui/aura/remote_root_window_host_win.cc (right):

https://codereview.chromium.org/23592024/diff/1/ui/aura/remote_root_window_ho...
ui/aura/remote_root_window_host_win.cc:176: const base::string16& shortcut,
const base::string16& url) {
Does this need to be a string16? You're converting it from a FilePath anyway.

[sky, 2013-08-30 17:06:56]

https://codereview.chromium.org/23592024/diff/1/chrome/browser/plugins/plugin...
File chrome/browser/plugins/plugin_infobar_delegates.cc (right):

https://codereview.chromium.org/23592024/diff/1/chrome/browser/plugins/plugin...
chrome/browser/plugins/plugin_infobar_delegates.cc:498: base::FilePath
shortcut_path(
nit: 4 space indent is correct here (meaning don't change this).

https://codereview.chromium.org/23592024/diff/1/chrome/browser/plugins/plugin...
chrome/browser/plugins/plugin_infobar_delegates.cc:506:
aura::RemoteRootWindowHostWin::Instance()->HandleOpenURLOnDesktop(
What about non-aura?

https://codereview.chromium.org/23592024/diff/1/ui/aura/remote_root_window_ho...
File ui/aura/remote_root_window_host_win.cc (right):

https://codereview.chromium.org/23592024/diff/1/ui/aura/remote_root_window_ho...
ui/aura/remote_root_window_host_win.cc:176: const base::string16& shortcut,
const base::string16& url) {
nit: when you wrap, one param per line.

[zturner, 2013-08-30 17:14:43]

https://codereview.chromium.org/23592024/diff/1/chrome/browser/plugins/plugin...
File chrome/browser/plugins/plugin_infobar_delegates.cc (right):

https://codereview.chromium.org/23592024/diff/1/chrome/browser/plugins/plugin...
chrome/browser/plugins/plugin_infobar_delegates.cc:506:
aura::RemoteRootWindowHostWin::Instance()->HandleOpenURLOnDesktop(
On 2013/08/30 17:06:56, sky wrote:
> What about non-aura?

This function is only used as a helper function from
PluginMetroModeInfoBarDelegate::Accept() (defined immediately below), which in
turn only calls this function #if defined(USE_AURA).

Based on that, however, I should probably wrap this function in the same set of
pre-processor defines so that it's compiled out in non-aura.

https://codereview.chromium.org/23592024/diff/1/ui/aura/remote_root_window_ho...
File ui/aura/remote_root_window_host_win.cc (right):

https://codereview.chromium.org/23592024/diff/1/ui/aura/remote_root_window_ho...
ui/aura/remote_root_window_host_win.cc:176: const base::string16& shortcut,
const base::string16& url) {
On 2013/08/30 07:50:34, Bernhard Bauer wrote:
> Does this need to be a string16? You're converting it from a FilePath anyway.

Just to make sure I'm following you, you're saying I should pass it through as a
FilePath?  I didn't think FilePath's were IPC serializable, but I suppose I'm
probably wrong about that.

[zturner, 2013-08-30 18:10:05]

https://codereview.chromium.org/23592024/diff/1/chrome/browser/plugins/plugin...
File chrome/browser/plugins/plugin_infobar_delegates.cc (right):

https://codereview.chromium.org/23592024/diff/1/chrome/browser/plugins/plugin...
chrome/browser/plugins/plugin_infobar_delegates.cc:498: base::FilePath
shortcut_path(
On 2013/08/30 17:06:56, sky wrote:
> nit: 4 space indent is correct here (meaning don't change this).

Done.

https://codereview.chromium.org/23592024/diff/1/chrome/browser/plugins/plugin...
chrome/browser/plugins/plugin_infobar_delegates.cc:506:
aura::RemoteRootWindowHostWin::Instance()->HandleOpenURLOnDesktop(
On 2013/08/30 17:14:44, zturner wrote:
> On 2013/08/30 17:06:56, sky wrote:
> > What about non-aura?
> 
> This function is only used as a helper function from
> PluginMetroModeInfoBarDelegate::Accept() (defined immediately below), which in
> turn only calls this function #if defined(USE_AURA).
> 
> Based on that, however, I should probably wrap this function in the same set
of
> pre-processor defines so that it's compiled out in non-aura.

Done.

https://codereview.chromium.org/23592024/diff/1/ui/aura/remote_root_window_ho...
File ui/aura/remote_root_window_host_win.cc (right):

https://codereview.chromium.org/23592024/diff/1/ui/aura/remote_root_window_ho...
ui/aura/remote_root_window_host_win.cc:176: const base::string16& shortcut,
const base::string16& url) {
On 2013/08/30 17:06:56, sky wrote:
> nit: when you wrap, one param per line.

Done.

https://codereview.chromium.org/23592024/diff/1/ui/aura/remote_root_window_ho...
ui/aura/remote_root_window_host_win.cc:176: const base::string16& shortcut,
const base::string16& url) {
On 2013/08/30 07:50:34, Bernhard Bauer wrote:
> Does this need to be a string16? You're converting it from a FilePath anyway.

Done.

[sky, 2013-08-30 19:51:20]

LGTM

[Bernhard Bauer, 2013-08-31 10:15:05]

LGTM w/ nits:

https://codereview.chromium.org/23592024/diff/9001/win8/metro_driver/chrome_a...
File win8/metro_driver/chrome_app_view_ash.cc (right):

https://codereview.chromium.org/23592024/diff/9001/win8/metro_driver/chrome_a...
win8/metro_driver/chrome_app_view_ash.cc:518: string16 file =
shortcut.AsUTF16Unsafe();
You could probably use FilePath::StringType here. It should be the same type (on
Window, both string16 and FilePath::StringType are typedef'd to std::wstring),
but it looks a bit nicer than calling a method that has "Unsafe" in its name
when we in fact know that it is safe.

https://codereview.chromium.org/23592024/diff/9001/win8/metro_driver/chrome_a...
File win8/metro_driver/chrome_app_view_ash.h (right):

https://codereview.chromium.org/23592024/diff/9001/win8/metro_driver/chrome_a...
win8/metro_driver/chrome_app_view_ash.h:20: class FilePath;
Nit: No indent (also below).

[cpu_(ooo_6.6-7.5), 2013-09-03 17:55:11]

lgtm

https://codereview.chromium.org/23592024/diff/9001/win8/metro_driver/chrome_a...
File win8/metro_driver/chrome_app_view_ash.cc (right):

https://codereview.chromium.org/23592024/diff/9001/win8/metro_driver/chrome_a...
win8/metro_driver/chrome_app_view_ash.cc:525: BOOL result =
ShellExecuteEx(&sei);
who knows what COM apartment this thread has... but I guess it works.

[commit-bot: I haz the power, 2013-09-03 20:39:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zturner@chromium.org/23592024/21001

[commit-bot: I haz the power, 2013-09-03 23:48:03]

Change committed as 221075