Remove ChoreographerCompat (#39775)

Summary: Pull Request resolved: #39775 ChoreographerCompat existed to support JellyBean, but given we target Android SDK 23+ now, this is safe to remove. Changelog: [Android][Removed] Deprecated ChoreographerCompat.FrameCallback, use Choreographer.FrameCallback Reviewed By: mdvacca Differential Revision: D49826889 fbshipit-source-id: 5158c470553327b70a199168f5b7ed7071cc8c48
facebook · Oct 3, 2023 · 248c324 · 248c324
1 parent 0e10ee6
commit 248c324
Show file tree

Hide file tree

Showing 14 changed files with 117 additions and 238 deletions.
diff --git a/...src/androidTest/java/com/facebook/react/testing/idledetection/ReactIdleDetectionUtil.java b/...src/androidTest/java/com/facebook/react/testing/idledetection/ReactIdleDetectionUtil.java
@@ -9,10 +9,11 @@
 
 import android.app.Instrumentation;
 import android.os.SystemClock;
+import android.view.Choreographer;
 import androidx.test.InstrumentationRegistry;
 import com.facebook.react.bridge.ReactContext;
 import com.facebook.react.bridge.UiThreadUtil;
-import com.facebook.react.modules.core.ChoreographerCompat;
+import com.facebook.react.modules.core.ReactChoreographer;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
@@ -48,26 +49,24 @@ private static void waitForChoreographer(long timeToWait) {
     final int waitFrameCount = 2;
     final CountDownLatch latch = new CountDownLatch(1);
     UiThreadUtil.runOnUiThread(
-        new Runnable() {
-          @Override
-          public void run() {
-            final ChoreographerCompat choreographerCompat = ChoreographerCompat.getInstance();
-            choreographerCompat.postFrameCallback(
-                new ChoreographerCompat.FrameCallback() {
+        () -> {
+          ReactChoreographer choreographer = ReactChoreographer.getInstance();
+          choreographer.postFrameCallback(
+              ReactChoreographer.CallbackType.IDLE_EVENT,
+              new Choreographer.FrameCallback() {
+                private int frameCount = 0;
 
-                  private int frameCount = 0;
-
-                  @Override
-                  public void doFrame(long frameTimeNanos) {
-                    frameCount++;
-                    if (frameCount == waitFrameCount) {
-                      latch.countDown();
-                    } else {
-                      choreographerCompat.postFrameCallback(this);
-                    }
+                @Override
+                public void doFrame(long frameTimeNanos) {
+                  frameCount++;
+                  if (frameCount == waitFrameCount) {
+                    latch.countDown();
+                  } else {
+                    choreographer.postFrameCallback(
+                        ReactChoreographer.CallbackType.IDLE_EVENT, this);
                   }
-                });
-          }
+                }
+              });
         });
     try {
       if (!latch.await(timeToWait, TimeUnit.MILLISECONDS)) {

diff --git a/...act-native/ReactAndroid/src/main/java/com/facebook/react/fabric/GuardedFrameCallback.java b/...act-native/ReactAndroid/src/main/java/com/facebook/react/fabric/GuardedFrameCallback.java
@@ -7,12 +7,12 @@
 
 package com.facebook.react.fabric;
 
+import android.view.Choreographer;
 import androidx.annotation.NonNull;
 import com.facebook.react.bridge.JSExceptionHandler;
 import com.facebook.react.bridge.ReactContext;
-import com.facebook.react.modules.core.ChoreographerCompat;
 
-public abstract class GuardedFrameCallback extends ChoreographerCompat.FrameCallback {
+public abstract class GuardedFrameCallback implements Choreographer.FrameCallback {
 
   @NonNull private final JSExceptionHandler mExceptionHandler;
 

diff --git a/...ative/ReactAndroid/src/main/java/com/facebook/react/modules/core/ChoreographerCompat.java b/...ative/ReactAndroid/src/main/java/com/facebook/react/modules/core/ChoreographerCompat.java
@@ -5,107 +5,12 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-// This file was pulled from the facebook/rebound repository.
-
 package com.facebook.react.modules.core;
 
-import android.os.Handler;
 import android.view.Choreographer;
-import com.facebook.react.bridge.UiThreadUtil;
 
-/**
- * Wrapper class for abstracting away availability of the JellyBean Choreographer. If Choreographer
- * is unavailable we fallback to using a normal Handler.
- */
 public class ChoreographerCompat {
-
-  private static final long ONE_FRAME_MILLIS = 17;
-  private static ChoreographerCompat sInstance;
-
-  private Handler mHandler;
-  private Choreographer mChoreographer;
-
-  public static ChoreographerCompat getInstance() {
-    UiThreadUtil.assertOnUiThread();
-    if (sInstance == null) {
-      sInstance = new ChoreographerCompat();
-    }
-    return sInstance;
-  }
-
-  private ChoreographerCompat() {
-    mChoreographer = getChoreographer();
-  }
-
-  public void postFrameCallback(FrameCallback callbackWrapper) {
-    choreographerPostFrameCallback(callbackWrapper.getFrameCallback());
-  }
-
-  public void postFrameCallbackDelayed(FrameCallback callbackWrapper, long delayMillis) {
-    choreographerPostFrameCallbackDelayed(callbackWrapper.getFrameCallback(), delayMillis);
-  }
-
-  public void removeFrameCallback(FrameCallback callbackWrapper) {
-    choreographerRemoveFrameCallback(callbackWrapper.getFrameCallback());
-  }
-
-  private Choreographer getChoreographer() {
-    return Choreographer.getInstance();
-  }
-
-  private void choreographerPostFrameCallback(Choreographer.FrameCallback frameCallback) {
-    mChoreographer.postFrameCallback(frameCallback);
-  }
-
-  private void choreographerPostFrameCallbackDelayed(
-      Choreographer.FrameCallback frameCallback, long delayMillis) {
-    mChoreographer.postFrameCallbackDelayed(frameCallback, delayMillis);
-  }
-
-  private void choreographerRemoveFrameCallback(Choreographer.FrameCallback frameCallback) {
-    mChoreographer.removeFrameCallback(frameCallback);
-  }
-
-  /**
-   * This class provides a compatibility wrapper around the JellyBean FrameCallback with methods to
-   * access cached wrappers for submitting a real FrameCallback to a Choreographer or a Runnable to
-   * a Handler.
-   */
-  public abstract static class FrameCallback {
-
-    private Runnable mRunnable;
-    private Choreographer.FrameCallback mFrameCallback;
-
-    Choreographer.FrameCallback getFrameCallback() {
-      if (mFrameCallback == null) {
-        mFrameCallback =
-            new Choreographer.FrameCallback() {
-              @Override
-              public void doFrame(long frameTimeNanos) {
-                FrameCallback.this.doFrame(frameTimeNanos);
-              }
-            };
-      }
-      return mFrameCallback;
-    }
-
-    Runnable getRunnable() {
-      if (mRunnable == null) {
-        mRunnable =
-            new Runnable() {
-              @Override
-              public void run() {
-                doFrame(System.nanoTime());
-              }
-            };
-      }
-      return mRunnable;
-    }
-
-    /**
-     * Just a wrapper for frame callback, see {@link
-     * android.view.Choreographer.FrameCallback#doFrame(long)}.
-     */
-    public abstract void doFrame(long frameTimeNanos);
-  }
+  /** @deprecated Use Choreographer.FrameCallback instead */
+  @Deprecated
+  public abstract static class FrameCallback implements Choreographer.FrameCallback {}
 }
diff --git a/...t-native/ReactAndroid/src/main/java/com/facebook/react/modules/core/JavaTimerManager.java b/...t-native/ReactAndroid/src/main/java/com/facebook/react/modules/core/JavaTimerManager.java
@@ -8,6 +8,7 @@
 package com.facebook.react.modules.core;
 
 import android.util.SparseArray;
+import android.view.Choreographer;
 import androidx.annotation.Nullable;
 import com.facebook.proguard.annotations.DoNotStrip;
 import com.facebook.react.bridge.Arguments;
@@ -51,7 +52,7 @@ private Timer(int callbackID, long initialTargetTime, int duration, boolean repe
     }
   }
 
-  private class TimerFrameCallback extends ChoreographerCompat.FrameCallback {
+  private class TimerFrameCallback implements Choreographer.FrameCallback {
 
     // Temporary map for constructing the individual arrays of timers to call
     private @Nullable WritableArray mTimersToCall = null;
@@ -89,7 +90,7 @@ public void doFrame(long frameTimeNanos) {
     }
   }
 
-  private class IdleFrameCallback extends ChoreographerCompat.FrameCallback {
+  private class IdleFrameCallback implements Choreographer.FrameCallback {
 
     @Override
     public void doFrame(long frameTimeNanos) {

diff --git a/...native/ReactAndroid/src/main/java/com/facebook/react/modules/core/ReactChoreographer.java b/...native/ReactAndroid/src/main/java/com/facebook/react/modules/core/ReactChoreographer.java
@@ -7,7 +7,7 @@
 
 package com.facebook.react.modules.core;
 
-import androidx.annotation.GuardedBy;
+import android.view.Choreographer;
 import androidx.annotation.Nullable;
 import com.facebook.common.logging.FLog;
 import com.facebook.infer.annotation.Assertions;
@@ -17,10 +17,10 @@
 
 /**
  * A simple wrapper around Choreographer that allows us to control the order certain callbacks are
- * executed within a given frame. The main difference is that we enforce this is accessed from the
- * UI thread: this is because this ordering cannot be guaranteed across multiple threads.
+ * executed within a given frame. The wrapped Choreographer instance will always be the main thread
+ * one and the API's are safe to use from any thread.
  */
-public class ReactChoreographer {
+public final class ReactChoreographer {
 
   public enum CallbackType {
 
@@ -67,37 +67,65 @@ public static ReactChoreographer getInstance() {
   }
 
   // This needs to be volatile due to double checked locking issue - https://fburl.com/z409owpf
-  private @Nullable volatile ChoreographerCompat mChoreographer;
-  private final ReactChoreographerDispatcher mReactChoreographerDispatcher;
-  private final Object mCallbackQueuesLock = new Object();
-
-  @GuardedBy("mCallbackQueuesLock")
-  private final ArrayDeque<ChoreographerCompat.FrameCallback>[] mCallbackQueues;
+  private @Nullable volatile Choreographer mChoreographer;
+
+  private final ArrayDeque<Choreographer.FrameCallback>[] mCallbackQueues;
+
+  private final Choreographer.FrameCallback mFrameCallback =
+      new Choreographer.FrameCallback() {
+        @Override
+        public void doFrame(long frameTimeNanos) {
+          synchronized (mCallbackQueues) {
+            // Callbacks run once and are then automatically removed, the callback will
+            // be posted again from postFrameCallback
+            mHasPostedCallback = false;
+
+            for (int i = 0; i < mCallbackQueues.length; i++) {
+              ArrayDeque<Choreographer.FrameCallback> callbackQueue = mCallbackQueues[i];
+              int initialLength = callbackQueue.size();
+              for (int callback = 0; callback < initialLength; callback++) {
+                Choreographer.FrameCallback frameCallback = callbackQueue.pollFirst();
+                if (frameCallback != null) {
+                  frameCallback.doFrame(frameTimeNanos);
+                  mTotalCallbacks--;
+                } else {
+                  FLog.e(ReactConstants.TAG, "Tried to execute non-existent frame callback");
+                }
+              }
+            }
+            maybeRemoveFrameCallback();
+          }
+        }
+      };
 
   private int mTotalCallbacks = 0;
   private boolean mHasPostedCallback = false;
 
   private ReactChoreographer() {
-    mReactChoreographerDispatcher = new ReactChoreographerDispatcher();
     mCallbackQueues = new ArrayDeque[CallbackType.values().length];
     for (int i = 0; i < mCallbackQueues.length; i++) {
       mCallbackQueues[i] = new ArrayDeque<>();
     }
-    initializeChoreographer(null);
+
+    UiThreadUtil.runOnUiThread(
+        () -> {
+          mChoreographer = Choreographer.getInstance();
+        });
   }
 
-  public void postFrameCallback(
-      CallbackType type, ChoreographerCompat.FrameCallback frameCallback) {
-    synchronized (mCallbackQueuesLock) {
+  public void postFrameCallback(CallbackType type, Choreographer.FrameCallback frameCallback) {
+    synchronized (mCallbackQueues) {
       mCallbackQueues[type.getOrder()].addLast(frameCallback);
       mTotalCallbacks++;
       Assertions.assertCondition(mTotalCallbacks > 0);
+
       if (!mHasPostedCallback) {
         if (mChoreographer == null) {
-          initializeChoreographer(
-              new Runnable() {
-                @Override
-                public void run() {
+          // Schedule on the main thread, at which point the constructor's async work will have
+          // completed
+          UiThreadUtil.runOnUiThread(
+              () -> {
+                synchronized (mCallbackQueues) {
                   postFrameCallbackOnChoreographer();
                 }
               });
@@ -110,33 +138,15 @@ public void run() {
 
   /**
    * This method writes on mHasPostedCallback and it should be called from another method that has
-   * the lock mCallbackQueuesLock
+   * the lock mCallbackQueues
    */
   private void postFrameCallbackOnChoreographer() {
-    mChoreographer.postFrameCallback(mReactChoreographerDispatcher);
+    mChoreographer.postFrameCallback(mFrameCallback);
     mHasPostedCallback = true;
   }
 
-  public void initializeChoreographer(@Nullable final Runnable runnable) {
-    UiThreadUtil.runOnUiThread(
-        new Runnable() {
-          @Override
-          public void run() {
-            synchronized (ReactChoreographer.class) {
-              if (mChoreographer == null) {
-                mChoreographer = ChoreographerCompat.getInstance();
-              }
-            }
-            if (runnable != null) {
-              runnable.run();
-            }
-          }
-        });
-  }
-
-  public void removeFrameCallback(
-      CallbackType type, ChoreographerCompat.FrameCallback frameCallback) {
-    synchronized (mCallbackQueuesLock) {
+  public void removeFrameCallback(CallbackType type, Choreographer.FrameCallback frameCallback) {
+    synchronized (mCallbackQueues) {
       if (mCallbackQueues[type.getOrder()].removeFirstOccurrence(frameCallback)) {
         mTotalCallbacks--;
         maybeRemoveFrameCallback();
@@ -148,39 +158,15 @@ public void removeFrameCallback(
 
   /**
    * This method reads and writes on mHasPostedCallback and it should be called from another method
-   * that already has the lock mCallbackQueuesLock.
+   * that already has the lock on mCallbackQueues.
    */
   private void maybeRemoveFrameCallback() {
     Assertions.assertCondition(mTotalCallbacks >= 0);
     if (mTotalCallbacks == 0 && mHasPostedCallback) {
       if (mChoreographer != null) {
-        mChoreographer.removeFrameCallback(mReactChoreographerDispatcher);
+        mChoreographer.removeFrameCallback(mFrameCallback);
       }
       mHasPostedCallback = false;
     }
   }
-
-  private class ReactChoreographerDispatcher extends ChoreographerCompat.FrameCallback {
-
-    @Override
-    public void doFrame(long frameTimeNanos) {
-      synchronized (mCallbackQueuesLock) {
-        mHasPostedCallback = false;
-        for (int i = 0; i < mCallbackQueues.length; i++) {
-          ArrayDeque<ChoreographerCompat.FrameCallback> callbackQueue = mCallbackQueues[i];
-          int initialLength = callbackQueue.size();
-          for (int callback = 0; callback < initialLength; callback++) {
-            ChoreographerCompat.FrameCallback frameCallback = callbackQueue.pollFirst();
-            if (frameCallback != null) {
-              frameCallback.doFrame(frameTimeNanos);
-              mTotalCallbacks--;
-            } else {
-              FLog.e(ReactConstants.TAG, "Tried to execute non-existent frame callback");
-            }
-          }
-        }
-        maybeRemoveFrameCallback();
-      }
-    }
-  }
 }