Add ash-enable-sticky-edges for 'sticky' instead of 'snap' behavior.

Add ash-enable-sticky-edges for 'sticky' instead of 'snap' behavior.

I renamed everything related to edges to 'sticky' to differentiate
it from SnapSizer in the code. I put the new behavior (which is
a pretty trivial change) behind a flag.

I moved ShouldStickToEdge to window_util.cc in case we want to use
the smae logic in snapping panels to / from the launcher. However,
my initial experimentation with that was not great so I left that
change out for now.

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

[flackr, 2013-04-16 00:32:14]

https://codereview.chromium.org/14273008/diff/1/ash/wm/window_util.cc
File ash/wm/window_util.cc (right):

https://codereview.chromium.org/14273008/diff/1/ash/wm/window_util.cc#newcode238
ash/wm/window_util.cc:238: const int sticky_size =
internal::WorkspaceWindowResizer::kScreenEdgeInset;
Assuming this method stays here we should probably move this constant to this
file and rename it more appropriately, i.e. kWindowSnapSize (The inset behavior
is only a consequence of restoring windows with snap behavior turned on). Then
there would be no need to assign to a local const int.

https://codereview.chromium.org/14273008/diff/1/ash/wm/window_util.h
File ash/wm/window_util.h (right):

https://codereview.chromium.org/14273008/diff/1/ash/wm/window_util.h#newcode134
ash/wm/window_util.h:134: ASH_EXPORT bool ShouldStickToEdge(int
distance_from_edge);
Currently, this is only used by workspace_window_resizer. I'm guessing it's
going to be used by panel_window_resizer soon though to implement the necessary
behavior?

https://codereview.chromium.org/14273008/diff/1/ash/wm/workspace/workspace_wi...
File ash/wm/workspace/workspace_window_resizer.cc (right):

https://codereview.chromium.org/14273008/diff/1/ash/wm/workspace/workspace_wi...
ash/wm/workspace/workspace_window_resizer.cc:181: // Converts a window comopnent
edge to the magnetic edge to stick to.
nit: s/comopnent/component

[varkha, 2013-04-16 02:12:12]

https://codereview.chromium.org/14273008/diff/1/ash/wm/base_layout_manager.cc
File ash/wm/base_layout_manager.cc (right):

https://codereview.chromium.org/14273008/diff/1/ash/wm/base_layout_manager.cc...
ash/wm/base_layout_manager.cc:58: switches::kAshEnableStickyEdges)) {
This changes the behavior of restore after a normal window was resized close to
or touching the edges, then maximized and then restored again. Under the flag
the window is no longer inset and is allowed to remain stuck to the edge(s).
This could be described in the bug report.

https://codereview.chromium.org/14273008/diff/1/ash/wm/workspace/workspace_wi...
File ash/wm/workspace/workspace_window_resizer.h (right):

https://codereview.chromium.org/14273008/diff/1/ash/wm/workspace/workspace_wi...
ash/wm/workspace/workspace_window_resizer.h:123: void
MagneticallyStickResizeToOtherWindows(gfx::Rect* bounds);
nit: To be consistent with StickyResizeToWorkAreaBounds the name should be
MagneticallyStickyResizeToOtherWindows.

[stevenjb, 2013-04-16 20:08:20]

PTAL

https://codereview.chromium.org/14273008/diff/1/ash/wm/base_layout_manager.cc
File ash/wm/base_layout_manager.cc (right):

https://codereview.chromium.org/14273008/diff/1/ash/wm/base_layout_manager.cc...
ash/wm/base_layout_manager.cc:58: switches::kAshEnableStickyEdges)) {
On 2013/04/16 02:12:12, varkha wrote:
> This changes the behavior of restore after a normal window was resized close
to
> or touching the edges, then maximized and then restored again. Under the flag
> the window is no longer inset and is allowed to remain stuck to the edge(s).
> This could be described in the bug report.

I'm going to revert this until discussing with UX.

https://codereview.chromium.org/14273008/diff/1/ash/wm/window_util.cc
File ash/wm/window_util.cc (right):

https://codereview.chromium.org/14273008/diff/1/ash/wm/window_util.cc#newcode238
ash/wm/window_util.cc:238: const int sticky_size =
internal::WorkspaceWindowResizer::kScreenEdgeInset;
On 2013/04/16 00:32:14, flackr wrote:
> Assuming this method stays here we should probably move this constant to this
> file and rename it more appropriately, i.e. kWindowSnapSize (The inset
behavior
> is only a consequence of restoring windows with snap behavior turned on). Then
> there would be no need to assign to a local const int.
Moved back.

https://codereview.chromium.org/14273008/diff/1/ash/wm/window_util.h
File ash/wm/window_util.h (right):

https://codereview.chromium.org/14273008/diff/1/ash/wm/window_util.h#newcode134
ash/wm/window_util.h:134: ASH_EXPORT bool ShouldStickToEdge(int
distance_from_edge);
On 2013/04/16 00:32:14, flackr wrote:
> Currently, this is only used by workspace_window_resizer. I'm guessing it's
> going to be used by panel_window_resizer soon though to implement the
necessary
> behavior?

That was my thought based on your suggestion, but as I mentioned in the CL,
after trying that it's less obvious to me that we will want to share logic, so
I'm putting it back.

https://codereview.chromium.org/14273008/diff/1/ash/wm/workspace/workspace_wi...
File ash/wm/workspace/workspace_window_resizer.cc (right):

https://codereview.chromium.org/14273008/diff/1/ash/wm/workspace/workspace_wi...
ash/wm/workspace/workspace_window_resizer.cc:181: // Converts a window comopnent
edge to the magnetic edge to stick to.
On 2013/04/16 00:32:14, flackr wrote:
> nit: s/comopnent/component

Done.

https://codereview.chromium.org/14273008/diff/1/ash/wm/workspace/workspace_wi...
File ash/wm/workspace/workspace_window_resizer.h (right):

https://codereview.chromium.org/14273008/diff/1/ash/wm/workspace/workspace_wi...
ash/wm/workspace/workspace_window_resizer.h:123: void
MagneticallyStickResizeToOtherWindows(gfx::Rect* bounds);
On 2013/04/16 02:12:12, varkha wrote:
> nit: To be consistent with StickyResizeToWorkAreaBounds the name should be
> MagneticallyStickyResizeToOtherWindows.

Done.

[flackr, 2013-04-16 21:07:00]

I do think it's odd that we pass sticky_size around when the only thing that
cares is ShouldStick... but this is how the code was. LGTM.

[flackr, 2013-04-16 21:08:21]

Sorry, just noticed there are snapping tests. Can you add sticky tests? I think
for the purposes of a test you could add a command line parameter.

[stevenjb, 2013-04-18 00:02:23]

Tests added.

[flackr, 2013-04-18 04:43:59]

lgtm

[commit-bot: I haz the power, 2013-04-18 15:21:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/14273008/21001

[commit-bot: I haz the power, 2013-04-18 15:21:17]

Presubmit check for 14273008-21001 failed and returned exit status 1.

INFO:root:Found 5 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/ash/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    ash/ash_switches.h
    ash/ash_switches.cc
    ash/wm/workspace/workspace_window_resizer.cc
    ash/wm/workspace/workspace_window_resizer.h
    ash/wm/workspace/workspace_window_resizer_unittest.cc

[stevenjb, 2013-04-18 15:22:24]

sky@ - OWNER review please.

[varkha, 2013-04-20 15:03:58]

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
File ash/wm/workspace/workspace_window_resizer_unittest.cc (right):

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_window_resizer_unittest.cc:170: virtual
~WorkspaceWindowResizerTestSticky() {}
Should the test clean the command line flag after itself to what it was set
before?

[sky, 2013-04-22 17:20:00]

Just nits

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
File ash/wm/workspace/workspace_window_resizer.cc (right):

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_window_resizer.cc:79: const int kStickySize = 64;
Comment.

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_window_resizer.cc:87: } else {
no else.

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_window_resizer.cc:93: // Returns the coordinate along
the secondary axis to stick to.
This isn't used in what you are calling sticking. It's used for the magnetic
code.

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_window_resizer.cc:322: int snap_size;
sticky_size

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
File ash/wm/workspace/workspace_window_resizer.h (right):

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_window_resizer.h:137: // Snaps the window bounds to
the work area edges if necessary.
If you're goal is to use 'stick' every where, which is should be, then update
this comment and 142.

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_window_resizer.h:143: void
StickyResizeToWorkAreaBounds(const gfx::Rect& work_area,
This name is awkward, maybe StickToWorkAreaBounds?

[stevenjb, 2013-04-22 17:49:01]

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
File ash/wm/workspace/workspace_window_resizer_unittest.cc (right):

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_window_resizer_unittest.cc:170: virtual
~WorkspaceWindowResizerTestSticky() {}
On 2013/04/20 15:03:58, varkha wrote:
> Should the test clean the command line flag after itself to what it was set
> before?
CommandLine is reset between tests, see TestClientInitializer in
base/test/test_suite.cc.

[stevenjb, 2013-04-22 18:09:41]

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
File ash/wm/workspace/workspace_window_resizer.cc (right):

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_window_resizer.cc:79: const int kStickySize = 64;
On 2013/04/22 17:20:00, sky wrote:
> Comment.

Done.

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_window_resizer.cc:87: } else {
On 2013/04/22 17:20:00, sky wrote:
> no else.

Done.

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_window_resizer.cc:93: // Returns the coordinate along
the secondary axis to stick to.
On 2013/04/22 17:20:00, sky wrote:
> This isn't used in what you are calling sticking. It's used for the magnetic
> code.

Done.

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_window_resizer.cc:322: int snap_size;
On 2013/04/22 17:20:00, sky wrote:
> sticky_size

Done.

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
File ash/wm/workspace/workspace_window_resizer.h (right):

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_window_resizer.h:137: // Snaps the window bounds to
the work area edges if necessary.
On 2013/04/22 17:20:00, sky wrote:
> If you're goal is to use 'stick' every where, which is should be, then update
> this comment and 142.

Done.

https://codereview.chromium.org/14273008/diff/21001/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_window_resizer.h:143: void
StickyResizeToWorkAreaBounds(const gfx::Rect& work_area,
On 2013/04/22 17:20:00, sky wrote:
> This name is awkward, maybe StickToWorkAreaBounds?
Changed to StickToWorkAreaOnResize

[stevenjb, 2013-04-22 22:27:52]

sky@ - PTAL
Note: linux_chromeos failure was flakiness.

[sky, 2013-04-22 22:31:44]

LGTM

[commit-bot: I haz the power, 2013-04-22 23:03:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/14273008/35001

[commit-bot: I haz the power, 2013-04-23 00:33:41]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win7_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[stevenjb, 2013-04-23 16:28:31]

Committed patchset #6 manually as r195809 (presubmit successful).