Converts DockedWindowLayoutManager to common ash/wm types

Converts DockedWindowLayoutManager to common ash/wm types

There are still a couple of aura specific values here
(ShellObserver). I will tackle that next.

BUG=603369
TEST=none
R=varkha@chromium.org

Committed: https://crrev.com/3f7af881b9e91331d387ea22c63db69b99e46836
Cr-Commit-Position: refs/heads/master@{#388847}

[varkha, 2016-04-21 14:56:09]

Shell::DeleteInstance deletes Shelf instance which resets
window_tree_host_manager_ and so the GetPrimaryRootWindow call crashes. Probably
need to delete WmGlobalsAura from inside ~Shelf similar to the rest of the
resets that happen there.
While there I think instance_ = NULL inside Shelf::DeleteInstance can be dropped
- there already is one in ~Shelf.

https://codereview.chromium.org/1907863002/diff/20001/ash/wm/aura/wm_globals_...
File ash/wm/aura/wm_globals_aura.cc (right):

https://codereview.chromium.org/1907863002/diff/20001/ash/wm/aura/wm_globals_...
ash/wm/aura/wm_globals_aura.cc:39:
aura::client::GetActivationClient(Shell::GetPrimaryRootWindow())
I think this fails in the tests. Is that because Shell is gone by now?

[sky, 2016-04-21 16:05:36]

Thanks for debugging my code. You were exactly right. Fixed. Take another look?

[varkha, 2016-04-21 17:46:56]

Nice!

https://codereview.chromium.org/1907863002/diff/40001/ash/root_window_control...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/1907863002/diff/40001/ash/root_window_control...
ash/root_window_controller.cc:813: aura::Window* docked_container =
GetContainer(kShellWindowId_DockedContainer);
Would it make sense to have WmWindow* docked_container =
wm::WmWindowAura::Get(Getcontainer(id)) here and use that below?
Also could wm::WmWindowAura::Get() ever fail in otherwise non-fatal scenario?

https://codereview.chromium.org/1907863002/diff/40001/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/1907863002/diff/40001/ash/shell.cc#newcode819
ash/shell.cc:819: // Needs to happen before |window_tree_host_manager_|.
Callback to Shell, so
nit: s/Callback/Calls back?

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/common/wm_root_w...
File ash/wm/common/wm_root_window_controller.h (right):

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/common/wm_root_w...
ash/wm/common/wm_root_window_controller.h:37: // |shell_container_id| is the id
of the container to parent the widget to.
nit: |widget|.

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/dock/docked_wind...
File ash/wm/dock/docked_window_layout_manager.cc (right):

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/dock/docked_wind...
ash/wm/dock/docked_window_layout_manager.cc:285: int maximum_height =
window->GetMaximumSize().height();
Much simpler, thanks!

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/dock/docked_wind...
ash/wm/dock/docked_window_layout_manager.cc:762: if (child->HasNonClientArea())
{
Is this how to check for resizable windows now?

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/dock/docked_wind...
ash/wm/dock/docked_window_layout_manager.cc:819: aura::Window* root_window) {
Will this be getting wm::WmWindow* in future?

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/dock/docked_wind...
ash/wm/dock/docked_window_layout_manager.cc:942: for (auto iter =
children.rbegin(); iter != children.rend(); ++iter) {
const auto iter?

[sky, 2016-04-21 18:03:05]

https://codereview.chromium.org/1907863002/diff/40001/ash/root_window_control...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/1907863002/diff/40001/ash/root_window_control...
ash/root_window_controller.cc:813: aura::Window* docked_container =
GetContainer(kShellWindowId_DockedContainer);
On 2016/04/21 17:46:55, varkha wrote:
> Would it make sense to have WmWindow* docked_container =
> wm::WmWindowAura::Get(Getcontainer(id)) here and use that below?

Done.

> Also could wm::WmWindowAura::Get() ever fail in otherwise non-fatal scenario?

No. The only potentially bad scenario is if someone calls WmWindowAura::Get()
after the aura::Window properties have been cleared (meaning part way through
the destructor). Other code has this restriction too (such as WindowState), so
I'm hoping it won't come up in practice.

https://codereview.chromium.org/1907863002/diff/40001/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/1907863002/diff/40001/ash/shell.cc#newcode819
ash/shell.cc:819: // Needs to happen before |window_tree_host_manager_|.
Callback to Shell, so
On 2016/04/21 17:46:55, varkha wrote:
> nit: s/Callback/Calls back?

Done.

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/common/wm_root_w...
File ash/wm/common/wm_root_window_controller.h (right):

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/common/wm_root_w...
ash/wm/common/wm_root_window_controller.h:37: // |shell_container_id| is the id
of the container to parent the widget to.
On 2016/04/21 17:46:55, varkha wrote:
> nit: |widget|.

Done.

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/dock/docked_wind...
File ash/wm/dock/docked_window_layout_manager.cc (right):

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/dock/docked_wind...
ash/wm/dock/docked_window_layout_manager.cc:762: if (child->HasNonClientArea())
{
On 2016/04/21 17:46:55, varkha wrote:
> Is this how to check for resizable windows now?

It's the equivalent of child->delegate() in the old. You can still can
WmWindow::GetMinimimumSize() if there is it doesn't have the non-client area and
you'll get 0x0.

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/dock/docked_wind...
ash/wm/dock/docked_window_layout_manager.cc:819: aura::Window* root_window) {
On 2016/04/21 17:46:55, varkha wrote:
> Will this be getting wm::WmWindow* in future?

I'm going to add a WmRootWindowControllerObserver interface that has this
function in it. That's coming up next.

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/dock/docked_wind...
ash/wm/dock/docked_window_layout_manager.cc:942: for (auto iter =
children.rbegin(); iter != children.rend(); ++iter) {
On 2016/04/21 17:46:55, varkha wrote:
> const auto iter?

That makes the iterator const, which you can't increment. So, it has to be this
way.

[varkha, 2016-04-21 18:17:41]

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/dock/docked_wind...
File ash/wm/dock/docked_window_layout_manager.cc (right):

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/dock/docked_wind...
ash/wm/dock/docked_window_layout_manager.cc:762: if (child->HasNonClientArea())
{
On 2016/04/21 18:03:04, sky wrote:
> On 2016/04/21 17:46:55, varkha wrote:
> > Is this how to check for resizable windows now?
> 
> It's the equivalent of child->delegate() in the old. You can still can
> WmWindow::GetMinimimumSize() if there is it doesn't have the non-client area
and
> you'll get 0x0.

If so don't you need to check that in other places that used to check for
window->delegate()?

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/dock/docked_wind...
ash/wm/dock/docked_window_layout_manager.cc:942: for (auto iter =
children.rbegin(); iter != children.rend(); ++iter) {
On 2016/04/21 18:03:04, sky wrote:
> On 2016/04/21 17:46:55, varkha wrote:
> > const auto iter?
> 
> That makes the iterator const, which you can't increment. So, it has to be
this
> way.

Oh I see. I wish there was an auto syntax that would infer const_iterator.

[sky, 2016-04-21 18:18:59]

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/dock/docked_wind...
File ash/wm/dock/docked_window_layout_manager.cc (right):

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/dock/docked_wind...
ash/wm/dock/docked_window_layout_manager.cc:762: if (child->HasNonClientArea())
{
On 2016/04/21 18:17:41, varkha wrote:
> On 2016/04/21 18:03:04, sky wrote:
> > On 2016/04/21 17:46:55, varkha wrote:
> > > Is this how to check for resizable windows now?
> > 
> > It's the equivalent of child->delegate() in the old. You can still can
> > WmWindow::GetMinimimumSize() if there is it doesn't have the non-client area
> and
> > you'll get 0x0.
> 
> If so don't you need to check that in other places that used to check for
> window->delegate()?

The other places correctly deal with the return value. For example, 271 new is
ok with the 0 return value as it's doing a max operation.

[varkha, 2016-04-21 18:26:01]

lgtm.

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/dock/docked_wind...
File ash/wm/dock/docked_window_layout_manager.cc (right):

https://codereview.chromium.org/1907863002/diff/40001/ash/wm/dock/docked_wind...
ash/wm/dock/docked_window_layout_manager.cc:762: if (child->HasNonClientArea())
{
On 2016/04/21 18:18:59, sky wrote:
> On 2016/04/21 18:17:41, varkha wrote:
> > On 2016/04/21 18:03:04, sky wrote:
> > > On 2016/04/21 17:46:55, varkha wrote:
> > > > Is this how to check for resizable windows now?
> > > 
> > > It's the equivalent of child->delegate() in the old. You can still can
> > > WmWindow::GetMinimimumSize() if there is it doesn't have the non-client
area
> > and
> > > you'll get 0x0.
> > 
> > If so don't you need to check that in other places that used to check for
> > window->delegate()?
> 
> The other places correctly deal with the return value. For example, 271 new is
> ok with the 0 return value as it's doing a max operation.

Yes I think you are right (checked that non-resizable windows behave correctly
which was a bit of a problem for me when I was writing this). Thanks!

[sky, 2016-04-21 18:59:53]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2016-04-21 19:00:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1907863002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1907863002/80001

[commit-bot: I haz the power, 2016-04-21 19:30:12]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-04-22 19:37:31]

Description was changed from

==========
Converts DockedWindowLayoutManager to common ash/wm types

There are still a couple of aura specific values here
(ShellObserver). I will tackle that next.

BUG=603369
TEST=none
R=varkha@chromium.org
==========

to

==========
Converts DockedWindowLayoutManager to common ash/wm types

There are still a couple of aura specific values here
(ShellObserver). I will tackle that next.

BUG=603369
TEST=none
R=varkha@chromium.org

Committed: https://crrev.com/3f7af881b9e91331d387ea22c63db69b99e46836
Cr-Commit-Position: refs/heads/master@{#388847}
==========

[commit-bot: I haz the power, 2016-04-22 19:37:32]

Patchset 5 (id:??) landed as
https://crrev.com/3f7af881b9e91331d387ea22c63db69b99e46836
Cr-Commit-Position: refs/heads/master@{#388847}