Maximize window in the display to be restored.

Maximize window in the display to be restored.

BUG=268987
TEST=covered by test.

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

[Jun Mukai, 2013-08-08 23:17:32]

lgtm

I think Intersect() is not ideal, since the display bounds may be changed (due
to layout change etc) after the window's restore bounds are set, but as you
described, currently it's the best guess.  Please do the TODO in the future.

https://codereview.chromium.org/22394003/diff/9002/ash/wm/workspace/workspace...
File ash/wm/workspace/workspace_layout_manager.cc (right):

https://codereview.chromium.org/22394003/diff/9002/ash/wm/workspace/workspace...
ash/wm/workspace/workspace_layout_manager.cc:62: aura::Window* new_container =
Shell::GetContainer(new_root, id);
better to use window->parent()->id() inline here.
or at least, it's better to exchange the order of declarations of id and
new_root (I got a bit confused by the display.id() and id).

[oshima, 2013-08-09 02:36:20]

https://codereview.chromium.org/22394003/diff/9002/ash/wm/workspace/workspace...
File ash/wm/workspace/workspace_layout_manager.cc (right):

https://codereview.chromium.org/22394003/diff/9002/ash/wm/workspace/workspace...
ash/wm/workspace/workspace_layout_manager.cc:62: aura::Window* new_container =
Shell::GetContainer(new_root, id);
On 2013/08/08 23:17:32, Jun Mukai wrote:
> better to use window->parent()->id() inline here.
> or at least, it's better to exchange the order of declarations of id and
> new_root (I got a bit confused by the display.id() and id).

Done.

[oshima, 2013-08-09 02:38:15]

jamescook -> ash/
sadrul -> ui/views

[sadrul, 2013-08-09 05:33:49]

ui/views/ lgtm

[James Cook, 2013-08-09 16:48:54]

LGTM with nits

https://codereview.chromium.org/22394003/diff/16001/ash/wm/workspace/workspac...
File ash/wm/workspace/workspace_layout_manager.cc (right):

https://codereview.chromium.org/22394003/diff/16001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_layout_manager.cc:45: void
MoveToDisplayToBeRestored(aura::Window* window,
MoveToDisplayForRestore?  ReparentToDisplayForRestore?  The current name kind of
sounds like the display is being restored.

https://codereview.chromium.org/22394003/diff/16001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_layout_manager.cc:48: // the root window. Thre is no
information about in which
Thre -> There

https://codereview.chromium.org/22394003/diff/16001/ash/wm/workspace/workspac...
File ash/wm/workspace/workspace_layout_manager_unittest.cc (right):

https://codereview.chromium.org/22394003/diff/16001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_layout_manager_unittest.cc:34: ui::WindowShowState*
show_state) const {
OVERRIDE?

https://codereview.chromium.org/22394003/diff/16001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_layout_manager_unittest.cc:170: EXPECT_EQ("400,0
30x40", w1->GetWindowBoundsInScreen().ToString());
Nice test - very thorough.

[oshima, 2013-08-09 17:30:06]

https://codereview.chromium.org/22394003/diff/16001/ash/wm/workspace/workspac...
File ash/wm/workspace/workspace_layout_manager.cc (right):

https://codereview.chromium.org/22394003/diff/16001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_layout_manager.cc:45: void
MoveToDisplayToBeRestored(aura::Window* window,
On 2013/08/09 16:48:54, James Cook wrote:
> MoveToDisplayForRestore?  ReparentToDisplayForRestore?  The current name kind
of
> sounds like the display is being restored.

Done.

https://codereview.chromium.org/22394003/diff/16001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_layout_manager.cc:48: // the root window. Thre is no
information about in which
On 2013/08/09 16:48:54, James Cook wrote:
> Thre -> There

Done.

https://codereview.chromium.org/22394003/diff/16001/ash/wm/workspace/workspac...
File ash/wm/workspace/workspace_layout_manager_unittest.cc (right):

https://codereview.chromium.org/22394003/diff/16001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_layout_manager_unittest.cc:34: ui::WindowShowState*
show_state) const {
On 2013/08/09 16:48:54, James Cook wrote:
> OVERRIDE?

Done.

[commit-bot: I haz the power, 2013-08-09 17:32:03]

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

[commit-bot: I haz the power, 2013-08-09 18:35:19]

Retried try job too often on linux_chromeos for step(s) ash_unittests,
unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-08-09 18:52:25]

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

[commit-bot: I haz the power, 2013-08-09 19:05:09]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2013-08-09 19:50:36]

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

[commit-bot: I haz the power, 2013-08-12 04:39:04]

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

[commit-bot: I haz the power, 2013-08-12 07:44:24]

Change committed as 216953