From fee0b82175dc387c372f527876c212ff72f1fee5 Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Tue, 17 Dec 2019 23:56:44 -0800 Subject: [PATCH] Add SurfaceControl.Transaction lock debugging utility. Add a subclass of SurfaceControl.Transaction which can be given a monitor object at construction. If the monitor is not held when other methods are called the method will throw an exception. This should help us debug those hard to find unlocked Transaction access issues that frequently show up. I think this has been common enough that it's worse keeping a utility around in trunk. Bug: 146473344 Test: Builds Change-Id: I3617181745b452fbf9a575fd8e55760df900dd6f --- core/java/android/view/SurfaceControl.java | 88 +++++++++++++++------- 1 file changed, 59 insertions(+), 29 deletions(-) diff --git a/core/java/android/view/SurfaceControl.java b/core/java/android/view/SurfaceControl.java index 87628da425993..f84daaaa98a08 100644 --- a/core/java/android/view/SurfaceControl.java +++ b/core/java/android/view/SurfaceControl.java @@ -2091,6 +2091,13 @@ public final class SurfaceControl implements Parcelable { private final ArrayMap mResizedSurfaces = new ArrayMap<>(); Runnable mFreeNativeResources; + /** + * @hide + */ + protected void checkPreconditions(SurfaceControl sc) { + sc.checkNotReleased(); + } + /** * Open a new transaction object. The transaction may be filed with commands to * manipulate {@link SurfaceControl} instances, and then applied atomically with @@ -2155,7 +2162,7 @@ public final class SurfaceControl implements Parcelable { */ @NonNull public Transaction setVisibility(@NonNull SurfaceControl sc, boolean visible) { - sc.checkNotReleased(); + checkPreconditions(sc); if (visible) { return show(sc); } else { @@ -2172,7 +2179,7 @@ public final class SurfaceControl implements Parcelable { */ @UnsupportedAppUsage public Transaction show(SurfaceControl sc) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetFlags(mNativeObject, sc.mNativeObject, 0, SURFACE_HIDDEN); return this; } @@ -2186,7 +2193,7 @@ public final class SurfaceControl implements Parcelable { */ @UnsupportedAppUsage public Transaction hide(SurfaceControl sc) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetFlags(mNativeObject, sc.mNativeObject, SURFACE_HIDDEN, SURFACE_HIDDEN); return this; } @@ -2196,7 +2203,7 @@ public final class SurfaceControl implements Parcelable { */ @UnsupportedAppUsage public Transaction setPosition(SurfaceControl sc, float x, float y) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetPosition(mNativeObject, sc.mNativeObject, x, y); return this; } @@ -2213,7 +2220,7 @@ public final class SurfaceControl implements Parcelable { @NonNull public Transaction setBufferSize(@NonNull SurfaceControl sc, @IntRange(from = 0) int w, @IntRange(from = 0) int h) { - sc.checkNotReleased(); + checkPreconditions(sc); mResizedSurfaces.put(sc, new Point(w, h)); nativeSetSize(mNativeObject, sc.mNativeObject, w, h); return this; @@ -2231,7 +2238,7 @@ public final class SurfaceControl implements Parcelable { @NonNull public Transaction setLayer(@NonNull SurfaceControl sc, @IntRange(from = Integer.MIN_VALUE, to = Integer.MAX_VALUE) int z) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetLayer(mNativeObject, sc.mNativeObject, z); return this; } @@ -2240,7 +2247,7 @@ public final class SurfaceControl implements Parcelable { * @hide */ public Transaction setRelativeLayer(SurfaceControl sc, SurfaceControl relativeTo, int z) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetRelativeLayer(mNativeObject, sc.mNativeObject, relativeTo.mNativeObject, z); return this; } @@ -2249,7 +2256,7 @@ public final class SurfaceControl implements Parcelable { * @hide */ public Transaction setTransparentRegionHint(SurfaceControl sc, Region transparentRegion) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetTransparentRegionHint(mNativeObject, sc.mNativeObject, transparentRegion); return this; @@ -2265,7 +2272,7 @@ public final class SurfaceControl implements Parcelable { @NonNull public Transaction setAlpha(@NonNull SurfaceControl sc, @FloatRange(from = 0.0, to = 1.0) float alpha) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetAlpha(mNativeObject, sc.mNativeObject, alpha); return this; } @@ -2274,7 +2281,7 @@ public final class SurfaceControl implements Parcelable { * @hide */ public Transaction setInputWindowInfo(SurfaceControl sc, InputWindowHandle handle) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetInputWindowInfo(mNativeObject, sc.mNativeObject, handle); return this; } @@ -2304,7 +2311,7 @@ public final class SurfaceControl implements Parcelable { @NonNull public Transaction setGeometry(@NonNull SurfaceControl sc, @Nullable Rect sourceCrop, @Nullable Rect destFrame, @Surface.Rotation int orientation) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetGeometry(mNativeObject, sc.mNativeObject, sourceCrop, destFrame, orientation); return this; } @@ -2315,7 +2322,7 @@ public final class SurfaceControl implements Parcelable { @UnsupportedAppUsage public Transaction setMatrix(SurfaceControl sc, float dsdx, float dtdx, float dtdy, float dsdy) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetMatrix(mNativeObject, sc.mNativeObject, dsdx, dtdx, dtdy, dsdy); return this; @@ -2349,7 +2356,7 @@ public final class SurfaceControl implements Parcelable { */ public Transaction setColorTransform(SurfaceControl sc, @Size(9) float[] matrix, @Size(3) float[] translation) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetColorTransform(mNativeObject, sc.mNativeObject, matrix, translation); return this; } @@ -2361,7 +2368,7 @@ public final class SurfaceControl implements Parcelable { * @hide */ public Transaction setColorSpaceAgnostic(SurfaceControl sc, boolean agnostic) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetColorSpaceAgnostic(mNativeObject, sc.mNativeObject, agnostic); return this; } @@ -2378,7 +2385,7 @@ public final class SurfaceControl implements Parcelable { */ @UnsupportedAppUsage public Transaction setWindowCrop(SurfaceControl sc, Rect crop) { - sc.checkNotReleased(); + checkPreconditions(sc); if (crop != null) { nativeSetWindowCrop(mNativeObject, sc.mNativeObject, crop.left, crop.top, crop.right, crop.bottom); @@ -2399,7 +2406,7 @@ public final class SurfaceControl implements Parcelable { * @hide */ public Transaction setWindowCrop(SurfaceControl sc, int width, int height) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetWindowCrop(mNativeObject, sc.mNativeObject, 0, 0, width, height); return this; } @@ -2413,7 +2420,7 @@ public final class SurfaceControl implements Parcelable { */ @UnsupportedAppUsage public Transaction setCornerRadius(SurfaceControl sc, float cornerRadius) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetCornerRadius(mNativeObject, sc.mNativeObject, cornerRadius); return this; @@ -2424,7 +2431,7 @@ public final class SurfaceControl implements Parcelable { */ @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.O) public Transaction setLayerStack(SurfaceControl sc, int layerStack) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetLayerStack(mNativeObject, sc.mNativeObject, layerStack); return this; } @@ -2438,7 +2445,7 @@ public final class SurfaceControl implements Parcelable { if (frameNumber < 0) { return this; } - sc.checkNotReleased(); + checkPreconditions(sc); nativeDeferTransactionUntil(mNativeObject, sc.mNativeObject, barrier.mNativeObject, frameNumber); return this; @@ -2453,7 +2460,7 @@ public final class SurfaceControl implements Parcelable { if (frameNumber < 0) { return this; } - sc.checkNotReleased(); + checkPreconditions(sc); nativeDeferTransactionUntilSurface(mNativeObject, sc.mNativeObject, barrierSurface.mNativeObject, frameNumber); return this; @@ -2463,7 +2470,7 @@ public final class SurfaceControl implements Parcelable { * @hide */ public Transaction reparentChildren(SurfaceControl sc, SurfaceControl newParent) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeReparentChildren(mNativeObject, sc.mNativeObject, newParent.mNativeObject); return this; } @@ -2480,7 +2487,7 @@ public final class SurfaceControl implements Parcelable { @NonNull public Transaction reparent(@NonNull SurfaceControl sc, @Nullable SurfaceControl newParent) { - sc.checkNotReleased(); + checkPreconditions(sc); long otherObject = 0; if (newParent != null) { newParent.checkNotReleased(); @@ -2494,7 +2501,7 @@ public final class SurfaceControl implements Parcelable { * @hide */ public Transaction detachChildren(SurfaceControl sc) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSeverChildren(mNativeObject, sc.mNativeObject); return this; } @@ -2503,7 +2510,7 @@ public final class SurfaceControl implements Parcelable { * @hide */ public Transaction setOverrideScalingMode(SurfaceControl sc, int overrideScalingMode) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetOverrideScalingMode(mNativeObject, sc.mNativeObject, overrideScalingMode); return this; @@ -2516,7 +2523,7 @@ public final class SurfaceControl implements Parcelable { */ @UnsupportedAppUsage public Transaction setColor(SurfaceControl sc, @Size(3) float[] color) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetColor(mNativeObject, sc.mNativeObject, color); return this; } @@ -2527,7 +2534,7 @@ public final class SurfaceControl implements Parcelable { * @hide */ public Transaction setSecure(SurfaceControl sc, boolean isSecure) { - sc.checkNotReleased(); + checkPreconditions(sc); if (isSecure) { nativeSetFlags(mNativeObject, sc.mNativeObject, SECURE, SECURE); } else { @@ -2542,7 +2549,7 @@ public final class SurfaceControl implements Parcelable { * @hide */ public Transaction setOpaque(SurfaceControl sc, boolean isOpaque) { - sc.checkNotReleased(); + checkPreconditions(sc); if (isOpaque) { nativeSetFlags(mNativeObject, sc.mNativeObject, SURFACE_OPAQUE, SURFACE_OPAQUE); } else { @@ -2657,7 +2664,7 @@ public final class SurfaceControl implements Parcelable { * @hide */ public Transaction setMetadata(SurfaceControl sc, int key, Parcel data) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetMetadata(mNativeObject, sc.mNativeObject, key, data); return this; } @@ -2680,7 +2687,7 @@ public final class SurfaceControl implements Parcelable { * @hide */ public Transaction setShadowRadius(SurfaceControl sc, float shadowRadius) { - sc.checkNotReleased(); + checkPreconditions(sc); nativeSetShadowRadius(mNativeObject, sc.mNativeObject, shadowRadius); return this; } @@ -2763,4 +2770,27 @@ public final class SurfaceControl implements Parcelable { } }; } + + /** + * A debugging utility subclass of SurfaceControl.Transaction. At construction + * you can pass in a monitor object, and all the other methods will throw an exception + * if the monitor is not held when they are called. + * @hide + */ + public static class LockDebuggingTransaction extends SurfaceControl.Transaction { + Object mMonitor; + + public LockDebuggingTransaction(Object o) { + mMonitor = o; + } + + @Override + protected void checkPreconditions(SurfaceControl sc) { + super.checkPreconditions(sc); + if (!Thread.holdsLock(mMonitor)) { + throw new RuntimeException( + "Unlocked access to synchronized SurfaceControl.Transaction"); + } + } + } }