Ensure that priority is not dropped from a progressed subtree

Must bubble up priority from a progressed subtree that was preempted
by a high priority update. We know this is the case if the progressed
priority is lower than the priority at which we're reconciling.

Author: acdlite

scripts/fiber/tests-passing.txt

@@ -1236,6 +1236,7 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js
 * updates a child even though the old props is empty
 * can defer side-effects and resume them later on
 * can defer side-effects and reuse them later - complex
+* does not drop priority from a progressed subtree
 * deprioritizes setStates that happens within a deprioritized tree
 * calls callback after update is flushed
 * calls setState callback even if component bails out

src/renderers/shared/fiber/ReactFiberBeginWork.js

@@ -629,6 +629,7 @@ module.exports = function<T, P, I, TI, PI, C, CX>(
     if (workInProgress.progressedPriority === priorityLevel) {
       // If we have progressed work on this priority level already, we can
       // proceed with that as the child.
+      workInProgress.pendingWorkPriority = priorityLevel;
       workInProgress.child = workInProgress.progressedChild;
     }
 

src/renderers/shared/fiber/ReactFiberCompleteWork.js

@@ -72,8 +72,22 @@ module.exports = function<T, P, I, TI, PI, C, CX>(
     popHostContainer,
   } = hostContext;
 
-  function resetWorkPriority(workInProgress : Fiber) {
-    let newPriority = NoWork;
+  function resetWorkPriority(workInProgress : Fiber, priorityLevel : PriorityLevel) {
+    // priorityLevel is the level we're currently reconciling at. It's called
+    // nextPriorityLevel in the scheduler. Can't think of a name that's
+    // not confusing.
+
+    // If the progressedPriority is less than the priority we're currently
+    // reconciling at, this was a bailout. Set the work priority to the
+    // progressed priority, otherwise reset it to NoWork.
+    let newPriority;
+    if (workInProgress.progressedPriority === NoWork ||
+        (workInProgress.progressedPriority > priorityLevel) && priorityLevel !== NoWork) {
+      newPriority = workInProgress.progressedPriority;
+    } else {
+      newPriority = NoWork;
+    }
+
     // progressedChild is going to be the child set with the highest priority.
     // Either it is the same as child, or it just bailed out because it choose
     // not to do the work.
@@ -196,12 +210,12 @@ module.exports = function<T, P, I, TI, PI, C, CX>(
 
     switch (workInProgress.tag) {
       case FunctionalComponent:
-        resetWorkPriority(workInProgress);
+        resetWorkPriority(workInProgress, priorityLevel);
         return null;
       case ClassComponent: {
         // We are leaving this subtree, so pop context if any.
         popContextProvider(workInProgress);
-        resetWorkPriority(workInProgress);
+        resetWorkPriority(workInProgress, priorityLevel);
         return null;
       }
       case HostRoot: {
@@ -211,7 +225,7 @@ module.exports = function<T, P, I, TI, PI, C, CX>(
           fiberRoot.context = fiberRoot.pendingContext;
           fiberRoot.pendingContext = null;
         }
-        resetWorkPriority(workInProgress);
+        resetWorkPriority(workInProgress, priorityLevel);
         return null;
       }
       case HostComponent:
@@ -278,7 +292,7 @@ module.exports = function<T, P, I, TI, PI, C, CX>(
           // offscreen priority, there's remaining work in the subtree.
           workInProgress.pendingWorkPriority = OffscreenPriority;
         } else {
-          resetWorkPriority(workInProgress);
+          resetWorkPriority(workInProgress, priorityLevel);
         }
         return null;
       case HostText:
@@ -304,29 +318,29 @@ module.exports = function<T, P, I, TI, PI, C, CX>(
           const textInstance = createTextInstance(newText, rootContainerInstance, currentHostContext, workInProgress);
           workInProgress.stateNode = textInstance;
         }
-        resetWorkPriority(workInProgress);
+        resetWorkPriority(workInProgress, priorityLevel);
         return null;
       case CoroutineComponent: {
         const next = moveCoroutineToHandlerPhase(current, workInProgress);
-        resetWorkPriority(workInProgress);
+        resetWorkPriority(workInProgress, priorityLevel);
         return next;
       }
       case CoroutineHandlerPhase:
         // Reset the tag to now be a first phase coroutine.
         workInProgress.tag = CoroutineComponent;
-        resetWorkPriority(workInProgress);
+        resetWorkPriority(workInProgress, priorityLevel);
         return null;
       case YieldComponent:
         // Does nothing.
         return null;
       case Fragment:
-        resetWorkPriority(workInProgress);
+        resetWorkPriority(workInProgress, priorityLevel);
         return null;
       case HostPortal:
         // TODO: Only mark this as an update if we have any pending callbacks.
         markUpdate(workInProgress);
         popHostContainer(workInProgress);
-        resetWorkPriority(workInProgress);
+        resetWorkPriority(workInProgress, priorityLevel);
         return null;
 
       // Error cases

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

@@ -756,6 +756,94 @@ describe('ReactIncrementalSideEffects', () => {
     expect(ops).toEqual(['Bar', 'Baz', 'Bar', 'Bar']);
   });
 
+  it('does not drop priority from a progressed subtree', () => {
+    let ops = [];
+    let lowPri;
+    let highPri;
+
+    function LowPriDidComplete() {
+      ops.push('LowPriDidComplete');
+      // Because this is terminal, beginning work on LowPriDidComplete implies
+      // that LowPri will be completed before the scheduler yields.
+      return null;
+    }
+
+    class LowPri extends React.Component {
+      state = { step: 0 };
+      render() {
+        ops.push('LowPri');
+        lowPri = this;
+        return [
+          <span prop={this.state.step} />,
+          <LowPriDidComplete />,
+        ];
+      }
+    }
+
+    function LowPriSibling() {
+      ops.push('LowPriSibling');
+      return null;
+    }
+
+    class HighPri extends React.Component {
+      state = { step: 0 };
+      render() {
+        ops.push('HighPri');
+        highPri = this;
+        return <span prop={this.state.step} />;
+      }
+    }
+
+    function App() {
+      ops.push('App');
+      return [
+        <div>
+          <LowPri />
+          <LowPriSibling />
+        </div>,
+        <div><HighPri /></div>,
+      ];
+    }
+
+    ReactNoop.render(<App />);
+    ReactNoop.flush();
+    expect(ReactNoop.getChildren()).toEqual([
+      div(span(0)),
+      div(span(0)),
+    ]);
+    ops = [];
+
+    lowPri.setState({ step: 1 });
+    // Do just enough work to begin LowPri
+    ReactNoop.flushDeferredPri(30);
+    expect(ops).toEqual(['LowPri']);
+    // Now we'll do one more tick of work to complete LowPri. Because LowPri
+    // has a sibling, the parent div of LowPri has not yet completed.
+    ReactNoop.flushDeferredPri(10);
+    expect(ops).toEqual(['LowPri', 'LowPriDidComplete']);
+    expect(ReactNoop.getChildren()).toEqual([
+      div(span(0)), // Complete, but not yet updated
+      div(span(0)),
+    ]);
+    ops = [];
+
+    // Interrupt with high pri update
+    ReactNoop.performAnimationWork(() => highPri.setState({ step: 1 }));
+    ReactNoop.flushAnimationPri();
+    expect(ops).toEqual(['HighPri']);
+    expect(ReactNoop.getChildren()).toEqual([
+      div(span(0)), // Completed, but not yet updated
+      div(span(1)),
+    ]);
+    ops = [];
+
+    ReactNoop.flush();
+    expect(ReactNoop.getChildren()).toEqual([
+      div(span(1)),
+      div(span(1)),
+    ]);
+  });
+
   it('deprioritizes setStates that happens within a deprioritized tree', () => {
     var ops = [];