MultiMonitor support

* MultiMonitorManager creates and manages mutliple monitor instance
* MonitorContrler controls root window <-> monitor mapping and resize/reposition root window to match monitor configuration.
* Moved use_fullscreen_host_window to MonitorManager
* Moved compositor initialize/terminate to Env as there can be more than one root window.

BUG=115510
TEST=none

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

[oshima, 2012-03-20 00:21:35]

piman -> compositor initialization change (root window & env)
ben -> everything

[piman, 2012-03-20 00:31:37]

http://codereview.chromium.org/9701098/diff/9012/ui/aura/env.cc
File ui/aura/env.cc (right):

http://codereview.chromium.org/9701098/diff/9012/ui/aura/env.cc#newcode44
ui/aura/env.cc:44: void Env::DeleteInstance() {
Just to confirm, where is this called from? It needs to happen after all the
root windows have been destroyed, but before we destroy the webkit thread.
I didn't see it called in ~Shell, but maybe that's done implicitly.

http://codereview.chromium.org/9701098/diff/9012/ui/aura/root_window.cc
File ui/aura/root_window.cc (right):

http://codereview.chromium.org/9701098/diff/9012/ui/aura/root_window.cc#newco...
ui/aura/root_window.cc:123: if (instance_ == this)
Should this go away now that we can have more than one RootWindow?

[oshima, 2012-03-20 02:29:58]

http://codereview.chromium.org/9701098/diff/9012/ui/aura/env.cc
File ui/aura/env.cc (right):

http://codereview.chromium.org/9701098/diff/9012/ui/aura/env.cc#newcode44
ui/aura/env.cc:44: void Env::DeleteInstance() {
On 2012/03/20 00:31:37, piman wrote:
> Just to confirm, where is this called from? It needs to happen after all the
> root windows have been destroyed, but before we destroy the webkit thread.
> I didn't see it called in ~Shell, but maybe that's done implicitly.

This is called in PostMessageLoopRun, which may be too late and 
after webkit thread was destroyed (I'm not certain). When does webkit thread get
destroyed?

 Alternatively, I can count the creation/deletion of RootWindow and Terminate
when root window count becomes 0. Let me know what you think.

http://codereview.chromium.org/9701098/diff/9012/ui/aura/root_window.cc
File ui/aura/root_window.cc (right):

http://codereview.chromium.org/9701098/diff/9012/ui/aura/root_window.cc#newco...
ui/aura/root_window.cc:123: if (instance_ == this)
On 2012/03/20 00:31:37, piman wrote:
> Should this go away now that we can have more than one RootWindow?

looks like this is obsolete code. deleted.

[piman, 2012-03-20 04:45:46]

lgtm

http://codereview.chromium.org/9701098/diff/9012/ui/aura/env.cc
File ui/aura/env.cc (right):

http://codereview.chromium.org/9701098/diff/9012/ui/aura/env.cc#newcode44
ui/aura/env.cc:44: void Env::DeleteInstance() {
On 2012/03/20 02:29:58, oshima wrote:
> On 2012/03/20 00:31:37, piman wrote:
> > Just to confirm, where is this called from? It needs to happen after all the
> > root windows have been destroyed, but before we destroy the webkit thread.
> > I didn't see it called in ~Shell, but maybe that's done implicitly.
> 
> This is called in PostMessageLoopRun, which may be too late and 
> after webkit thread was destroyed (I'm not certain). When does webkit thread
get
> destroyed?

Ok, this is fine then, according to
http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/browser_main_...

> 
>  Alternatively, I can count the creation/deletion of RootWindow and Terminate
> when root window count becomes 0. Let me know what you think.

I'm thinking, it would be useful to have a DCHECK just in case.

[oshima, 2012-03-20 16:31:13]

http://codereview.chromium.org/9701098/diff/9012/ui/aura/env.cc
File ui/aura/env.cc (right):

http://codereview.chromium.org/9701098/diff/9012/ui/aura/env.cc#newcode44
ui/aura/env.cc:44: void Env::DeleteInstance() {
On 2012/03/20 04:45:46, piman wrote:
> On 2012/03/20 02:29:58, oshima wrote:
> > On 2012/03/20 00:31:37, piman wrote:
> > > Just to confirm, where is this called from? It needs to happen after all
the
> > > root windows have been destroyed, but before we destroy the webkit thread.
> > > I didn't see it called in ~Shell, but maybe that's done implicitly.
> > 
> > This is called in PostMessageLoopRun, which may be too late and 
> > after webkit thread was destroyed (I'm not certain). When does webkit thread
> get
> > destroyed?
> 
> Ok, this is fine then, according to
>
http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/browser_main_...
> 
> > 
> >  Alternatively, I can count the creation/deletion of RootWindow and
Terminate
> > when root window count becomes 0. Let me know what you think.
> 
> I'm thinking, it would be useful to have a DCHECK just in case.

Let me clarify. Do you want to count # of current root windows and check in
~Env?

[piman, 2012-03-20 16:49:33]

On 2012/03/20 16:31:13, oshima wrote:
> http://codereview.chromium.org/9701098/diff/9012/ui/aura/env.cc
> File ui/aura/env.cc (right):
> 
> http://codereview.chromium.org/9701098/diff/9012/ui/aura/env.cc#newcode44
> ui/aura/env.cc:44: void Env::DeleteInstance() {
> On 2012/03/20 04:45:46, piman wrote:
> > On 2012/03/20 02:29:58, oshima wrote:
> > > On 2012/03/20 00:31:37, piman wrote:
> > > > Just to confirm, where is this called from? It needs to happen after all
> the
> > > > root windows have been destroyed, but before we destroy the webkit
thread.
> > > > I didn't see it called in ~Shell, but maybe that's done implicitly.
> > > 
> > > This is called in PostMessageLoopRun, which may be too late and 
> > > after webkit thread was destroyed (I'm not certain). When does webkit
thread
> > get
> > > destroyed?
> > 
> > Ok, this is fine then, according to
> >
>
http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/browser_main_...
> > 
> > > 
> > >  Alternatively, I can count the creation/deletion of RootWindow and
> Terminate
> > > when root window count becomes 0. Let me know what you think.
> > 
> > I'm thinking, it would be useful to have a DCHECK just in case.
> 
> Let me clarify. Do you want to count # of current root windows and check in
> ~Env?

Now I'm thinking, it should probably be done in Compositor::Terminate (counting
Compositors).

[piman, 2012-03-20 16:50:29]

On 2012/03/20 16:49:33, piman wrote:
> On 2012/03/20 16:31:13, oshima wrote:
> > http://codereview.chromium.org/9701098/diff/9012/ui/aura/env.cc
> > File ui/aura/env.cc (right):
> > 
> > http://codereview.chromium.org/9701098/diff/9012/ui/aura/env.cc#newcode44
> > ui/aura/env.cc:44: void Env::DeleteInstance() {
> > On 2012/03/20 04:45:46, piman wrote:
> > > On 2012/03/20 02:29:58, oshima wrote:
> > > > On 2012/03/20 00:31:37, piman wrote:
> > > > > Just to confirm, where is this called from? It needs to happen after
all
> > the
> > > > > root windows have been destroyed, but before we destroy the webkit
> thread.
> > > > > I didn't see it called in ~Shell, but maybe that's done implicitly.
> > > > 
> > > > This is called in PostMessageLoopRun, which may be too late and 
> > > > after webkit thread was destroyed (I'm not certain). When does webkit
> thread
> > > get
> > > > destroyed?
> > > 
> > > Ok, this is fine then, according to
> > >
> >
>
http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/browser_main_...
> > > 
> > > > 
> > > >  Alternatively, I can count the creation/deletion of RootWindow and
> > Terminate
> > > > when root window count becomes 0. Let me know what you think.
> > > 
> > > I'm thinking, it would be useful to have a DCHECK just in case.
> > 
> > Let me clarify. Do you want to count # of current root windows and check in
> > ~Env?
> 
> Now I'm thinking, it should probably be done in Compositor::Terminate
(counting
> Compositors).

We can leave it for a future CL.

[Ben Goodger (Google), 2012-03-20 17:09:02]

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/monitor_control...
File ash/monitor/monitor_controller.cc (right):

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/monitor_control...
ash/monitor/monitor_controller.cc:53:
root_windows_[monitor]->SetHostBounds(monitor->bounds());
nit: no braces

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/monitor_control...
ash/monitor/monitor_controller.cc:64: // primary monitor
// Primary monitor.

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/monitor_control...
File ash/monitor/monitor_controller.h (right):

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/monitor_control...
ash/monitor/monitor_controller.h:22: // MonitorController controls RootWindow to
match monitor's configuration.
"MonitorController creates and maintains RootWindows for each attached monitor,
keeping them in sync with monitor configuration changes."

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/monitor_control...
ash/monitor/monitor_controller.h:34: // Mapping from monitor to RootWindows.
comment is not that useful :-)

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/multi_monitor_m...
File ash/monitor/multi_monitor_manager.cc (right):

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/multi_monitor_m...
ash/monitor/multi_monitor_manager.cc:74: // the primary wnidow.
window

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/multi_monitor_m...
File ash/monitor/multi_monitor_manager.h (right):

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/multi_monitor_m...
ash/monitor/multi_monitor_manager.h:45: typedef std::vector<aura::Monitor*>
Monitors;
typedefs go at the start of the section

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/secondary_monit...
File ash/monitor/secondary_monitor_view.h (right):

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/secondary_monit...
ash/monitor/secondary_monitor_view.h:19: views::Widget*
CreateSecondaryMonitorWidget(aura::Window* parent);
// Creates the widget that hosts the static message displayed on the secondary
monitor.

http://codereview.chromium.org/9701098/diff/20023/ash/shell.cc
File ash/shell.cc (right):

http://codereview.chromium.org/9701098/diff/20023/ash/shell.cc#newcode410
ash/shell.cc:410: CreateRootWindowForPrimaryMonitor()),
and replace this with your helper function too (that I mention in another
comment in your CL)

http://codereview.chromium.org/9701098/diff/20023/ui/aura/demo/demo_main.cc
File ui/aura/demo/demo_main.cc (right):

http://codereview.chromium.org/9701098/diff/20023/ui/aura/demo/demo_main.cc#n...
ui/aura/demo/demo_main.cc:116: aura::Env::GetInstance()->monitor_manager()->
this seems convoluted. can you add a global helper function in env.h that does
this?

[oshima, 2012-03-20 19:26:36]

new patch uploaded. PTAL

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/monitor_control...
File ash/monitor/monitor_controller.cc (right):

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/monitor_control...
ash/monitor/monitor_controller.cc:53:
root_windows_[monitor]->SetHostBounds(monitor->bounds());
On 2012/03/20 17:09:02, Ben Goodger (Google) wrote:
> nit: no braces

Done.

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/monitor_control...
ash/monitor/monitor_controller.cc:64: // primary monitor
On 2012/03/20 17:09:02, Ben Goodger (Google) wrote:
> // Primary monitor.

Done.

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/monitor_control...
File ash/monitor/monitor_controller.h (right):

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/monitor_control...
ash/monitor/monitor_controller.h:22: // MonitorController controls RootWindow to
match monitor's configuration.
On 2012/03/20 17:09:02, Ben Goodger (Google) wrote:
> "MonitorController creates and maintains RootWindows for each attached
monitor,
> keeping them in sync with monitor configuration changes."

Done.

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/monitor_control...
ash/monitor/monitor_controller.h:34: // Mapping from monitor to RootWindows.
On 2012/03/20 17:09:02, Ben Goodger (Google) wrote:
> comment is not that useful :-)

Remoevd

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/multi_monitor_m...
File ash/monitor/multi_monitor_manager.cc (right):

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/multi_monitor_m...
ash/monitor/multi_monitor_manager.cc:74: // the primary wnidow.
On 2012/03/20 17:09:02, Ben Goodger (Google) wrote:
> window

Done.

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/multi_monitor_m...
File ash/monitor/multi_monitor_manager.h (right):

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/multi_monitor_m...
ash/monitor/multi_monitor_manager.h:45: typedef std::vector<aura::Monitor*>
Monitors;
On 2012/03/20 17:09:02, Ben Goodger (Google) wrote:
> typedefs go at the start of the section

Done.

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/secondary_monit...
File ash/monitor/secondary_monitor_view.h (right):

http://codereview.chromium.org/9701098/diff/20023/ash/monitor/secondary_monit...
ash/monitor/secondary_monitor_view.h:19: views::Widget*
CreateSecondaryMonitorWidget(aura::Window* parent);
On 2012/03/20 17:09:02, Ben Goodger (Google) wrote:
> // Creates the widget that hosts the static message displayed on the secondary
> monitor.

Done.

http://codereview.chromium.org/9701098/diff/20023/ash/shell.cc
File ash/shell.cc (right):

http://codereview.chromium.org/9701098/diff/20023/ash/shell.cc#newcode410
ash/shell.cc:410: CreateRootWindowForPrimaryMonitor()),
On 2012/03/20 17:09:02, Ben Goodger (Google) wrote:
> and replace this with your helper function too (that I mention in another
> comment in your CL)

Done.

http://codereview.chromium.org/9701098/diff/20023/ui/aura/demo/demo_main.cc
File ui/aura/demo/demo_main.cc (right):

http://codereview.chromium.org/9701098/diff/20023/ui/aura/demo/demo_main.cc#n...
ui/aura/demo/demo_main.cc:116: aura::Env::GetInstance()->monitor_manager()->
On 2012/03/20 17:09:02, Ben Goodger (Google) wrote:
> this seems convoluted. can you add a global helper function in env.h that does
> this?

Done.

[Ben Goodger (Google), 2012-03-20 21:30:06]

lgtm

[oshima, 2012-03-20 22:44:22]

On 2012/03/20 21:30:06, Ben Goodger (Google) wrote:
> lgtm

TestLoginAfterRepost is failing on other try jobs too. Other two seems to be
flake (Tested locally).

[oshima, 2012-03-20 22:44:22]

On 2012/03/20 21:30:06, Ben Goodger (Google) wrote:
> lgtm

TestLoginAfterRepost is failing on other try jobs too. Other two seems to be
flake (Tested locally).