Merge changes I7167e29d,I1b48c892,Ic6b27d38

* changes: Use NativeAllocationRegistry for DisplayEventReceiver Use camel case to name the tests Rearrange SurfaceHolder callback methods Update the Attached Choreographer test to use divisor for refresh rate
2023-02-03 20:22:03 +00:00
parent aeeff9a2d1 1585f19982
commit ec9f456321
4 changed files with 62 additions and 82 deletions
--- a/core/java/android/view/DisplayEventReceiver.java
+++ b/core/java/android/view/DisplayEventReceiver.java
@@ -27,6 +27,8 @@ import com.android.internal.annotations.VisibleForTesting;

 import dalvik.annotation.optimization.FastNative;

+import libcore.util.NativeAllocationRegistry;
+
 import java.lang.ref.WeakReference;

 /**
@@ -81,11 +83,17 @@ public abstract class DisplayEventReceiver {

    private static native long nativeInit(WeakReference<DisplayEventReceiver> receiver,
            MessageQueue messageQueue, int vsyncSource, int eventRegistration, long layerHandle);
-    private static native void nativeDispose(long receiverPtr);
+    private static native long nativeGetDisplayEventReceiverFinalizer();
    @FastNative
    private static native void nativeScheduleVsync(long receiverPtr);
    private static native VsyncEventData nativeGetLatestVsyncEventData(long receiverPtr);

+    private static final NativeAllocationRegistry sNativeAllocationRegistry =
+            NativeAllocationRegistry.createMalloced(
+                    DisplayEventReceiver.class.getClassLoader(),
+                    nativeGetDisplayEventReceiverFinalizer());
+    private Runnable mFreeNativeResources;
+
    /**
     * Creates a display event receiver.
     *
@@ -118,27 +126,16 @@ public abstract class DisplayEventReceiver {
        mMessageQueue = looper.getQueue();
        mReceiverPtr = nativeInit(new WeakReference<DisplayEventReceiver>(this), mMessageQueue,
                vsyncSource, eventRegistration, layerHandle);
-    }
-
-    @Override
-    protected void finalize() throws Throwable {
-        try {
-            dispose(true);
-        } finally {
-            super.finalize();
-        }
+        mFreeNativeResources = sNativeAllocationRegistry.registerNativeAllocation(this,
+                mReceiverPtr);
    }

    /**
     * Disposes the receiver.
     */
    public void dispose() {
-        dispose(false);
-    }
-
-    private void dispose(boolean finalized) {
        if (mReceiverPtr != 0) {
-            nativeDispose(mReceiverPtr);
+            mFreeNativeResources.run();
            mReceiverPtr = 0;
        }
        mMessageQueue = null;
--- a/core/java/android/view/SurfaceControl.java
+++ b/core/java/android/view/SurfaceControl.java
@@ -1386,7 +1386,6 @@ public final class SurfaceControl implements Parcelable {
            synchronized (mChoreographerLock) {
                if (mChoreographer != null) {
                    mChoreographer.invalidate();
-                    // TODO(b/266121235): Use NativeAllocationRegistry to clean up Choreographer.
                    mChoreographer = null;
                }
            }
--- a/core/jni/android_view_DisplayEventReceiver.cpp
+++ b/core/jni/android_view_DisplayEventReceiver.cpp
@@ -240,13 +240,15 @@ static jlong nativeInit(JNIEnv* env, jclass clazz, jobject receiverWeak, jobject
    return reinterpret_cast<jlong>(receiver.get());
 }

-static void nativeDispose(JNIEnv* env, jclass clazz, jlong receiverPtr) {
-    NativeDisplayEventReceiver* receiver =
-            reinterpret_cast<NativeDisplayEventReceiver*>(receiverPtr);
+static void release(NativeDisplayEventReceiver* receiver) {
    receiver->dispose();
    receiver->decStrong(gDisplayEventReceiverClassInfo.clazz); // drop reference held by the object
 }

+static jlong nativeGetDisplayEventReceiverFinalizer(JNIEnv*, jclass) {
+    return static_cast<jlong>(reinterpret_cast<uintptr_t>(&release));
+}
+
 static void nativeScheduleVsync(JNIEnv* env, jclass clazz, jlong receiverPtr) {
    sp<NativeDisplayEventReceiver> receiver =
            reinterpret_cast<NativeDisplayEventReceiver*>(receiverPtr);
@@ -274,7 +276,8 @@ static const JNINativeMethod gMethods[] = {
        /* name, signature, funcPtr */
        {"nativeInit", "(Ljava/lang/ref/WeakReference;Landroid/os/MessageQueue;IIJ)J",
         (void*)nativeInit},
-        {"nativeDispose", "(J)V", (void*)nativeDispose},
+        {"nativeGetDisplayEventReceiverFinalizer", "()J",
+         (void*)nativeGetDisplayEventReceiverFinalizer},
        // @FastNative
        {"nativeScheduleVsync", "(J)V", (void*)nativeScheduleVsync},
        {"nativeGetLatestVsyncEventData", "(J)Landroid/view/DisplayEventReceiver$VsyncEventData;",
--- a/tests/ChoreographerTests/src/main/java/android/view/choreographertests/AttachedChoreographerTest.java
+++ b/tests/ChoreographerTests/src/main/java/android/view/choreographertests/AttachedChoreographerTest.java
@@ -50,16 +50,13 @@ import java.util.concurrent.TimeUnit;
 public class AttachedChoreographerTest {
    private static final String TAG = "AttachedChoreographerTest";
    private static final long DISPLAY_MODE_RETURNS_PHYSICAL_REFRESH_RATE_CHANGEID = 170503758;
-    private static final int THRESHOLD_MS = 10;
-    private static final int CALLBACK_TIME_10_FPS = 100;
-    private static final int CALLBACK_TIME_30_FPS = 33;
+    private static final long THRESHOLD_MS = 10;
    private static final int FRAME_ITERATIONS = 21;
    private static final int CALLBACK_MISSED_THRESHOLD = 2;

    private final CountDownLatch mTestCompleteSignal = new CountDownLatch(2);
    private final CountDownLatch mSurfaceCreationCountDown = new CountDownLatch(1);
    private final CountDownLatch mNoCallbackSignal = new CountDownLatch(1);
-    private final CountDownLatch mFramesSignal = new CountDownLatch(FRAME_ITERATIONS);

    private ActivityScenario<GraphicsActivity> mScenario;
    private int mInitialMatchContentFrameRate;
@@ -73,29 +70,27 @@ public class AttachedChoreographerTest {
    public void setUp() throws Exception {
        mScenario = ActivityScenario.launch(GraphicsActivity.class);
        mScenario.moveToState(Lifecycle.State.CREATED);
-        mCallbackMissedCounter = 0;
        mScenario.onActivity(activity -> {
            mSurfaceView = activity.findViewById(R.id.surface);
            mSurfaceHolder = mSurfaceView.getHolder();
            mSurfaceHolder.addCallback(new SurfaceHolder.Callback() {

-                @Override
-                public void surfaceChanged(SurfaceHolder holder, int format, int width,
-                        int height) {
-                }
-
                @Override
                public void surfaceCreated(SurfaceHolder holder) {
                    mSurfaceCreationCountDown.countDown();
                }

+                @Override
+                public void surfaceChanged(SurfaceHolder holder, int format, int width,
+                        int height) {
+                }
+
                @Override
                public void surfaceDestroyed(SurfaceHolder holder) {
                }
            });
        });

-
        UiDevice uiDevice = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation());
        uiDevice.wakeUp();
        uiDevice.executeShellCommand("wm dismiss-keyguard");
@@ -131,7 +126,7 @@ public class AttachedChoreographerTest {
    }

    @Test
-    public void test_create_choreographer() {
+    public void testCreateChoreographer() {
        mScenario.onActivity(activity -> {
            if (waitForCountDown(mSurfaceCreationCountDown, /* timeoutInSeconds */ 1L)) {
                fail("Unable to create surface within 1 Second");
@@ -166,7 +161,7 @@ public class AttachedChoreographerTest {
    }

    @Test
-    public void test_copy_surface_control() {
+    public void testCopySurfaceControl() {
        mScenario.onActivity(activity -> {
            if (waitForCountDown(mSurfaceCreationCountDown, /* timeoutInSeconds */ 1L)) {
                fail("Unable to create surface within 1 Second");
@@ -199,7 +194,7 @@ public class AttachedChoreographerTest {
    }

    @Test
-    public void test_mirror_surface_control() {
+    public void testMirrorSurfaceControl() {
        mScenario.onActivity(activity -> {
            if (waitForCountDown(mSurfaceCreationCountDown, /* timeoutInSeconds */ 1L)) {
                fail("Unable to create surface within 1 Second");
@@ -231,7 +226,7 @@ public class AttachedChoreographerTest {
    }

    @Test
-    public void test_postFrameCallback() {
+    public void testPostFrameCallback() {
        mScenario.onActivity(activity -> {
            if (waitForCountDown(mSurfaceCreationCountDown, /* timeoutInSeconds */ 1L)) {
                fail("Unable to create surface within 1 Second");
@@ -256,7 +251,7 @@ public class AttachedChoreographerTest {
    }

    @Test
-    public void test_postFrameCallbackDelayed() {
+    public void testPostFrameCallbackDelayed() {
        mScenario.onActivity(activity -> {
            if (waitForCountDown(mSurfaceCreationCountDown, /* timeoutInSeconds */ 1L)) {
                fail("Unable to create surface within 1 Second");
@@ -283,7 +278,7 @@ public class AttachedChoreographerTest {
    }

    @Test
-    public void test_postCallback() {
+    public void testPostCallback() {
        mScenario.onActivity(activity -> {
            if (waitForCountDown(mSurfaceCreationCountDown, /* timeoutInSeconds */ 1L)) {
                fail("Unable to create surface within 1 Second");
@@ -309,7 +304,7 @@ public class AttachedChoreographerTest {
    }

    @Test
-    public void test_postCallbackDelayed() {
+    public void testPostCallbackDelayed() {
        mScenario.onActivity(activity -> {
            if (waitForCountDown(mSurfaceCreationCountDown, /* timeoutInSeconds */ 1L)) {
                fail("Unable to create surface within 1 Second");
@@ -335,7 +330,7 @@ public class AttachedChoreographerTest {
    }

    @Test
-    public void test_postVsyncCallback() {
+    public void testPostVsyncCallback() {
        mScenario.onActivity(activity -> {
            if (waitForCountDown(mSurfaceCreationCountDown, /* timeout */ 1L)) {
                fail("Unable to create surface within 1 Second");
@@ -359,52 +354,37 @@ public class AttachedChoreographerTest {
    }

    @Test
-    public void test_choreographer_10Hz_refreshRate() {
-        mScenario.onActivity(activity -> {
-            if (waitForCountDown(mSurfaceCreationCountDown, /* timeoutInSeconds */ 1L)) {
-                fail("Unable to create surface within 1 Second");
+    public void testChoreographerDivisorRefreshRate() {
+        for (int divisor : new int[]{2, 3}) {
+            CountDownLatch continueLatch = new CountDownLatch(1);
+            mScenario.onActivity(activity -> {
+                if (waitForCountDown(mSurfaceCreationCountDown, /* timeoutInSeconds */ 1L)) {
+                    fail("Unable to create surface within 1 Second");
+                }
+                SurfaceControl sc = mSurfaceView.getSurfaceControl();
+                Choreographer choreographer = sc.getChoreographer();
+                SurfaceControl.Transaction transaction = new SurfaceControl.Transaction();
+                float displayRefreshRate = activity.getDisplay().getMode().getRefreshRate();
+                float fps = displayRefreshRate / divisor;
+                long callbackDurationMs = Math.round(1000 / fps);
+                mCallbackMissedCounter = 0;
+                transaction.setFrameRate(sc, fps, Surface.FRAME_RATE_COMPATIBILITY_FIXED_SOURCE)
+                        .addTransactionCommittedListener(Runnable::run,
+                                () -> verifyVsyncCallbacks(choreographer,
+                                        callbackDurationMs, continueLatch, FRAME_ITERATIONS))
+                        .apply();
+            });
+            // wait for the previous callbacks to finish before moving to the next divisor
+            if (waitForCountDown(continueLatch, /* timeoutInSeconds */ 5L)) {
+                fail("Test not finished in 5 Seconds");
            }
-            SurfaceControl sc = mSurfaceView.getSurfaceControl();
-            Choreographer choreographer = sc.getChoreographer();
-            SurfaceControl.Transaction transaction = new SurfaceControl.Transaction();
-            transaction.setFrameRate(sc, 10.0f, Surface.FRAME_RATE_COMPATIBILITY_FIXED_SOURCE)
-                    .addTransactionCommittedListener(Runnable::run,
-                            () -> verifyVsyncCallbacks(choreographer,
-                                    CALLBACK_TIME_10_FPS))
-                    .apply();
-            mTestCompleteSignal.countDown();
-        });
-        if (waitForCountDown(mTestCompleteSignal, /* timeoutInSeconds */ 5L)) {
-            fail("Test not finished in 5 Seconds");
        }
    }

-    @Test
-    public void test_choreographer_30Hz_refreshRate() {
-        mScenario.onActivity(activity -> {
-            if (waitForCountDown(mSurfaceCreationCountDown, /* timeoutInSeconds */ 1L)) {
-                fail("Unable to create surface within 1 Second");
-            }
-            SurfaceControl sc = mSurfaceView.getSurfaceControl();
-            Choreographer choreographer = sc.getChoreographer();
-            SurfaceControl.Transaction transaction = new SurfaceControl.Transaction();
-            transaction.setFrameRate(sc, 30.0f, Surface.FRAME_RATE_COMPATIBILITY_FIXED_SOURCE)
-                    .addTransactionCommittedListener(Runnable::run,
-                            () -> verifyVsyncCallbacks(choreographer,
-                                    CALLBACK_TIME_30_FPS))
-                    .apply();
-            mTestCompleteSignal.countDown();
-        });
-        if (waitForCountDown(mTestCompleteSignal, /* timeoutInSeconds */ 5L)) {
-            fail("Test not finished in 5 Seconds");
-        }
-    }
-
-    private void verifyVsyncCallbacks(Choreographer choreographer, int callbackDurationMs) {
+    private void verifyVsyncCallbacks(Choreographer choreographer, long callbackDurationMs,
+            CountDownLatch continueLatch, int frameCount) {
        long callbackRequestedTimeNs = System.nanoTime();
        choreographer.postVsyncCallback(frameData -> {
-            mFramesSignal.countDown();
-            final long frameCount = mFramesSignal.getCount();
            if (frameCount > 0) {
                if (!mIsFirstCallback) {
                    // Skip the first callback as it takes 1 frame
@@ -422,18 +402,19 @@ public class AttachedChoreographerTest {
                    }
                }
                mIsFirstCallback = false;
-                verifyVsyncCallbacks(choreographer, callbackDurationMs);
+                verifyVsyncCallbacks(choreographer, callbackDurationMs,
+                        continueLatch, frameCount - 1);
            } else {
                assertTrue("Missed timeline for " + mCallbackMissedCounter + " callbacks, while "
                                + CALLBACK_MISSED_THRESHOLD + " missed callbacks are allowed",
                        mCallbackMissedCounter <= CALLBACK_MISSED_THRESHOLD);
-                mTestCompleteSignal.countDown();
+                continueLatch.countDown();
            }
        });
    }

    private long getCallbackDurationDiffInMs(long callbackTimeNs, long requestedTimeNs,
-            int expectedCallbackMs) {
+            long expectedCallbackMs) {
        long actualTimeMs = TimeUnit.NANOSECONDS.toMillis(callbackTimeNs)
                - TimeUnit.NANOSECONDS.toMillis(requestedTimeNs);
        return Math.abs(expectedCallbackMs - actualTimeMs);