Temporarily remember the primary root window while replacing the display.

Temporarily remember the primary root window while replacing the display.

|root_windows_| map becomes empty while replacing display.
This CL remembers the primary root window temporarily and
use it when the display is added.

This is a bit ugly but less risky to merge to 25 branch.
For 26, I'll refactor and add "replace" protocol to
DisplayObserver.

BUG=166673
TEST=covered by test

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

[Daniel Erat, 2012-12-19 16:49:13]

https://codereview.chromium.org/11644012/diff/2015/ash/display/display_contro...
File ash/display/display_controller.h (right):

https://codereview.chromium.org/11644012/diff/2015/ash/display/display_contro...
ash/display/display_controller.h:186: aura::RootWindow*
primary_root_window_for_replace_;
can you store this in a scoped_ptr instead so it won't be leaked?

[oshima, 2012-12-19 16:59:51]

https://codereview.chromium.org/11644012/diff/2015/ash/display/display_contro...
File ash/display/display_controller.h (right):

https://codereview.chromium.org/11644012/diff/2015/ash/display/display_contro...
ash/display/display_controller.h:186: aura::RootWindow*
primary_root_window_for_replace_;
On 2012/12/19 16:49:13, Daniel Erat wrote:
> can you store this in a scoped_ptr instead so it won't be leaked?

If it ever leaked, many bad things will happen, and will most likely causes
crash. I'd rather add CHECK where appropriate. What do you think?

[Daniel Erat, 2012-12-19 17:02:26]

https://codereview.chromium.org/11644012/diff/2015/ash/display/display_contro...
File ash/display/display_controller.h (right):

https://codereview.chromium.org/11644012/diff/2015/ash/display/display_contro...
ash/display/display_controller.h:186: aura::RootWindow*
primary_root_window_for_replace_;
On 2012/12/19 16:59:51, oshima wrote:
> On 2012/12/19 16:49:13, Daniel Erat wrote:
> > can you store this in a scoped_ptr instead so it won't be leaked?
> 
> If it ever leaked, many bad things will happen, and will most likely causes
> crash. I'd rather add CHECK where appropriate. What do you think?

Doesn't this get leaked if OnDisplayRemoved() is called but the
DisplayController is destroyed before OnDisplayAdded() is called to reclaim this
pointer?  Using scoped_ptr seems like it makes owership clearer.

[oshima, 2012-12-19 17:23:03]

On 2012/12/19 17:02:26, Daniel Erat wrote:
>
https://codereview.chromium.org/11644012/diff/2015/ash/display/display_contro...
> File ash/display/display_controller.h (right):
> 
>
https://codereview.chromium.org/11644012/diff/2015/ash/display/display_contro...
> ash/display/display_controller.h:186: aura::RootWindow*
> primary_root_window_for_replace_;
> On 2012/12/19 16:59:51, oshima wrote:
> > On 2012/12/19 16:49:13, Daniel Erat wrote:
> > > can you store this in a scoped_ptr instead so it won't be leaked?
> > 
> > If it ever leaked, many bad things will happen, and will most likely causes
> > crash. I'd rather add CHECK where appropriate. What do you think?
> 
> Doesn't this get leaked if OnDisplayRemoved() is called but the
> DisplayController is destroyed before OnDisplayAdded() is called to reclaim
this
> pointer?  Using scoped_ptr seems like it makes owership clearer.

displays never becomes empty after each configuration changes, and it's
guaranteed
that OnDisplaysAdded is called after OnDisplayRemoved in the same event
dispatch,
so the scenario above should never happen. If it ever happens, it will cause a
lot of issues
and using scoped_ptr won't help (it will most likely just crash somewhere).

[Daniel Erat, 2012-12-19 17:29:42]

Thanks for the explanation.  LGTM.

[commit-bot: I haz the power, 2012-12-19 18:00:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/11644012/2015

[commit-bot: I haz the power, 2012-12-19 22:05:03]

Change committed as 174008