Early ViewCommand dispatch: retry exactly once, log soft errors in on…

…e place

Summary:
As a followup to T63997094, I think it is better to attempt to execute early once, and then to execute the ViewCommand after the current batch of mount instructions if that fails. If both fail, then log a soft exception.

This is to support the use-case here, when ScrollView.scrollToEnd is called during the render pass, before any Views are mounted on the screen: https://our.intern.facebook.com/intern/diffusion/FBS/browse/master/xplat/js/RKJSModules/Apps/Profile/ProfileEdit/ui/featured_highlights_migration/ProfileEditFeaturedHighlightsMigrationProfilePreview.js?commit=75f66a9077abb81b7797079b567df759dd023a79&lines=221-229

Changelog: [Internal] Fix for dispatching ViewCommands early

Reviewed By: mdvacca

Differential Revision: D20478681

fbshipit-source-id: 052b3ecf4913a0096f590637faf0f68a072e2427

ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java

@@ -24,7 +24,6 @@
 import com.facebook.react.bridge.Callback;
 import com.facebook.react.bridge.JSApplicationIllegalArgumentException;
 import com.facebook.react.bridge.ReactContext;
-import com.facebook.react.bridge.ReactSoftException;
 import com.facebook.react.bridge.ReadableArray;
 import com.facebook.react.bridge.ReadableMap;
 import com.facebook.react.bridge.SoftAssertions;
@@ -766,13 +765,12 @@ public synchronized void dispatchCommand(
     UiThreadUtil.assertOnUiThread();
     View view = mTagsToViews.get(reactTag);
     if (view == null) {
-      ReactSoftException.logSoftException(
-          TAG,
-          new IllegalViewOperationException(
-              "Trying to send command to a non-existing view " + "with tag " + reactTag));
-      return;
+      throw new IllegalViewOperationException(
+          "Trying to send command to a non-existing view with tag ["
+              + reactTag
+              + "] and command "
+              + commandId);
     }
-
     ViewManager viewManager = resolveViewManager(reactTag);
     viewManager.receiveCommand(view, commandId, args);
   }
@@ -782,13 +780,12 @@ public synchronized void dispatchCommand(
     UiThreadUtil.assertOnUiThread();
     View view = mTagsToViews.get(reactTag);
     if (view == null) {
-      ReactSoftException.logSoftException(
-          TAG,
-          new IllegalViewOperationException(
-              "Trying to send command to a non-existing view " + "with tag " + reactTag));
-      return;
+      throw new IllegalViewOperationException(
+          "Trying to send command to a non-existing view with tag ["
+              + reactTag
+              + "] and command "
+              + commandId);
     }
-
     ViewManager viewManager = resolveViewManager(reactTag);
     viewManager.receiveCommand(view, commandId, args);
   }

ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java

@@ -11,11 +11,13 @@
 import android.view.View;
 import androidx.annotation.GuardedBy;
 import androidx.annotation.Nullable;
+import androidx.annotation.UiThread;
 import com.facebook.common.logging.FLog;
 import com.facebook.react.bridge.Callback;
 import com.facebook.react.bridge.GuardedRunnable;
 import com.facebook.react.bridge.ReactApplicationContext;
 import com.facebook.react.bridge.ReactContext;
+import com.facebook.react.bridge.ReactSoftException;
 import com.facebook.react.bridge.ReadableArray;
 import com.facebook.react.bridge.ReadableMap;
 import com.facebook.react.bridge.SoftAssertions;
@@ -44,6 +46,7 @@
 public class UIViewOperationQueue {
 
   public static final int DEFAULT_MIN_TIME_LEFT_IN_FRAME_FOR_NONBATCHED_OPERATION_MS = 8;
+  private static final String TAG = UIViewOperationQueue.class.getSimpleName();
 
   private final int[] mMeasureBuffer = new int[4];
 
@@ -260,12 +263,35 @@ public void execute() {
     }
   }
 
+  /**
+   * This is a common interface for View Command operations. Once we delete the deprecated {@link
+   * DispatchCommandOperation}, we can delete this interface too. It provides a set of common
+   * operations to simplify generic operations on all types of ViewCommands.
+   */
+  private interface DispatchCommandViewOperation {
+
+    /**
+     * Like the execute function, but throws real exceptions instead of logging soft errors and
+     * returning silently.
+     */
+    void executeWithExceptions();
+
+    /** Increment retry counter. */
+    void incrementRetries();
+
+    /** Get retry counter. */
+    int getRetries();
+  }
+
   @Deprecated
-  private final class DispatchCommandOperation extends ViewOperation {
+  private final class DispatchCommandOperation extends ViewOperation
+      implements DispatchCommandViewOperation {
 
     private final int mCommand;
     private final @Nullable ReadableArray mArgs;
 
+    private int numRetries = 0;
+
     public DispatchCommandOperation(int tag, int command, @Nullable ReadableArray args) {
       super(tag);
       mCommand = command;
@@ -274,14 +300,38 @@ public DispatchCommandOperation(int tag, int command, @Nullable ReadableArray ar
 
     @Override
     public void execute() {
+      try {
+        mNativeViewHierarchyManager.dispatchCommand(mTag, mCommand, mArgs);
+      } catch (Throwable e) {
+        ReactSoftException.logSoftException(
+            TAG, new RuntimeException("Error dispatching View Command", e));
+      }
+    }
+
+    @Override
+    public void executeWithExceptions() {
       mNativeViewHierarchyManager.dispatchCommand(mTag, mCommand, mArgs);
     }
+
+    @Override
+    @UiThread
+    public void incrementRetries() {
+      numRetries++;
+    }
+
+    @Override
+    @UiThread
+    public int getRetries() {
+      return numRetries;
+    }
   }
 
-  private final class DispatchStringCommandOperation extends ViewOperation {
+  private final class DispatchStringCommandOperation extends ViewOperation
+      implements DispatchCommandViewOperation {
 
     private final String mCommand;
     private final @Nullable ReadableArray mArgs;
+    private int numRetries = 0;
 
     public DispatchStringCommandOperation(int tag, String command, @Nullable ReadableArray args) {
       super(tag);
@@ -291,8 +341,30 @@ public DispatchStringCommandOperation(int tag, String command, @Nullable Readabl
 
     @Override
     public void execute() {
+      try {
+        mNativeViewHierarchyManager.dispatchCommand(mTag, mCommand, mArgs);
+      } catch (Throwable e) {
+        ReactSoftException.logSoftException(
+            TAG, new RuntimeException("Error dispatching View Command", e));
+      }
+    }
+
+    @Override
+    @UiThread
+    public void executeWithExceptions() {
       mNativeViewHierarchyManager.dispatchCommand(mTag, mCommand, mArgs);
     }
+
+    @Override
+    @UiThread
+    public void incrementRetries() {
+      numRetries++;
+    }
+
+    @Override
+    public int getRetries() {
+      return numRetries;
+    }
   }
 
   private final class ShowPopupMenuOperation extends ViewOperation {
@@ -520,7 +592,7 @@ public void execute() {
   private final ReactApplicationContext mReactApplicationContext;
 
   private final boolean mAllowViewCommandsQueue;
-  private ArrayList<UIOperation> mViewCommandOperations = new ArrayList<>();
+  private ArrayList<DispatchCommandViewOperation> mViewCommandOperations = new ArrayList<>();
 
   // Only called from the UIManager queue?
   private ArrayList<UIOperation> mOperations = new ArrayList<>();
@@ -759,7 +831,7 @@ public void dispatchViewUpdates(
 
       // Store the current operation queues to dispatch and create new empty ones to continue
       // receiving new operations
-      final ArrayList<UIOperation> viewCommandOperations;
+      final ArrayList<DispatchCommandViewOperation> viewCommandOperations;
       if (!mViewCommandOperations.isEmpty()) {
         viewCommandOperations = mViewCommandOperations;
         mViewCommandOperations = new ArrayList<>();
@@ -799,10 +871,35 @@ public void run() {
               try {
                 long runStartTime = SystemClock.uptimeMillis();
 
-                // All ViewCommands should be executed first as a perf optimization
+                // All ViewCommands should be executed first as a perf optimization.
+                // This entire block is only executed if there's a separate viewCommand queue,
+                // which is currently gated by a ReactFeatureFlag.
                 if (viewCommandOperations != null) {
-                  for (UIOperation viewCommandOp : viewCommandOperations) {
-                    viewCommandOp.execute();
+                  for (DispatchCommandViewOperation op : viewCommandOperations) {
+                    try {
+                      op.executeWithExceptions();
+                    } catch (Throwable e) {
+                      // Catch errors in DispatchCommands. We allow all commands to be retried
+                      // exactly
+                      // once, after the current batch of other mountitems. If the second attempt
+                      // fails,
+                      // then  we log a soft error. This will still crash only in debug.
+                      // We do this because it is a ~relatively common pattern to dispatch a command
+                      // during render, for example, to scroll to the bottom of a ScrollView in
+                      // render.
+                      // This dispatches the command before that View is even mounted. By retrying
+                      // once,
+                      // we can still dispatch the vast majority of commands faster, avoid errors,
+                      // and
+                      // still operate correctly for most commands even when they're executed too
+                      // soon.
+                      if (op.getRetries() == 0) {
+                        op.incrementRetries();
+                        mViewCommandOperations.add(op);
+                      } else {
+                        ReactSoftException.logSoftException(TAG, e);
+                      }
+                    }
                   }
                 }