Hide docked windows behind a flag

Hide docked windows behind a flag

As a first step before removing this feature we will hide it behind   
a flag in this milestone, and remove it completely in the next one.

BUG=668355
TEST=manually, with flag disabled, press Alt+] or Alt+[ or dragging windows 
to either edges of the screen, windows only snap, no docked windows.
TEST=manually, with flag enabled, windows are dockable and snappable.
TEST=ash_unittests --gtest_filter=AcceleratorControllerTest.WindowSnapWithoutDocking

Review-Url: https://codereview.chromium.org/2617733002
Cr-Commit-Position: refs/heads/master@{#442620}
Committed: https://chromium.googlesource.com/chromium/src/+/b9147616bf449986d216f2a67f1bd48924c793a7

[afakhry, 2017-01-05 18:49:49]

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

[commit-bot: I haz the power, 2017-01-05 18:50:15]

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, 2017-01-05 20:05:44]

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

[commit-bot: I haz the power, 2017-01-05 20:05:44]

Dry run: This issue passed the CQ dry run.

[afakhry, 2017-01-05 22:10:01]

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

[commit-bot: I haz the power, 2017-01-05 22:10:37]

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, 2017-01-05 23:06:10]

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

[commit-bot: I haz the power, 2017-01-05 23:06:11]

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

[afakhry, 2017-01-06 19:45:58]

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

[commit-bot: I haz the power, 2017-01-06 19:46:50]

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, 2017-01-06 20:51:00]

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

[commit-bot: I haz the power, 2017-01-06 20:51:01]

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

[afakhry, 2017-01-06 21:08:42]

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

[commit-bot: I haz the power, 2017-01-06 21:09:18]

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, 2017-01-06 21:11:31]

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

[commit-bot: I haz the power, 2017-01-06 21:11:32]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[afakhry, 2017-01-06 22:39:58]

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

[commit-bot: I haz the power, 2017-01-06 22:40:35]

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, 2017-01-06 23:29:39]

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

[commit-bot: I haz the power, 2017-01-06 23:29:40]

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

[afakhry, 2017-01-07 00:44:53]

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

[commit-bot: I haz the power, 2017-01-07 00:45:20]

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

[afakhry, 2017-01-07 01:02:28]

Patchset #6 (id:100001) has been deleted

[commit-bot: I haz the power, 2017-01-07 01:03:48]

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_...)

[afakhry, 2017-01-07 01:04:09]

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

[commit-bot: I haz the power, 2017-01-07 01:04:22]

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, 2017-01-07 01:51:25]

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

[commit-bot: I haz the power, 2017-01-07 01:51:25]

Dry run: This issue passed the CQ dry run.

[afakhry, 2017-01-09 18:59:42]

Description was changed from

==========
[WIP] Hide docked windows behind a flag

BUG=668355
==========

to

==========
Hide docked windows behind a flag

As a first step before removing this feature we will hide it behind   
a flag in this milestone, and remove it completely in the next one.

BUG=668355
TEST=manually, with flag disabled, press Alt+] or Alt+[ or dragging windows 
to either edges of the screen, windows only snap, no docked windows.
TEST=manually, with flag enabled, windows are dockable and snappable.
TEST=ash_unittests
--gtest_filter=AcceleratorControllerTest.WindowSnapWithoutDocking
==========

[afakhry, 2017-01-09 19:01:11]

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

[commit-bot: I haz the power, 2017-01-09 19:01:35]

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, 2017-01-09 20:36:27]

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

[commit-bot: I haz the power, 2017-01-09 20:36:27]

Dry run: This issue passed the CQ dry run.

[afakhry, 2017-01-09 20:41:23]

afakhry@chromium.org changed reviewers:
+ oshima@chromium.org

[afakhry, 2017-01-09 20:41:24]

Oshima-san, please review this CL and let me know if there's anything else I
skipped that needs to be hidden behind the flag. Thanks!

[oshima, 2017-01-09 21:57:10]

lgtm

https://codereview.chromium.org/2617733002/diff/140001/ash/common/wm/default_...
File ash/common/wm/default_state.cc (right):

https://codereview.chromium.org/2617733002/diff/140001/ash/common/wm/default_...
ash/common/wm/default_state.cc:692: // TODO(afakhry): Remove in M58.
can you add DCHECK here?

[afakhry, 2017-01-09 22:09:34]

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

[commit-bot: I haz the power, 2017-01-09 22:10:07]

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

[afakhry, 2017-01-09 22:13:47]

afakhry@chromium.org changed reviewers:
+ holte@chromium.org, pkasting@chromium.org

[afakhry, 2017-01-09 22:13:48]

+pkasting for tab_drag_controller_interactive_uitest.cc

+holte for histograms.xml

https://codereview.chromium.org/2617733002/diff/140001/ash/common/wm/default_...
File ash/common/wm/default_state.cc (right):

https://codereview.chromium.org/2617733002/diff/140001/ash/common/wm/default_...
ash/common/wm/default_state.cc:692: // TODO(afakhry): Remove in M58.
On 2017/01/09 21:57:09, oshima wrote:
> can you add DCHECK here?

Done.

[Peter Kasting, 2017-01-09 22:31:48]

LGTM

[commit-bot: I haz the power, 2017-01-09 23:58:49]

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

[commit-bot: I haz the power, 2017-01-09 23:58:50]

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

[Steven Holte, 2017-01-10 00:15:31]

histograms lgtm

[afakhry, 2017-01-10 01:21:46]

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

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

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, 2017-01-10 03:24:18]

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

[commit-bot: I haz the power, 2017-01-10 03:24:19]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[afakhry, 2017-01-10 17:07:19]

A few more docked windows tests needed to explicitly enable the flag after the
added DCHECK.

[afakhry, 2017-01-10 17:22:50]

The CQ bit was checked by afakhry@chromium.org

[afakhry, 2017-01-10 17:22:51]

The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org,
pkasting@chromium.org, holte@chromium.org
Link to the patchset: https://codereview.chromium.org/2617733002/#ps180001
(title: "Fix tests failed because of DCHECK")

[commit-bot: I haz the power, 2017-01-10 17:23:14]

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, 2017-01-10 17:28:28]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1484068970949620,
"parent_rev": "ab5ae32c6233fe7938b9a0b53cf1c74918310d7a", "commit_rev":
"b9147616bf449986d216f2a67f1bd48924c793a7"}

[commit-bot: I haz the power, 2017-01-10 17:29:00]

Description was changed from

==========
Hide docked windows behind a flag

As a first step before removing this feature we will hide it behind   
a flag in this milestone, and remove it completely in the next one.

BUG=668355
TEST=manually, with flag disabled, press Alt+] or Alt+[ or dragging windows 
to either edges of the screen, windows only snap, no docked windows.
TEST=manually, with flag enabled, windows are dockable and snappable.
TEST=ash_unittests
--gtest_filter=AcceleratorControllerTest.WindowSnapWithoutDocking
==========

to

==========
Hide docked windows behind a flag

As a first step before removing this feature we will hide it behind   
a flag in this milestone, and remove it completely in the next one.

BUG=668355
TEST=manually, with flag disabled, press Alt+] or Alt+[ or dragging windows 
to either edges of the screen, windows only snap, no docked windows.
TEST=manually, with flag enabled, windows are dockable and snappable.
TEST=ash_unittests
--gtest_filter=AcceleratorControllerTest.WindowSnapWithoutDocking

Review-Url: https://codereview.chromium.org/2617733002
Cr-Commit-Position: refs/heads/master@{#442620}
Committed:
https://chromium.googlesource.com/chromium/src/+/b9147616bf449986d216f2a67f1b...
==========

[commit-bot: I haz the power, 2017-01-10 17:29:01]

Committed patchset #9 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/b9147616bf449986d216f2a67f1b...

[varkha, 2017-01-18 20:58:31]

varkha@chromium.org changed reviewers:
+ varkha@chromium.org

[varkha, 2017-01-18 20:58:32]

I see that the user events that trigger docking is now blocked. However
currently if a user has docked windows belonging to specific apps before (using
a version before this CL), those windows will still be docked even when running
without --ash-enable-docked-windows. Persistent behavior was added in CLs linked
from http://crbug.com/271582 - it should be possible to roll those changes back.
Sorry for a late comment here - you can always cc me on this work.

https://codereview.chromium.org/2617733002/diff/180001/ash/common/ash_switches.h
File ash/common/ash_switches.h (right):

https://codereview.chromium.org/2617733002/diff/180001/ash/common/ash_switche...
ash/common/ash_switches.h:30: ASH_EXPORT extern const char
kAshEnableDockedWindows[];
nit: add new flags in alphabetical order.

https://codereview.chromium.org/2617733002/diff/180001/chrome/browser/about_f...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2617733002/diff/180001/chrome/browser/about_f...
chrome/browser/about_flags.cc:1026:
IDS_FLAGS_ASH_ENABLE_DOCKED_WINDOWS_DESCRIPTION, kOsAll,
Should this be a Chrome OS only flag?