Remove recursion from handleErrors

Changed the algorithm of handleErrors a bit to ensure that boundaries
are not revisited once they are acknowledged.

This passes all the error boundary tests and all the incremental tests;
however, it's causing an infinite loop in one of the files when I try
to run all the tests. Haven't figured out which one yet. It also means
I can't run the record-tests script to see which tests might be failing.

I feel pretty good about the overall approach here, though.

Author: acdlite

src/renderers/shared/fiber/ReactFiberErrorBoundary.js

@@ -12,7 +12,6 @@
 
 'use strict';
 
-import type { TrappedError } from 'ReactFiberErrorBoundary';
 import type { Fiber } from 'ReactFiber';
 import type { FiberRoot } from 'ReactFiberRoot';
 import type { HostConfig, Deadline } from 'ReactFiberReconciler';
@@ -24,7 +23,6 @@ var ReactFiberCommitWork = require('ReactFiberCommitWork');
 var ReactCurrentOwner = require('ReactCurrentOwner');
 
 var { cloneFiber } = require('ReactFiber');
-var { trapError, acknowledgeErrorInBoundary } = require('ReactFiberErrorBoundary');
 
 var {
   NoWork,
@@ -51,6 +49,7 @@ var {
 
 var {
   HostContainer,
+  ClassComponent,
 } = require('ReactTypeOfWork');
 
 if (__DEV__) {
@@ -89,6 +88,9 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
   let isAnimationCallbackScheduled : boolean = false;
   let isDeferredCallbackScheduled : boolean = false;
 
+  let nextTrappedErrors : Array<TrappedError> | null = null;
+  let isHandlingErrors : boolean = false;
+
   function scheduleAnimationCallback(callback) {
     if (!isAnimationCallbackScheduled) {
       isAnimationCallbackScheduled = true;
@@ -246,10 +248,10 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
       }
     }
 
-    // Now that the tree has been committed, we can handle errors.
-    if (allTrappedErrors) {
-      handleErrors(allTrappedErrors);
-    }
+    nextTrappedErrors = nextTrappedErrors ?
+      nextTrappedErrors.push.apply(nextTrappedErrors, allTrappedErrors) :
+      allTrappedErrors;
+
     performTaskWork();
   }
 
@@ -512,13 +514,13 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
       switch (priorityLevel) {
         case SynchronousPriority:
           performSynchronousWorkUnsafe();
-          return;
+          break;
         case TaskPriority:
           performTaskWorkUnsafe(false);
-          return;
+          break;
         case AnimationPriority:
           performAnimationWorkUnsafe();
-          return;
+          break;
         case HighPriority:
         case LowPriority:
         case OffscreenPriority:
@@ -527,9 +529,9 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
           } else {
             performDeferredWorkUnsafe(deadline);
           }
-          return;
+          break;
         default:
-          return;
+          break;
       }
     } catch (error) {
       const failedUnitOfWork = nextUnitOfWork;
@@ -541,79 +543,143 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
         throw error;
       }
       const trappedError = trapError(failedUnitOfWork, error);
-      handleErrors([trappedError]);
+      if (nextTrappedErrors) {
+        nextTrappedErrors.push(trappedError);
+      } else {
+        nextTrappedErrors = [trappedError];
+      }
+    }
+
+    // If there were any errors, handle them now
+    if (nextTrappedErrors) {
+      handleErrors();
     }
   }
 
-  function handleErrors(initialTrappedErrors : Array<TrappedError>) : void {
-    let nextTrappedErrors = initialTrappedErrors;
+  // Boundaries that have been acknowledged
+  let knownBoundaries : Set<Fiber | null> = new Set();
+
+  function handleErrors() : void {
+    // Prevent recursion
+    if (isHandlingErrors) {
+      return;
+    }
+    isHandlingErrors = true;
+
     let firstUncaughtError = null;
 
+    // All work created by error boundaries should have Task priority
+    // so that it finishes before this function exits
     const previousPriorityContext = priorityContext;
     priorityContext = TaskPriority;
 
-    // In each phase, we will attempt to pass errors to boundaries and re-render them.
-    // If we get more errors, we propagate them to higher boundaries in the next iterations.
-
-    while (nextTrappedErrors) {
-      const trappedErrors = nextTrappedErrors;
-      nextTrappedErrors = null;
-
-      // Pass errors to all affected boundaries.
-      const affectedBoundaries : Set<Fiber> = new Set();
-      trappedErrors.forEach(trappedError => {
+    // Keep looping until there are no more trapped errors
+    while (true) {
+      while (nextTrappedErrors && nextTrappedErrors.length) {
+        const trappedError = nextTrappedErrors.shift();
         const boundary = trappedError.boundary;
         const error = trappedError.error;
         if (!boundary) {
           firstUncaughtError = firstUncaughtError || error;
-          return;
+          continue;
         }
         // Don't visit boundaries twice.
-        if (affectedBoundaries.has(boundary)) {
-          return;
+        if (knownBoundaries.has(boundary)) {
+          continue;
         }
-        // Give error boundary a chance to update its state.
+
         try {
+          knownBoundaries.add(boundary);
+          // Give error boundary a chance to update its state.
+          // Updates will be scheduled with Task priority.
           acknowledgeErrorInBoundary(boundary, error);
-          affectedBoundaries.add(boundary);
         } catch (nextError) {
-          // If it throws, propagate the error.
-          nextTrappedErrors = nextTrappedErrors || [];
-          nextTrappedErrors.push(trapError(boundary, nextError));
+          // If an error is thrown, propagate the error to the next boundary
+          const te = trapError(boundary, nextError);
+          if (te.boundary) {
+            // If a boundary is found, push trapped error onto array
+            nextTrappedErrors.push(te);
+          } else {
+            // Otherwise, we'll need to throw
+            firstUncaughtError = firstUncaughtError || te.error;
+          }
         }
-      });
+      }
 
-      // We will process an update caused by each error boundary synchronously.
-      affectedBoundaries.forEach(boundary => {
-        scheduleUpdate(boundary);
-        try {
-          nextUnitOfWork = findNextUnitOfWork();
-          performTaskWorkUnsafe(true);
-        } catch (nextError) {
-          // If it throws, propagate the error.
-          nextTrappedErrors = nextTrappedErrors || [];
-          nextTrappedErrors.push(trapError(boundary, nextError));
+
+      // Now that we attempt to flush any work that was scheduled by the boundaries
+      // If this creates errors, they will be pushed to nextTrappedErrors and the loop will continue
+      try {
+        performTaskWorkUnsafe(true);
+      } catch (error) {
+        const failedUnitOfWork = nextUnitOfWork;
+        // Reset because it points to the error boundary:
+        nextUnitOfWork = null;
+        if (!failedUnitOfWork) {
+          // We shouldn't end up here because nextUnitOfWork
+          // should always be set while work is being performed.
+          throw error;
         }
-      });
-    }
+        const trappedError = trapError(failedUnitOfWork, error);
+        if (nextTrappedErrors) {
+          nextTrappedErrors.push(trappedError);
+        } else {
+          nextTrappedErrors = [trappedError];
+        }
+      }
 
-    ReactCurrentOwner.current = null;
+      if (!nextTrappedErrors || firstUncaughtError) {
+        break;
+      }
+    }
 
+    nextTrappedErrors = null;
+    knownBoundaries = new Set();
     priorityContext = previousPriorityContext;
+    isHandlingErrors = false;
 
-    // Surface the first error uncaught by the boundaries to the user.
     if (firstUncaughtError) {
       // We need to make sure any future root can get scheduled despite these errors.
       // Currently after throwing, nothing gets scheduled because these fields are set.
       // FIXME: this is likely a wrong fix! It's still better than ignoring updates though.
       nextScheduledRoot = null;
       lastScheduledRoot = null;
-
-      // Throw any unhandled errors.
       throw firstUncaughtError;
     }
   }
 
+  type TrappedError = {
+    boundary: Fiber | null,
+    error: any,
+  };
+
+  function findClosestErrorBoundary(fiber : Fiber): Fiber | null {
+    let maybeErrorBoundary = fiber.return;
+    while (maybeErrorBoundary) {
+      if (maybeErrorBoundary.tag === ClassComponent) {
+        const instance = maybeErrorBoundary.stateNode;
+        if (typeof instance.unstable_handleError === 'function' &&
+            !knownBoundaries.has(maybeErrorBoundary)) {
+          return maybeErrorBoundary;
+        }
+      }
+      maybeErrorBoundary = maybeErrorBoundary.return;
+    }
+    return null;
+  }
+
+  function trapError(fiber : Fiber, error : any) : TrappedError {
+    return {
+      boundary: findClosestErrorBoundary(fiber),
+      error,
+    };
+  }
+
+  function acknowledgeErrorInBoundary(boundary : Fiber, error : any) {
+    const instance = boundary.stateNode;
+    instance.unstable_handleError(error);
+  }
+
   function scheduleWork(root : FiberRoot) {
     let priorityLevel = priorityContext;