From 751f7e97ba299eb8f06231126df0a7ecbc41d607 Mon Sep 17 00:00:00 2001 From: Nikita Lutsenko Date: Tue, 31 Oct 2023 11:51:09 -0700 Subject: [PATCH] rn-android | Add abstraction layer between ReactChoreographer and android.view.Choreographer that allows substituting current implementation. Summary: We want to have an extension point for choreographer, so we can override default behavior and have either rate-limiting, or testing or other form of manual control. For all those cases allow substitution of choreographer that ReactChoreographer would use by default with a custom one. Changelog: [Android][Added] ReactChoreographer can now use an implementation substitution instead of relying on android.view.Choreographer directly. Reviewed By: javache Differential Revision: D50827975 fbshipit-source-id: 0fd78e1f4f96ffd832e5d8cdc6c805f9a9e272cf --- .../testing/ReactIntegrationTestCase.java | 3 +- .../react/testing/ReactTestHelper.java | 3 +- .../facebook/react/ReactInstanceManager.java | 3 +- .../AndroidChoreographerProvider.java | 39 +++++++++++++++++++ .../react/internal/ChoreographerProvider.java | 30 ++++++++++++++ .../modules/core/ReactChoreographer.java | 12 +++--- .../facebook/react/runtime/ReactInstance.java | 3 +- 7 files changed, 83 insertions(+), 10 deletions(-) create mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/AndroidChoreographerProvider.java create mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/ChoreographerProvider.java diff --git a/packages/react-native/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactIntegrationTestCase.java b/packages/react-native/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactIntegrationTestCase.java index d13ef19aa56f57..5faec5a81e0885 100644 --- a/packages/react-native/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactIntegrationTestCase.java +++ b/packages/react-native/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactIntegrationTestCase.java @@ -22,6 +22,7 @@ import com.facebook.react.bridge.UiThreadUtil; import com.facebook.react.common.futures.SimpleSettableFuture; import com.facebook.react.devsupport.interfaces.DevSupportManager; +import com.facebook.react.internal.AndroidChoreographerProvider; import com.facebook.react.modules.core.ReactChoreographer; import com.facebook.react.modules.core.TimingModule; import com.facebook.react.testing.idledetection.ReactBridgeIdleSignaler; @@ -134,7 +135,7 @@ protected TimingModule createTimingModule() { new SimpleSettableFuture(); UiThreadUtil.runOnUiThread( () -> { - ReactChoreographer.initialize(); + ReactChoreographer.initialize(AndroidChoreographerProvider.getInstance()); TimingModule timingModule = new TimingModule(getContext(), Mockito.mock(DevSupportManager.class)); simpleSettableFuture.set(timingModule); diff --git a/packages/react-native/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java b/packages/react-native/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java index 11affeb80ffbc0..8506f9d6b7a72b 100644 --- a/packages/react-native/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java +++ b/packages/react-native/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java @@ -29,6 +29,7 @@ import com.facebook.react.bridge.NativeModule; import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.queue.ReactQueueConfigurationSpec; +import com.facebook.react.internal.AndroidChoreographerProvider; import com.facebook.react.modules.core.ReactChoreographer; import com.facebook.react.uimanager.ViewManager; import java.util.Arrays; @@ -149,7 +150,7 @@ public CatalystInstance build() { new Runnable() { @Override public void run() { - ReactChoreographer.initialize(); + ReactChoreographer.initialize(AndroidChoreographerProvider.getInstance()); instance.initialize(); } }); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java index 1710271335107e..419e60486af536 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java @@ -87,6 +87,7 @@ import com.facebook.react.devsupport.interfaces.DevSupportManager; import com.facebook.react.devsupport.interfaces.PackagerStatusCallback; import com.facebook.react.devsupport.interfaces.RedBoxHandler; +import com.facebook.react.internal.AndroidChoreographerProvider; import com.facebook.react.internal.turbomodule.core.TurboModuleManager; import com.facebook.react.internal.turbomodule.core.TurboModuleManagerDelegate; import com.facebook.react.modules.appearance.AppearanceModule; @@ -293,7 +294,7 @@ public void invokeDefaultOnBackPressed() { mJSIModulePackage = jsiModulePackage; // Instantiate ReactChoreographer in UI thread. - ReactChoreographer.initialize(); + ReactChoreographer.initialize(AndroidChoreographerProvider.getInstance()); if (mUseDeveloperSupport) { mDevSupportManager.startInspector(); } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/AndroidChoreographerProvider.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/AndroidChoreographerProvider.java new file mode 100644 index 00000000000000..a3b3de670d9d35 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/AndroidChoreographerProvider.java @@ -0,0 +1,39 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.react.internal; + +import com.facebook.react.bridge.UiThreadUtil; + +/** An implementation of ChoreographerProvider that directly uses android.view.Choreographer. */ +public final class AndroidChoreographerProvider implements ChoreographerProvider { + + public static final class AndroidChoreographer implements ChoreographerProvider.Choreographer { + private final android.view.Choreographer sInstance = android.view.Choreographer.getInstance(); + + public void postFrameCallback(android.view.Choreographer.FrameCallback callback) { + sInstance.postFrameCallback(callback); + } + + public void removeFrameCallback(android.view.Choreographer.FrameCallback callback) { + sInstance.removeFrameCallback(callback); + } + } + + private static class Holder { + private static final AndroidChoreographerProvider INSTANCE = new AndroidChoreographerProvider(); + } + + public static AndroidChoreographerProvider getInstance() { + return Holder.INSTANCE; + } + + public Choreographer getChoreographer() { + UiThreadUtil.assertOnUiThread(); + return new AndroidChoreographer(); + } +} diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/ChoreographerProvider.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/ChoreographerProvider.java new file mode 100644 index 00000000000000..1cfcb2268e45b1 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/ChoreographerProvider.java @@ -0,0 +1,30 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.react.internal; + +public interface ChoreographerProvider { + /** + * The interface to a android.view.Choreographer-like object, that can either use the + * android.view.Choreographer or a mock one for testing purposes, or override built-in + * android.view.Choreographer's behaviors. + */ + interface Choreographer { + + /** Posts a frame callback to run on the next frame. */ + void postFrameCallback(android.view.Choreographer.FrameCallback callback); + /** Removes a previously posted frame callback. */ + void removeFrameCallback(android.view.Choreographer.FrameCallback callback); + } + + /** + * Get an instance of Choreographer. + * + * @return An instance of Choreographer. + */ + Choreographer getChoreographer(); +} diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/core/ReactChoreographer.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/core/ReactChoreographer.java index 2abb9bf37f78d8..a05be50df4b4f6 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/core/ReactChoreographer.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/core/ReactChoreographer.java @@ -13,6 +13,7 @@ import com.facebook.infer.annotation.Assertions; import com.facebook.react.bridge.UiThreadUtil; import com.facebook.react.common.ReactConstants; +import com.facebook.react.internal.ChoreographerProvider; import java.util.ArrayDeque; /** @@ -55,9 +56,9 @@ public enum CallbackType { private static ReactChoreographer sInstance; - public static void initialize() { + public static void initialize(ChoreographerProvider choreographerProvider) { if (sInstance == null) { - sInstance = new ReactChoreographer(); + sInstance = new ReactChoreographer(choreographerProvider); } } @@ -66,8 +67,7 @@ public static ReactChoreographer getInstance() { return sInstance; } - // This needs to be volatile due to double checked locking issue - https://fburl.com/z409owpf - private @Nullable volatile Choreographer mChoreographer; + private @Nullable ChoreographerProvider.Choreographer mChoreographer; private final ArrayDeque[] mCallbackQueues; @@ -101,7 +101,7 @@ public void doFrame(long frameTimeNanos) { private int mTotalCallbacks = 0; private boolean mHasPostedCallback = false; - private ReactChoreographer() { + private ReactChoreographer(ChoreographerProvider choreographerProvider) { mCallbackQueues = new ArrayDeque[CallbackType.values().length]; for (int i = 0; i < mCallbackQueues.length; i++) { mCallbackQueues[i] = new ArrayDeque<>(); @@ -109,7 +109,7 @@ private ReactChoreographer() { UiThreadUtil.runOnUiThread( () -> { - mChoreographer = Choreographer.getInstance(); + mChoreographer = choreographerProvider.getChoreographer(); }); } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.java index 4da3827ab8d708..60497013ba31d3 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.java @@ -44,6 +44,7 @@ import com.facebook.react.fabric.ReactNativeConfig; import com.facebook.react.fabric.events.EventBeatManager; import com.facebook.react.interfaces.exceptionmanager.ReactJsExceptionHandler; +import com.facebook.react.internal.AndroidChoreographerProvider; import com.facebook.react.internal.turbomodule.core.CallInvokerHolderImpl; import com.facebook.react.internal.turbomodule.core.NativeMethodCallInvokerHolderImpl; import com.facebook.react.internal.turbomodule.core.TurboModuleManager; @@ -137,7 +138,7 @@ final class ReactInstance { MessageQueueThread nativeModulesMessageQueueThread = mQueueConfiguration.getNativeModulesQueueThread(); - ReactChoreographer.initialize(); + ReactChoreographer.initialize(AndroidChoreographerProvider.getInstance()); if (useDevSupport) { devSupportManager.startInspector(); }