Skip to content

Commit

Permalink
[M110] Fix current queue not correctly updated when Remove_front is c…
Browse files Browse the repository at this point in the history
…ancelled

In following scenario:
1. [m1, m2] is showing
2. m1 is removed, so transit [m1, m2] to [m2, null]
3. before step 2 is finished, queue is suspended.

Before step 2 is finished, currentQueue has been updated to [m2, null]
but m1 is not actually removed from container because
SingleActionMessage's callback is not triggered due to the  cancellation
of animator.

This CL forces the animator to end in order to trigger the callback
to ensure the message is actually removed from container.

(cherry picked from commit 75f0260)

Bug: 1405389
Change-Id: I2f774fda99f5ac9add39857a58589afdd5915d9d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4148054
Reviewed-by: Aishwarya Rajesh <aishwaryarj@google.com>
Commit-Queue: Lijin Shen <lazzzis@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1091114}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4163378
Cr-Commit-Position: refs/branch-heads/5481@{#255}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
Lijin Shen authored and Chromium LUCI CQ committed Jan 13, 2023
1 parent b54c914 commit 4b33170
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 1 deletion.
Expand Up @@ -159,7 +159,12 @@ public void updateWithoutStacking(
public void updateWithStacking(
@NonNull List<MessageState> candidates, boolean isSuspended, Runnable onFinished) {
// Wait until the current animation is done, unless we need to hide them immediately.
if (!isSuspended && mAnimatorSet.isStarted()) {
if (mAnimatorSet.isStarted()) {
if (isSuspended) {
// crbug.com/1405389: Force animation to end in order to trigger callbacks.
mAnimatorSet.end();
onFinished.run();
}
return;
}
var currentFront = mCurrentDisplayedMessages.get(0); // Currently front.
Expand Down Expand Up @@ -358,4 +363,8 @@ public void onEnd(Animator animator) {
mOnFinished.run();
}
}

AnimatorSet getAnimatorSetForTesting() {
return mAnimatorSet;
}
}
Expand Up @@ -10,6 +10,7 @@
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyInt;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.never;
Expand Down Expand Up @@ -442,6 +443,51 @@ public void onAnimationEnd() {}
verify(m2.handler).hide(anyInt(), anyInt(), anyBoolean());
}

// Test when suspension cancels a hiding animation.
@Test
@SmallTest
public void testSuspensionCancellingHidingAnimation() throws TimeoutException {
var currentMessages = mAnimationCoordinator.getCurrentDisplayedMessages();
Assert.assertArrayEquals(new MessageState[] {null, null}, currentMessages.toArray());
MessageState m1 = buildMessageState();
setMessageIdentifier(m1, 1);
MessageState m2 = buildMessageState();
setMessageIdentifier(m2, 2);
doAnswer(invocation -> {
Runnable runnable = invocation.getArgument(0);
runnable.run();
return null;
})
.when(mContainer)
.runAfterInitialMessageLayout(any(Runnable.class));
mAnimationCoordinator.updateWithStacking(Arrays.asList(m1, m2), false, () -> {});
verify(m1.handler).show(Position.INVISIBLE, Position.FRONT);
verify(m2.handler).show(Position.FRONT, Position.BACK);
currentMessages = mAnimationCoordinator.getCurrentDisplayedMessages();
Assert.assertArrayEquals(new MessageState[] {m1, m2}, currentMessages.toArray());
shadowOf(getMainLooper()).idle();

var animator = ValueAnimator.ofInt(0, 1);
animator.setDuration(100000);
doReturn(animator).when(m1.handler).hide(anyInt(), anyInt(), anyBoolean());
mAnimationCoordinator.updateWithStacking(Arrays.asList(m2, null), false, () -> {});
verify(m1.handler).hide(anyInt(), anyInt(), anyBoolean());
verify(m2.handler).show(Position.BACK, Position.FRONT);

currentMessages = mAnimationCoordinator.getCurrentDisplayedMessages();
Assert.assertArrayEquals(new MessageState[] {m2, null}, currentMessages.toArray());
Assert.assertTrue(mAnimationCoordinator.getAnimatorSetForTesting().isStarted());

mAnimationCoordinator.updateWithStacking(Arrays.asList(null, null), true, () -> {
// Simulate triggering the callback given by MessageQueueManager after animation is
// finished; equivalent to calling MessageQueueManager's updateWithStacking.
mAnimationCoordinator.updateWithStacking(Arrays.asList(null, null), true, () -> {});
});
Assert.assertFalse(mAnimationCoordinator.getAnimatorSetForTesting().isStarted());
currentMessages = mAnimationCoordinator.getCurrentDisplayedMessages();
Assert.assertArrayEquals(new MessageState[] {null, null}, currentMessages.toArray());
}

private void setMessageIdentifier(MessageState message, int messageIdentifier) {
doReturn(messageIdentifier).when(message.handler).getMessageIdentifier();
}
Expand Down

0 comments on commit 4b33170

Please sign in to comment.