Avoid "previous current" work-in-progress scenario

It's possible for a work-in-progress fiber to be the most progressed
work (last fiber whose children were reconciled) but be older than
the current commit. This scenario, where the work-in-progress fiber
is really the "previous current," happens after a bailout. To avoid,
in addition to keeping track of the most progressed fiber, we also
keep track of the most recent fiber to enter the begin phase,
regardless of whether it bailed out. Then we only resume on the work-
in-progress if its the most recent fiber.

It'd be nice to come up with a heuristic here that didn't require an
additional fiber field.

Also, the naming of "progressedWork" versus "newestWork" is not great.
Maybe "lastUpdatedWork" and "lastUpdatedOrBailedOutWork"? We can
bikeshed later.

src/renderers/shared/fiber/ReactFiber.js

@@ -154,6 +154,7 @@ export type Fiber = {
   // progressed work or if it is better to continue from the current state.
   progressedPriority: PriorityLevel,
   progressedWork: ProgressedWork,
+  newestWork: Fiber,
 
   // This is a pooled version of a Fiber. Every fiber that gets updated will
   // eventually have a pair. There are cases when we can clean up pairs to save
@@ -244,11 +245,13 @@ var createFiber = function(
     // TODO: Express this circular reference properly
     progressedWork: (null: any),
     progressedPriority: NoWork,
+    newestWork: (null: any),
 
     alternate: null,
   };
 
   fiber.progressedWork = fiber;
+  fiber.newestWork = fiber;
 
   if (__DEV__) {
     fiber._debugID = debugCounter++;
@@ -301,6 +304,7 @@ exports.createWorkInProgress = function(
 
     workInProgress.progressedPriority = current.progressedPriority;
     workInProgress.progressedWork = current.progressedWork;
+    workInProgress.newestWork = current.newestWork;
 
     // Clone child from current.
     workInProgress.child = current.child;

src/renderers/shared/fiber/ReactFiberBeginWork.js

@@ -901,6 +901,15 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
     workInProgress.memoizedProps = nextProps;
     workInProgress.memoizedState = nextState;
 
+    // Mark this as the newest work. This is not the same as progressed work,
+    // which only includes re-renders/updates. Keeping track of the lastest
+    // update OR bailout lets us make sure that we don't mistake a "previous
+    // current" fiber for a fresh work-in-progress.
+    workInProgress.newestWork = workInProgress;
+    if (current !== null) {
+      current.newestWork = workInProgress;
+    }
+
     // If the child is null, this is terminal. The work is done.
     if (workInProgress.child === null) {
       return null;
@@ -917,8 +926,8 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
       // The children do not have sufficient priority. We should skip the
       // children. If they have low-pri work, we'll come back to them later.
 
-      // Before exiting, we need to check if we have progressed work.
-      if (current === null || workInProgress.child !== current.child) {
+      // Before exiting, we need to check if this work is the progressed work
+      if (workInProgress === progressedWork) {
         if (workInProgress.progressedPriority === renderPriority) {
           // We have progressed work that completed at this level. Because the
           // remaining priority (pendingWorkPriority) is less than the priority
@@ -957,7 +966,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
     // continue working on it.
 
     // Check to see if we have progressed work since the last commit.
-    if (progressedWork !== current) {
+    if (current === null || workInProgress.child !== current.child) {
       // We already have progressed work. We can reuse the children. But we
       // need to reset the return fiber since we'll traverse down into them.
       let child = workInProgress.child;
@@ -1081,10 +1090,12 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
     // Keep track of the priority at which this work was performed.
     workInProgress.progressedPriority = renderPriority;
     workInProgress.progressedWork = workInProgress;
+    workInProgress.newestWork = workInProgress;
     if (current !== null) {
       // Set the progressed work on both fibers
       current.progressedPriority = renderPriority;
       current.progressedWork = workInProgress;
+      current.newestWork = workInProgress;
     }
   }
 
@@ -1154,7 +1165,29 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
   ): void {
     const progressedPriority = workInProgress.progressedPriority;
     const progressedWork = workInProgress.progressedWork;
-    if (progressedPriority === renderPriority && progressedWork !== current) {
+    const newestWork = workInProgress.newestWork;
+
+    if (
+      // Only resume on the progressed work if it rendered at the
+      // same priority.
+      progressedPriority === renderPriority &&
+      // Don't resume on current; use the reset path below.
+      progressedWork !== current &&
+      // It's possible for a work-in-progress fiber to be the most progressed
+      // work (last fiber whose children were reconciled) but be older than
+      // the current commit. This scenario, where the work-in-progress fiber
+      // is really the "previous current," happens after a bailout. To avoid,
+      // in addition to keeping track of the most progressed fiber, we also
+      // keep track of the most recent fiber to enter the begin phase,
+      // regardless of whether it bailed out. Then we only resume on the work-
+      // in-progress if its the most recent fiber.
+      // TODO: It'd be nice to come up with a heuristic here that didn't require
+      // an additional fiber field.
+      !(progressedWork === workInProgress && workInProgress !== newestWork)
+      // If the progressed work is not the current or the work-in-progress
+      // fiber, then it must point to a forked ProgressedWork object, which
+      // we can resume on.
+    ) {
       // We have progressed work at this priority. Reuse it.
       return resumeAlreadyProgressedWork(workInProgress, progressedWork);
     }

src/renderers/shared/fiber/__tests__/ReactIncremental-test.js

@@ -183,7 +183,7 @@ describe('ReactIncremental', () => {
     expect(ops).toEqual(['Foo', 'Bar', 'Bar']);
   });
 
-  xit('should call callbacks even if updates are aborted', () => {
+  it('should call callbacks even if updates are aborted', () => {
     const ops = [];
     let inst;
 
@@ -346,7 +346,7 @@ describe('ReactIncremental', () => {
     expect(ops).toEqual(['Middle', 'Middle']);
   });
 
-  xit('can resume work in a subtree even when a parent bails out', () => {
+  it('can resume work in a subtree even when a parent bails out', () => {
     var ops = [];
 
     function Bar(props) {
@@ -871,7 +871,7 @@ describe('ReactIncremental', () => {
     expect(ops).toEqual(['Middle']);
   });
 
-  xit('memoizes work even if shouldComponentUpdate returns false', () => {
+  it('memoizes work even if shouldComponentUpdate returns false', () => {
     let ops = [];
     class Foo extends React.Component {
       shouldComponentUpdate(nextProps) {
@@ -1949,7 +1949,7 @@ describe('ReactIncremental', () => {
     ]);
   });
 
-  xit('provides context when reusing work', () => {
+  it('provides context when reusing work', () => {
     var ops = [];
 
     class Intl extends React.Component {
@@ -2006,7 +2006,7 @@ describe('ReactIncremental', () => {
     ]);
   });
 
-  xit('reads context when setState is below the provider', () => {
+  it('reads context when setState is below the provider', () => {
     var ops = [];
     var statefulInst;
 
@@ -2096,7 +2096,7 @@ describe('ReactIncremental', () => {
     expect(ops).toEqual([]);
   });
 
-  xit('reads context when setState is above the provider', () => {
+  it('reads context when setState is above the provider', () => {
     var ops = [];
     var statefulInst;
 
@@ -2199,7 +2199,7 @@ describe('ReactIncremental', () => {
     ]);
   });
 
-  xit('maintains the correct context when providers bail out due to low priority', () => {
+  it('maintains the correct context when providers bail out due to low priority', () => {
     class Root extends React.Component {
       render() {
         return <Middle {...this.props} />;
@@ -2242,7 +2242,7 @@ describe('ReactIncremental', () => {
     ReactNoop.flush();
   });
 
-  xit('maintains the correct context when unwinding due to an error in render', () => {
+  it('maintains the correct context when unwinding due to an error in render', () => {
     class Root extends React.Component {
       unstable_handleError(error) {
         // If context is pushed/popped correctly,
@@ -2289,7 +2289,7 @@ describe('ReactIncremental', () => {
     ReactNoop.flush();
   });
 
-  xit('should not recreate masked context unless inputs have changed', () => {
+  it('should not recreate masked context unless inputs have changed', () => {
     const ops = [];
 
     let scuCounter = 0;
@@ -2335,7 +2335,7 @@ describe('ReactIncremental', () => {
     ]);
   });
 
-  xit('should reuse memoized work if pointers are updated before calling lifecycles', () => {
+  it('should reuse memoized work if pointers are updated before calling lifecycles', () => {
     let cduNextProps = [];
     let cduPrevProps = [];
     let scuNextProps = [];