New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 848475 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Fix StackLayoutBase and Stack so CompositorAnimations work properly

Project Member Reported by rlanday@chromium.org, May 31 2018

Issue description

I'm working on adding a Stack animation in https://chromium-review.googlesource.com/c/chromium/src/+/1074212. It currently doesn't work properly without adding an AnimationUpdateListener and manually calling Stack#requestUpdate().

This can be fixed by doing the following:

- Adding a call to requestStackUpdate() near the beginning of StackLayoutBase#updateLayout()

- Removing the call to mLayout.requestUpdate() in Stack#requestUpdate() (this is necessary to avoid unnecessarily generating a bunch of compositor frames)

- Adding calls to Layout#requestUpdate() where necessary in code that handles gesture events (I think we need to do this in both StackLayoutBase and Stack)
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 5 2018

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

commit 236583123d600ff8e270ad09bf236c81d60d026e
Author: Matthew Jones <mdjones@chromium.org>
Date: Tue Jun 05 16:45:48 2018

Update tab positions every frame in the tab switcher

This patch removes a flag in the tab stack that conditionally allows
tab positions to be updated. The issue is that this flag is generally
only set when input events occur, requiring animations to manually
set it for every frame. This patch removes the flag and cleans up some
of the logic, the idea being that if a frame is requested, it is not
unlikely that a tab moved.

Bug:  848475 
Change-Id: I74658e80c977c43fed4198b055e2c917084edcb3
Reviewed-on: https://chromium-review.googlesource.com/1086000
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Ryan Landay <rlanday@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564539}
[modify] https://crrev.com/236583123d600ff8e270ad09bf236c81d60d026e/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/StackLayoutBase.java
[modify] https://crrev.com/236583123d600ff8e270ad09bf236c81d60d026e/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/stack/NonOverlappingStack.java
[modify] https://crrev.com/236583123d600ff8e270ad09bf236c81d60d026e/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/stack/OverlappingStack.java
[modify] https://crrev.com/236583123d600ff8e270ad09bf236c81d60d026e/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/stack/Stack.java

Labels: M-68 Merge-Request-68
Requesting merge into M68 to avoid conflicts with patches for the horizontal tab switcher experiment.
Project Member

Comment 3 by sheriffbot@chromium.org, Jun 6 2018

Labels: -Merge-Request-68 Hotlist-Merge-Approved Merge-Approved-68
Your change meets the bar and is auto-approved for M68. Please go ahead and merge the CL to branch 3440 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

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

Comment 4 by sheriffbot@chromium.org, Jun 11 2018

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Comment 5 by bugdroid1@chromium.org, Jun 11 2018

Labels: -merge-approved-68 merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7dec4e7d791957b83340901de54a8e5d0bbedb97

commit 7dec4e7d791957b83340901de54a8e5d0bbedb97
Author: Matthew Jones <mdjones@chromium.org>
Date: Mon Jun 11 20:35:28 2018

Update tab positions every frame in the tab switcher

This patch removes a flag in the tab stack that conditionally allows
tab positions to be updated. The issue is that this flag is generally
only set when input events occur, requiring animations to manually
set it for every frame. This patch removes the flag and cleans up some
of the logic, the idea being that if a frame is requested, it is not
unlikely that a tab moved.

TBR=mdjones@chromium.org

(cherry picked from commit 236583123d600ff8e270ad09bf236c81d60d026e)

Bug:  848475 
Change-Id: I74658e80c977c43fed4198b055e2c917084edcb3
Reviewed-on: https://chromium-review.googlesource.com/1086000
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Ryan Landay <rlanday@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#564539}
Reviewed-on: https://chromium-review.googlesource.com/1096097
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#287}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/7dec4e7d791957b83340901de54a8e5d0bbedb97/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/StackLayoutBase.java
[modify] https://crrev.com/7dec4e7d791957b83340901de54a8e5d0bbedb97/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/stack/NonOverlappingStack.java
[modify] https://crrev.com/7dec4e7d791957b83340901de54a8e5d0bbedb97/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/stack/OverlappingStack.java
[modify] https://crrev.com/7dec4e7d791957b83340901de54a8e5d0bbedb97/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/stack/Stack.java

Status: Fixed (was: Assigned)

Sign in to add a comment