exo: Improve window placement.

exo: Improve window placement.

This improves placement of normal top-level windows by
delaying widget creation until a time when an initial
size has been provided by the client.

This also makes the standard usage of shell surfaces
simpler by requiring bindings that want to prevent the
surface from being mapped to mark it as disabled. Shell
surfaces are enabled by default.

BUG=549781
TEST=exo_unittests

Committed: https://crrev.com/09c5e9878c57ee3011080ab9a2fc0f170173e3bd
Cr-Commit-Position: refs/heads/master@{#372180}

[reveman, 2016-01-28 20:12:26]

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

[reveman, 2016-01-28 20:13:06]

reveman@chromium.org changed reviewers:
+ lpique@chromium.org

[commit-bot: I haz the power, 2016-01-28 20:13:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1645043003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1645043003/1

[commit-bot: I haz the power, 2016-01-28 20:34:57]

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

[commit-bot: I haz the power, 2016-01-28 20:34:58]

Dry run: This issue passed the CQ dry run.

[lpique, 2016-01-28 20:48:32]

lgtm

Just some nits/checks.

https://codereview.chromium.org/1645043003/diff/1/components/exo/shell_surfac...
File components/exo/shell_surface.cc (right):

https://codereview.chromium.org/1645043003/diff/1/components/exo/shell_surfac...
components/exo/shell_surface.cc:92: DCHECK(enabled());
Should this be a precondition check inside CreateShellSurfaceWidget() instead?

https://codereview.chromium.org/1645043003/diff/1/components/exo/shell_surfac...
components/exo/shell_surface.cc:261: 
Was dropping the TRACE_EVENT intentional?

https://codereview.chromium.org/1645043003/diff/1/components/exo/wayland/serv...
File components/exo/wayland/server.cc (right):

https://codereview.chromium.org/1645043003/diff/1/components/exo/wayland/serv...
components/exo/wayland/server.cc:645: return;
Note: SetEnabled() basically does this same early out check (DRY).

https://codereview.chromium.org/1645043003/diff/1/components/exo/wayland/serv...
components/exo/wayland/server.cc:668: return;
If you remove this new check here or below (as above), note that
SetFullscreen/Maximize() do not check that if it is already full screen, though
this means that it is no worse than the code prior to this change.

[reveman, 2016-01-28 21:35:19]

The CQ bit was checked by reveman@chromium.org

[reveman, 2016-01-28 21:35:20]

https://codereview.chromium.org/1645043003/diff/1/components/exo/shell_surfac...
File components/exo/shell_surface.cc (right):

https://codereview.chromium.org/1645043003/diff/1/components/exo/shell_surfac...
components/exo/shell_surface.cc:92: DCHECK(enabled());
On 2016/01/28 at 20:48:32, lpique wrote:
> Should this be a precondition check inside CreateShellSurfaceWidget() instead?

Good idea. Done.

https://codereview.chromium.org/1645043003/diff/1/components/exo/shell_surfac...
components/exo/shell_surface.cc:261: 
On 2016/01/28 at 20:48:32, lpique wrote:
> Was dropping the TRACE_EVENT intentional?

Yes, I've been limiting TRACE_EVENT statements to public API entry points and
code that is known to take a significant time to execute. FYI, the first is
useful in combination with --trace-to-console=exo when debugging clients.

https://codereview.chromium.org/1645043003/diff/1/components/exo/wayland/serv...
File components/exo/wayland/server.cc (right):

https://codereview.chromium.org/1645043003/diff/1/components/exo/wayland/serv...
components/exo/wayland/server.cc:645: return;
On 2016/01/28 at 20:48:32, lpique wrote:
> Note: SetEnabled() basically does this same early out check (DRY).

Good point. Changed this to unconditionally call SetEnabled(true).

https://codereview.chromium.org/1645043003/diff/1/components/exo/wayland/serv...
components/exo/wayland/server.cc:668: return;
On 2016/01/28 at 20:48:32, lpique wrote:
> If you remove this new check here or below (as above), note that
SetFullscreen/Maximize() do not check that if it is already full screen, though
this means that it is no worse than the code prior to this change.

Correct. It shouldn't be possible to change the state of a wl_shell_surface once
mapped if I understand the interface correctly but I changed the code to match
the old behavior and if we want to improve it we can instead do that in a
separate patch.

[reveman, 2016-01-28 21:35:20]

The patchset sent to the CQ was uploaded after l-g-t-m from lpique@chromium.org
Link to the patchset: https://codereview.chromium.org/1645043003/#ps20001
(title: " ")

[commit-bot: I haz the power, 2016-01-28 21:36:13]

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

[commit-bot: I haz the power, 2016-01-28 22:11:10]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-28 22:12:30]

Description was changed from

==========
exo: Improve window placement.

This improves placement of normal top-level windows by
delaying widget creation until a time when an initial
size has been provided by the client.

This also makes the standard usage of shell surfaces
simpler by requiring bindings that want to prevent the
surface from being mapped to mark it as disabled. Shell
surfaces are enabled by default.

BUG=549781
TEST=exo_unittests
==========

to

==========
exo: Improve window placement.

This improves placement of normal top-level windows by
delaying widget creation until a time when an initial
size has been provided by the client.

This also makes the standard usage of shell surfaces
simpler by requiring bindings that want to prevent the
surface from being mapped to mark it as disabled. Shell
surfaces are enabled by default.

BUG=549781
TEST=exo_unittests

Committed: https://crrev.com/09c5e9878c57ee3011080ab9a2fc0f170173e3bd
Cr-Commit-Position: refs/heads/master@{#372180}
==========

[commit-bot: I haz the power, 2016-01-28 22:12:31]

Patchset 2 (id:??) landed as
https://crrev.com/09c5e9878c57ee3011080ab9a2fc0f170173e3bd
Cr-Commit-Position: refs/heads/master@{#372180}