From 5d0cbb03960541d943202b2ce60ada92cb2791b1 Mon Sep 17 00:00:00 2001 From: shawnlin Date: Wed, 29 Jul 2020 17:01:45 +0800 Subject: [PATCH] Update WM display info when nonOverrideDisplayInfo changes Fixes an issue where a changed nonOverrideDisplayInfo would not be propagated to window manager. This causes an issue where if we change cutout shapes, the non override display info changes, but window manager is never informed about this. It happens to work most times, because other things trigger a refresh of the non override display info (e.g. freezing the screen or going to an app that overrides orientation). Bug: 113090662 Test: Switch between cutout emulation modes a few times. Verify that the cutout change is always applied. Change-Id: I46989e658929877775753ee453a270fa22193f11 --- .../server/display/DisplayManagerService.java | 21 +++- .../display/DisplayManagerServiceTest.java | 106 ++++++++++++++++++ 2 files changed, 123 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/display/DisplayManagerService.java b/services/core/java/com/android/server/display/DisplayManagerService.java index 2b13bcddb8f10..4a12ee71adbe1 100644 --- a/services/core/java/com/android/server/display/DisplayManagerService.java +++ b/services/core/java/com/android/server/display/DisplayManagerService.java @@ -284,6 +284,7 @@ public final class DisplayManagerService extends SystemService { // Temporary display info, used for comparing display configurations. private final DisplayInfo mTempDisplayInfo = new DisplayInfo(); + private final DisplayInfo mTempNonOverrideDisplayInfo = new DisplayInfo(); // Temporary viewports, used when sending new viewport information to the // input system. May be used outside of the lock but only on the handler thread. @@ -508,7 +509,8 @@ public final class DisplayManagerService extends SystemService { mDisplayTransactionListeners.remove(listener); } - private void setDisplayInfoOverrideFromWindowManagerInternal( + @VisibleForTesting + void setDisplayInfoOverrideFromWindowManagerInternal( int displayId, DisplayInfo info) { synchronized (mSyncRoot) { LogicalDisplay display = mLogicalDisplays.get(displayId); @@ -936,7 +938,8 @@ public final class DisplayManagerService extends SystemService { adapter.registerLocked(); } - private void handleDisplayDeviceAdded(DisplayDevice device) { + @VisibleForTesting + void handleDisplayDeviceAdded(DisplayDevice device) { synchronized (mSyncRoot) { handleDisplayDeviceAddedLocked(device); } @@ -948,7 +951,6 @@ public final class DisplayManagerService extends SystemService { Slog.w(TAG, "Attempted to add already added display device: " + info); return; } - Slog.i(TAG, "Display device added: " + info); device.mDebugLastLoggedDeviceInfo = info; @@ -961,7 +963,8 @@ public final class DisplayManagerService extends SystemService { scheduleTraversalLocked(false); } - private void handleDisplayDeviceChanged(DisplayDevice device) { + @VisibleForTesting + void handleDisplayDeviceChanged(DisplayDevice device) { synchronized (mSyncRoot) { DisplayDeviceInfo info = device.getDisplayDeviceInfoLocked(); if (!mDisplayDevices.contains(device)) { @@ -1235,6 +1238,7 @@ public final class DisplayManagerService extends SystemService { LogicalDisplay display = mLogicalDisplays.valueAt(i); mTempDisplayInfo.copyFrom(display.getDisplayInfoLocked()); + display.getNonOverrideDisplayInfoLocked(mTempNonOverrideDisplayInfo); display.updateLocked(mDisplayDevices); if (!display.isValidLocked()) { mLogicalDisplays.removeAt(i); @@ -1243,6 +1247,15 @@ public final class DisplayManagerService extends SystemService { } else if (!mTempDisplayInfo.equals(display.getDisplayInfoLocked())) { handleLogicalDisplayChanged(displayId, display); changed = true; + } else { + // While applications shouldn't know nor care about the non-overridden info, we + // still need to let WindowManager know so it can update its own internal state for + // things like display cutouts. + display.getNonOverrideDisplayInfoLocked(mTempDisplayInfo); + if (!mTempNonOverrideDisplayInfo.equals(mTempDisplayInfo)) { + handleLogicalDisplayChanged(displayId, display); + changed = true; + } } } return changed; diff --git a/services/tests/servicestests/src/com/android/server/display/DisplayManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/display/DisplayManagerServiceTest.java index b1f3871274ace..73dda0736d2f0 100644 --- a/services/tests/servicestests/src/com/android/server/display/DisplayManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/display/DisplayManagerServiceTest.java @@ -19,6 +19,7 @@ package com.android.server.display; import static com.android.server.display.VirtualDisplayAdapter.UNIQUE_ID_PREFIX; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -28,18 +29,23 @@ import static org.mockito.Mockito.when; import android.app.PropertyInvalidatedCache; import android.content.Context; +import android.graphics.Insets; +import android.graphics.Rect; import android.hardware.display.BrightnessConfiguration; import android.hardware.display.Curve; import android.hardware.display.DisplayManager; +import android.hardware.display.DisplayManagerGlobal; import android.hardware.display.DisplayViewport; import android.hardware.display.DisplayedContentSample; import android.hardware.display.DisplayedContentSamplingAttributes; +import android.hardware.display.IDisplayManagerCallback; import android.hardware.display.IVirtualDisplayCallback; import android.hardware.display.VirtualDisplayConfig; import android.hardware.input.InputManagerInternal; import android.os.Handler; import android.os.IBinder; import android.view.Display; +import android.view.DisplayCutout; import android.view.DisplayInfo; import android.view.Surface; import android.view.SurfaceControl; @@ -281,6 +287,68 @@ public class DisplayManagerServiceTest { displayManager.onBootPhase(SystemService.PHASE_WAIT_FOR_DEFAULT_DISPLAY); } + /** + * Tests that there should be a display change notification to WindowManager to update its own + * internal state for things like display cutout when nonOverrideDisplayInfo is changed. + */ + @Test + public void testShouldNotifyChangeWhenNonOverrideDisplayInfoChanged() throws Exception { + DisplayManagerService displayManager = + new DisplayManagerService(mContext, mShortMockedInjector); + registerDefaultDisplays(displayManager); + displayManager.onBootPhase(SystemService.PHASE_WAIT_FOR_DEFAULT_DISPLAY); + + // Add the FakeDisplayDevice + FakeDisplayDevice displayDevice = new FakeDisplayDevice(); + DisplayDeviceInfo displayDeviceInfo = new DisplayDeviceInfo(); + displayDeviceInfo.width = 100; + displayDeviceInfo.height = 200; + final Rect zeroRect = new Rect(); + displayDeviceInfo.displayCutout = new DisplayCutout( + Insets.of(0, 10, 0, 0), + zeroRect, new Rect(0, 0, 10, 10), zeroRect, zeroRect); + displayDeviceInfo.flags = DisplayDeviceInfo.FLAG_DEFAULT_DISPLAY; + displayDevice.setDisplayDeviceInfo(displayDeviceInfo); + displayManager.handleDisplayDeviceAdded(displayDevice); + + // Find the display id of the added FakeDisplayDevice + DisplayManagerService.BinderService bs = displayManager.new BinderService(); + final int[] displayIds = bs.getDisplayIds(); + assertTrue(displayIds.length > 0); + int displayId = Display.INVALID_DISPLAY; + for (int i = 0; i < displayIds.length; i++) { + DisplayDeviceInfo ddi = displayManager.getDisplayDeviceInfoInternal(displayIds[i]); + if (displayDeviceInfo.equals(ddi)) { + displayId = displayIds[i]; + break; + } + } + assertFalse(displayId == Display.INVALID_DISPLAY); + + // Setup override DisplayInfo + DisplayInfo overrideInfo = bs.getDisplayInfo(displayId); + displayManager.setDisplayInfoOverrideFromWindowManagerInternal(displayId, overrideInfo); + + Handler handler = displayManager.getDisplayHandler(); + handler.runWithScissors(() -> { + }, 0 /* now */); + + // register display listener callback + FakeDisplayManagerCallback callback = new FakeDisplayManagerCallback(displayId); + bs.registerCallback(callback); + + // Simulate DisplayDevice change + DisplayDeviceInfo displayDeviceInfo2 = new DisplayDeviceInfo(); + displayDeviceInfo2.copyFrom(displayDeviceInfo); + displayDeviceInfo2.displayCutout = null; + displayDevice.setDisplayDeviceInfo(displayDeviceInfo2); + displayManager.handleDisplayDeviceChanged(displayDevice); + + handler.runWithScissors(() -> { + }, 0 /* now */); + assertTrue(callback.mCalled); + } + /** * Tests that we get a Runtime exception when we cannot initialize the default display. */ @@ -512,4 +580,42 @@ public class DisplayManagerServiceTest { // flush the handler handler.runWithScissors(() -> {}, 0 /* now */); } + + private class FakeDisplayManagerCallback extends IDisplayManagerCallback.Stub { + int mDisplayId; + boolean mCalled = false; + + FakeDisplayManagerCallback(int displayId) { + mDisplayId = displayId; + } + + @Override + public void onDisplayEvent(int displayId, int event) { + if (displayId == mDisplayId && event == DisplayManagerGlobal.EVENT_DISPLAY_CHANGED) { + mCalled = true; + } + } + } + + private class FakeDisplayDevice extends DisplayDevice { + private DisplayDeviceInfo mDisplayDeviceInfo; + + FakeDisplayDevice() { + super(null, null, ""); + } + + public void setDisplayDeviceInfo(DisplayDeviceInfo displayDeviceInfo) { + mDisplayDeviceInfo = displayDeviceInfo; + } + + @Override + public boolean hasStableUniqueId() { + return false; + } + + @Override + public DisplayDeviceInfo getDisplayDeviceInfoLocked() { + return mDisplayDeviceInfo; + } + } }