From 427b32900eeb30be6a665087d22d267416d64bd7 Mon Sep 17 00:00:00 2001 From: Marin Shalamanov Date: Mon, 8 Mar 2021 17:45:48 +0100 Subject: [PATCH 1/2] Display API: Do not expose internal locks Currently the display class synchronizes on this, which is against the API guidelines. Bug: 171240622 Test: presubmit Change-Id: I5706cb38803c9868cc4bae0803aa3fd0c5991bde --- core/java/android/view/Display.java | 69 +++++++++++++++-------------- 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/core/java/android/view/Display.java b/core/java/android/view/Display.java index b98ef1410ebf4..d11136ec98222 100644 --- a/core/java/android/view/Display.java +++ b/core/java/android/view/Display.java @@ -84,6 +84,7 @@ public final class Display { private static final String TAG = "Display"; private static final boolean DEBUG = false; + private final Object mLock = new Object(); private final DisplayManagerGlobal mGlobal; private final int mDisplayId; private final int mFlags; @@ -569,7 +570,7 @@ public final class Display { * @return True if the display is still valid. */ public boolean isValid() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mIsValid; } @@ -584,7 +585,7 @@ public final class Display { */ @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P) public boolean getDisplayInfo(DisplayInfo outDisplayInfo) { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); outDisplayInfo.copyFrom(mDisplayInfo); return mIsValid; @@ -601,7 +602,7 @@ public final class Display { * @hide */ public int getLayerStack() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mDisplayInfo.layerStack; } @@ -648,7 +649,7 @@ public final class Display { * @hide */ public DisplayAddress getAddress() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mDisplayInfo.address; } @@ -708,7 +709,7 @@ public final class Display { * @return The display's name. */ public String getName() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mDisplayInfo.name; } @@ -721,7 +722,7 @@ public final class Display { * @hide */ public float getBrightnessDefault() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mDisplayInfo.brightnessDefault; } @@ -760,7 +761,7 @@ public final class Display { */ @Deprecated public void getSize(Point outSize) { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); mDisplayInfo.getAppMetrics(mTempMetrics, getDisplayAdjustments()); outSize.x = mTempMetrics.widthPixels; @@ -777,7 +778,7 @@ public final class Display { */ @Deprecated public void getRectSize(Rect outSize) { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); mDisplayInfo.getAppMetrics(mTempMetrics, getDisplayAdjustments()); outSize.set(0, 0, mTempMetrics.widthPixels, mTempMetrics.heightPixels); @@ -815,7 +816,7 @@ public final class Display { * for example, screen decorations like the status bar are being hidden. */ public void getCurrentSizeRange(Point outSmallestSize, Point outLargestSize) { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); outSmallestSize.x = mDisplayInfo.smallestNominalAppWidth; outSmallestSize.y = mDisplayInfo.smallestNominalAppHeight; @@ -831,7 +832,7 @@ public final class Display { */ @UnsupportedAppUsage public int getMaximumSizeDimension() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return Math.max(mDisplayInfo.logicalWidth, mDisplayInfo.logicalHeight); } @@ -842,7 +843,7 @@ public final class Display { */ @Deprecated public int getWidth() { - synchronized (this) { + synchronized (mLock) { updateCachedAppSizeIfNeededLocked(); return mCachedAppWidthCompat; } @@ -853,7 +854,7 @@ public final class Display { */ @Deprecated public int getHeight() { - synchronized (this) { + synchronized (mLock) { updateCachedAppSizeIfNeededLocked(); return mCachedAppHeightCompat; } @@ -878,7 +879,7 @@ public final class Display { */ @Surface.Rotation public int getRotation() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mMayAdjustByFixedRotation ? getDisplayAdjustments().getRotation(mDisplayInfo.rotation) @@ -904,7 +905,7 @@ public final class Display { */ @Nullable public DisplayCutout getCutout() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mMayAdjustByFixedRotation ? getDisplayAdjustments().getDisplayCutout(mDisplayInfo.displayCutout) @@ -922,7 +923,7 @@ public final class Display { @SuppressLint("VisiblySynchronized") @Nullable public RoundedCorner getRoundedCorner(@RoundedCorner.Position int position) { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); RoundedCorners roundedCorners; if (mMayAdjustByFixedRotation) { @@ -954,7 +955,7 @@ public final class Display { * Gets the refresh rate of this display in frames per second. */ public float getRefreshRate() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mDisplayInfo.getRefreshRate(); } @@ -970,7 +971,7 @@ public final class Display { */ @Deprecated public float[] getSupportedRefreshRates() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mDisplayInfo.getDefaultRefreshRates(); } @@ -980,7 +981,7 @@ public final class Display { * Returns the active mode of the display. */ public Mode getMode() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mDisplayInfo.getMode(); } @@ -990,7 +991,7 @@ public final class Display { * Gets the supported modes of this display. */ public Mode[] getSupportedModes() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); final Display.Mode[] modes = mDisplayInfo.supportedModes; return Arrays.copyOf(modes, modes.length); @@ -1016,7 +1017,7 @@ public final class Display { */ @SuppressLint("VisiblySynchronized") public boolean isMinimalPostProcessingSupported() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mDisplayInfo.minimalPostProcessingSupported; } @@ -1036,7 +1037,7 @@ public final class Display { * @hide */ public int getColorMode() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mDisplayInfo.colorMode; } @@ -1063,7 +1064,7 @@ public final class Display { * @see #isHdr() */ public HdrCapabilities getHdrCapabilities() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mDisplayInfo.hdrCapabilities; } @@ -1076,7 +1077,7 @@ public final class Display { * @see HdrCapabilities#getSupportedHdrTypes() */ public boolean isHdr() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mDisplayInfo.isHdr(); } @@ -1089,7 +1090,7 @@ public final class Display { * {@link Configuration#isScreenWideColorGamut()}. */ public boolean isWideColorGamut() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mDisplayInfo.isWideColorGamut(); } @@ -1104,7 +1105,7 @@ public final class Display { */ @Nullable public ColorSpace getPreferredWideGamutColorSpace() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); if (mDisplayInfo.isWideColorGamut()) { return mGlobal.getPreferredWideGamutColorSpace(); @@ -1118,7 +1119,7 @@ public final class Display { * @hide */ public int[] getSupportedColorModes() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); int[] colorModes = mDisplayInfo.supportedColorModes; return Arrays.copyOf(colorModes, colorModes.length); @@ -1135,7 +1136,7 @@ public final class Display { @NonNull @TestApi public @ColorMode ColorSpace[] getSupportedWideColorGamut() { - synchronized (this) { + synchronized (mLock) { final ColorSpace[] defaultColorSpaces = new ColorSpace[0]; updateDisplayInfoLocked(); if (!isWideColorGamut()) { @@ -1169,7 +1170,7 @@ public final class Display { * A/V synchronization. */ public long getAppVsyncOffsetNanos() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mDisplayInfo.appVsyncOffsetNanos; } @@ -1187,7 +1188,7 @@ public final class Display { * ({@link System#nanoTime}). */ public long getPresentationDeadlineNanos() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mDisplayInfo.presentationDeadlineNanos; } @@ -1235,7 +1236,7 @@ public final class Display { */ @Deprecated public void getMetrics(DisplayMetrics outMetrics) { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); mDisplayInfo.getAppMetrics(outMetrics, getDisplayAdjustments()); } @@ -1288,7 +1289,7 @@ public final class Display { */ @Deprecated public void getRealSize(Point outSize) { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); if (shouldReportMaxBounds()) { final Rect bounds = mResources.getConfiguration() @@ -1358,7 +1359,7 @@ public final class Display { */ @Deprecated public void getRealMetrics(DisplayMetrics outMetrics) { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); if (shouldReportMaxBounds()) { mDisplayInfo.getMaxBoundsMetrics(outMetrics, @@ -1434,7 +1435,7 @@ public final class Display { * {@link #STATE_UNKNOWN}. */ public int getState() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); return mIsValid ? mDisplayInfo.state : STATE_UNKNOWN; } @@ -1518,7 +1519,7 @@ public final class Display { // For debugging purposes @Override public String toString() { - synchronized (this) { + synchronized (mLock) { updateDisplayInfoLocked(); final DisplayAdjustments adjustments = getDisplayAdjustments(); mDisplayInfo.getAppMetrics(mTempMetrics, adjustments); From 254351a6a784cf9deca8e89fed70287073efb163 Mon Sep 17 00:00:00 2001 From: Marin Shalamanov Date: Mon, 8 Mar 2021 19:27:53 +0100 Subject: [PATCH 2/2] Update device info on Display.getDeviceProductInfo() Each getter of the Display class should first call updateDisplayInfoLocked(). Test: presubmit Bug: 179775994 Change-Id: I4df9945cd030296da0e170411af8c66c89f57adf --- core/java/android/view/Display.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/java/android/view/Display.java b/core/java/android/view/Display.java index d11136ec98222..d484f4d47e990 100644 --- a/core/java/android/view/Display.java +++ b/core/java/android/view/Display.java @@ -1203,7 +1203,10 @@ public final class Display { */ @Nullable public DeviceProductInfo getDeviceProductInfo() { - return mDisplayInfo.deviceProductInfo; + synchronized (mLock) { + updateDisplayInfoLocked(); + return mDisplayInfo.deviceProductInfo; + } } /**