Don't prematurely commit visibility in consecutive shell transitions

Originally, upon finish, any participating activity would immediately
any commitVisibility(false) if the activity was invisible. This can
cause "glitches" in quick sequences of animations (like opening and
then immediately closing an activity) because even if a transition
was created to make an activity visible, if it happened to be
invisibleRequested by the time the transition finished, it'd
immediately hide the surface when that surface is still needed
for the follow-on animation.

This CL records the "intended" endVisibility of each window token
in a transition so that it can be used during finishTransition to
prevent commitVisibility if a follow-on transition is responible
for animating it.

Bug: 183993924
Test: atest TransitionTests#testIntermediateVisibility
Change-Id: Ib830ccfa924191091a152037156ae62cd98ffa09
This commit is contained in:
Evan Rosky
2021-08-10 16:13:21 -07:00
parent 3ed6fed8cf
commit 91eee7fdcb
3 changed files with 106 additions and 9 deletions

View File

@@ -139,6 +139,12 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe
/** The final animation targets derived from participants after promotion. */
private ArraySet<WindowContainer> mTargets = null;
/**
* Set of participating windowtokens (activity/wallpaper) which are visible at the end of
* the transition animation.
*/
private final ArraySet<WindowToken> mVisibleAtTransitionEndTokens = new ArraySet<>();
/** Custom activity-level animation options and callbacks. */
private TransitionInfo.AnimationOptions mOverrideOptions;
private IRemoteCallback mClientAnimationStartCallback = null;
@@ -345,7 +351,14 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe
for (int i = 0; i < mParticipants.size(); ++i) {
final ActivityRecord ar = mParticipants.valueAt(i).asActivityRecord();
if (ar != null) {
if (!ar.isVisibleRequested()) {
boolean visibleAtTransitionEnd = mVisibleAtTransitionEndTokens.contains(ar);
// We need both the expected visibility AND current requested-visibility to be
// false. If it is expected-visible but not currently visible, it means that
// another animation is queued-up to animate this to invisibility, so we can't
// remove the surfaces yet. If it is currently visible, but not expected-visible,
// then doing commitVisibility here would actually be out-of-order and leave the
// activity in a bad state.
if (!visibleAtTransitionEnd && !ar.isVisibleRequested()) {
boolean commitVisibility = true;
if (ar.getDeferHidingClient() && ar.getTask() != null) {
if (ar.pictureInPictureArgs != null
@@ -368,17 +381,20 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe
activitiesWentInvisible = true;
}
}
if (mChanges.get(ar).mVisible != ar.isVisibleRequested()) {
if (mChanges.get(ar).mVisible != visibleAtTransitionEnd) {
// Legacy dispatch relies on this (for now).
ar.mEnteringAnimation = ar.isVisibleRequested();
ar.mEnteringAnimation = visibleAtTransitionEnd;
}
mController.dispatchLegacyAppTransitionFinished(ar);
}
final WallpaperWindowToken wt = mParticipants.valueAt(i).asWallpaperToken();
if (wt != null && !wt.isVisibleRequested()) {
ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS,
" Commit wallpaper becoming invisible: %s", wt);
wt.commitVisibility(false /* visible */);
if (wt != null) {
final boolean visibleAtTransitionEnd = mVisibleAtTransitionEndTokens.contains(wt);
if (!visibleAtTransitionEnd && !wt.isVisibleRequested()) {
ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS,
" Commit wallpaper becoming invisible: %s", wt);
wt.commitVisibility(false /* visible */);
}
}
}
if (activitiesWentInvisible) {
@@ -488,6 +504,15 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe
}
}
// Record windowtokens (activity/wallpaper) that are expected to be visible after the
// transition animation. This will be used in finishTransition to prevent prematurely
// committing visibility.
for (int i = mParticipants.size() - 1; i >= 0; --i) {
final WindowContainer wc = mParticipants.valueAt(i);
if (wc.asWindowToken() == null || !wc.isVisibleRequested()) continue;
mVisibleAtTransitionEndTokens.add(wc.asWindowToken());
}
mStartTransaction = transaction;
mFinishTransaction = mController.mAtm.mWindowManager.mTransactionFactory.get();
buildFinishTransaction(mFinishTransaction, info.getRootLeash());

View File

@@ -117,11 +117,16 @@ class TransitionController {
void registerTransitionPlayer(@Nullable ITransitionPlayer player) {
try {
// Note: asBinder() can be null if player is same process (likely in a test).
if (mTransitionPlayer != null) {
mTransitionPlayer.asBinder().unlinkToDeath(mTransitionPlayerDeath, 0);
if (mTransitionPlayer.asBinder() != null) {
mTransitionPlayer.asBinder().unlinkToDeath(mTransitionPlayerDeath, 0);
}
mTransitionPlayer = null;
}
player.asBinder().linkToDeath(mTransitionPlayerDeath, 0);
if (player.asBinder() != null) {
player.asBinder().linkToDeath(mTransitionPlayerDeath, 0);
}
mTransitionPlayer = player;
} catch (RemoteException e) {
throw new RuntimeException("Unable to set transition player");

View File

@@ -20,6 +20,7 @@ import static android.app.WindowConfiguration.ACTIVITY_TYPE_STANDARD;
import static android.app.WindowConfiguration.WINDOWING_MODE_FREEFORM;
import static android.app.WindowConfiguration.WINDOWING_MODE_FULLSCREEN;
import static android.view.WindowManager.LayoutParams.TYPE_WALLPAPER;
import static android.view.WindowManager.TRANSIT_CLOSE;
import static android.view.WindowManager.TRANSIT_OLD_TASK_OPEN;
import static android.view.WindowManager.TRANSIT_OPEN;
import static android.view.WindowManager.TRANSIT_TO_BACK;
@@ -41,6 +42,7 @@ import android.platform.test.annotations.Presubmit;
import android.util.ArrayMap;
import android.util.ArraySet;
import android.window.ITaskOrganizer;
import android.window.ITransitionPlayer;
import android.window.TransitionInfo;
import androidx.test.filters.SmallTest;
@@ -444,6 +446,71 @@ public class TransitionTests extends WindowTestsBase {
info.getChange(openInChangeTask.mRemoteToken.toWindowContainerToken()), info));
}
@Test
public void testIntermediateVisibility() {
final TransitionController controller = new TransitionController(mAtm);
final ITransitionPlayer player = new ITransitionPlayer.Default();
controller.registerTransitionPlayer(player);
ITaskOrganizer mockOrg = mock(ITaskOrganizer.class);
final Transition openTransition = controller.createTransition(TRANSIT_OPEN);
// Start out with task2 visible and set up a transition that closes task2 and opens task1
final Task task1 = createTask(mDisplayContent);
task1.mTaskOrganizer = mockOrg;
final ActivityRecord activity1 = createActivityRecord(task1);
activity1.mVisibleRequested = false;
activity1.setVisible(false);
final Task task2 = createTask(mDisplayContent);
task2.mTaskOrganizer = mockOrg;
final ActivityRecord activity2 = createActivityRecord(task1);
activity2.mVisibleRequested = true;
activity2.setVisible(true);
openTransition.collectExistenceChange(task1);
openTransition.collectExistenceChange(activity1);
openTransition.collectExistenceChange(task2);
openTransition.collectExistenceChange(activity2);
activity1.mVisibleRequested = true;
activity1.setVisible(true);
activity2.mVisibleRequested = false;
// Using abort to force-finish the sync (since we can't wait for drawing in unit test).
// We didn't call abort on the transition itself, so it will still run onTransactionReady
// normally.
mWm.mSyncEngine.abort(openTransition.getSyncId());
// Before finishing openTransition, we are now going to simulate closing task1 to return
// back to (open) task2.
final Transition closeTransition = controller.createTransition(TRANSIT_CLOSE);
closeTransition.collectExistenceChange(task1);
closeTransition.collectExistenceChange(activity1);
closeTransition.collectExistenceChange(task2);
closeTransition.collectExistenceChange(activity2);
activity1.mVisibleRequested = false;
activity2.mVisibleRequested = true;
openTransition.finishTransition();
// We finished the openTransition. Even though activity1 is visibleRequested=false, since
// the closeTransition animation hasn't played yet, make sure that we didn't commit
// visible=false on activity1 since it needs to remain visible for the animation.
assertTrue(activity1.isVisible());
assertTrue(activity2.isVisible());
// Using abort to force-finish the sync (since we obviously can't wait for drawing).
// We didn't call abort on the actual transition, so it will still run onTransactionReady
// normally.
mWm.mSyncEngine.abort(closeTransition.getSyncId());
closeTransition.finishTransition();
assertFalse(activity1.isVisible());
assertTrue(activity2.isVisible());
}
/** Fill the change map with all the parents of top. Change maps are usually fully populated */
private static void fillChangeMap(ArrayMap<WindowContainer, Transition.ChangeInfo> changes,
WindowContainer top) {