Fix for ViewManager commands being run before view updates.

Summary: Since Nodes' manageChildren doesn't enqueue the child updates immediately, commands were being directed to non-updated views.  Previously we applied updates for the shadow node before dispatching the command, but we can instead wait to fire commands until after we update the view hierarchy.

Reviewed By: ahmedre

Differential Revision: D3568541

ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIImplementation.java

@@ -71,7 +71,7 @@ public class FlatUIImplementation extends UIImplementation {
   /**
    * Helper class that sorts moveTo/moveFrom arrays passed to #manageChildren().
    * Not used outside of the said method.
-   */ 
+   */
   private final MoveProxy mMoveProxy = new MoveProxy();
   private final StateBuilder mStateBuilder;
   private @Nullable ReactImageManager mReactImageManager;
@@ -224,14 +224,13 @@ public class FlatUIImplementation extends UIImplementation {
         callback);
   }
 
-  private boolean ensureMountsToViewAndBackingViewIsCreated(int reactTag) {
+  private void ensureMountsToViewAndBackingViewIsCreated(int reactTag) {
     FlatShadowNode node = (FlatShadowNode) resolveShadowNode(reactTag);
     if (node.isBackingViewCreated()) {
-      return false;
+      return;
     }
     node.forceMountToView();
     mStateBuilder.ensureBackingViewIsCreated(node);
-    return true;
   }
 
   @Override
@@ -253,16 +252,10 @@ public class FlatUIImplementation extends UIImplementation {
 
   @Override
   public void dispatchViewManagerCommand(int reactTag, int commandId, ReadableArray commandArgs) {
-    if (ensureMountsToViewAndBackingViewIsCreated(reactTag)) {
-      // need to make sure any ui operations (UpdateViewGroup, for example, etc) have already
-      // happened before we actually dispatch the view manager command (since otherwise, the command
-      // may go to an empty shell parent without its children, which is against the specs). note
-      // that we only want to applyUpdates if the view has not yet been created so that it does
-      // get created (otherwise, we may end up changing the View's position when we're not supposed
-      // to, for example).
-      mStateBuilder.applyUpdates((FlatShadowNode) resolveShadowNode(reactTag));
-    }
-    super.dispatchViewManagerCommand(reactTag, commandId, commandArgs);
+    // Make sure that our target view is actually a view, then delay command dispatch until after
+    // we have updated the view hierarchy.
+    ensureMountsToViewAndBackingViewIsCreated(reactTag);
+    mStateBuilder.enqueueViewManagerCommand(reactTag, commandId, commandArgs);
   }
 
   @Override
@@ -414,7 +407,7 @@ public class FlatUIImplementation extends UIImplementation {
 
         ++addToIndex;
         if (addToIndex == numNodesToAdd) {
-          addToChildIndex = Integer.MAX_VALUE;  
+          addToChildIndex = Integer.MAX_VALUE;
         } else {
           addToChildIndex = addAtIndices.getInt(addToIndex);
         }

ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIViewOperationQueue.java

@@ -17,6 +17,7 @@ import android.view.ViewGroup;
 
 import com.facebook.react.bridge.Callback;
 import com.facebook.react.bridge.ReactApplicationContext;
+import com.facebook.react.bridge.ReadableArray;
 import com.facebook.react.uimanager.IllegalViewOperationException;
 import com.facebook.react.uimanager.NoSuchNativeViewException;
 import com.facebook.react.uimanager.PixelUtil;
@@ -314,6 +315,31 @@ import com.facebook.react.uimanager.UIViewOperationQueue;
     }
   }
 
+  /**
+   * Used to delay view manager command dispatch until after the view hierarchy is updated.
+   * Mirrors command operation dispatch, but is only used in Nodes for view manager commands.
+   */
+  public final class ViewManagerCommand implements UIOperation {
+
+    private final int mReactTag;
+    private final int mCommand;
+    private final @Nullable ReadableArray mArgs;
+
+    public ViewManagerCommand(
+        int reactTag,
+        int command,
+        @Nullable ReadableArray args) {
+      mReactTag = reactTag;
+      mCommand = command;
+      mArgs = args;
+    }
+
+    @Override
+    public void execute() {
+      mNativeViewHierarchyManager.dispatchCommand(mReactTag, mCommand, mArgs);
+    }
+  }
+
   public FlatUIViewOperationQueue(
       ReactApplicationContext reactContext,
       FlatNativeViewHierarchyManager nativeViewHierarchyManager) {
@@ -359,6 +385,17 @@ import com.facebook.react.uimanager.UIViewOperationQueue;
     enqueueUIOperation(updateViewBounds);
   }
 
+  public ViewManagerCommand createViewManagerCommand(
+      int reactTag,
+      int command,
+      @Nullable ReadableArray args) {
+    return new ViewManagerCommand(reactTag, command, args);
+  }
+
+  public void enqueueViewManagerCommand(ViewManagerCommand viewManagerCommand) {
+    enqueueUIOperation(viewManagerCommand);
+  }
+
   public void enqueueSetPadding(
       int reactTag,
       int paddingLeft,

ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java

@@ -14,6 +14,7 @@ import javax.annotation.Nullable;
 import java.util.ArrayList;
 
 import com.facebook.csslayout.Spacing;
+import com.facebook.react.bridge.ReadableArray;
 import com.facebook.react.uimanager.OnLayoutEvent;
 import com.facebook.react.uimanager.ReactShadowNode;
 import com.facebook.react.uimanager.ReactStylesDiffMap;
@@ -48,6 +49,8 @@ import com.facebook.react.uimanager.events.EventDispatcher;
   private final ArrayList<OnLayoutEvent> mOnLayoutEvents = new ArrayList<>();
   private final ArrayList<FlatUIViewOperationQueue.UpdateViewBounds> mUpdateViewBoundsOperations =
       new ArrayList<>();
+  private final ArrayList<FlatUIViewOperationQueue.ViewManagerCommand> mViewManagerCommands =
+      new ArrayList<>();
 
   private @Nullable FlatUIViewOperationQueue.DetachAllChildrenFromViews mDetachAllChildrenFromViews;
 
@@ -99,6 +102,14 @@ import com.facebook.react.uimanager.events.EventDispatcher;
     }
     mUpdateViewBoundsOperations.clear();
 
+    // Process view manager commands after bounds operations, so that any ui operations have already
+    // happened before we actually dispatch the view manager command.  This prevents things like
+    // commands going to empty parents and views not yet being created.
+    for (int i = 0, size = mViewManagerCommands.size(); i != size; i++) {
+      mOperationsQueue.enqueueViewManagerCommand(mViewManagerCommands.get(i));
+    }
+    mViewManagerCommands.clear();
+
     // This could be more efficient if EventDispatcher had a batch mode
     // to avoid multiple synchronized calls.
     for (int i = 0, size = mOnLayoutEvents.size(); i != size; ++i) {
@@ -134,6 +145,14 @@ import com.facebook.react.uimanager.events.EventDispatcher;
     mAttachDetachListeners.add(listener);
   }
 
+  /* package */ void enqueueViewManagerCommand(
+      int reactTag,
+      int commandId,
+      ReadableArray commandArgs) {
+    mViewManagerCommands.add(
+        mOperationsQueue.createViewManagerCommand(reactTag, commandId, commandArgs));
+  }
+
   /* package */ void enqueueCreateOrUpdateView(
       FlatShadowNode node,
       @Nullable ReactStylesDiffMap styles) {