Remove SizeMonitoringFrameLayout

Summary: SizeMonitoringFrameLayout was used to set layout contraints on the root shadow node when the native view's size changes. Since then, we introduced ways for the root node to use proper layout contraints using the root view's measure specs, which provides more accurate constraints for the root node, so SizeMonitoringFrameLayout is no longer needed. This ends up making a lot of UIManagerModule's method signatures cleaner

Reviewed By: mdvacca

Differential Revision: D9565720

fbshipit-source-id: c569cd15991a09987cc01e89612dc9193ad99b45

ReactAndroid/src/androidTest/java/com/facebook/react/testing/rule/ReactNativeTestRule.java

@@ -173,4 +173,8 @@ public class ReactNativeTestRule implements TestRule {
   public ReactRootView getView() {
     return mView;
   }
+
+  public ReactContext getContext() {
+    return mReactInstanceManager.getCurrentReactContext();
+  }
 }

ReactAndroid/src/androidTest/java/com/facebook/react/tests/core/ReactRootViewTest.java

@@ -10,17 +10,20 @@ import static org.fest.assertions.api.Assertions.assertThat;
 import android.app.Instrumentation;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.runner.AndroidJUnit4;
+import android.util.DisplayMetrics;
 import android.view.View;
 import android.view.ViewGroup;
 import android.widget.FrameLayout;
 import com.facebook.react.ReactPackage;
 import com.facebook.react.ReactRootView;
+import com.facebook.react.bridge.JavaScriptModule;
 import com.facebook.react.bridge.ModuleSpec;
 import com.facebook.react.bridge.NativeModule;
 import com.facebook.react.bridge.ReactApplicationContext;
 import com.facebook.react.shell.MainReactPackage;
 import com.facebook.react.testing.StringRecordingModule;
 import com.facebook.react.testing.rule.ReactNativeTestRule;
+import com.facebook.react.uimanager.PixelUtil;
 import java.util.ArrayList;
 import java.util.List;
 import javax.inject.Provider;
@@ -32,6 +35,10 @@ import org.junit.runner.RunWith;
 @RunWith(AndroidJUnit4.class)
 public class ReactRootViewTest {
 
+  private interface ReactRootViewTestModule extends JavaScriptModule {
+    void setHeight(int height);
+  }
+
   final StringRecordingModule mRecordingModule = new StringRecordingModule();
   final ReactPackage mReactPackage = new MainReactPackage() {
     @Override
@@ -85,6 +92,40 @@ public class ReactRootViewTest {
     assertThat(newWidth).isEqualTo(childView.getWidth());
   }
 
+  @Test
+  public void testRootViewWrapContent() {
+    final ReactRootView rootView = mReactNativeRule.getView();
+
+    Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation();
+    instrumentation.runOnMainSync(
+      new Runnable() {
+        @Override
+        public void run() {
+          rootView.setLayoutParams(
+              new FrameLayout.LayoutParams(
+                  ViewGroup.LayoutParams.WRAP_CONTENT,
+                  ViewGroup.LayoutParams.WRAP_CONTENT));
+        }
+      });
+
+    instrumentation.waitForIdleSync();
+    mReactNativeRule.waitForIdleSync();
+
+    int newComponentHeight = 500;
+    mReactNativeRule
+      .getContext()
+      .getJSModule(ReactRootViewTestModule.class)
+      .setHeight(newComponentHeight);
+
+    instrumentation.waitForIdleSync();
+    mReactNativeRule.waitForIdleSync();
+    instrumentation.waitForIdleSync();
+
+    // added 0.5 to account for rounding issues
+    assertThat(rootView.getMeasuredHeight())
+      .isEqualTo((int) (PixelUtil.toPixelFromDIP(newComponentHeight) + 0.5));
+  }
+
   /**
    * Verify that removing the root view from hierarchy will trigger subviews removal both on JS and
    * native side

ReactAndroid/src/androidTest/js/CatalystRootViewTestModule.js

@@ -9,27 +9,54 @@
 
 'use strict';
 
+const BatchedBridge = require('BatchedBridge');
 const React = require('React');
 const Recording = require('NativeModules').Recording;
 const StyleSheet = require('StyleSheet');
 const View = require('View');
 
+let that;
+
 class CatalystRootViewTestApp extends React.Component {
+  state = {
+    height: 300,
+  };
+
+  componentDidMount() {
+    that = this;
+  }
+
   componentWillUnmount() {
     Recording.record('RootComponentWillUnmount');
   }
 
   render() {
-    return <View collapsable={false} style={styles.container} />;
+    return (
+      <View
+        collapsable={false}
+        style={[styles.container, {height: this.state.height}]}
+      />
+    );
   }
 }
 
+const ReactRootViewTestModule = {
+  setHeight: function(height) {
+    that.setState({height: height});
+  },
+};
+
 const styles = StyleSheet.create({
   container: {
     alignSelf: 'stretch',
   },
 });
 
+BatchedBridge.registerCallableModule(
+  'ReactRootViewTestModule',
+  ReactRootViewTestModule,
+);
+
 module.exports = {
   CatalystRootViewTestApp: CatalystRootViewTestApp,
 };

ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java

@@ -25,6 +25,7 @@ import android.view.View;
 import android.view.ViewGroup;
 import android.view.ViewTreeObserver;
 import android.view.WindowManager;
+import android.widget.FrameLayout;
 import com.facebook.common.logging.FLog;
 import com.facebook.infer.annotation.Assertions;
 import com.facebook.react.bridge.Arguments;
@@ -48,7 +49,6 @@ import com.facebook.react.uimanager.RootView;
 import com.facebook.react.uimanager.UIManagerHelper;
 import com.facebook.react.uimanager.UIManagerModule;
 import com.facebook.react.uimanager.common.MeasureSpecProvider;
-import com.facebook.react.uimanager.common.SizeMonitoringFrameLayout;
 import com.facebook.react.uimanager.common.UIManagerType;
 import com.facebook.react.uimanager.events.EventDispatcher;
 import com.facebook.systrace.Systrace;
@@ -66,8 +66,7 @@ import javax.annotation.Nullable;
  * subsequent touch events related to that gesture (in case when JS code wants to handle that
  * gesture).
  */
-public class ReactRootView extends SizeMonitoringFrameLayout
-    implements RootView, MeasureSpecProvider {
+public class ReactRootView extends FrameLayout implements RootView, MeasureSpecProvider {
 
   /**
    * Listener interface for react root view events

ReactAndroid/src/main/java/com/facebook/react/bridge/UIManager.java

@@ -8,9 +8,8 @@
 package com.facebook.react.bridge;
 
 import com.facebook.react.bridge.WritableMap;
+import android.view.View;
 import com.facebook.react.uimanager.common.MeasureSpecProvider;
-import com.facebook.react.uimanager.common.SizeMonitoringFrameLayout;
-
 import javax.annotation.Nullable;
 
 public interface UIManager extends JSIModule, PerformanceCounter {
@@ -18,7 +17,7 @@ public interface UIManager extends JSIModule, PerformanceCounter {
   /**
    * Registers a new root view.
    */
-  <T extends SizeMonitoringFrameLayout & MeasureSpecProvider> int addRootView(final T rootView, WritableMap initialProps, @Nullable String initialUITemplate);
+  <T extends View & MeasureSpecProvider> int addRootView(final T rootView, WritableMap initialProps, @Nullable String initialUITemplate);
 
   /**
    * Unregisters a new root view.

ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java

@@ -18,6 +18,7 @@ import android.os.SystemClock;
 import android.support.annotation.GuardedBy;
 import android.support.annotation.Nullable;
 import android.support.annotation.UiThread;
+import android.view.View;
 import com.facebook.common.logging.FLog;
 import com.facebook.infer.annotation.Assertions;
 import com.facebook.infer.annotation.ThreadConfined;
@@ -57,7 +58,6 @@ import com.facebook.react.uimanager.ThemedReactContext;
 import com.facebook.react.uimanager.ViewManagerPropertyUpdater;
 import com.facebook.react.uimanager.ViewManagerRegistry;
 import com.facebook.react.uimanager.common.MeasureSpecProvider;
-import com.facebook.react.uimanager.common.SizeMonitoringFrameLayout;
 import com.facebook.react.uimanager.events.EventDispatcher;
 import com.facebook.systrace.Systrace;
 import java.util.ArrayList;
@@ -132,7 +132,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
   }
 
   @Override
-  public <T extends SizeMonitoringFrameLayout & MeasureSpecProvider> int addRootView(
+  public <T extends View & MeasureSpecProvider> int addRootView(
       final T rootView, final WritableMap initialProps, final @Nullable String initialUITemplate) {
     final int rootTag = ReactRootViewTagGenerator.getNextRootViewTag();
     ThemedReactContext reactContext =

ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java

@@ -31,7 +31,6 @@ import com.facebook.react.uimanager.ThemedReactContext;
 import com.facebook.react.uimanager.ViewGroupManager;
 import com.facebook.react.uimanager.ViewManager;
 import com.facebook.react.uimanager.ViewManagerRegistry;
-import com.facebook.react.uimanager.common.SizeMonitoringFrameLayout;
 import com.facebook.yoga.YogaMeasureMode;
 import java.util.concurrent.ConcurrentHashMap;
 
@@ -53,7 +52,7 @@ public class MountingManager {
   }
 
   @UiThread
-  public void addRootView(int reactRootTag, SizeMonitoringFrameLayout rootView) {
+  public void addRootView(int reactRootTag, View rootView) {
     if (rootView.getId() != View.NO_ID) {
       throw new IllegalViewOperationException(
           "Trying to add a root view with an explicit id already set. React Native uses "

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

@@ -8,7 +8,7 @@
 package com.facebook.react.uimanager;
 
 import android.content.res.Resources;
-import com.facebook.common.logging.FLog;
+import android.os.Build;
 import android.util.SparseArray;
 import android.util.SparseBooleanArray;
 import android.view.Menu;
@@ -17,8 +17,9 @@ import android.view.View;
 import android.view.ViewGroup;
 import android.view.ViewParent;
 import android.widget.PopupMenu;
-import com.facebook.react.R;
+import com.facebook.common.logging.FLog;
 import com.facebook.infer.annotation.Assertions;
+import com.facebook.react.R;
 import com.facebook.react.animation.Animation;
 import com.facebook.react.animation.AnimationListener;
 import com.facebook.react.animation.AnimationRegistry;
@@ -30,7 +31,6 @@ import com.facebook.react.bridge.ReadableMap;
 import com.facebook.react.bridge.SoftAssertions;
 import com.facebook.react.bridge.UiThreadUtil;
 import com.facebook.react.touch.JSResponderHandler;
-import com.facebook.react.uimanager.common.SizeMonitoringFrameLayout;
 import com.facebook.react.uimanager.layoutanimation.LayoutAnimationController;
 import com.facebook.react.uimanager.layoutanimation.LayoutAnimationListener;
 import com.facebook.systrace.Systrace;
@@ -532,17 +532,11 @@ public class NativeViewHierarchyManager {
   /**
    * See {@link UIManagerModule#addRootView}.
    */
-  public synchronized void addRootView(
-      int tag,
-      SizeMonitoringFrameLayout view,
-      ThemedReactContext themedContext) {
-    addRootViewGroup(tag, view, themedContext);
+  public synchronized void addRootView(int tag, View view) {
+    addRootViewGroup(tag, view);
   }
 
-  protected synchronized final void addRootViewGroup(
-      int tag,
-      ViewGroup view,
-      ThemedReactContext themedContext) {
+  protected synchronized final void addRootViewGroup(int tag, View view) {
     if (view.getId() != View.NO_ID) {
       FLog.e(
         TAG,

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

@@ -8,7 +8,7 @@
 package com.facebook.react.uimanager;
 
 import android.view.ViewGroup;
-import com.facebook.react.uimanager.common.SizeMonitoringFrameLayout;
+import android.widget.FrameLayout;
 
 /**
  * View manager for ReactRootView components.
@@ -24,6 +24,6 @@ public class RootViewManager extends ViewGroupManager<ViewGroup> {
 
   @Override
   protected ViewGroup createViewInstance(ThemedReactContext reactContext) {
-    return new SizeMonitoringFrameLayout(reactContext);
+    return new FrameLayout(reactContext);
   }
 }

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

@@ -11,6 +11,7 @@ import static android.view.View.MeasureSpec.EXACTLY;
 import static android.view.View.MeasureSpec.UNSPECIFIED;
 
 import android.os.SystemClock;
+import android.view.View;
 import android.view.View.MeasureSpec;
 import com.facebook.common.logging.FLog;
 import com.facebook.infer.annotation.Assertions;
@@ -25,7 +26,6 @@ import com.facebook.react.bridge.WritableArray;
 import com.facebook.react.common.ReactConstants;
 import com.facebook.react.modules.i18nmanager.I18nUtil;
 import com.facebook.react.uimanager.common.MeasureSpecProvider;
-import com.facebook.react.uimanager.common.SizeMonitoringFrameLayout;
 import com.facebook.react.uimanager.debug.NotThreadSafeViewHierarchyUpdateDebugListener;
 import com.facebook.react.uimanager.events.EventDispatcher;
 import com.facebook.systrace.Systrace;
@@ -195,7 +195,7 @@ public class UIImplementation {
    * Registers a root node with a given tag, size and ThemedReactContext and adds it to a node
    * registry.
    */
-  public <T extends SizeMonitoringFrameLayout & MeasureSpecProvider> void registerRootView(
+  public <T extends View & MeasureSpecProvider> void registerRootView(
       T rootView, int tag, ThemedReactContext context) {
     final ReactShadowNode rootCSSNode = createRootShadowNode();
     rootCSSNode.setReactTag(tag);
@@ -213,7 +213,7 @@ public class UIImplementation {
     });
 
     // register it within NativeViewHierarchyManager
-    mOperationsQueue.addRootView(tag, rootView, context);
+    mOperationsQueue.addRootView(tag, rootView);
   }
 
   /**

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

@@ -15,6 +15,7 @@ import android.content.Context;
 import android.content.res.Configuration;
 import android.media.AudioManager;
 import android.util.ArrayMap;
+import android.view.View;
 import com.facebook.common.logging.FLog;
 import com.facebook.debug.holder.PrinterHolder;
 import com.facebook.debug.tags.ReactDebugOverlayTags;
@@ -37,7 +38,6 @@ import com.facebook.react.common.ReactConstants;
 import com.facebook.react.common.annotations.VisibleForTesting;
 import com.facebook.react.module.annotations.ReactModule;
 import com.facebook.react.uimanager.common.MeasureSpecProvider;
-import com.facebook.react.uimanager.common.SizeMonitoringFrameLayout;
 import com.facebook.react.uimanager.common.ViewUtil;
 import com.facebook.react.uimanager.debug.NotThreadSafeViewHierarchyUpdateDebugListener;
 import com.facebook.react.uimanager.events.EventDispatcher;
@@ -376,7 +376,7 @@ public class UIManagerModule extends ReactContextBaseJavaModule
     return mUIImplementation.getProfiledBatchPerfCounters();
   }
 
-  public <T extends SizeMonitoringFrameLayout & MeasureSpecProvider> int addRootView(
+  public <T extends View & MeasureSpecProvider> int addRootView(
       final T rootView) {
         return addRootView(rootView, null, null);
       }
@@ -391,7 +391,7 @@ public class UIManagerModule extends ReactContextBaseJavaModule
    * <p>TODO(6242243): Make addRootView thread safe NB: this method is horribly not-thread-safe.
    */
   @Override
-  public <T extends SizeMonitoringFrameLayout & MeasureSpecProvider> int addRootView(
+  public <T extends View & MeasureSpecProvider> int addRootView(
       final T rootView, WritableMap initialProps, @Nullable String initialUITemplate) {
     Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "UIManagerModule.addRootView");
     final int tag = ReactRootViewTagGenerator.getNextRootViewTag();
@@ -400,21 +400,6 @@ public class UIManagerModule extends ReactContextBaseJavaModule
         new ThemedReactContext(reactApplicationContext, rootView.getContext());
 
     mUIImplementation.registerRootView(rootView, tag, themedRootContext);
-
-    rootView.setOnSizeChangedListener(
-        new SizeMonitoringFrameLayout.OnSizeChangedListener() {
-          @Override
-          public void onSizeChanged(final int width, final int height, int oldW, int oldH) {
-            reactApplicationContext.runOnNativeModulesQueueThread(
-                new GuardedRunnable(reactApplicationContext) {
-                  @Override
-                  public void runGuarded() {
-                    updateNodeSize(tag, width, height);
-                  }
-                });
-          }
-        });
-
     Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE);
     return tag;
   }

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

@@ -8,6 +8,7 @@
 package com.facebook.react.uimanager;
 
 import android.os.SystemClock;
+import android.view.View;
 import com.facebook.common.logging.FLog;
 import com.facebook.react.animation.Animation;
 import com.facebook.react.animation.AnimationRegistry;
@@ -21,7 +22,6 @@ import com.facebook.react.bridge.SoftAssertions;
 import com.facebook.react.bridge.UiThreadUtil;
 import com.facebook.react.common.ReactConstants;
 import com.facebook.react.modules.core.ReactChoreographer;
-import com.facebook.react.uimanager.common.SizeMonitoringFrameLayout;
 import com.facebook.react.uimanager.debug.NotThreadSafeViewHierarchyUpdateDebugListener;
 import com.facebook.systrace.Systrace;
 import com.facebook.systrace.SystraceMessage;
@@ -673,11 +673,8 @@ public class UIViewOperationQueue {
     return mOperations.isEmpty();
   }
 
-  public void addRootView(
-    final int tag,
-    final SizeMonitoringFrameLayout rootView,
-    final ThemedReactContext themedRootContext) {
-    mNativeViewHierarchyManager.addRootView(tag, rootView, themedRootContext);
+  public void addRootView(final int tag, final View rootView) {
+    mNativeViewHierarchyManager.addRootView(tag, rootView);
   }
 
   /**

ReactAndroid/src/main/java/com/facebook/react/uimanager/common/SizeMonitoringFrameLayout.java

@@ -1,53 +0,0 @@
-/**
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree.
- */
-
-package com.facebook.react.uimanager.common;
-
-import javax.annotation.Nullable;
-
-import android.content.Context;
-import android.util.AttributeSet;
-import android.widget.FrameLayout;
-
-/**
- * Subclass of {@link FrameLayout} that allows registering for size change events. The main purpose
- * for this class is to hide complexity of {@link ReactRootView} from the code under
- * {@link com.facebook.react.uimanager} package.
- */
-public class SizeMonitoringFrameLayout extends FrameLayout {
-
-  public interface OnSizeChangedListener {
-    void onSizeChanged(int width, int height, int oldWidth, int oldHeight);
-  }
-
-  private @Nullable OnSizeChangedListener mOnSizeChangedListener;
-
-  public SizeMonitoringFrameLayout(Context context) {
-    super(context);
-  }
-
-  public SizeMonitoringFrameLayout(Context context, AttributeSet attrs) {
-    super(context, attrs);
-  }
-
-  public SizeMonitoringFrameLayout(Context context, AttributeSet attrs, int defStyle) {
-    super(context, attrs, defStyle);
-  }
-
-  public void setOnSizeChangedListener(OnSizeChangedListener onSizeChangedListener) {
-    mOnSizeChangedListener = onSizeChangedListener;
-  }
-
-  @Override
-  protected void onSizeChanged(int w, int h, int oldw, int oldh) {
-    super.onSizeChanged(w, h, oldw, oldh);
-
-    if (mOnSizeChangedListener != null) {
-      mOnSizeChangedListener.onSizeChanged(w, h, oldw, oldh);
-    }
-  }
-}