Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Catch exception for deferred window transaction

Browse Source 
Currently a queued window container transaction may execute
on display thread. Because the transaction might be deferred a
while, its operations may be no longer valid. And since
applyTransaction may throw: UnsupportedOperationException,
IllegalArgumentException and RuntimeException, catch the
exceptions for the deferred case to avoid crashing the system.

Also ignore some hierarchy operations if the target container
was detached.

Fix: 282560294
Test: atest TransitionTests#testQueueWithLegacy

Change-Id: Id5fc8c31f51b6def5f1b155f3a5a0d2c9d55df63
This commit is contained in:
Riddle Hsu
2023-06-14 16:34:17 +08:00
parent f80ef93530
commit 0ceb15e492
3 changed files with 50 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
services/core/java/com/android/server/wm/TransitionController.java
View File

@@ -58,6 +58,7 @@ import com.android.internal.protolog.common.ProtoLog;
import com.android.server.FgThread;
import java.util.ArrayList;
import java.util.function.Consumer;
import java.util.function.LongConsumer;
/**
@@ -1314,18 +1315,18 @@ class TransitionController {
return transit;
}
/** Returns {@code true} if it started collecting, {@code false} if it was queued. */
boolean startLegacySyncOrQueue(BLASTSyncEngine.SyncGroup syncGroup, Runnable applySync) {
/** Starts the sync set if there is no pending or active syncs, otherwise enqueue the sync. */
void startLegacySyncOrQueue(BLASTSyncEngine.SyncGroup syncGroup, Consumer<Boolean> applySync) {
if (!mQueuedTransitions.isEmpty() || mSyncEngine.hasActiveSync()) {
// Just add to queue since we already have a queue.
mQueuedTransitions.add(new QueuedTransition(syncGroup, (d) -> applySync.run()));
mQueuedTransitions.add(new QueuedTransition(syncGroup,
(deferred) -> applySync.accept(true /* deferred */)));
ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS_MIN,
"Queueing legacy sync-set: %s", syncGroup.mSyncId);
return false;
return;
}
mSyncEngine.startSyncSet(syncGroup);
applySync.run();
return true;
applySync.accept(false /* deferred */);
}
interface OnStartCollect {

44
services/core/java/com/android/server/wm/WindowOrganizerController.java
View File

@@ -232,8 +232,8 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
final BLASTSyncEngine.SyncGroup syncGroup = prepareSyncWithOrganizer(callback);
final int syncId = syncGroup.mSyncId;
if (mTransitionController.isShellTransitionsEnabled()) {
mTransitionController.startLegacySyncOrQueue(syncGroup, () -> {
applyTransaction(t, syncId, null /*transition*/, caller);
mTransitionController.startLegacySyncOrQueue(syncGroup, (deferred) -> {
applyTransaction(t, syncId, null /* transition */, caller, deferred);
setSyncReady(syncId);
});
} else {
@@ -304,7 +304,8 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
(deferred) -> {
nextTransition.start();
nextTransition.mLogger.mStartWCT = wct;
applyTransaction(wct, -1 /*syncId*/, nextTransition, caller);
applyTransaction(wct, -1 /* syncId */, nextTransition, caller,
deferred);
if (needsSetReady) {
nextTransition.setAllReady();
}
@@ -456,7 +457,7 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
transition.abort();
return;
}
if (applyTransaction(wct, -1 /* syncId */, transition, caller)
if (applyTransaction(wct, -1 /* syncId */, transition, caller, deferred)
== TRANSACT_EFFECTS_NONE && transition.mParticipants.isEmpty()) {
transition.abort();
return;
@@ -476,6 +477,23 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
return applyTransaction(t, syncId, transition, caller, null /* finishTransition */);
}
private int applyTransaction(@NonNull WindowContainerTransaction t, int syncId,
@Nullable Transition transition, @NonNull CallerInfo caller, boolean deferred) {
if (deferred) {
try {
return applyTransaction(t, syncId, transition, caller);
} catch (RuntimeException e) {
// If the transaction is deferred, the caller could be from TransitionController
// #tryStartCollectFromQueue that executes on system's worker thread rather than
// binder thread. And the operation in the WCT may be outdated that violates the
// current state. So catch the exception to avoid crashing the system.
Slog.e(TAG, "Failed to execute deferred applyTransaction", e);
}
return TRANSACT_EFFECTS_NONE;
}
return applyTransaction(t, syncId, transition, caller);
}
/**
* @param syncId If non-null, this will be a sync-transaction.
* @param transition A transition to collect changes into.
@@ -838,13 +856,21 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
switch (type) {
case HIERARCHY_OP_TYPE_REMOVE_TASK: {
final WindowContainer wc = WindowContainer.fromBinder(hop.getContainer());
final Task task = wc != null ? wc.asTask() : null;
if (wc == null || wc.asTask() == null || !wc.isAttached()) {
Slog.e(TAG, "Attempt to remove invalid task: " + wc);
break;
}
final Task task = wc.asTask();
task.remove(true, "Applying remove task Hierarchy Op");
break;
}
case HIERARCHY_OP_TYPE_SET_LAUNCH_ROOT: {
final WindowContainer wc = WindowContainer.fromBinder(hop.getContainer());
final Task task = wc != null ? wc.asTask() : null;
if (wc == null || !wc.isAttached()) {
Slog.e(TAG, "Attempt to set launch root to a detached container: " + wc);
break;
}
final Task task = wc.asTask();
if (task == null) {
throw new IllegalArgumentException("Cannot set non-task as launch root: " + wc);
} else if (task.getTaskDisplayArea() == null) {
@@ -858,7 +884,11 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
}
case HIERARCHY_OP_TYPE_SET_LAUNCH_ADJACENT_FLAG_ROOT: {
final WindowContainer wc = WindowContainer.fromBinder(hop.getContainer());
final Task task = wc != null ? wc.asTask() : null;
if (wc == null || !wc.isAttached()) {
Slog.e(TAG, "Attempt to set launch adjacent to a detached container: " + wc);
break;
}
final Task task = wc.asTask();
final boolean clearRoot = hop.getToTop();
if (task == null) {
throw new IllegalArgumentException("Cannot set non-task as launch root: " + wc);

8
services/tests/wmtests/src/com/android/server/wm/TransitionTests.java
View File

@@ -2191,8 +2191,11 @@ public class TransitionTests extends WindowTestsBase {
BLASTSyncEngine.SyncGroup legacySync = mSyncEngine.prepareSyncSet(
mock(BLASTSyncEngine.TransactionReadyListener.class), "test");
final boolean[] applyLegacy = new boolean[]{false};
controller.startLegacySyncOrQueue(legacySync, () -> applyLegacy[0] = true);
final boolean[] applyLegacy = new boolean[2];
controller.startLegacySyncOrQueue(legacySync, (deferred) -> {
applyLegacy[0] = true;
applyLegacy[1] = deferred;
});
assertFalse(applyLegacy[0]);
waitUntilHandlersIdle();
@@ -2208,6 +2211,7 @@ public class TransitionTests extends WindowTestsBase {
assertTrue(transitA.isPlaying());
// legacy sync should start now
assertTrue(applyLegacy[0]);
assertTrue(applyLegacy[1]);
// transitB must wait
assertTrue(transitB.isPending());