mash: Remove WorkspaceLayoutManagerDelegate.

mash: Remove WorkspaceLayoutManagerDelegate.

Call WmShelf::UpdateVisibilityState directly.
Move and call WmShell::NotifyFullscreenStateChanged.

Check for the default container instead of |delegate|.
This should mirror existing always-on-top behavior.
See related: https://codereview.chromium.org/1841803002

Nix WmRootWindowControllerObserver::OnFullscreenStateChanged.
(use the original ShellObserver observer pattern instead)

BUG=615155
TEST=Automated; no cros resgressions.
R=jamescook@chromium.org

Committed: https://crrev.com/5fabdd65c794131e3abf9093b4a6f9eb256a4615
Cr-Commit-Position: refs/heads/master@{#411222}

[msw, 2016-08-10 19:07:20]

Description was changed from

==========
mash: Remove WorkspaceLayoutManagerDelegate.

BUG=615155
==========

to

==========
mash: Remove WorkspaceLayoutManagerDelegate.

Call WmShelf::UpdateVisibilityState directly.
Move and call WmShell::NotifyFullscreenStateChanged.

BUG=615155
TEST=Automated; no cros resgressions.
R=jamescook@chromium.org
==========

[msw, 2016-08-10 19:07:20]

msw@chromium.org changed reviewers:
+ jamescook@chromium.org

[msw, 2016-08-10 19:07:36]

Hey James, please take a look; thanks!

[msw, 2016-08-10 19:08:07]

The CQ bit was checked by msw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 19:08:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 19:47:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-10 19:47:54]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[James Cook, 2016-08-10 19:55:45]

Looks fine to me -- I'm not sure why the bot is unhappy. One question though.

https://codereview.chromium.org/2234033002/diff/1/ash/common/wm/workspace/wor...
File ash/common/wm/workspace/workspace_layout_manager.cc (right):

https://codereview.chromium.org/2234033002/diff/1/ash/common/wm/workspace/wor...
ash/common/wm/workspace/workspace_layout_manager.cc:341: if
(root_window_controller_->GetShelf())
Maybe this should be HasShelf()? I'm not sure. It might be possible for this to
return a WmShelf* before the ShelfWidget is constructed.

https://codereview.chromium.org/2234033002/diff/1/ash/common/wm/workspace/wor...
File ash/common/wm/workspace/workspace_layout_manager.h (left):

https://codereview.chromium.org/2234033002/diff/1/ash/common/wm/workspace/wor...
ash/common/wm/workspace/workspace_layout_manager.h:49: void DeleteDelegate();
Nice to get rid of calls like this.

https://codereview.chromium.org/2234033002/diff/1/ash/common/wm_shell.h
File ash/common/wm_shell.h (right):

https://codereview.chromium.org/2234033002/diff/1/ash/common/wm_shell.h#newco...
ash/common/wm_shell.h:286: // Notifies observers on entering or exiting
fullscreen mode in |root_window|.
nit: "on" -> "after", assuming that's true

[msw, 2016-08-10 20:43:02]

Description was changed from

==========
mash: Remove WorkspaceLayoutManagerDelegate.

Call WmShelf::UpdateVisibilityState directly.
Move and call WmShell::NotifyFullscreenStateChanged.

BUG=615155
TEST=Automated; no cros resgressions.
R=jamescook@chromium.org
==========

to

==========
mash: Remove WorkspaceLayoutManagerDelegate.

Call WmShelf::UpdateVisibilityState directly.
Move and call WmShell::NotifyFullscreenStateChanged.

Check for the default container instead of |delegate|.
This should mirror existing always-on-top behavior.
See related: https://codereview.chromium.org/1841803002

BUG=615155
TEST=Automated; no cros resgressions.
R=jamescook@chromium.org
==========

[msw, 2016-08-10 20:47:41]

Please take another look; thanks!

https://codereview.chromium.org/2234033002/diff/1/ash/common/wm/workspace/wor...
File ash/common/wm/workspace/workspace_layout_manager.cc (right):

https://codereview.chromium.org/2234033002/diff/1/ash/common/wm/workspace/wor...
ash/common/wm/workspace/workspace_layout_manager.cc:341: if
(root_window_controller_->GetShelf())
On 2016/08/10 19:55:41, James Cook wrote:
> Maybe this should be HasShelf()? I'm not sure. It might be possible for this
to
> return a WmShelf* before the ShelfWidget is constructed.

Done.

https://codereview.chromium.org/2234033002/diff/1/ash/common/wm/workspace/wor...
File ash/common/wm/workspace/workspace_layout_manager.h (left):

https://codereview.chromium.org/2234033002/diff/1/ash/common/wm/workspace/wor...
ash/common/wm/workspace/workspace_layout_manager.h:49: void DeleteDelegate();
On 2016/08/10 19:55:41, James Cook wrote:
> Nice to get rid of calls like this.

Acknowledged.

https://codereview.chromium.org/2234033002/diff/1/ash/common/wm_shell.h
File ash/common/wm_shell.h (right):

https://codereview.chromium.org/2234033002/diff/1/ash/common/wm_shell.h#newco...
ash/common/wm_shell.h:286: // Notifies observers on entering or exiting
fullscreen mode in |root_window|.
On 2016/08/10 19:55:42, James Cook wrote:
> nit: "on" -> "after", assuming that's true

Done.

[msw, 2016-08-10 20:47:48]

The CQ bit was checked by msw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 20:48:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[James Cook, 2016-08-10 21:01:20]

LGTM. Nice patch.

https://codereview.chromium.org/2234033002/diff/20001/ash/common/wm/workspace...
File ash/common/wm/workspace/workspace_layout_manager.h (right):

https://codereview.chromium.org/2234033002/diff/20001/ash/common/wm/workspace...
ash/common/wm/workspace/workspace_layout_manager.h:119: WmWindow* window_;
optional idea: It would be nice if there was a comment here or on the
constructor saying that |window| is a container.

[commit-bot: I haz the power, 2016-08-10 21:29:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-10 21:29:23]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[msw, 2016-08-11 00:09:59]

Description was changed from

==========
mash: Remove WorkspaceLayoutManagerDelegate.

Call WmShelf::UpdateVisibilityState directly.
Move and call WmShell::NotifyFullscreenStateChanged.

Check for the default container instead of |delegate|.
This should mirror existing always-on-top behavior.
See related: https://codereview.chromium.org/1841803002

BUG=615155
TEST=Automated; no cros resgressions.
R=jamescook@chromium.org
==========

to

==========
mash: Remove WorkspaceLayoutManagerDelegate.

Call WmShelf::UpdateVisibilityState directly.
Move and call WmShell::NotifyFullscreenStateChanged.

Check for the default container instead of |delegate|.
This should mirror existing always-on-top behavior.
See related: https://codereview.chromium.org/1841803002

Nix WmRootWindowControllerObserver::OnFullscreenStateChanged.
(use the original ShellObserver observer pattern instead)

BUG=615155
TEST=Automated; no cros resgressions.
R=jamescook@chromium.org
==========

[msw, 2016-08-11 00:10:46]

Hey James, please take another look; thanks!
+CC Scott FYI for WmRootWindowControllerObserver change.

https://codereview.chromium.org/2234033002/diff/20001/ash/common/wm/workspace...
File ash/common/wm/workspace/workspace_layout_manager.h (right):

https://codereview.chromium.org/2234033002/diff/20001/ash/common/wm/workspace...
ash/common/wm/workspace/workspace_layout_manager.h:119: WmWindow* window_;
On 2016/08/10 21:01:17, James Cook wrote:
> optional idea: It would be nice if there was a comment here or on the
> constructor saying that |window| is a container.

Done.

[msw, 2016-08-11 00:12:59]

The CQ bit was checked by msw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-11 00:14:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[James Cook, 2016-08-11 00:35:58]

LGTM. Nice how you collapsed all the fullscreen notifications into a single
place and single observer.

[commit-bot: I haz the power, 2016-08-11 00:54:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-11 00:54:44]

Dry run: This issue passed the CQ dry run.

[msw, 2016-08-11 01:42:33]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2016-08-11 01:43:21]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 01:48:46]

Description was changed from

==========
mash: Remove WorkspaceLayoutManagerDelegate.

Call WmShelf::UpdateVisibilityState directly.
Move and call WmShell::NotifyFullscreenStateChanged.

Check for the default container instead of |delegate|.
This should mirror existing always-on-top behavior.
See related: https://codereview.chromium.org/1841803002

Nix WmRootWindowControllerObserver::OnFullscreenStateChanged.
(use the original ShellObserver observer pattern instead)

BUG=615155
TEST=Automated; no cros resgressions.
R=jamescook@chromium.org
==========

to

==========
mash: Remove WorkspaceLayoutManagerDelegate.

Call WmShelf::UpdateVisibilityState directly.
Move and call WmShell::NotifyFullscreenStateChanged.

Check for the default container instead of |delegate|.
This should mirror existing always-on-top behavior.
See related: https://codereview.chromium.org/1841803002

Nix WmRootWindowControllerObserver::OnFullscreenStateChanged.
(use the original ShellObserver observer pattern instead)

BUG=615155
TEST=Automated; no cros resgressions.
R=jamescook@chromium.org
==========

[commit-bot: I haz the power, 2016-08-11 01:48:48]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-11 01:50:21]

Description was changed from

==========
mash: Remove WorkspaceLayoutManagerDelegate.

Call WmShelf::UpdateVisibilityState directly.
Move and call WmShell::NotifyFullscreenStateChanged.

Check for the default container instead of |delegate|.
This should mirror existing always-on-top behavior.
See related: https://codereview.chromium.org/1841803002

Nix WmRootWindowControllerObserver::OnFullscreenStateChanged.
(use the original ShellObserver observer pattern instead)

BUG=615155
TEST=Automated; no cros resgressions.
R=jamescook@chromium.org
==========

to

==========
mash: Remove WorkspaceLayoutManagerDelegate.

Call WmShelf::UpdateVisibilityState directly.
Move and call WmShell::NotifyFullscreenStateChanged.

Check for the default container instead of |delegate|.
This should mirror existing always-on-top behavior.
See related: https://codereview.chromium.org/1841803002

Nix WmRootWindowControllerObserver::OnFullscreenStateChanged.
(use the original ShellObserver observer pattern instead)

BUG=615155
TEST=Automated; no cros resgressions.
R=jamescook@chromium.org

Committed: https://crrev.com/5fabdd65c794131e3abf9093b4a6f9eb256a4615
Cr-Commit-Position: refs/heads/master@{#411222}
==========

[commit-bot: I haz the power, 2016-08-11 01:50:22]

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