From a63b443e62aac6ad0c6745fb01a38cc49b88778d Mon Sep 17 00:00:00 2001
From: Xin Chen <xch@meta.com>
Date: Thu, 7 Sep 2023 20:23:59 -0700
Subject: [PATCH] Fix FpsDebugFrameCallback so that we properly cancel frame
 loop to avoid race (#38671)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38671

Fix a race condition when we unmount and mount a view using FpsView too frequently. In this case, the frame loop callback didn't get a chance to unset the  `mShouldStop` flag, causing the old frame loop continues to run unexpectedly.

The fix here guarantees `stop` would queue logic that removes the frame loop callback, and a later `start` would queue logic that attaches a new frame loop callback. Since both of them happens on UI thread, they are in sync.

Changelog:
[Android][Fixed] - Fix a race with FpsView on using FpsDebugFrameCallback.

Reviewed By: hoxyq

Differential Revision: D47849848

fbshipit-source-id: 8c4be40e86be128734bfa3f571fd3a1735976c7c
---
 .../modules/debug/FpsDebugFrameCallback.java     | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/debug/FpsDebugFrameCallback.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/debug/FpsDebugFrameCallback.java
index f4197d63b7a9a0..20f2bc3a5a103c 100644
--- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/debug/FpsDebugFrameCallback.java
+++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/debug/FpsDebugFrameCallback.java
@@ -63,7 +63,6 @@ public FpsInfo(
   private final UIManagerModule mUIManagerModule;
   private final DidJSUpdateUiDuringFrameDetector mDidJSUpdateUiDuringFrameDetector;
 
-  private boolean mShouldStop = false;
   private long mFirstFrameTime = -1;
   private long mLastFrameTime = -1;
   private int mNumFrameCallbacks = 0;
@@ -82,10 +81,6 @@ public FpsDebugFrameCallback(ReactContext reactContext) {
 
   @Override
   public void doFrame(long l) {
-    if (mShouldStop) {
-      return;
-    }
-
     if (mFirstFrameTime == -1) {
       mFirstFrameTime = l;
     }
@@ -124,7 +119,6 @@ public void doFrame(long l) {
   }
 
   public void start() {
-    mShouldStop = false;
     mReactContext
         .getCatalystInstance()
         .addBridgeIdleDebugListener(mDidJSUpdateUiDuringFrameDetector);
@@ -147,11 +141,19 @@ public void startAndRecordFpsAtEachFrame() {
   }
 
   public void stop() {
-    mShouldStop = true;
     mReactContext
         .getCatalystInstance()
         .removeBridgeIdleDebugListener(mDidJSUpdateUiDuringFrameDetector);
     mUIManagerModule.setViewHierarchyUpdateDebugListener(null);
+    final FpsDebugFrameCallback fpsDebugFrameCallback = this;
+    UiThreadUtil.runOnUiThread(
+        new Runnable() {
+          @Override
+          public void run() {
+            mChoreographer = ChoreographerCompat.getInstance();
+            mChoreographer.removeFrameCallback(fpsDebugFrameCallback);
+          }
+        });
   }
 
   public double getFPS() {