From e380d6d0fd361a2c5aef951f0a246d43ea68bda0 Mon Sep 17 00:00:00 2001
From: Ahmed El-Helw <ahmedre@fb.com>
Date: Wed, 28 Dec 2016 18:40:40 -0800
Subject: [PATCH] Ignore touch events on layout only Nodes

Summary:
Before this patch, each Node would always generate a node
region, representing the bounds of this particular Node. This set of Nodes was
used when handling touch to figure out whether we should intercept touch (i.e.
a flat Node is catching the draw), or let Android handle touch (i.e. a Node
mounted to a View will handle the touch).

This patch modifies the list of NodeRegions to exclude any Nodes that draw
nothing at all. These Nodes, having no draw output, are effectively layout
containers used to group items, so they shouldn't handle touch.

Reviewed By: sriramramani

Differential Revision: D4369484

fbshipit-source-id: 71b41611873580631f1639f368fa8d971995874f
---
 .../com/facebook/react/flat/FlatShadowNode.java   | 15 +++++++++++++++
 .../com/facebook/react/flat/RCTImageView.java     |  5 +++++
 .../java/com/facebook/react/flat/RCTText.java     |  7 +++++++
 .../java/com/facebook/react/flat/RCTView.java     |  5 +++++
 .../com/facebook/react/flat/StateBuilder.java     |  4 +++-
 5 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatShadowNode.java b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatShadowNode.java
index 2152cc7cb9a..e8ec9afc5a4 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatShadowNode.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatShadowNode.java
@@ -144,6 +144,21 @@ import com.facebook.react.uimanager.ReactClippingViewGroupHelper;
     }
   }
 
+  /**
+   * Return whether or not this node draws anything
+   *
+   * This is used to decide whether or not to collect the NodeRegion for this node. This ensures
+   * that any FlatShadowNode that does not emit any DrawCommands should not bother handling touch
+   * (i.e. if it draws absolutely nothing, it is, for all intents and purposes, a layout only node).
+   *
+   * @return whether or not this is node draws anything
+   */
+  boolean doesDraw() {
+    // if it mounts to view or draws a background, we can collect it - otherwise, no, unless a
+    // child suggests some alternative behavior
+    return mDrawView != null || mDrawBackground != null;
+  }
+
   @ReactProp(name = ViewProps.BACKGROUND_COLOR)
   public void setBackgroundColor(int backgroundColor) {
     mDrawBackground = (backgroundColor == 0) ? null : new DrawBackgroundColor(backgroundColor);
diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTImageView.java b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTImageView.java
index 3330d321d2d..592d83fed16 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTImageView.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTImageView.java
@@ -81,6 +81,11 @@ import com.facebook.react.views.image.ImageResizeMode;
     }
   }
 
+  @Override
+  boolean doesDraw() {
+    return mDrawImage.hasImageRequest() || super.doesDraw();
+  }
+
   @ReactProp(name = "shouldNotifyLoadEvents")
   public void setShouldNotifyLoadEvents(boolean shouldNotifyLoadEvents) {
     getMutableDrawImage().setReactTag(shouldNotifyLoadEvents ? getReactTag() : 0);
diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTText.java b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTText.java
index 9a264451c50..fb97bdeab59 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTText.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTText.java
@@ -198,6 +198,13 @@ import com.facebook.react.uimanager.annotations.ReactProp;
     performCollectAttachDetachListeners(stateBuilder);
   }
 
+  @Override
+  boolean doesDraw() {
+    // assume text always draws - this is a performance optimization to avoid having to
+    // getText() when layout was skipped and when collectState wasn't yet called
+    return true;
+  }
+
   @ReactProp(name = ViewProps.LINE_HEIGHT, defaultDouble = Double.NaN)
   public void setLineHeight(double lineHeight) {
     if (Double.isNaN(lineHeight)) {
diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTView.java b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTView.java
index 2d2a9ff4eec..547fe5f1838 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTView.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTView.java
@@ -85,6 +85,11 @@ import com.facebook.react.uimanager.annotations.ReactPropGroup;
     }
   }
 
+  @Override
+  boolean doesDraw() {
+    return mDrawBorder != null || super.doesDraw();
+  }
+
   @Override
   public void setBackgroundColor(int backgroundColor) {
     getMutableBorder().setBackgroundColor(backgroundColor);
diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java b/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java
index a8bde56f41c..0060022057c 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java
@@ -261,7 +261,9 @@ import com.facebook.react.uimanager.events.EventDispatcher;
     }
 
     node.updateNodeRegion(left, top, right, bottom, isVirtual);
-    mNodeRegions.add(node.getNodeRegion());
+    if (node.doesDraw()) {
+      mNodeRegions.add(node.getNodeRegion());
+    }
   }
 
   /**