Ensure that Metro chrome in ASH is able to connect to an existing desktop chrome AURA process.

Ensure that Metro chrome in ASH is able to connect to an existing desktop chrome AURA process.

To achieve this the following changes were made.

1. Moved the MetroViewerProcessHost instance from the ChromeBrowserMainPartsWin class to the BrowserProcessImpl class.
2. We instantiate the MetroViewerProcessHost class as needed, i.e. when the command line for the current browser process
   specifies that a metro viewer is being created and when a chrome browser process delegates to an existing desktop chrome
   browser process passing in the metro viewer command line.
3. The functionality to process a commmand line is provided by the new virtual function PlatformSpecificCommandLineProcessing
   which is invoked during startup and when a newly launched chrome browser process delegates to an already running instance.

I also added a call to CloseAsh in the channel error handling in the MetroViewerProcessHost class, to ensure that subsequent launches
of Chrome Ash in metro succeed.

BUG=151718
R=sky
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=169777

[sky, 2012-11-27 16:30:49]

I'm going to pass this to Ben, who I think understand all these pieces a bit
better than I do. My initial reaction is adding
PlatformSpecificCommandLineProcessing to BrowserProcess is the wrong place for
the code, but I'll let Ben make that call.

[sky, 2012-11-27 16:31:40]

Also note, I saw an AddRef without a corresponding ReleaseModule. Is that right?

[Ben Goodger (Google), 2012-11-27 16:56:41]

https://codereview.chromium.org/11299192/diff/4002/chrome/browser/browser_pro...
File chrome/browser/browser_process_impl.h (right):

https://codereview.chromium.org/11299192/diff/4002/chrome/browser/browser_pro...
chrome/browser/browser_process_impl.h:275: void PerformInitForWindowsAura(const
CommandLine& command_line);
nit: NL after

https://codereview.chromium.org/11299192/diff/4002/chrome/browser/process_sin...
File chrome/browser/process_singleton_win.cc (right):

https://codereview.chromium.org/11299192/diff/4002/chrome/browser/process_sin...
chrome/browser/process_singleton_win.cc:374: else if (!remote_window_) {
nit: add braces to entire if..else.

https://codereview.chromium.org/11299192/diff/4002/chrome/browser/ui/startup/...
File chrome/browser/ui/startup/startup_browser_creator.cc (right):

https://codereview.chromium.org/11299192/diff/4002/chrome/browser/ui/startup/...
chrome/browser/ui/startup/startup_browser_creator.cc:591:
g_browser_process->PlatformSpecificCommandLineProcessing(cmd_line);
to the uninitiated (I know this code only slightly better than sky... I renamed
this file from browser_init to sbc a while back because aspirationally I wanted
it to be the code that created the initial set of browser objects on session
start.

You're calling this after doing the initial session startup by the looks of it.
Doesn't this need to happen earlier? Is there a common chokepoint back up in
main somewhere where you could do this?

[ananta, 2012-11-27 19:15:45]

On 2012/11/27 16:31:40, sky wrote:
> Also note, I saw an AddRef without a corresponding ReleaseModule. Is that
right?

I moved the AddRefModule and ReleaseModule to the MetroViewerProcessHost
class.

[ananta, 2012-11-27 19:16:03]

https://codereview.chromium.org/11299192/diff/4002/chrome/browser/browser_pro...
File chrome/browser/browser_process_impl.h (right):

https://codereview.chromium.org/11299192/diff/4002/chrome/browser/browser_pro...
chrome/browser/browser_process_impl.h:275: void PerformInitForWindowsAura(const
CommandLine& command_line);
On 2012/11/27 16:56:41, Ben Goodger (Google) wrote:
> nit: NL after

Done.

https://codereview.chromium.org/11299192/diff/4002/chrome/browser/process_sin...
File chrome/browser/process_singleton_win.cc (right):

https://codereview.chromium.org/11299192/diff/4002/chrome/browser/process_sin...
chrome/browser/process_singleton_win.cc:374: else if (!remote_window_) {
On 2012/11/27 16:56:41, Ben Goodger (Google) wrote:
> nit: add braces to entire if..else.

Done.

https://codereview.chromium.org/11299192/diff/4002/chrome/browser/ui/startup/...
File chrome/browser/ui/startup/startup_browser_creator.cc (right):

https://codereview.chromium.org/11299192/diff/4002/chrome/browser/ui/startup/...
chrome/browser/ui/startup/startup_browser_creator.cc:591:
g_browser_process->PlatformSpecificCommandLineProcessing(cmd_line);
On 2012/11/27 16:56:41, Ben Goodger (Google) wrote:
> to the uninitiated (I know this code only slightly better than sky... I
renamed
> this file from browser_init to sbc a while back because aspirationally I
wanted
> it to be the code that created the initial set of browser objects on session
> start.
> 
> You're calling this after doing the initial session startup by the looks of
it.
> Doesn't this need to happen earlier? Is there a common chokepoint back up in
> main somewhere where you could do this?

Moved this to the ProcessSingletonNotificationCallback function which gets
called when a running chrome browser process handles command line parameters
invoked from a launched chrome browser exe which defers to the running instance.

[ananta, 2012-11-27 19:17:24]

https://codereview.chromium.org/11299192/diff/10007/chrome/browser/metro_view...
File chrome/browser/metro_viewer/metro_viewer_process_host_win.cc (left):

https://codereview.chromium.org/11299192/diff/10007/chrome/browser/metro_view...
chrome/browser/metro_viewer/metro_viewer_process_host_win.cc:54:
browser::CloseAllBrowsers();
Removed the call to CloseAllBrowsers as it closes desktop browsers as well. The
CloseAsh function which we now call does the required cleanup.

[Ben Goodger (Google), 2012-11-27 19:25:31]

lgtm