diff options
4 files changed, 138 insertions, 118 deletions
diff --git a/core/java/android/app/servertransaction/ClientTransactionListenerController.java b/core/java/android/app/servertransaction/ClientTransactionListenerController.java index e2aee839b0c3..7418c06a2dd1 100644 --- a/core/java/android/app/servertransaction/ClientTransactionListenerController.java +++ b/core/java/android/app/servertransaction/ClientTransactionListenerController.java @@ -20,46 +20,31 @@ import static com.android.window.flags.Flags.syncWindowConfigUpdateFlag; import static java.util.Objects.requireNonNull; -import android.annotation.CallbackExecutor; import android.annotation.NonNull; +import android.app.ActivityThread; +import android.hardware.display.DisplayManagerGlobal; import android.os.Process; -import android.util.ArrayMap; -import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; -import java.util.ArrayList; -import java.util.concurrent.Executor; -import java.util.function.IntConsumer; - /** * Singleton controller to manage listeners to individual {@link ClientTransaction}. * - * TODO(b/260873529) make as TestApi to allow CTS. * @hide */ public class ClientTransactionListenerController { private static ClientTransactionListenerController sController; - private final Object mLock = new Object(); - - /** - * Mapping from client registered listener for display change to the corresponding - * {@link Executor} to invoke the listener on. - * @see #registerDisplayChangeListener(IntConsumer, Executor) - */ - @GuardedBy("mLock") - private final ArrayMap<IntConsumer, Executor> mDisplayChangeListeners = new ArrayMap<>(); - - private final ArrayList<IntConsumer> mTmpDisplayChangeListeners = new ArrayList<>(); + private final DisplayManagerGlobal mDisplayManager; /** Gets the singleton controller. */ @NonNull public static ClientTransactionListenerController getInstance() { synchronized (ClientTransactionListenerController.class) { if (sController == null) { - sController = new ClientTransactionListenerController(); + sController = new ClientTransactionListenerController( + DisplayManagerGlobal.getInstance()); } return sController; } @@ -68,45 +53,13 @@ public class ClientTransactionListenerController { /** Creates a new instance for test only. */ @VisibleForTesting @NonNull - public static ClientTransactionListenerController createInstanceForTesting() { - return new ClientTransactionListenerController(); + public static ClientTransactionListenerController createInstanceForTesting( + @NonNull DisplayManagerGlobal displayManager) { + return new ClientTransactionListenerController(displayManager); } - private ClientTransactionListenerController() {} - - /** - * Registers a new listener for display change. It will be invoked when receives a - * {@link ClientTransaction} that is updating display-related window configuration, such as - * bounds and rotation. - * - * WHen triggered, the listener will be invoked with the logical display id that was changed. - * - * @param listener the listener to invoke when receives a transaction with Display change. - * @param executor the executor on which callback method will be invoked. - */ - public void registerDisplayChangeListener(@NonNull IntConsumer listener, - @NonNull @CallbackExecutor Executor executor) { - if (!isSyncWindowConfigUpdateFlagEnabled()) { - return; - } - requireNonNull(listener); - requireNonNull(executor); - synchronized (mLock) { - mDisplayChangeListeners.put(listener, executor); - } - } - - /** - * Unregisters the listener for display change that was previously registered through - * {@link #registerDisplayChangeListener}. - */ - public void unregisterDisplayChangeListener(@NonNull IntConsumer listener) { - if (!isSyncWindowConfigUpdateFlagEnabled()) { - return; - } - synchronized (mLock) { - mDisplayChangeListeners.remove(listener); - } + private ClientTransactionListenerController(@NonNull DisplayManagerGlobal displayManager) { + mDisplayManager = requireNonNull(displayManager); } /** @@ -117,24 +70,14 @@ public class ClientTransactionListenerController { if (!isSyncWindowConfigUpdateFlagEnabled()) { return; } - synchronized (mLock) { - // Make a copy of the list to avoid listener removal during callback. - mTmpDisplayChangeListeners.addAll(mDisplayChangeListeners.keySet()); - final int num = mTmpDisplayChangeListeners.size(); - try { - for (int i = 0; i < num; i++) { - final IntConsumer listener = mTmpDisplayChangeListeners.get(i); - final Executor executor = mDisplayChangeListeners.get(listener); - executor.execute(() -> listener.accept(displayId)); - } - } finally { - mTmpDisplayChangeListeners.clear(); - } + if (ActivityThread.isSystem()) { + // Not enable for system server. + return; } + mDisplayManager.handleDisplayChangeFromWindowManager(displayId); } /** Whether {@link #syncWindowConfigUpdateFlag} feature flag is enabled. */ - @VisibleForTesting public boolean isSyncWindowConfigUpdateFlagEnabled() { // Can't read flag from isolated process. return !Process.isIsolated() && syncWindowConfigUpdateFlag(); diff --git a/core/java/android/hardware/display/DisplayManagerGlobal.java b/core/java/android/hardware/display/DisplayManagerGlobal.java index 2b5f5ee35a26..b2a281980939 100644 --- a/core/java/android/hardware/display/DisplayManagerGlobal.java +++ b/core/java/android/hardware/display/DisplayManagerGlobal.java @@ -42,7 +42,6 @@ import android.os.Handler; import android.os.HandlerExecutor; import android.os.IBinder; import android.os.Looper; -import android.os.Message; import android.os.RemoteException; import android.os.ServiceManager; import android.os.Trace; @@ -412,6 +411,18 @@ public final class DisplayManagerGlobal { } } + /** + * Called when there is a display-related window configuration change. Reroutes the event from + * WindowManager to make sure the {@link Display} fields are up-to-date in the last callback. + * @param displayId the logical display that was changed. + */ + public void handleDisplayChangeFromWindowManager(int displayId) { + // There can be racing condition between DMS and WMS callbacks, so force triggering the + // listener to make sure the client can get the onDisplayChanged callback even if + // DisplayInfo is not changed (Display read from both DisplayInfo and WindowConfiguration). + handleDisplayEvent(displayId, EVENT_DISPLAY_CHANGED, true /* forceUpdate */); + } + private static Looper getLooperForHandler(@Nullable Handler handler) { Looper looper = handler != null ? handler.getLooper() : Looper.myLooper(); if (looper == null) { @@ -470,7 +481,7 @@ public final class DisplayManagerGlobal { } } - private void handleDisplayEvent(int displayId, @DisplayEvent int event) { + private void handleDisplayEvent(int displayId, @DisplayEvent int event, boolean forceUpdate) { final DisplayInfo info; synchronized (mLock) { if (USE_CACHE) { @@ -501,7 +512,7 @@ public final class DisplayManagerGlobal { // Accepting an Executor means the listener may be synchronously invoked, so we must // not be holding mLock when we do so for (DisplayListenerDelegate listener : mDisplayListeners) { - listener.sendDisplayEvent(displayId, event, info); + listener.sendDisplayEvent(displayId, event, info, forceUpdate); } } @@ -1176,7 +1187,7 @@ public final class DisplayManagerGlobal { Log.d(TAG, "onDisplayEvent: displayId=" + displayId + ", event=" + eventToString( event)); } - handleDisplayEvent(displayId, event); + handleDisplayEvent(displayId, event, false /* forceUpdate */); } } @@ -1197,87 +1208,86 @@ public final class DisplayManagerGlobal { mPackageName = packageName; } - public void sendDisplayEvent(int displayId, @DisplayEvent int event, DisplayInfo info) { + void sendDisplayEvent(int displayId, @DisplayEvent int event, @Nullable DisplayInfo info, + boolean forceUpdate) { if (extraLogging()) { Slog.i(TAG, "Sending Display Event: " + eventToString(event)); } long generationId = mGenerationId.get(); - Message msg = Message.obtain(null, event, displayId, 0, info); mExecutor.execute(() -> { - // If the generation id's don't match we were canceled but still need to recycle() + // If the generation id's don't match we were canceled if (generationId == mGenerationId.get()) { - handleMessage(msg); + handleDisplayEventInner(displayId, event, info, forceUpdate); } - msg.recycle(); }); } - public void clearEvents() { + void clearEvents() { mGenerationId.incrementAndGet(); } - public void setEventsMask(@EventsMask long newEventsMask) { + void setEventsMask(@EventsMask long newEventsMask) { mEventsMask = newEventsMask; } - private void handleMessage(Message msg) { + private void handleDisplayEventInner(int displayId, @DisplayEvent int event, + @Nullable DisplayInfo info, boolean forceUpdate) { if (extraLogging()) { - Slog.i(TAG, "DLD(" + eventToString(msg.what) - + ", display=" + msg.arg1 + Slog.i(TAG, "DLD(" + eventToString(event) + + ", display=" + displayId + ", mEventsMask=" + Long.toBinaryString(mEventsMask) + ", mPackageName=" + mPackageName - + ", msg.obj=" + msg.obj + + ", displayInfo=" + info + ", listener=" + mListener.getClass() + ")"); } if (DEBUG) { Trace.beginSection( TextUtils.trimToSize( - "DLD(" + eventToString(msg.what) - + ", display=" + msg.arg1 + "DLD(" + eventToString(event) + + ", display=" + displayId + ", listener=" + mListener.getClass() + ")", 127)); } - switch (msg.what) { + switch (event) { case EVENT_DISPLAY_ADDED: if ((mEventsMask & DisplayManager.EVENT_FLAG_DISPLAY_ADDED) != 0) { - mListener.onDisplayAdded(msg.arg1); + mListener.onDisplayAdded(displayId); } break; case EVENT_DISPLAY_CHANGED: if ((mEventsMask & DisplayManager.EVENT_FLAG_DISPLAY_CHANGED) != 0) { - DisplayInfo newInfo = (DisplayInfo) msg.obj; - if (newInfo != null && !newInfo.equals(mDisplayInfo)) { + if (info != null && (forceUpdate || !info.equals(mDisplayInfo))) { if (extraLogging()) { Slog.i(TAG, "Sending onDisplayChanged: Display Changed. Info: " - + newInfo); + + info); } - mDisplayInfo.copyFrom(newInfo); - mListener.onDisplayChanged(msg.arg1); + mDisplayInfo.copyFrom(info); + mListener.onDisplayChanged(displayId); } } break; case EVENT_DISPLAY_BRIGHTNESS_CHANGED: if ((mEventsMask & DisplayManager.EVENT_FLAG_DISPLAY_BRIGHTNESS) != 0) { - mListener.onDisplayChanged(msg.arg1); + mListener.onDisplayChanged(displayId); } break; case EVENT_DISPLAY_REMOVED: if ((mEventsMask & DisplayManager.EVENT_FLAG_DISPLAY_REMOVED) != 0) { - mListener.onDisplayRemoved(msg.arg1); + mListener.onDisplayRemoved(displayId); } break; case EVENT_DISPLAY_HDR_SDR_RATIO_CHANGED: if ((mEventsMask & DisplayManager.EVENT_FLAG_HDR_SDR_RATIO_CHANGED) != 0) { - mListener.onDisplayChanged(msg.arg1); + mListener.onDisplayChanged(displayId); } break; case EVENT_DISPLAY_CONNECTED: if ((mEventsMask & DisplayManager.EVENT_FLAG_DISPLAY_CONNECTION_CHANGED) != 0) { - mListener.onDisplayConnected(msg.arg1); + mListener.onDisplayConnected(displayId); } break; case EVENT_DISPLAY_DISCONNECTED: if ((mEventsMask & DisplayManager.EVENT_FLAG_DISPLAY_CONNECTION_CHANGED) != 0) { - mListener.onDisplayDisconnected(msg.arg1); + mListener.onDisplayDisconnected(displayId); } break; } diff --git a/core/tests/coretests/src/android/app/servertransaction/ClientTransactionListenerControllerTest.java b/core/tests/coretests/src/android/app/servertransaction/ClientTransactionListenerControllerTest.java index 0f62b1c23ab4..930b1a4dde26 100644 --- a/core/tests/coretests/src/android/app/servertransaction/ClientTransactionListenerControllerTest.java +++ b/core/tests/coretests/src/android/app/servertransaction/ClientTransactionListenerControllerTest.java @@ -16,14 +16,19 @@ package android.app.servertransaction; -import static org.mockito.ArgumentMatchers.anyInt; -import static org.mockito.Mockito.clearInvocations; +import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation; + import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; +import android.hardware.display.DisplayManager; +import android.hardware.display.DisplayManagerGlobal; +import android.hardware.display.IDisplayManager; +import android.os.Handler; +import android.os.RemoteException; import android.platform.test.annotations.Presubmit; +import android.view.DisplayInfo; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; @@ -34,8 +39,6 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import java.util.function.IntConsumer; - /** * Tests for {@link ClientTransactionListenerController}. * @@ -47,30 +50,36 @@ import java.util.function.IntConsumer; @Presubmit public class ClientTransactionListenerControllerTest { @Mock - private IntConsumer mDisplayChangeListener; + private IDisplayManager mIDisplayManager; + @Mock + private DisplayManager.DisplayListener mListener; + private DisplayManagerGlobal mDisplayManager; + private Handler mHandler; private ClientTransactionListenerController mController; @Before public void setup() { MockitoAnnotations.initMocks(this); - mController = spy(ClientTransactionListenerController.createInstanceForTesting()); + mDisplayManager = new DisplayManagerGlobal(mIDisplayManager); + mHandler = getInstrumentation().getContext().getMainThreadHandler(); + mController = spy(ClientTransactionListenerController.createInstanceForTesting( + mDisplayManager)); doReturn(true).when(mController).isSyncWindowConfigUpdateFlagEnabled(); } @Test - public void testRegisterDisplayChangeListener() { - mController.registerDisplayChangeListener(mDisplayChangeListener, Runnable::run); + public void testOnDisplayChanged() throws RemoteException { + // Mock IDisplayManager to return a display info to trigger display change. + final DisplayInfo newDisplayInfo = new DisplayInfo(); + doReturn(newDisplayInfo).when(mIDisplayManager).getDisplayInfo(123); - mController.onDisplayChanged(123); + mDisplayManager.registerDisplayListener(mListener, mHandler, + DisplayManager.EVENT_FLAG_DISPLAY_CHANGED, null /* packageName */); - verify(mDisplayChangeListener).accept(123); - - clearInvocations(mDisplayChangeListener); - mController.unregisterDisplayChangeListener(mDisplayChangeListener); - - mController.onDisplayChanged(321); + mController.onDisplayChanged(123); + mHandler.runWithScissors(() -> { }, 0); - verify(mDisplayChangeListener, never()).accept(anyInt()); + verify(mListener).onDisplayChanged(123); } } diff --git a/core/tests/coretests/src/android/hardware/display/DisplayManagerGlobalTest.java b/core/tests/coretests/src/android/hardware/display/DisplayManagerGlobalTest.java index c2e6b60cd680..969ae8e819da 100644 --- a/core/tests/coretests/src/android/hardware/display/DisplayManagerGlobalTest.java +++ b/core/tests/coretests/src/android/hardware/display/DisplayManagerGlobalTest.java @@ -16,12 +16,19 @@ package android.hardware.display; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import android.content.Context; import android.os.Handler; import android.os.RemoteException; +import android.platform.test.annotations.Presubmit; +import android.view.DisplayInfo; import androidx.test.InstrumentationRegistry; import androidx.test.filters.SmallTest; @@ -37,6 +44,13 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; +/** + * Tests for {@link DisplayManagerGlobal}. + * + * Build/Install/Run: + * atest FrameworksCoreTests:DisplayManagerGlobalTest + */ +@Presubmit @SmallTest @RunWith(AndroidJUnit4.class) public class DisplayManagerGlobalTest { @@ -51,6 +65,9 @@ public class DisplayManagerGlobalTest { @Mock private DisplayManager.DisplayListener mListener; + @Mock + private DisplayManager.DisplayListener mListener2; + @Captor private ArgumentCaptor<IDisplayManagerCallback> mCallbackCaptor; @@ -82,7 +99,11 @@ public class DisplayManagerGlobalTest { Mockito.verifyNoMoreInteractions(mListener); Mockito.reset(mListener); - callback.onDisplayEvent(1, DisplayManagerGlobal.EVENT_DISPLAY_CHANGED); + // Mock IDisplayManager to return a different display info to trigger display change. + final DisplayInfo newDisplayInfo = new DisplayInfo(); + newDisplayInfo.rotation++; + doReturn(newDisplayInfo).when(mDisplayManager).getDisplayInfo(displayId); + callback.onDisplayEvent(displayId, DisplayManagerGlobal.EVENT_DISPLAY_CHANGED); waitForHandler(); Mockito.verify(mListener).onDisplayChanged(eq(displayId)); Mockito.verifyNoMoreInteractions(mListener); @@ -161,7 +182,44 @@ public class DisplayManagerGlobalTest { mDisplayManagerGlobal.unregisterDisplayListener(mListener); inOrder.verify(mDisplayManager) .registerCallbackWithEventMask(mCallbackCaptor.capture(), eq(0L)); + } + + @Test + public void testHandleDisplayChangeFromWindowManager() throws RemoteException { + // Mock IDisplayManager to return a display info to trigger display change. + final DisplayInfo newDisplayInfo = new DisplayInfo(); + doReturn(newDisplayInfo).when(mDisplayManager).getDisplayInfo(123); + doReturn(newDisplayInfo).when(mDisplayManager).getDisplayInfo(321); + + // Nothing happens when there is no listener. + mDisplayManagerGlobal.handleDisplayChangeFromWindowManager(123); + + // One listener listens on add/remove, and the other one listens on change. + mDisplayManagerGlobal.registerDisplayListener(mListener, mHandler, + DisplayManager.EVENT_FLAG_DISPLAY_ADDED + | DisplayManager.EVENT_FLAG_DISPLAY_REMOVED, null /* packageName */); + mDisplayManagerGlobal.registerDisplayListener(mListener2, mHandler, + DisplayManager.EVENT_FLAG_DISPLAY_CHANGED, null /* packageName */); + + mDisplayManagerGlobal.handleDisplayChangeFromWindowManager(321); + waitForHandler(); + + verify(mListener, never()).onDisplayChanged(anyInt()); + verify(mListener2).onDisplayChanged(321); + + // Trigger the callback again even if the display info is not changed. + clearInvocations(mListener2); + mDisplayManagerGlobal.handleDisplayChangeFromWindowManager(321); + waitForHandler(); + + verify(mListener2).onDisplayChanged(321); + + // No callback for non-existing display (no display info returned from IDisplayManager). + clearInvocations(mListener2); + mDisplayManagerGlobal.handleDisplayChangeFromWindowManager(456); + waitForHandler(); + verify(mListener2, never()).onDisplayChanged(anyInt()); } private void waitForHandler() { |