Fix Suspense throttling mechanism (#26802)

The throttling mechanism for fallbacks should apply to both their
appearance _and_ disappearance.

This was mostly addressed by #26611. See that PR for additional context.

However, a flaw in the implementation is that we only update the the
timestamp used for throttling when the fallback initially appears. We
don't update it when the real content pops in. If lots of content in
separate Suspense trees loads around the same time, you can still get
jank.

The issue is fixed by updating the throttling timestamp whenever the
visibility of a fallback changes. Not just when it appears.

DiffTrain build for [4bfcd02](4bfcd02)

Author: acdlite

compiled/facebook-www/REVISION

@@ -1 +1 @@
-4cd7065665ea2cf33c306265c8d817904bb401ca
+4bfcd02b2cebcb390f5aff0d7747c60a55012d5d

compiled/facebook-www/React-dev.modern.js

@@ -27,7 +27,7 @@ if (
 }
           "use strict";
 
-var ReactVersion = "18.3.0-www-modern-6995898a";
+var ReactVersion = "18.3.0-www-modern-102b0f04";
 
 // ATTENTION
 // When adding new symbols to this file,

compiled/facebook-www/ReactART-dev.classic.js

@@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
   return self;
 }
 
-var ReactVersion = "18.3.0-www-classic-d297891e";
+var ReactVersion = "18.3.0-www-classic-ed32f0c6";
 
 var LegacyRoot = 0;
 var ConcurrentRoot = 1;
@@ -22110,20 +22110,35 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
 
     case SuspenseComponent: {
       recursivelyTraverseMutationEffects(root, finishedWork);
-      commitReconciliationEffects(finishedWork);
+      commitReconciliationEffects(finishedWork); // TODO: We should mark a flag on the Suspense fiber itself, rather than
+      // relying on the Offscreen fiber having a flag also being marked. The
+      // reason is that this offscreen fiber might not be part of the work-in-
+      // progress tree! It could have been reused from a previous render. This
+      // doesn't lead to incorrect behavior because we don't rely on the flag
+      // check alone; we also compare the states explicitly below. But for
+      // modeling purposes, we _should_ be able to rely on the flag check alone.
+      // So this is a bit fragile.
+      //
+      // Also, all this logic could/should move to the passive phase so it
+      // doesn't block paint.
+
       var offscreenFiber = finishedWork.child;
 
       if (offscreenFiber.flags & Visibility) {
-        var newState = offscreenFiber.memoizedState;
-        var isHidden = newState !== null;
-
-        if (isHidden) {
-          var wasHidden =
-            offscreenFiber.alternate !== null &&
-            offscreenFiber.alternate.memoizedState !== null;
-
-          if (!wasHidden) {
-            // TODO: Move to passive phase
+        // Throttle the appearance and disappearance of Suspense fallbacks.
+        var isShowingFallback = finishedWork.memoizedState !== null;
+        var wasShowingFallback =
+          current !== null && current.memoizedState !== null;
+
+        if (alwaysThrottleRetries) {
+          if (isShowingFallback !== wasShowingFallback) {
+            // A fallback is either appearing or disappearing.
+            markCommitTimeOfFallback();
+          }
+        } else {
+          if (isShowingFallback && !wasShowingFallback) {
+            // Old behavior. Only mark when a fallback appears, not when
+            // it disappears.
             markCommitTimeOfFallback();
           }
         }
@@ -22154,20 +22169,18 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
         }
       }
 
-      var _newState = finishedWork.memoizedState;
-
-      var _isHidden = _newState !== null;
-
-      var _wasHidden = current !== null && current.memoizedState !== null;
+      var newState = finishedWork.memoizedState;
+      var isHidden = newState !== null;
+      var wasHidden = current !== null && current.memoizedState !== null;
 
       if (finishedWork.mode & ConcurrentMode) {
         // Before committing the children, track on the stack whether this
         // offscreen subtree was already hidden, so that we don't unmount the
         // effects again.
         var prevOffscreenSubtreeIsHidden = offscreenSubtreeIsHidden;
         var prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden;
-        offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden || _isHidden;
-        offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || _wasHidden;
+        offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden || isHidden;
+        offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || wasHidden;
         recursivelyTraverseMutationEffects(root, finishedWork);
         offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
         offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden;
@@ -22188,21 +22201,21 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
       if (flags & Visibility) {
         // Track the current state on the Offscreen instance so we can
         // read it during an event
-        if (_isHidden) {
+        if (isHidden) {
           offscreenInstance._visibility &= ~OffscreenVisible;
         } else {
           offscreenInstance._visibility |= OffscreenVisible;
         }
 
-        if (_isHidden) {
+        if (isHidden) {
           var isUpdate = current !== null;
           var wasHiddenByAncestorOffscreen =
             offscreenSubtreeIsHidden || offscreenSubtreeWasHidden; // Only trigger disapper layout effects if:
           //   - This is an update, not first mount.
           //   - This Offscreen was not hidden before.
           //   - Ancestor Offscreen was not hidden in previous commit.
 
-          if (isUpdate && !_wasHidden && !wasHiddenByAncestorOffscreen) {
+          if (isUpdate && !wasHidden && !wasHiddenByAncestorOffscreen) {
             if ((finishedWork.mode & ConcurrentMode) !== NoMode) {
               // Disappear the layout effects of all the children
               recursivelyTraverseDisappearLayoutEffects(finishedWork);
@@ -22213,7 +22226,7 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
         if (!isOffscreenManual(finishedWork)) {
           // TODO: This needs to run whenever there's an insertion or update
           // inside a hidden Offscreen tree.
-          hideOrUnhideAllChildren(finishedWork, _isHidden);
+          hideOrUnhideAllChildren(finishedWork, isHidden);
         }
       } // TODO: Move to passive phase
 
@@ -24000,8 +24013,10 @@ var workInProgressRootPingedLanes = NoLanes; // Errors that are thrown during th
 var workInProgressRootConcurrentErrors = null; // These are errors that we recovered from without surfacing them to the UI.
 // We will log them once the tree commits.
 
-var workInProgressRootRecoverableErrors = null; // The most recent time we committed a fallback. This lets us ensure a train
-// model where we don't commit new loading states in too quick succession.
+var workInProgressRootRecoverableErrors = null; // The most recent time we either committed a fallback, or when a fallback was
+// filled in with the resolved UI. This lets us throttle the appearance of new
+// content as it streams in, to minimize jank.
+// TODO: Think of a better name for this variable?
 
 var globalMostRecentFallbackTime = 0;
 var FALLBACK_THROTTLE_MS = 500; // The absolute time for when we should start giving up on rendering

compiled/facebook-www/ReactART-dev.modern.js

@@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
   return self;
 }
 
-var ReactVersion = "18.3.0-www-modern-2f2a7948";
+var ReactVersion = "18.3.0-www-modern-c4ac5f42";
 
 var LegacyRoot = 0;
 var ConcurrentRoot = 1;
@@ -21775,20 +21775,35 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
 
     case SuspenseComponent: {
       recursivelyTraverseMutationEffects(root, finishedWork);
-      commitReconciliationEffects(finishedWork);
+      commitReconciliationEffects(finishedWork); // TODO: We should mark a flag on the Suspense fiber itself, rather than
+      // relying on the Offscreen fiber having a flag also being marked. The
+      // reason is that this offscreen fiber might not be part of the work-in-
+      // progress tree! It could have been reused from a previous render. This
+      // doesn't lead to incorrect behavior because we don't rely on the flag
+      // check alone; we also compare the states explicitly below. But for
+      // modeling purposes, we _should_ be able to rely on the flag check alone.
+      // So this is a bit fragile.
+      //
+      // Also, all this logic could/should move to the passive phase so it
+      // doesn't block paint.
+
       var offscreenFiber = finishedWork.child;
 
       if (offscreenFiber.flags & Visibility) {
-        var newState = offscreenFiber.memoizedState;
-        var isHidden = newState !== null;
-
-        if (isHidden) {
-          var wasHidden =
-            offscreenFiber.alternate !== null &&
-            offscreenFiber.alternate.memoizedState !== null;
-
-          if (!wasHidden) {
-            // TODO: Move to passive phase
+        // Throttle the appearance and disappearance of Suspense fallbacks.
+        var isShowingFallback = finishedWork.memoizedState !== null;
+        var wasShowingFallback =
+          current !== null && current.memoizedState !== null;
+
+        if (alwaysThrottleRetries) {
+          if (isShowingFallback !== wasShowingFallback) {
+            // A fallback is either appearing or disappearing.
+            markCommitTimeOfFallback();
+          }
+        } else {
+          if (isShowingFallback && !wasShowingFallback) {
+            // Old behavior. Only mark when a fallback appears, not when
+            // it disappears.
             markCommitTimeOfFallback();
           }
         }
@@ -21819,20 +21834,18 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
         }
       }
 
-      var _newState = finishedWork.memoizedState;
-
-      var _isHidden = _newState !== null;
-
-      var _wasHidden = current !== null && current.memoizedState !== null;
+      var newState = finishedWork.memoizedState;
+      var isHidden = newState !== null;
+      var wasHidden = current !== null && current.memoizedState !== null;
 
       if (finishedWork.mode & ConcurrentMode) {
         // Before committing the children, track on the stack whether this
         // offscreen subtree was already hidden, so that we don't unmount the
         // effects again.
         var prevOffscreenSubtreeIsHidden = offscreenSubtreeIsHidden;
         var prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden;
-        offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden || _isHidden;
-        offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || _wasHidden;
+        offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden || isHidden;
+        offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || wasHidden;
         recursivelyTraverseMutationEffects(root, finishedWork);
         offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
         offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden;
@@ -21853,21 +21866,21 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
       if (flags & Visibility) {
         // Track the current state on the Offscreen instance so we can
         // read it during an event
-        if (_isHidden) {
+        if (isHidden) {
           offscreenInstance._visibility &= ~OffscreenVisible;
         } else {
           offscreenInstance._visibility |= OffscreenVisible;
         }
 
-        if (_isHidden) {
+        if (isHidden) {
           var isUpdate = current !== null;
           var wasHiddenByAncestorOffscreen =
             offscreenSubtreeIsHidden || offscreenSubtreeWasHidden; // Only trigger disapper layout effects if:
           //   - This is an update, not first mount.
           //   - This Offscreen was not hidden before.
           //   - Ancestor Offscreen was not hidden in previous commit.
 
-          if (isUpdate && !_wasHidden && !wasHiddenByAncestorOffscreen) {
+          if (isUpdate && !wasHidden && !wasHiddenByAncestorOffscreen) {
             if ((finishedWork.mode & ConcurrentMode) !== NoMode) {
               // Disappear the layout effects of all the children
               recursivelyTraverseDisappearLayoutEffects(finishedWork);
@@ -21878,7 +21891,7 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
         if (!isOffscreenManual(finishedWork)) {
           // TODO: This needs to run whenever there's an insertion or update
           // inside a hidden Offscreen tree.
-          hideOrUnhideAllChildren(finishedWork, _isHidden);
+          hideOrUnhideAllChildren(finishedWork, isHidden);
         }
       } // TODO: Move to passive phase
 
@@ -23665,8 +23678,10 @@ var workInProgressRootPingedLanes = NoLanes; // Errors that are thrown during th
 var workInProgressRootConcurrentErrors = null; // These are errors that we recovered from without surfacing them to the UI.
 // We will log them once the tree commits.
 
-var workInProgressRootRecoverableErrors = null; // The most recent time we committed a fallback. This lets us ensure a train
-// model where we don't commit new loading states in too quick succession.
+var workInProgressRootRecoverableErrors = null; // The most recent time we either committed a fallback, or when a fallback was
+// filled in with the resolved UI. This lets us throttle the appearance of new
+// content as it streams in, to minimize jank.
+// TODO: Think of a better name for this variable?
 
 var globalMostRecentFallbackTime = 0;
 var FALLBACK_THROTTLE_MS = 500; // The absolute time for when we should start giving up on rendering