New issue
Advanced search Search tips

Issue 842957 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

WindowGrid::PositionWindows ignoring a window does not quite work properly.

Project Member Reported by sammiequon@chromium.org, May 15 2018

Issue description

code is here [1].

This is used by splitview but needs to be adapted for overview swipe to close. The problem was the method, when told to ignore a window would still calculate based on that window being there, but skip it when it came to window placement.

ie. 6 windows -> we want to ignore window 3
PositionWindows will calculate the positions of 6 windows, place and shift window 4 bounds to where window 3 should be if not ignored and shift window 5 bounds to where window 4 should be if window 3 not ignored.

It should be 6 window -> ignore window 3.
PositionWindows will calcualte the positions of 5 windows and place them accordingly.

This didn't cause much problem for splitview but it will look bad for overview swipe to close.

[1] https://cs.chromium.org/chromium/src/ash/wm/overview/window_grid.cc?rcl=54fe82667f8cd11b9acd7560addaad4f3baf3c0b&l=264


 
Project Member

Comment 1 by bugdroid1@chromium.org, May 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ccbeab673ac7218b9d5636cb507362d7e153ab3a

commit ccbeab673ac7218b9d5636cb507362d7e153ab3a
Author: Sammie Quon <sammiequon@google.com>
Date: Tue May 15 16:53:24 2018

overview: Modify WindowGrid::PositionWindows when ignoring an item.

Previously it would calculate positions including the ignored item and
shift the indices of the items placed behind the ignored item. Now it
calculates positions ignoring the item.

Test: Added tests
Bug:  842957 
Change-Id: Idc56917e8be76dc91721d3b02357834ba22a9258
Reviewed-on: https://chromium-review.googlesource.com/1058833
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558753}
[modify] https://crrev.com/ccbeab673ac7218b9d5636cb507362d7e153ab3a/ash/wm/overview/window_grid.cc
[modify] https://crrev.com/ccbeab673ac7218b9d5636cb507362d7e153ab3a/ash/wm/overview/window_grid.h
[modify] https://crrev.com/ccbeab673ac7218b9d5636cb507362d7e153ab3a/ash/wm/overview/window_selector_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, May 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/80a6c46d2af6059666ef324ff9040149e2ff4df7

commit 80a6c46d2af6059666ef324ff9040149e2ff4df7
Author: Sammie Quon <sammiequon@google.com>
Date: Thu May 17 15:41:05 2018

overview: Remove swipe to close drag -> splitview drag state change.

Ui review does not like being able to return to splitview drag mode
while already in swipe to close mode.

Also fixes:
 - add animation when item is long pressed
 - fix a bug with dragging the last item caused by:
   https://chromium-review.googlesource.com/c/chromium/src/+/1058833
 - fix a bug with close button not returning after a unsuccessful fling

Test: Manual
Bug: 828646, 839492,  842957 
Change-Id: Iaf66f112ac0f2c6f814ab7efb655926206d8937d
Reviewed-on: https://chromium-review.googlesource.com/1063179
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559544}
[modify] https://crrev.com/80a6c46d2af6059666ef324ff9040149e2ff4df7/ash/wm/overview/overview_window_drag_controller.cc
[modify] https://crrev.com/80a6c46d2af6059666ef324ff9040149e2ff4df7/ash/wm/overview/window_grid.cc
[modify] https://crrev.com/80a6c46d2af6059666ef324ff9040149e2ff4df7/ash/wm/overview/window_selector.cc
[modify] https://crrev.com/80a6c46d2af6059666ef324ff9040149e2ff4df7/ash/wm/overview/window_selector_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment