From 050d4fd564dccb119e2f7cbe4e41a130b497504d Mon Sep 17 00:00:00 2001
From: Riddle Hsu <riddlehsu@google.com>
Date: Fri, 27 Oct 2023 13:57:37 +0000
Subject: [PATCH] Consume pending display config change with multiple remote
callbacks
For example:
Callback1 { sendNewConfiguration }
Callback2 { applySomething }
If Callback1 completes first, its sendNewConfiguration won't take
effect because isWaitingForRemoteDisplayChange is true by the
pending Callback2. And then when Callback2 completes, it only
perform its own operations.
So this change provides a callback from display to make sure its
pending config change can be handled when all remote callbacks
are done.
Fix: 305655839
Test: atest DisplayContentTests#testRemoteDisplayChange
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:c793afe3bd45cd5e5a820e67e681483b2c14b8fb)
Merged-In: I64897baeaa0b5d81c904d75f2b1e17c0eefec3bf
Change-Id: I64897baeaa0b5d81c904d75f2b1e17c0eefec3bf
---
.../com/android/server/wm/DisplayContent.java | 2 +-
.../wm/RemoteDisplayChangeController.java | 35 ++++++++++----
.../server/wm/DisplayContentTests.java | 48 +++++++++++++++++++
3 files changed, 76 insertions(+), 9 deletions(-)
diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java
index 47e90ceaca79..36c7930c50e8 100644
--- a/services/core/java/com/android/server/wm/DisplayContent.java
+++ b/services/core/java/com/android/server/wm/DisplayContent.java
@@ -1162,7 +1162,7 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp
mUnknownAppVisibilityController = new UnknownAppVisibilityController(mWmService, this);
mDisplaySwitchTransitionLauncher = new PhysicalDisplaySwitchTransitionLauncher(this,
mTransitionController);
- mRemoteDisplayChangeController = new RemoteDisplayChangeController(mWmService, mDisplayId);
+ mRemoteDisplayChangeController = new RemoteDisplayChangeController(this);
final InputChannel inputChannel = mWmService.mInputManager.monitorInput(
"PointerEventDispatcher" + mDisplayId, mDisplayId);
diff --git a/services/core/java/com/android/server/wm/RemoteDisplayChangeController.java b/services/core/java/com/android/server/wm/RemoteDisplayChangeController.java
index 43baebc7255a..bad0c01c8f66 100644
--- a/services/core/java/com/android/server/wm/RemoteDisplayChangeController.java
+++ b/services/core/java/com/android/server/wm/RemoteDisplayChangeController.java
@@ -26,6 +26,7 @@ import android.view.IDisplayChangeWindowCallback;
import android.window.DisplayAreaInfo;
import android.window.WindowContainerTransaction;
+import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.protolog.common.ProtoLog;
import java.util.ArrayList;
@@ -44,16 +45,16 @@ public class RemoteDisplayChangeController {
private static final int REMOTE_DISPLAY_CHANGE_TIMEOUT_MS = 800;
private final WindowManagerService mService;
- private final int mDisplayId;
+ private final DisplayContent mDisplayContent;
private final Runnable mTimeoutRunnable = this::onContinueTimedOut;
// all remote changes that haven't finished yet.
private final List<ContinueRemoteDisplayChangeCallback> mCallbacks = new ArrayList<>();
- public RemoteDisplayChangeController(WindowManagerService service, int displayId) {
- mService = service;
- mDisplayId = displayId;
+ RemoteDisplayChangeController(@NonNull DisplayContent displayContent) {
+ mService = displayContent.mWmService;
+ mDisplayContent = displayContent;
}
/**
@@ -99,8 +100,8 @@ public class RemoteDisplayChangeController {
try {
mService.mH.removeCallbacks(mTimeoutRunnable);
mService.mH.postDelayed(mTimeoutRunnable, REMOTE_DISPLAY_CHANGE_TIMEOUT_MS);
- mService.mDisplayChangeController.onDisplayChange(mDisplayId, fromRotation, toRotation,
- newDisplayAreaInfo, remoteCallback);
+ mService.mDisplayChangeController.onDisplayChange(mDisplayContent.mDisplayId,
+ fromRotation, toRotation, newDisplayAreaInfo, remoteCallback);
return true;
} catch (RemoteException e) {
Slog.e(TAG, "Exception while dispatching remote display-change", e);
@@ -117,10 +118,23 @@ public class RemoteDisplayChangeController {
mCallbacks.get(i).onContinueRemoteDisplayChange(null /* transaction */);
}
mCallbacks.clear();
+ onCompleted();
}
}
- private void continueDisplayChange(@NonNull ContinueRemoteDisplayChangeCallback callback,
+ /** Called when all remote callbacks are done. */
+ private void onCompleted() {
+ // Because DisplayContent#sendNewConfiguration() will be skipped if there are pending remote
+ // changes, check again when all remote callbacks are done. E.g. callback X is done but
+ // there is a pending callback Y so its invocation is skipped, and when the callback Y is
+ // done, it doesn't call sendNewConfiguration().
+ if (mDisplayContent.mWaitingForConfig) {
+ mDisplayContent.sendNewConfiguration();
+ }
+ }
+
+ @VisibleForTesting
+ void continueDisplayChange(@NonNull ContinueRemoteDisplayChangeCallback callback,
@Nullable WindowContainerTransaction transaction) {
synchronized (mService.mGlobalLock) {
int idx = mCallbacks.indexOf(callback);
@@ -133,11 +147,16 @@ public class RemoteDisplayChangeController {
// ordering by continuing everything up until this one with empty transactions.
mCallbacks.get(i).onContinueRemoteDisplayChange(null /* transaction */);
}
+ // The "toIndex" is exclusive, so it needs +1 to clear the current calling callback.
mCallbacks.subList(0, idx + 1).clear();
- if (mCallbacks.isEmpty()) {
+ final boolean completed = mCallbacks.isEmpty();
+ if (completed) {
mService.mH.removeCallbacks(mTimeoutRunnable);
}
callback.onContinueRemoteDisplayChange(transaction);
+ if (completed) {
+ onCompleted();
+ }
}
}
diff --git a/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java b/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java
index 9b22efdae7da..89fc65a54ab3 100644
--- a/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java
@@ -144,6 +144,7 @@ import android.window.DisplayAreaInfo;
import android.window.IDisplayAreaOrganizer;
import android.window.ScreenCapture;
import android.window.WindowContainerToken;
+import android.window.WindowContainerTransaction;
import androidx.test.filters.SmallTest;
@@ -2012,6 +2013,53 @@ public class DisplayContentTests extends WindowTestsBase {
assertFalse(mWm.mDisplayFrozen);
}
+ @Test
+ public void testRemoteDisplayChange() {
+ mWm.mDisplayChangeController = mock(IDisplayChangeWindowController.class);
+ final Boolean[] isWaitingForRemote = new Boolean[2];
+ final var callbacks = new RemoteDisplayChangeController.ContinueRemoteDisplayChangeCallback[
+ isWaitingForRemote.length];
+ for (int i = 0; i < isWaitingForRemote.length; i++) {
+ final int index = i;
+ var callback = new RemoteDisplayChangeController.ContinueRemoteDisplayChangeCallback() {
+ @Override
+ public void onContinueRemoteDisplayChange(WindowContainerTransaction transaction) {
+ isWaitingForRemote[index] =
+ mDisplayContent.mRemoteDisplayChangeController
+ .isWaitingForRemoteDisplayChange();
+ }
+ };
+ mDisplayContent.mRemoteDisplayChangeController.performRemoteDisplayChange(
+ ROTATION_0, ROTATION_0, null /* newDisplayAreaInfo */, callback);
+ callbacks[i] = callback;
+ }
+
+ // The last callback is completed, all callbacks should be notified.
+ mDisplayContent.mRemoteDisplayChangeController.continueDisplayChange(callbacks[1],
+ null /* transaction */);
+ // When notifying 0, the callback 1 still exists.
+ assertTrue(isWaitingForRemote[0]);
+ assertFalse(isWaitingForRemote[1]);
+
+ // The first callback is completed, other callbacks after it should remain.
+ for (int i = 0; i < isWaitingForRemote.length; i++) {
+ isWaitingForRemote[i] = null;
+ mDisplayContent.mRemoteDisplayChangeController.performRemoteDisplayChange(
+ ROTATION_0, ROTATION_0, null /* newDisplayAreaInfo */, callbacks[i]);
+ }
+ mDisplayContent.mRemoteDisplayChangeController.continueDisplayChange(callbacks[0],
+ null /* transaction */);
+ assertTrue(isWaitingForRemote[0]);
+ assertNull(isWaitingForRemote[1]);
+
+ // Complete the last callback. It should be able to consume pending config change.
+ mDisplayContent.mWaitingForConfig = true;
+ mDisplayContent.mRemoteDisplayChangeController.continueDisplayChange(callbacks[1],
+ null /* transaction */);
+ assertFalse(isWaitingForRemote[1]);
+ assertFalse(mDisplayContent.mWaitingForConfig);
+ }
+
@Test
public void testShellTransitRotation() {
DisplayContent dc = createNewDisplay();
--
GitLab