New issue
Advanced search Search tips

Issue 710750 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Android ToolbarPhone: isTabSwitcherAnimationRunning is incorrect

Project Member Reported by dtrainor@chromium.org, Apr 12 2017

Issue description

When in the tab switcher isTabSwitcherAnimationRunning always returns true.  It looks like we never call onTabSwitcherTransitionFinished() for tab switcher enter animations.

We should either
(1) Find another place to reset this state properly based on the toolbar animation itself.
(2) Properly call and maybe add an OverviewModeObserver method to let us know when this happens.

This has some impact on some of the other code in that file, since it makes it difficult to hide and show things based on the state of the toolbar transition.  Not super high priority but will require a workaround without it to show IPH in the tab switcher.

Assigning to mdjones@ to triage.
 
Status: Assigned (was: Untriaged)
If that function always returns true, there is a much bigger problem of mTabSwitcherState being incorrect. I'll take a look.

Comment 2 Deleted

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 26 2018

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

commit 99c1a2c3394f7bfab9e11f846aaa708853fb8095
Author: Matthew Jones <mdjones@chromium.org>
Date: Mon Feb 26 18:32:14 2018

Update tab switcher state in ToolbarPhone after animation completes

Previously onTabSwitcherTransitionFinished was only called when
exiting the tab switcher. This change implements
onOverviewModeFinishedShowing to call this method.

BUG=710750

Change-Id: I1a34551a6cd93a54a9461441c25ce022144efa9e
Reviewed-on: https://chromium-review.googlesource.com/929991
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539205}
[modify] https://crrev.com/99c1a2c3394f7bfab9e11f846aaa708853fb8095/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java

Status: Fixed (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 9 2018

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

commit ce362d27961115aa97aba0b91c1ee728ead7898c
Author: Matthew Jones <mdjones@chromium.org>
Date: Fri Mar 09 16:52:39 2018

Revert "Update tab switcher state in ToolbarPhone after animation completes"

This reverts commit 99c1a2c3394f7bfab9e11f846aaa708853fb8095.

Reason for revert: Causes bugs 820349 & 816858

Original change's description:
> Update tab switcher state in ToolbarPhone after animation completes
> 
> Previously onTabSwitcherTransitionFinished was only called when
> exiting the tab switcher. This change implements
> onOverviewModeFinishedShowing to call this method.
> 
> BUG=710750
> 
> Change-Id: I1a34551a6cd93a54a9461441c25ce022144efa9e
> Reviewed-on: https://chromium-review.googlesource.com/929991
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Commit-Queue: Matthew Jones <mdjones@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#539205}

TBR=dtrainor@chromium.org,tedchoc@chromium.org,mdjones@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 710750
Change-Id: Ia5dd268f289f1fbb5bb0e0614c6242ec905a4ecc
Reviewed-on: https://chromium-review.googlesource.com/956937
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542143}
[modify] https://crrev.com/ce362d27961115aa97aba0b91c1ee728ead7898c/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java

Labels: -Pri-2 Pri-3
Status: Assigned (was: Fixed)
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 13 2018

Labels: merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/71c954dec87bbc952ba4ef55ccdc0ad6d7da1493

commit 71c954dec87bbc952ba4ef55ccdc0ad6d7da1493
Author: Matthew Jones <mdjones@chromium.org>
Date: Tue Mar 13 15:12:38 2018

Revert "Update tab switcher state in ToolbarPhone after animation completes"

This reverts commit 99c1a2c3394f7bfab9e11f846aaa708853fb8095.

Reason for revert: Causes bugs 820349 & 816858

Original change's description:
> Update tab switcher state in ToolbarPhone after animation completes
>
> Previously onTabSwitcherTransitionFinished was only called when
> exiting the tab switcher. This change implements
> onOverviewModeFinishedShowing to call this method.
>
> BUG=710750
>
> Change-Id: I1a34551a6cd93a54a9461441c25ce022144efa9e
> Reviewed-on: https://chromium-review.googlesource.com/929991
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Commit-Queue: Matthew Jones <mdjones@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#539205}

TBR=dtrainor@chromium.org, mdjones@chromium.org, tedchoc@chromium.org


(cherry picked from commit ce362d27961115aa97aba0b91c1ee728ead7898c)

Bug: 710750,816858,820349
Change-Id: Ia5dd268f289f1fbb5bb0e0614c6242ec905a4ecc
Reviewed-on: https://chromium-review.googlesource.com/956937
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#542143}
Reviewed-on: https://chromium-review.googlesource.com/960567
Cr-Commit-Position: refs/branch-heads/3359@{#199}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/71c954dec87bbc952ba4ef55ccdc0ad6d7da1493/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java

Comment 8 Deleted

Sign in to add a comment