Fix system crash when ActivityEmbedding process died with pending events

Before, when the organizer binder died, it will trigger TaskFragment
removal that can cause surface placement. Now, we make sure to cleanup
all pending events before removing organized TaskFragments.

Bug: 251583070
Bug: 254322068
Test: atest WmTests:TaskFragmentOrganizerControllerTest
Change-Id: Ifec6f763a7e7819c93cc28d71bd0314f3ac9ac0d
This commit is contained in:
Chris Li
2022-10-19 11:40:45 +08:00
parent 5dafb54f14
commit 61ba70f1d1
2 changed files with 73 additions and 11 deletions

View File

@@ -184,19 +184,30 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr
}
void dispose() {
while (!mOrganizedTaskFragments.isEmpty()) {
final TaskFragment taskFragment = mOrganizedTaskFragments.get(0);
// Cleanup before remove to prevent it from sending any additional event, such as
// #onTaskFragmentVanished, to the removed organizer.
for (int i = mOrganizedTaskFragments.size() - 1; i >= 0; i--) {
// Cleanup the TaskFragmentOrganizer from all TaskFragments it organized before
// removing the windows to prevent it from adding any additional TaskFragment
// pending event.
final TaskFragment taskFragment = mOrganizedTaskFragments.get(i);
taskFragment.onTaskFragmentOrganizerRemoved();
taskFragment.removeImmediately();
mOrganizedTaskFragments.remove(taskFragment);
}
// Defer to avoid unnecessary layout when there are multiple TaskFragments removal.
mAtmService.deferWindowLayout();
try {
while (!mOrganizedTaskFragments.isEmpty()) {
final TaskFragment taskFragment = mOrganizedTaskFragments.remove(0);
taskFragment.removeImmediately();
}
} finally {
mAtmService.continueWindowLayout();
}
for (int i = mDeferredTransitions.size() - 1; i >= 0; i--) {
// Cleanup any running transaction to unblock the current transition.
onTransactionFinished(mDeferredTransitions.keyAt(i));
}
mOrganizer.asBinder().unlinkToDeath(this, 0 /*flags*/);
mOrganizer.asBinder().unlinkToDeath(this, 0 /* flags */);
}
@NonNull
@@ -426,7 +437,6 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr
@Override
public void unregisterOrganizer(@NonNull ITaskFragmentOrganizer organizer) {
validateAndGetState(organizer);
final int pid = Binder.getCallingPid();
final long uid = Binder.getCallingUid();
final long origId = Binder.clearCallingIdentity();
@@ -697,11 +707,17 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr
}
private void removeOrganizer(@NonNull ITaskFragmentOrganizer organizer) {
final TaskFragmentOrganizerState state = validateAndGetState(organizer);
final TaskFragmentOrganizerState state = mTaskFragmentOrganizerState.get(
organizer.asBinder());
if (state == null) {
Slog.w(TAG, "The organizer has already been removed.");
return;
}
// Remove any pending event of this organizer first because state.dispose() may trigger
// event dispatch as result of surface placement.
mPendingTaskFragmentEvents.remove(organizer.asBinder());
// remove all of the children of the organized TaskFragment
state.dispose();
// Remove any pending event of this organizer.
mPendingTaskFragmentEvents.remove(organizer.asBinder());
mTaskFragmentOrganizerState.remove(organizer.asBinder());
}

View File

@@ -56,6 +56,7 @@ import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
@@ -91,6 +92,7 @@ import org.mockito.ArgumentCaptor;
import org.mockito.Captor;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockito.stubbing.Answer;
import java.util.List;
@@ -761,6 +763,50 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
assertNotNull(mWindowOrganizerController.getTaskFragment(fragmentToken));
}
@Test
public void testOrganizerRemovedWithPendingEvents() {
final TaskFragment tf0 = new TaskFragmentBuilder(mAtm)
.setCreateParentTask()
.setOrganizer(mOrganizer)
.setFragmentToken(mFragmentToken)
.build();
final TaskFragment tf1 = new TaskFragmentBuilder(mAtm)
.setCreateParentTask()
.setOrganizer(mOrganizer)
.setFragmentToken(new Binder())
.build();
assertTrue(tf0.isOrganizedTaskFragment());
assertTrue(tf1.isOrganizedTaskFragment());
assertTrue(tf0.isAttached());
assertTrue(tf0.isAttached());
// Mock the behavior that remove TaskFragment can trigger event dispatch.
final Answer<Void> removeImmediately = invocation -> {
invocation.callRealMethod();
mController.dispatchPendingEvents();
return null;
};
doAnswer(removeImmediately).when(tf0).removeImmediately();
doAnswer(removeImmediately).when(tf1).removeImmediately();
// Add pending events.
mController.onTaskFragmentAppeared(mIOrganizer, tf0);
mController.onTaskFragmentAppeared(mIOrganizer, tf1);
// Remove organizer.
mController.unregisterOrganizer(mIOrganizer);
mController.dispatchPendingEvents();
// Nothing should happen after the organizer is removed.
verify(mOrganizer, never()).onTransactionReady(any());
// TaskFragments should be removed.
assertFalse(tf0.isOrganizedTaskFragment());
assertFalse(tf1.isOrganizedTaskFragment());
assertFalse(tf0.isAttached());
assertFalse(tf0.isAttached());
}
@Test
public void testTaskFragmentInPip_startActivityInTaskFragment() {
setupTaskFragmentInPip();