Clear restore bounds for user resize/drag actions.

Clear restore bounds for user resize/drag actions.

When the window is not snapped, user resizes should always clear restore
bounds. This allows user initiated resizes to not be unexpectedly
ignored.

BUG=405563
TEST=WorkspaceWindowResizerTest.RestoreClearedOnResize

Committed: https://crrev.com/56e39c64d4538396ce669727ae1dd0c462478f74
Cr-Commit-Position: refs/heads/master@{#292228}

[Peter Wen, 2014-08-26 15:21:57]

wnwen@chromium.org changed reviewers:
+ flackr@chromium.org

[flackr, 2014-08-26 15:59:51]

LGTM, can you name the specific test in CL description. I.e.
TEST=WorkspaceWindowResizerTest.RestoreClearedOnResize

[Peter Wen, 2014-08-26 17:21:56]

Done. Thanks for letting me know the proper way to note the new unittest.

[Peter Wen, 2014-08-26 17:22:02]

The CQ bit was checked by wnwen@chromium.org

[Peter Wen, 2014-08-26 17:22:43]

The CQ bit was unchecked by wnwen@chromium.org

[Peter Wen, 2014-08-26 17:23:28]

The CQ bit was checked by wnwen@chromium.org

[commit-bot: I haz the power, 2014-08-26 17:25:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wnwen@chromium.org/477823003/20001

[commit-bot: I haz the power, 2014-08-26 18:30:44]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-26 19:00:15]

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

[commit-bot: I haz the power, 2014-08-26 19:00:16]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Peter Wen, 2014-08-26 20:28:02]

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

[Peter Wen, 2014-08-26 20:28:02]

+varkha@ to verify docked windows behave correctly when restore bounds are
cleared on user resize/move.

[varkha, 2014-08-26 21:10:35]

On 2014/08/26 20:28:02, Peter Wen wrote:
> +varkha@ to verify docked windows behave correctly when restore bounds are
> cleared on user resize/move.

So was it that the docked panels were getting undocked at the end of a
resize-drag or do I misread the test failures before the last patch? How does
avoiding ClearRestoreBounds resolve this?

[varkha, 2014-08-26 21:20:03]

https://codereview.chromium.org/477823003/diff/40001/ash/wm/workspace/workspa...
File ash/wm/workspace/workspace_window_resizer.cc (right):

https://codereview.chromium.org/477823003/diff/40001/ash/wm/workspace/workspa...
ash/wm/workspace/workspace_window_resizer.cc:470: if
(!dock_layout_->is_dragged_window_docked()) {
nit: Can be merged into an else if.
nit: The code as it is here does not need parens with a one-line block under if
(recommended at
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Condit...).

[varkha, 2014-08-26 22:03:35]

wnwen@, what is the bad scenario that you are trying to fix? Does this scenario
involved virtual keyboard?

[Peter Wen, 2014-08-27 13:32:03]

On 2014/08/26 22:03:35, varkha wrote:
> wnwen@, what is the bad scenario that you are trying to fix? Does this
scenario
> involved virtual keyboard?

In the unit tests the docked panels were getting undocked at the end of a
resize-drag. In the case of docked windows, their restore bounds should not be
cleared when users initially drag them away from the docked state because the
window should restore to the previous bounds if it becomes undocked. Just like
snapped windows do not get their restore bounds cleared on user resize/drag.

The bad scenario does involve virtual keyboard, but it can be generalized. When
a keyboard is shown, windows are automatically resized so the keyboard does not
obstruct the windows. Then when the keyboard is hidden, windows are restored to
their original bounds. But if a user resizes/drags the window while the keyboard
is shown, it is more natural to expect that their manual resizing would remain
after keyboard is hidden rather than be reverted to a previous size before
keyboard was shown.

[Peter Wen, 2014-08-27 13:32:57]

https://codereview.chromium.org/477823003/diff/40001/ash/wm/workspace/workspa...
File ash/wm/workspace/workspace_window_resizer.cc (right):

https://codereview.chromium.org/477823003/diff/40001/ash/wm/workspace/workspa...
ash/wm/workspace/workspace_window_resizer.cc:470: if
(!dock_layout_->is_dragged_window_docked()) {
On 2014/08/26 21:20:03, varkha wrote:
> nit: Can be merged into an else if.
> nit: The code as it is here does not need parens with a one-line block under
if
> (recommended at
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Condit...).

Done.

[varkha, 2014-08-27 14:55:28]

I see. So is it the opening of virtual keyboard that sets the restore bounds? If
so maybe there should be a comment at a place where that happens about where the
restore bounds should be cleared.
Also please explain this scenario in the bug because what you are fixing now is
not at all what is described in the original bug description.

https://codereview.chromium.org/477823003/diff/60001/ash/wm/workspace/workspa...
File ash/wm/workspace/workspace_window_resizer.cc (right):

https://codereview.chromium.org/477823003/diff/60001/ash/wm/workspace/workspa...
ash/wm/workspace/workspace_window_resizer.cc:469: } else if
(!dock_layout_->is_dragged_window_docked())
nit: Now that the code is changed it should follow another clause in the guide -
"However, if one part of an if-else statement uses curly braces, the other part
must too".

[Peter Wen, 2014-08-27 15:07:31]

On 2014/08/27 14:55:28, varkha wrote:
> I see. So is it the opening of virtual keyboard that sets the restore bounds?
If
> so maybe there should be a comment at a place where that happens about where
the
> restore bounds should be cleared.
> Also please explain this scenario in the bug because what you are fixing now
is
> not at all what is described in the original bug description.

Done.

>
https://codereview.chromium.org/477823003/diff/60001/ash/wm/workspace/workspa...
> File ash/wm/workspace/workspace_window_resizer.cc (right):
> 
>
https://codereview.chromium.org/477823003/diff/60001/ash/wm/workspace/workspa...
> ash/wm/workspace/workspace_window_resizer.cc:469: } else if
> (!dock_layout_->is_dragged_window_docked())
> nit: Now that the code is changed it should follow another clause in the guide
-
> "However, if one part of an if-else statement uses curly braces, the other
part
> must too".

Thanks for explaining. I missed that last clause when reading the style guide.
:)

[varkha, 2014-08-27 15:32:42]

lgtm

[Peter Wen, 2014-08-27 15:37:40]

The CQ bit was checked by wnwen@chromium.org

[commit-bot: I haz the power, 2014-08-27 15:38:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wnwen@chromium.org/477823003/80001

[commit-bot: I haz the power, 2014-08-27 15:50:05]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/47028)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/52387)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-27 15:51:55]

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

[commit-bot: I haz the power, 2014-08-27 15:51:56]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/47032)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Peter Wen, 2014-08-27 19:08:36]

The CQ bit was checked by wnwen@chromium.org

[Peter Wen, 2014-08-27 19:08:40]

The CQ bit was unchecked by wnwen@chromium.org

[Peter Wen, 2014-08-27 19:08:41]

The CQ bit was checked by wnwen@chromium.org

[commit-bot: I haz the power, 2014-08-27 19:09:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wnwen@chromium.org/477823003/80001

[commit-bot: I haz the power, 2014-08-27 19:21:57]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/47117)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/52475)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-27 19:23:15]

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

[commit-bot: I haz the power, 2014-08-27 19:23:16]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/47127)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[varkha, 2014-08-27 20:05:45]

rebase?

[Peter Wen, 2014-08-27 20:27:26]

The CQ bit was checked by wnwen@chromium.org

[commit-bot: I haz the power, 2014-08-27 20:29:08]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wnwen@chromium.org/477823003/100001

[commit-bot: I haz the power, 2014-08-27 21:29:24]

Committed patchset #6 (id:100001) as 7e4eb56b2e8b13acd831fb251aa3f429991d99b5

[commit-bot: I haz the power, 2014-09-10 02:54:13]

Patchset 6 (id:??) landed as
https://crrev.com/56e39c64d4538396ce669727ae1dd0c462478f74
Cr-Commit-Position: refs/heads/master@{#292228}