Wiring metro mouse events to aura viewer

In-chorme viewer for metro (part 2)

- Wires metro mouse move, mouse click to aura viewer
- Introduces a new RootWindowHost, one that is remote

BUG=151718
TEST=see bug
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=160489

[Ben Goodger (Google), 2012-10-03 19:52:59]

direction looks good

http://codereview.chromium.org/11047012/diff/7001/chrome/browser/metro_viewer...
File chrome/browser/metro_viewer/metro_viewer_process_host_win.cc (right):

http://codereview.chromium.org/11047012/diff/7001/chrome/browser/metro_viewer...
chrome/browser/metro_viewer/metro_viewer_process_host_win.cc:57:
aura::RemoteRootWindowHostWin::Instance()->OnMouseMoved(x, y, modifiers);
I wonder if it'd be nicer to have this guy have a ptr to the RootWindowHost.

http://codereview.chromium.org/11047012/diff/7001/ui/aura/aura.gyp
File ui/aura/aura.gyp (right):

http://codereview.chromium.org/11047012/diff/7001/ui/aura/aura.gyp#newcode91
ui/aura/aura.gyp:91: 'remote_root_window_host_win.cc',
what are you editing this code in, notepad? :p

[cpu_(ooo_6.6-7.5), 2012-10-04 02:32:43]

Ok, most warts removed, please review now.

[jam, 2012-10-04 15:27:38]

removing myself as a reviewer since Ben is an owner in the content code

nit: no need to forward declare the method in the file, put it first?

[Ben Goodger (Google), 2012-10-04 15:48:30]

http://codereview.chromium.org/11047012/diff/27003/ash/display/multi_display_...
File ash/display/multi_display_manager.cc (right):

http://codereview.chromium.org/11047012/diff/27003/ash/display/multi_display_...
ash/display/multi_display_manager.cc:272: }
Write this like so:

  RootWindow::CreateParams params(display.bounds_in_pixel());
#if defined(OS_WIN)
  if (base::win::GetVersion() >= base::win::VERSION_WIN8)
    params.host = aura::RemoteRootWindowHostWin::GetInstance();
#endif
  RootWindow* root_window = new RootWindow(params);
  ...

http://codereview.chromium.org/11047012/diff/27003/ui/aura/remote_root_window...
File ui/aura/remote_root_window_host_win.cc (right):

http://codereview.chromium.org/11047012/diff/27003/ui/aura/remote_root_window...
ui/aura/remote_root_window_host_win.cc:32: RemoteRootWindowHostWin*
RemoteRootWindowHostWin::Instance() {
This should be GetInstance() and create on first call.

http://codereview.chromium.org/11047012/diff/27003/ui/aura/remote_root_window...
ui/aura/remote_root_window_host_win.cc:36: RootWindow*
RemoteRootWindowHostWin::CreateRootWindow(
This function can go away per my comment in the multi-display-manager.

http://codereview.chromium.org/11047012/diff/27003/ui/aura/remote_root_window...
File ui/aura/remote_root_window_host_win.h (right):

http://codereview.chromium.org/11047012/diff/27003/ui/aura/remote_root_window...
ui/aura/remote_root_window_host_win.h:22: class AURA_EXPORT
RemoteRootWindowHostWin : public RootWindowHost {
// RWH implementation that receives events from a separate viewer process.

http://codereview.chromium.org/11047012/diff/27003/ui/aura/remote_root_window...
ui/aura/remote_root_window_host_win.h:30: RemoteRootWindowHostWin(
ctor/dtors -> start of private section, don't forget to update order in .cc

http://codereview.chromium.org/11047012/diff/27003/ui/aura/remote_root_window...
ui/aura/remote_root_window_host_win.h:35: protected:
do you expect this to get overridden? if not, private:

[cpu_(ooo_6.6-7.5), 2012-10-04 19:57:56]

comments addressed, please take a look again.

http://codereview.chromium.org/11047012/diff/27003/ash/display/multi_display_...
File ash/display/multi_display_manager.cc (right):

http://codereview.chromium.org/11047012/diff/27003/ash/display/multi_display_...
ash/display/multi_display_manager.cc:272: }
On 2012/10/04 15:48:30, Ben Goodger (Google) wrote:
> Write this like so:
> 
>   RootWindow::CreateParams params(display.bounds_in_pixel());
> #if defined(OS_WIN)
>   if (base::win::GetVersion() >= base::win::VERSION_WIN8)
>     params.host = aura::RemoteRootWindowHostWin::GetInstance();
> #endif
>   RootWindow* root_window = new RootWindow(params);
>   ...

The problem is that I need to update the delegate_ member of the RRWH with
root_window, but RWH does not have a method to set the delegate except for the
constructor.

That is why I think the DesktopRootWindowHostWin::Init does it the same way I
do.

http://codereview.chromium.org/11047012/diff/27003/ui/aura/remote_root_window...
File ui/aura/remote_root_window_host_win.cc (right):

http://codereview.chromium.org/11047012/diff/27003/ui/aura/remote_root_window...
ui/aura/remote_root_window_host_win.cc:36: RootWindow*
RemoteRootWindowHostWin::CreateRootWindow(
On 2012/10/04 15:48:30, Ben Goodger (Google) wrote:
> This function can go away per my comment in the multi-display-manager.

See my comment on the multi-display-manager file.

http://codereview.chromium.org/11047012/diff/27003/ui/aura/remote_root_window...
File ui/aura/remote_root_window_host_win.h (right):

http://codereview.chromium.org/11047012/diff/27003/ui/aura/remote_root_window...
ui/aura/remote_root_window_host_win.h:22: class AURA_EXPORT
RemoteRootWindowHostWin : public RootWindowHost {
On 2012/10/04 15:48:30, Ben Goodger (Google) wrote:
> // RWH implementation that receives events from a separate viewer process.

Done.

http://codereview.chromium.org/11047012/diff/27003/ui/aura/remote_root_window...
ui/aura/remote_root_window_host_win.h:30: RemoteRootWindowHostWin(
On 2012/10/04 15:48:30, Ben Goodger (Google) wrote:
> ctor/dtors -> start of private section, don't forget to update order in .cc

Done.

http://codereview.chromium.org/11047012/diff/27003/ui/aura/remote_root_window...
ui/aura/remote_root_window_host_win.h:35: protected:
On 2012/10/04 15:48:30, Ben Goodger (Google) wrote:
> do you expect this to get overridden? if not, private:

Done.

[Ben Goodger (Google), 2012-10-04 20:03:41]

On 2012/10/04 19:57:56, cpu wrote:
> The problem is that I need to update the delegate_ member of the RRWH with
> root_window, but RWH does not have a method to set the delegate except for the
> constructor.

So, instead of doing this can you add a SetDelegate() method to RWH, and rewrite
CreateHost() in ui/aura/root_window.cc like so:

RootWindowHost* CreateHost(RootWindow* root_window,
                           const RootWindow::CreateParams& params) {
  RootWindowHost* host = params.host ||
      RootWindowHost::Create(params.initial_bounds);
  host->SetDelegate(root_window);
  return host;
}

... you would also change RootWindowHost::Create() to just take bounds and not
the delegate.

This lets us consolidate everything more neatly.

[cpu_(ooo_6.6-7.5), 2012-10-05 20:34:33]

refactored RWH per your suggestion. Hopefully I didn't break anything. Please
take a look again.

[Ben Goodger (Google), 2012-10-05 20:47:55]

lgtm