New issue
Advanced search Search tips

Issue 774236 link

Starred by 3 users
Status: Fixed
Owner:
osh...@chromium.org
Closed: Oct 2017
Cc:
flackr@chromium.org
jonr...@chromium.org
osh...@chromium.org
omrilio@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug


Participants' hotlists:
Fixing-touch


Sign in to add a comment

Make sure no screen rotation animation when exiting tablet mode

Project Member Reported by osh...@chromium.org, Oct 12 2017 
ScreenRotation animation is async, so if there is scren rotation animation running during tablet mode switch, it may end up with
wrong orientation in clamshell mode.

wutao@ can you check if this can happen? If so, then we should stop
it before tablet mode transition is complete.

Comment 1 by wutao@chromium.org, Oct 12 2017 

In a recent change [1], we make the screen rotation animation sync call.
[1] https://bugs.chromium.org/p/chromium/issues/detail?id=757851#c7

I need to double check if this could happen. Seems not.
If an animation is ongoing, and then exiting the tablet mode, a) if the target orientation is same as user's rotation, then we do nothing, b) if the target orientation is different as user's rotation, then we will use sync call animation, and finish the ongoing animation to previous target and then start new animation.

But any how, at last, we will have consistent final orientation as the user_rotation?

Comment 2 by tbuck...@chromium.org, Oct 13 2017

Components: UI>Shell>TouchView

Comment 3 by osh...@chromium.org, Oct 14 2017

Owner: osh...@chromium.org
let me triple check before closing

Comment 4 by osh...@chromium.org, Oct 18 2017

Status: Started (was: Assigned)
I could reproduce the case where the rotation animation ends after tablet mode ended on soraka. I don't know exactly how yet. Investigating.

Comment 5 by osh...@chromium.org, Oct 18 2017 

found the bug. will send a fix shortly.
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 20 2017 

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

commit be565c8c9c1707db77149ab5a803af132b370953
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Fri Oct 20 18:13:06 2017

Fix the current rotation in DisplayConfiguration

BUG= 774236 
TEST=manually tested on soraka.

Change-Id: I6b6b12afdfd36e4062e779494c4fa652762011d2
Reviewed-on: https://chromium-review.googlesource.com/729124
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510493}
[modify] https://crrev.com/be565c8c9c1707db77149ab5a803af132b370953/ash/display/display_configuration_controller.cc
[modify] https://crrev.com/be565c8c9c1707db77149ab5a803af132b370953/ash/display/display_configuration_controller.h
[modify] https://crrev.com/be565c8c9c1707db77149ab5a803af132b370953/ash/display/display_configuration_controller_test_api.cc
[modify] https://crrev.com/be565c8c9c1707db77149ab5a803af132b370953/ash/display/display_configuration_controller_test_api.h
[modify] https://crrev.com/be565c8c9c1707db77149ab5a803af132b370953/ash/display/screen_orientation_controller_chromeos.cc
[modify] https://crrev.com/be565c8c9c1707db77149ab5a803af132b370953/ash/display/screen_orientation_controller_test_api.cc
[modify] https://crrev.com/be565c8c9c1707db77149ab5a803af132b370953/ash/display/screen_orientation_controller_test_api.h
[modify] https://crrev.com/be565c8c9c1707db77149ab5a803af132b370953/ash/rotator/screen_rotation_animator_unittest.cc

Comment 7 by osh...@chromium.org, Oct 20 2017

Labels: ReleaseBlock-Stable Merge-Request-63 Merge-Request-62 M-62
This can happen on 62, and the risk is small. Can we merge this to 62 as well as to 63?
Project Member

Comment 8 by sheriffbot@chromium.org, Oct 20 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 9 by gkihumba@google.com, Oct 20 2017

Labels: -Merge-Request-63 Merge-Approved-63

Comment 10 by bhthompson@google.com, Oct 23 2017

Labels: -Hotlist-Merge-Review -Merge-Review-62 Merge-Approved-62

Comment 11 by bhthompson@google.com, Oct 23 2017

Labels: -ReleaseBlock-Stable
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 23 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/602028f2de6b4ce9ff36beba676cfae7d7c43d72

commit 602028f2de6b4ce9ff36beba676cfae7d7c43d72
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Mon Oct 23 20:14:32 2017

Fix the current rotation in DisplayConfiguration

BUG= 774236 
TEST=manually tested on soraka.

(cherry picked from commit be565c8c9c1707db77149ab5a803af132b370953)

Change-Id: I6b6b12afdfd36e4062e779494c4fa652762011d2
Reviewed-on: https://chromium-review.googlesource.com/729124
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#510493}
Reviewed-on: https://chromium-review.googlesource.com/734207
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#729}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/602028f2de6b4ce9ff36beba676cfae7d7c43d72/ash/display/display_configuration_controller.cc
[modify] https://crrev.com/602028f2de6b4ce9ff36beba676cfae7d7c43d72/ash/display/display_configuration_controller.h
[modify] https://crrev.com/602028f2de6b4ce9ff36beba676cfae7d7c43d72/ash/display/display_configuration_controller_test_api.cc
[modify] https://crrev.com/602028f2de6b4ce9ff36beba676cfae7d7c43d72/ash/display/display_configuration_controller_test_api.h
[modify] https://crrev.com/602028f2de6b4ce9ff36beba676cfae7d7c43d72/ash/display/screen_orientation_controller_chromeos.cc
[modify] https://crrev.com/602028f2de6b4ce9ff36beba676cfae7d7c43d72/ash/display/screen_orientation_controller_test_api.cc
[modify] https://crrev.com/602028f2de6b4ce9ff36beba676cfae7d7c43d72/ash/display/screen_orientation_controller_test_api.h
[modify] https://crrev.com/602028f2de6b4ce9ff36beba676cfae7d7c43d72/ash/rotator/screen_rotation_animator_unittest.cc
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 23 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cb8b4540ebe6c0b7ede0fa987c9367f5702308d6

commit cb8b4540ebe6c0b7ede0fa987c9367f5702308d6
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Mon Oct 23 20:15:26 2017

Fix the current rotation in DisplayConfiguration

BUG= 774236 
TEST=manually tested on soraka.

Change-Id: I6b6b12afdfd36e4062e779494c4fa652762011d2
Reviewed-on: https://chromium-review.googlesource.com/729124
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#510493}(cherry picked from commit be565c8c9c1707db77149ab5a803af132b370953)
Reviewed-on: https://chromium-review.googlesource.com/733881
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#162}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/cb8b4540ebe6c0b7ede0fa987c9367f5702308d6/ash/display/display_configuration_controller.cc
[modify] https://crrev.com/cb8b4540ebe6c0b7ede0fa987c9367f5702308d6/ash/display/display_configuration_controller.h
[modify] https://crrev.com/cb8b4540ebe6c0b7ede0fa987c9367f5702308d6/ash/display/display_configuration_controller_test_api.cc
[modify] https://crrev.com/cb8b4540ebe6c0b7ede0fa987c9367f5702308d6/ash/display/display_configuration_controller_test_api.h
[modify] https://crrev.com/cb8b4540ebe6c0b7ede0fa987c9367f5702308d6/ash/display/screen_orientation_controller_chromeos.cc
[modify] https://crrev.com/cb8b4540ebe6c0b7ede0fa987c9367f5702308d6/ash/display/screen_orientation_controller_test_api.cc
[modify] https://crrev.com/cb8b4540ebe6c0b7ede0fa987c9367f5702308d6/ash/display/screen_orientation_controller_test_api.h
[modify] https://crrev.com/cb8b4540ebe6c0b7ede0fa987c9367f5702308d6/ash/rotator/screen_rotation_animator_unittest.cc

Comment 14 by osh...@chromium.org, Oct 23 2017

Status: Fixed (was: Started)

Comment 15 by osh...@chromium.org, Oct 26 2017

Cc: osh...@chromium.org
 Issue 776657  has been merged into this issue.
►

Sign in to add a comment