Only show virtual keyboard on primary root window

Only show virtual keyboard on primary root window

In order to display virtual keyboard(VK) only on primary root window,
this CL did
1. Shell takes ownership of keyboard controller(KC) instead of
RootWindowController
2. keyboard container window is owned by KC instead of its parent

There should only be one KC and one keyboard container at any time after
this change. keyboard container can be dynamically enabled/disabled on a
RootWindowController at runtime. If you want to do that,
you should DisableKeyboard on the previous RootWindowController first and
then EnableKeyboard on the new RootWindowController.

BUG=297858
TEST=
1. enable virtual keyboard from about::/flags on a Chromebook
2. plug in an external monitor

verify only one virtual keyboard shows

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

[bshe, 2013-09-27 19:17:19]

Hey Oshima.

Would you mind to take a look at this CL? It restricts the virtual keyboard to
primary root window.

The crash that I talked to you when changing primary root window is actually a
different issue (because of resize triggered by move root window to new display)
and it should fixed here: https://codereview.chromium.org/25105002/

Thanks!

[oshima, 2013-09-28 04:37:26]

https://codereview.chromium.org/25111002/diff/4001/ash/root_window_controller.cc
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/25111002/diff/4001/ash/root_window_controller...
ash/root_window_controller.cc:529: keyboard_controller_.reset(
If there should be only one KeyboardCOntroller, shouldn't it
be in shell? Then shell call this function in Init() like this

if (KeyboardEnabled()) {
  primary_root->InitKeyboard(keyboard_controller);
}

https://codereview.chromium.org/25111002/diff/4001/ash/root_window_controller...
File ash/root_window_controller_unittest.cc (right):

https://codereview.chromium.org/25111002/diff/4001/ash/root_window_controller...
ash/root_window_controller_unittest.cc:576: root_windows[1] : root_windows[0];
root_windows[ root_windows[0] == primary_root_window ? 1 : 0 ]

https://codereview.chromium.org/25111002/diff/4001/ash/root_window_controller...
ash/root_window_controller_unittest.cc:580: ASSERT_TRUE(keyboard_container);
EXPECT_TRUE(Shell::GetContainer(...))
EXPECT_FALSE(Shell::GetCOntainer(...))

no need to define extra variable.

[bshe, 2013-09-30 18:20:49]

https://codereview.chromium.org/25111002/diff/4001/ash/root_window_controller.cc
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/25111002/diff/4001/ash/root_window_controller...
ash/root_window_controller.cc:529: keyboard_controller_.reset(
+sadrul

I am happy to do the suggested change. But I dont know the code very well to be
certain that it wont cause any other unexpected behavior. I suspect it should be
fine. Just want to +sadrul to confirm before I do it.

On 2013/09/28 04:37:27, oshima wrote:
> If there should be only one KeyboardCOntroller, shouldn't it
> be in shell? Then shell call this function in Init() like this
> 
> if (KeyboardEnabled()) {
>   primary_root->InitKeyboard(keyboard_controller);
> }

[sadrul, 2013-09-30 18:31:50]

https://codereview.chromium.org/25111002/diff/4001/ash/root_window_controller.cc
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/25111002/diff/4001/ash/root_window_controller...
ash/root_window_controller.cc:529: keyboard_controller_.reset(
On 2013/09/30 18:20:50, bshe wrote:
> +sadrul
> 
> I am happy to do the suggested change. But I dont know the code very well to
be
> certain that it wont cause any other unexpected behavior. I suspect it should
be
> fine. Just want to +sadrul to confirm before I do it.

Yeah, if we decide to show the keyboard just on the primary root window, then as
oshima suggests, it should be in the Shell.

I believe the primary root-window can change at runtime (e.g. when
attaching/detaching monitors), so you would need to make sure the VK deals with
that case correctly.

Also note that the primary root-window may not be the one that has touch
capability, and it doesn't sound like a good UI to have the input-box in one
monitor and the VK in another. The decision to only show the virtual keyboard on
the primary root-window should perhaps be revisited (e.g. if we show the VK in
just one monitor, then perhaps don't show it for textboxes in any other monitor,
or allow the user to move the keyboard across monitors etc.).

[oshima, 2013-09-30 18:52:14]

On 2013/09/30 18:31:50, sadrul wrote:
>
https://codereview.chromium.org/25111002/diff/4001/ash/root_window_controller.cc
> File ash/root_window_controller.cc (right):
> 
>
https://codereview.chromium.org/25111002/diff/4001/ash/root_window_controller...
> ash/root_window_controller.cc:529: keyboard_controller_.reset(
> On 2013/09/30 18:20:50, bshe wrote:
> > +sadrul
> > 
> > I am happy to do the suggested change. But I dont know the code very well to
> be
> > certain that it wont cause any other unexpected behavior. I suspect it
should
> be
> > fine. Just want to +sadrul to confirm before I do it.
> 
> Yeah, if we decide to show the keyboard just on the primary root window, then
as
> oshima suggests, it should be in the Shell.
> 
> I believe the primary root-window can change at runtime (e.g. when
> attaching/detaching monitors), so you would need to make sure the VK deals
with
> that case correctly.

Primary display can change (internal/external, 1st/2nd) but
but Primary RootWindow never changes. That is, the primary root window
stays primary until shutdown.

> 
> Also note that the primary root-window may not be the one that has touch
> capability, and it doesn't sound like a good UI to have the input-box in one
> monitor and the VK in another. The decision to only show the virtual keyboard
on
> the primary root-window should perhaps be revisited (e.g. if we show the VK in
> just one monitor, then perhaps don't show it for textboxes in any other
monitor,
> or allow the user to move the keyboard across monitors etc.).

I agree. What was the motivation to show it only on Primary RootWindow?

[bshe, 2013-10-03 01:05:43]

I have made the suggested change (move KC to Shell). Now, only one KC is created
and it is currently bind to PrimaryRootWindow. Could you please take another
look?

https://codereview.chromium.org/25111002/diff/16001/ash/root_window_controlle...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/25111002/diff/16001/ash/root_window_controlle...
ash/root_window_controller.cc:534: 
Note: |DisableKeyboard| is not used anywhere yet. Just put it here to be
complete. It may potentially be used if we want to change VK container at run
time.

[sadrul, 2013-10-03 15:26:28]

https://codereview.chromium.org/25111002/diff/16001/ui/keyboard/keyboard_cont...
File ui/keyboard/keyboard_controller.cc (right):

https://codereview.chromium.org/25111002/diff/16001/ui/keyboard/keyboard_cont...
ui/keyboard/keyboard_controller.cc:136: container_->SetLayoutManager(new
KeyboardLayoutManager(container_));
Is this change necessary?

[bshe, 2013-10-03 15:57:13]

https://codereview.chromium.org/25111002/diff/16001/ui/keyboard/keyboard_cont...
File ui/keyboard/keyboard_controller.cc (right):

https://codereview.chromium.org/25111002/diff/16001/ui/keyboard/keyboard_cont...
ui/keyboard/keyboard_controller.cc:136: container_->SetLayoutManager(new
KeyboardLayoutManager(container_));
dooh. noop. It was from one of my experimental refactor. It is not necessary
with this patch. I will move it back.

On 2013/10/03 15:26:28, sadrul wrote:
> Is this change necessary?

[oshima, 2013-10-03 16:38:46]

don't you have to disable VK when shutdown?

https://codereview.chromium.org/25111002/diff/23001/ash/root_window_controlle...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/25111002/diff/23001/ash/root_window_controlle...
ash/root_window_controller.cc:523: if (keyboard::IsKeyboardEnabled()) {
Please add check if the keyboard controller is already added.

https://codereview.chromium.org/25111002/diff/23001/ash/root_window_controlle...
ash/root_window_controller.cc:540: if
(root_window()->Contains(keyboard_container)) {
keyboard_container->GetRootWindow() == root_window() 

is more efficient

[varkha, 2013-10-03 18:39:41]

https://codereview.chromium.org/25111002/diff/23001/ash/root_window_controlle...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/25111002/diff/23001/ash/root_window_controlle...
ash/root_window_controller.cc:525:
keyboard_controller->AddObserver(panel_layout_manager_);
You will need to add (move here)
keyboard_controller->AddObserver(docked_layout_manager_);
if https://codereview.chromium.org/25740005/ lands first.

https://codereview.chromium.org/25111002/diff/23001/ash/root_window_controlle...
ash/root_window_controller.cc:543:
keyboard_controller->RemoveObserver(panel_layout_manager_);
You will need to add
keyboard_controller->RemoveObserver(docked_layout_manager_);
if https://codereview.chromium.org/25740005/ lands first.

[bshe, 2013-10-04 00:58:08]

Address comments. Could you please take another look?

https://codereview.chromium.org/25111002/diff/16001/ui/keyboard/keyboard_cont...
File ui/keyboard/keyboard_controller.cc (right):

https://codereview.chromium.org/25111002/diff/16001/ui/keyboard/keyboard_cont...
ui/keyboard/keyboard_controller.cc:136: container_->SetLayoutManager(new
KeyboardLayoutManager(container_));
On 2013/10/03 15:26:28, sadrul wrote:
> Is this change necessary?

Done.

https://codereview.chromium.org/25111002/diff/23001/ash/root_window_controlle...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/25111002/diff/23001/ash/root_window_controlle...
ash/root_window_controller.cc:523: if (keyboard::IsKeyboardEnabled()) {
On 2013/10/03 16:38:46, oshima wrote:
> Please add check if the keyboard controller is already added.

Done.

https://codereview.chromium.org/25111002/diff/23001/ash/root_window_controlle...
ash/root_window_controller.cc:525:
keyboard_controller->AddObserver(panel_layout_manager_);
Aha. Looks like your CL has landed. I will add it after rebase.

On 2013/10/03 18:39:41, varkha wrote:
> You will need to add (move here)
> keyboard_controller->AddObserver(docked_layout_manager_);
> if https://codereview.chromium.org/25740005/ lands first.

https://codereview.chromium.org/25111002/diff/23001/ash/root_window_controlle...
ash/root_window_controller.cc:540: if
(root_window()->Contains(keyboard_container)) {
On 2013/10/03 16:38:46, oshima wrote:
> keyboard_container->GetRootWindow() == root_window() 
> 
> is more efficient

Done.

https://codereview.chromium.org/25111002/diff/23001/ash/root_window_controlle...
ash/root_window_controller.cc:543:
keyboard_controller->RemoveObserver(panel_layout_manager_);
ditto
On 2013/10/03 18:39:41, varkha wrote:
> You will need to add
> keyboard_controller->RemoveObserver(docked_layout_manager_);
> if https://codereview.chromium.org/25740005/ lands first.

[sadrul, 2013-10-04 01:19:52]

ui/keyboard/ LGTM

Please update the CL description to briefly explain the change, and also please
run the tests through the memory bots to make sure there are no leaks etc.

[bshe, 2013-10-04 01:48:27]

Thanks. I have updated the issue description.

+miket for
chrome/browser/extensions/api/input/input.cc

[varkha, 2013-10-04 01:54:06]

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
ash/root_window_controller.cc:409: // Disable keyboard container before close
child windows and shutdown
nit: s/close/closing, s/shutdown/shutting down.

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
ash/root_window_controller.cc:525: void RootWindowController::EnableKeyboard(
If I understand it right, "enabled" in keyboard::IsKeyboardEnabled and
EnableKeyboard mean different things. Consider renaming these methods to
something like ActivateKeyboard / DeactivateKeyboard.

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
ash/root_window_controller.cc:528:
!GetContainer(kShellWindowId_VirtualKeyboardContainer)) {
Could reverse the condition and return early.

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
ash/root_window_controller.cc:531:
keyboard_controller->AddObserver(panel_layout_manager_);
AddObserver(docked_layout_manager_)

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
ash/root_window_controller.cc:543: if (keyboard::IsKeyboardEnabled()) {
Favor early return.

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
ash/root_window_controller.cc:550:
keyboard_controller->RemoveObserver(panel_layout_manager_);
RemoveObserver(docked_layout_manager_)

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
File ash/root_window_controller_unittest.cc (right):

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
ash/root_window_controller_unittest.cc:564: 
Comment what this test does.

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
ash/root_window_controller_unittest.cc:580: ASSERT_TRUE(keyboard_container);
nit: consider inlining ASSERT_TRUE(Shell::GetContainer(...));

https://codereview.chromium.org/25111002/diff/35001/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/25111002/diff/35001/ash/shell.cc#newcode162
ash/shell.cc:162: Shell::Shell(ShellDelegate* delegate)
Ctor is private. Also verify that the methods are in the same order as in the
header - they seem to have diverged.

https://codereview.chromium.org/25111002/diff/35001/ash/shell.cc#newcode402
ash/shell.cc:402: void Shell::Init() {
Should go down in private: section.

https://codereview.chromium.org/25111002/diff/35001/ash/shell.cc#newcode886
ash/shell.cc:886: iter != controllers.end(); ++iter) {
nit: align with RootWindowControllerList above.

https://codereview.chromium.org/25111002/diff/35001/ash/shell.cc#newcode887
ash/shell.cc:887: (*iter)->DisableKeyboard(keyboard_controller_.get());
Can there be a situation when you are disabling keyboard here in some root only
to enable it below? If so, consider skipping it in the loop and then skipping
the EnableKeyboard call (unless there is some positive side effect from
disabling and re-enabling the keyboard in the same root.

https://codereview.chromium.org/25111002/diff/35001/chrome/browser/ui/ash/ash...
File chrome/browser/ui/ash/ash_keyboard_controller_proxy.cc (right):

https://codereview.chromium.org/25111002/diff/35001/chrome/browser/ui/ash/ash...
chrome/browser/ui/ash/ash_keyboard_controller_proxy.cc:143: // TODO(bshe):
Implements logic to decide which root window should display
nit: s/Implements/Implement ?

[bshe, 2013-10-04 03:04:38]

Thanks for review. Please see patchset 7 for reviews.

patchset 8 is for fixing keyboard_unittests.

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
ash/root_window_controller.cc:409: // Disable keyboard container before close
child windows and shutdown
On 2013/10/04 01:54:07, varkha wrote:
> nit: s/close/closing, s/shutdown/shutting down.

Done.

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
ash/root_window_controller.cc:525: void RootWindowController::EnableKeyboard(
On 2013/10/04 01:54:07, varkha wrote:
> If I understand it right, "enabled" in keyboard::IsKeyboardEnabled and
> EnableKeyboard mean different things. Consider renaming these methods to
> something like ActivateKeyboard / DeactivateKeyboard.

Done.

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
ash/root_window_controller.cc:528:
!GetContainer(kShellWindowId_VirtualKeyboardContainer)) {
On 2013/10/04 01:54:07, varkha wrote:
> Could reverse the condition and return early.

Done.

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
ash/root_window_controller.cc:531:
keyboard_controller->AddObserver(panel_layout_manager_);
Added in patchset 7 after rebase.

On 2013/10/04 01:54:07, varkha wrote:
> AddObserver(docked_layout_manager_)

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
ash/root_window_controller.cc:543: if (keyboard::IsKeyboardEnabled()) {
On 2013/10/04 01:54:07, varkha wrote:
> Favor early return.

Done.

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
ash/root_window_controller.cc:550:
keyboard_controller->RemoveObserver(panel_layout_manager_);
Removed in patchset 7 after rebase
On 2013/10/04 01:54:07, varkha wrote:
> RemoveObserver(docked_layout_manager_)

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
File ash/root_window_controller_unittest.cc (right):

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
ash/root_window_controller_unittest.cc:564: 
On 2013/10/04 01:54:07, varkha wrote:
> Comment what this test does.

Done.

https://codereview.chromium.org/25111002/diff/35001/ash/root_window_controlle...
ash/root_window_controller_unittest.cc:580: ASSERT_TRUE(keyboard_container);
On 2013/10/04 01:54:07, varkha wrote:
> nit: consider inlining ASSERT_TRUE(Shell::GetContainer(...));

Done.

https://codereview.chromium.org/25111002/diff/35001/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/25111002/diff/35001/ash/shell.cc#newcode162
ash/shell.cc:162: Shell::Shell(ShellDelegate* delegate)
On 2013/10/04 01:54:07, varkha wrote:
> Ctor is private. Also verify that the methods are in the same order as in the
> header - they seem to have diverged.

Done.

https://codereview.chromium.org/25111002/diff/35001/ash/shell.cc#newcode402
ash/shell.cc:402: void Shell::Init() {
On 2013/10/04 01:54:07, varkha wrote:
> Should go down in private: section.

Done.

https://codereview.chromium.org/25111002/diff/35001/ash/shell.cc#newcode886
ash/shell.cc:886: iter != controllers.end(); ++iter) {
On 2013/10/04 01:54:07, varkha wrote:
> nit: align with RootWindowControllerList above.

Done.

https://codereview.chromium.org/25111002/diff/35001/ash/shell.cc#newcode887
ash/shell.cc:887: (*iter)->DisableKeyboard(keyboard_controller_.get());
We reset the keyboard controller before we enable keyboard controller again.
When keyboard controller got reset, the old keyboard container is deleted and a
new one should be created. This is mainly for reload keyboard upon user login.
We want to recreate keyboard controller and keyboard container. 

On 2013/10/04 01:54:07, varkha wrote:
> Can there be a situation when you are disabling keyboard here in some root
only
> to enable it below? If so, consider skipping it in the loop and then skipping
> the EnableKeyboard call (unless there is some positive side effect from
> disabling and re-enabling the keyboard in the same root.

https://codereview.chromium.org/25111002/diff/35001/chrome/browser/ui/ash/ash...
File chrome/browser/ui/ash/ash_keyboard_controller_proxy.cc (right):

https://codereview.chromium.org/25111002/diff/35001/chrome/browser/ui/ash/ash...
chrome/browser/ui/ash/ash_keyboard_controller_proxy.cc:143: // TODO(bshe):
Implements logic to decide which root window should display
On 2013/10/04 01:54:07, varkha wrote:
> nit: s/Implements/Implement ?

Done.

[varkha, 2013-10-04 03:20:06]

lgtm

[oshima, 2013-10-04 16:50:17]

https://codereview.chromium.org/25111002/diff/118001/ash/root_window_controll...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/25111002/diff/118001/ash/root_window_controll...
ash/root_window_controller.cc:549:
keyboard_container->SetBounds(parent->bounds());
won't RootWindowLayoutManager handle this?

https://codereview.chromium.org/25111002/diff/118001/ash/shell.cc
File ash/shell.cc (left):

https://codereview.chromium.org/25111002/diff/118001/ash/shell.cc#oldcode160
ash/shell.cc:160: Shell::Shell(ShellDelegate* delegate)
thank you for fixing this. next time, please do this in separate CL as 
it makes hard to see the diff.

https://codereview.chromium.org/25111002/diff/118001/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/25111002/diff/118001/ash/shell.cc#newcode660
ash/shell.cc:660: 
maybe you should reset the controller before settings instance_ to NULL.

[bshe, 2013-10-04 17:38:32]

https://codereview.chromium.org/25111002/diff/118001/ash/root_window_controll...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/25111002/diff/118001/ash/root_window_controll...
ash/root_window_controller.cc:549:
keyboard_container->SetBounds(parent->bounds());
Sorry, just to confirm, do you mean the SetBounds here is not necessary? In the
RootWindowLayoutManager, it seems OnWindowAddedToLayout didn't do anything (I
assume it is the place to SetBounds of child?). Or maybe we rely on
OnWindowResized to set the bounds?

On 2013/10/04 16:50:17, oshima wrote:
> won't RootWindowLayoutManager handle this?

https://codereview.chromium.org/25111002/diff/118001/ash/shell.cc
File ash/shell.cc (left):

https://codereview.chromium.org/25111002/diff/118001/ash/shell.cc#oldcode160
ash/shell.cc:160: Shell::Shell(ShellDelegate* delegate)
Sorry about it. I should have done it in a separate CL. It is pain to rebase
too.

On 2013/10/04 16:50:17, oshima wrote:
> thank you for fixing this. next time, please do this in separate CL as 
> it makes hard to see the diff.

https://codereview.chromium.org/25111002/diff/118001/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/25111002/diff/118001/ash/shell.cc#newcode660
ash/shell.cc:660: 
Done. keyboard_controller doesn't depend on shell instance so I thought it is ok
to skip the reset. But no harm to reset it too. So added reset here

On 2013/10/04 16:50:17, oshima wrote:
> maybe you should reset the controller before settings instance_ to NULL.

[oshima, 2013-10-04 17:51:35]

lgtm

https://codereview.chromium.org/25111002/diff/118001/ash/root_window_controll...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/25111002/diff/118001/ash/root_window_controll...
ash/root_window_controller.cc:549:
keyboard_container->SetBounds(parent->bounds());
On 2013/10/04 17:38:32, bshe wrote:
> Sorry, just to confirm, do you mean the SetBounds here is not necessary? In
the
> RootWindowLayoutManager, it seems OnWindowAddedToLayout didn't do anything (I
> assume it is the place to SetBounds of child?). Or maybe we rely on
> OnWindowResized to set the bounds?
> 
> On 2013/10/04 16:50:17, oshima wrote:
> > won't RootWindowLayoutManager handle this?
> 

I see. Maybe it's doing this way to avoid some issue in resizing container when
not all containers are added. Can you add TODO(oshima): here to fix
RootWindowLayoutManager and remove this?

[miket_OOO, 2013-10-04 17:54:32]

chrome/browser/extensions/api/input/input.cc LGTM

[bshe, 2013-10-04 18:05:52]

Thanks for reviews!

https://codereview.chromium.org/25111002/diff/118001/ash/root_window_controll...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/25111002/diff/118001/ash/root_window_controll...
ash/root_window_controller.cc:549:
keyboard_container->SetBounds(parent->bounds());
Thanks! Added a TODO.

On 2013/10/04 17:51:35, oshima wrote:
> On 2013/10/04 17:38:32, bshe wrote:
> > Sorry, just to confirm, do you mean the SetBounds here is not necessary? In
> the
> > RootWindowLayoutManager, it seems OnWindowAddedToLayout didn't do anything
(I
> > assume it is the place to SetBounds of child?). Or maybe we rely on
> > OnWindowResized to set the bounds?
> > 
> > On 2013/10/04 16:50:17, oshima wrote:
> > > won't RootWindowLayoutManager handle this?
> > 
> 
> I see. Maybe it's doing this way to avoid some issue in resizing container
when
> not all containers are added. Can you add TODO(oshima): here to fix
> RootWindowLayoutManager and remove this?

[commit-bot: I haz the power, 2013-10-04 18:06:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bshe@chromium.org/25111002/45001

[commit-bot: I haz the power, 2013-10-04 21:38:52]

Change committed as 227088