Commit 96f0ee80 authored by Yue Zhang's avatar Yue Zhang Committed by Commit Bot

Fix a bug in TabGridDialog animation setup

http://crrev.com/c/2119188 exposes two bugs in the setup introduced in
http://crrev.com/c/2143671, which jointly lead to a corner case where
the dialog animation is not setup correctly. This CL fixes this issue
by 1) restoring dialog view size and position after animation 2)
restoring the animation view alpha in the correct position.

This CL also revives a disabled test that could have caught this issue
by removing the flaky parts and add some bug-specific checkings.

Bug: 1075677, 1099563
Change-Id: Ie722d77f806dd556f69cb2df6b758500739cdc65
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2278923Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Yue Zhang <yuezhanggg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788801}
parent e7e9150e
......@@ -442,6 +442,16 @@ public class TabGridDialogView extends FrameLayout
dialogZoomInAnimatorSet.setDuration(DIALOG_ANIMATION_DURATION);
dialogZoomInAnimatorSet.setInterpolator(Interpolators.FAST_OUT_SLOW_IN_INTERPOLATOR);
dialogZoomInAnimatorSet.addListener(new AnimatorListenerAdapter() {
@Override
public void onAnimationEnd(Animator animation) {
mDialogContainerView.setTranslationX(0f);
mDialogContainerView.setTranslationY(0f);
mDialogContainerView.setScaleX(1f);
mDialogContainerView.setScaleY(1f);
}
});
// In the first half of the dialog hiding animation, the dialog fades out while it moves and
// scales down.
final ObjectAnimator dialogZoomInAlphaAnimator =
......@@ -484,6 +494,14 @@ public class TabGridDialogView extends FrameLayout
mBackgroundFrame.bringToFront();
mAnimationCardView.bringToFront();
}
@Override
public void onAnimationEnd(Animator animation) {
// At the end of the hiding animation, reset the alpha of animation related views to
// 0.
mBackgroundFrame.setAlpha(0f);
mAnimationCardView.setAlpha(0f);
}
});
// During the whole dialog hiding animation, the frame background scales down and moves so
......@@ -512,14 +530,6 @@ public class TabGridDialogView extends FrameLayout
// restored to 1.
mBackgroundFrame.setAlpha(1f);
}
@Override
public void onAnimationEnd(Animator animation) {
// At the end of the hiding animation, reset the alpha of animation related views to
// 0.
mBackgroundFrame.setAlpha(0f);
mAnimationCardView.setAlpha(0f);
}
});
// At the end of the dialog hiding animation, the original tab grid card fades in.
......
......@@ -25,7 +25,6 @@ import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.chrome.tab_ui.R;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.components.browser_ui.widget.scrim.ScrimCoordinator;
......@@ -272,8 +271,10 @@ public class TabGridDialogViewTest extends DummyUiActivityTestCase {
@Test
@MediumTest
@DisabledTest(message = "crbug.com/1075677")
public void testDialog_ZoomInZoomOut() {
// TODO(crbug.com/1075677): figure out a stable way to separate different stages of the
// animation so that we can verify the alpha and view hierarchy of the animation-related
// views.
AtomicReference<ViewGroup> parentViewReference = new AtomicReference<>();
// Setup the animation with a dummy animation source view.
TestThreadUtils.runOnUiThreadBlocking(() -> {
......@@ -285,18 +286,7 @@ public class TabGridDialogViewTest extends DummyUiActivityTestCase {
// Show the dialog with zoom-out animation.
TestThreadUtils.runOnUiThreadBlocking(() -> {
// At the very beginning of showing animation, the animation card should be on the
// top and the background frame should be the view below it. Both view should have
// alpha set to be 1.
mTabGridDialogView.showDialog();
if (areAnimatorsEnabled()) {
Assert.assertSame(
mAnimationCardView, parent.getChildAt(parent.getChildCount() - 1));
Assert.assertSame(
mBackgroundFrameView, parent.getChildAt(parent.getChildCount() - 2));
}
Assert.assertEquals(1f, mAnimationCardView.getAlpha(), 0.0);
Assert.assertEquals(1f, mBackgroundFrameView.getAlpha(), 0.0);
Assert.assertNotNull(mTabGridDialogView.getCurrentDialogAnimatorForTesting());
Assert.assertEquals(View.VISIBLE, mTabGridDialogView.getVisibility());
});
......@@ -320,14 +310,6 @@ public class TabGridDialogViewTest extends DummyUiActivityTestCase {
// Hide the dialog with zoom-in animation.
TestThreadUtils.runOnUiThreadBlocking(() -> {
mTabGridDialogView.hideDialog();
// At the very beginning of hiding animation, the dialog view should be on the top,
// and alpha of animation related views should remain 0.
if (areAnimatorsEnabled()) {
Assert.assertSame(
mTabGridDialogContainer, parent.getChildAt(parent.getChildCount() - 1));
}
Assert.assertEquals(1f, mTabGridDialogContainer.getAlpha(), 0.0);
Assert.assertEquals(1f, mBackgroundFrameView.getAlpha(), 0.0);
Assert.assertNotNull(mTabGridDialogView.getCurrentDialogAnimatorForTesting());
// PopupWindow is still showing for the hide animation.
Assert.assertEquals(View.VISIBLE, mTabGridDialogView.getVisibility());
......@@ -350,6 +332,10 @@ public class TabGridDialogViewTest extends DummyUiActivityTestCase {
Assert.assertEquals(View.GONE, mTabGridDialogView.getVisibility());
Assert.assertEquals(0f, mAnimationCardView.getAlpha(), 0.0);
Assert.assertEquals(0f, mBackgroundFrameView.getAlpha(), 0.0);
Assert.assertEquals(0f, mTabGridDialogContainer.getTranslationX(), 0.0);
Assert.assertEquals(0f, mTabGridDialogContainer.getTranslationY(), 0.0);
Assert.assertEquals(1f, mTabGridDialogContainer.getScaleX(), 0.0);
Assert.assertEquals(1f, mTabGridDialogContainer.getScaleY(), 0.0);
});
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment