From bbd925cdd1b364104014eb4d023aa19ba1ce0c31 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Mon, 15 Apr 2019 01:41:47 -0700 Subject: [PATCH] MountingManager can create views with props in one step instead of two Summary: Create views with props in one call instead of two. Backwards-compatible. Reviewed By: shergin Differential Revision: D14846424 fbshipit-source-id: cb53225579089f7e51d4e9d1fc9fc2e331a994c1 --- .../fabric/mounting/ContextBasedViewPool.java | 10 ++++--- .../fabric/mounting/MountingManager.java | 20 ++++++++----- .../react/fabric/mounting/ViewFactory.java | 3 +- .../fabric/mounting/ViewManagerFactory.java | 5 ++-- .../react/fabric/mounting/ViewPool.java | 9 +++--- .../uimanager/NativeViewHierarchyManager.java | 2 +- .../facebook/react/uimanager/ViewManager.java | 29 +++++++++++++++++-- .../uimanager/SimpleViewPropertyTest.java | 4 +-- 8 files changed, 59 insertions(+), 23 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/ContextBasedViewPool.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/ContextBasedViewPool.java index 88324df35ac..579e8b73005 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/ContextBasedViewPool.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/ContextBasedViewPool.java @@ -8,6 +8,7 @@ package com.facebook.react.fabric.mounting; import android.view.View; import androidx.annotation.UiThread; +import com.facebook.react.uimanager.ReactStylesDiffMap; import com.facebook.react.uimanager.ThemedReactContext; import com.facebook.react.uimanager.ViewManagerRegistry; import java.util.WeakHashMap; @@ -22,15 +23,16 @@ public final class ContextBasedViewPool implements ViewFactory { mViewManagerRegistry = viewManagerRegistry; } + @UiThread - void createView(ThemedReactContext context, String componentName) { - getViewPool(context).createView(componentName, context); + void createView(ThemedReactContext context, ReactStylesDiffMap props, String componentName) { + getViewPool(context).createView(componentName, props, context); } @UiThread @Override - public View getOrCreateView(String componentName, ThemedReactContext context) { - return getViewPool(context).getOrCreateView(componentName, context); + public View getOrCreateView(String componentName, ReactStylesDiffMap props, ThemedReactContext context) { + return getViewPool(context).getOrCreateView(componentName, props, context); } @UiThread diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java index 5f0b65c5915..4daf4021802 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java @@ -171,10 +171,11 @@ public class MountingManager { } @UiThread - public void createView( + public void createViewWithProps( ThemedReactContext themedReactContext, String componentName, int reactTag, + ReadableMap props, boolean isLayoutable) { if (mTagToViewState.get(reactTag) != null) { return; @@ -183,13 +184,21 @@ public class MountingManager { View view = null; ViewManager viewManager = null; + ReactStylesDiffMap diffMap = null; + if (props != null) { + diffMap = new ReactStylesDiffMap(props); + } + if (isLayoutable) { viewManager = mViewManagerRegistry.get(componentName); - view = mViewFactory.getOrCreateView(componentName, themedReactContext); + view = mViewFactory.getOrCreateView(componentName, diffMap, themedReactContext); view.setId(reactTag); } - mTagToViewState.put(reactTag, new ViewState(reactTag, view, viewManager)); + ViewState viewState = new ViewState(reactTag, view, viewManager); + viewState.mCurrentProps = diffMap; + + mTagToViewState.put(reactTag, viewState); } @UiThread @@ -313,10 +322,7 @@ public class MountingManager { throw new IllegalStateException("View for component " + componentName + " with tag " + reactTag + " already exists."); } - createView(reactContext, componentName, reactTag, isLayoutable); - if (isLayoutable) { - updateProps(reactTag, props); - } + createViewWithProps(reactContext, componentName, reactTag, props, isLayoutable); } @UiThread diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/ViewFactory.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/ViewFactory.java index 408119dc553..b92f576ebda 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/ViewFactory.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/ViewFactory.java @@ -7,11 +7,12 @@ package com.facebook.react.fabric.mounting; import android.view.View; +import com.facebook.react.uimanager.ReactStylesDiffMap; import com.facebook.react.uimanager.ThemedReactContext; public interface ViewFactory { - View getOrCreateView(String componentName, ThemedReactContext context); + View getOrCreateView(String componentName, ReactStylesDiffMap props, ThemedReactContext context); void recycle(ThemedReactContext context, String componentName, View view); diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/ViewManagerFactory.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/ViewManagerFactory.java index 46a053c994b..dffac26f5a6 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/ViewManagerFactory.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/ViewManagerFactory.java @@ -8,6 +8,7 @@ package com.facebook.react.fabric.mounting; import androidx.annotation.UiThread; import android.view.View; +import com.facebook.react.uimanager.ReactStylesDiffMap; import com.facebook.react.uimanager.ThemedReactContext; import com.facebook.react.uimanager.ViewManagerRegistry; @@ -22,8 +23,8 @@ public class ViewManagerFactory implements ViewFactory { @UiThread @Override public View getOrCreateView( - String componentName, ThemedReactContext context) { - return mViewManagerRegistry.get(componentName).createView(context, null); + String componentName, ReactStylesDiffMap props, ThemedReactContext context) { + return mViewManagerRegistry.get(componentName).createViewWithProps(context, props, null); } @UiThread diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/ViewPool.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/ViewPool.java index e253a98d2df..69a2c911c8d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/ViewPool.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/ViewPool.java @@ -9,6 +9,7 @@ package com.facebook.react.fabric.mounting; import androidx.annotation.UiThread; import android.view.View; import com.facebook.react.common.ClearableSynchronizedPool; +import com.facebook.react.uimanager.ReactStylesDiffMap; import com.facebook.react.uimanager.ThemedReactContext; import com.facebook.react.uimanager.ViewManager; import com.facebook.react.uimanager.ViewManagerRegistry; @@ -25,20 +26,20 @@ public final class ViewPool { } @UiThread - void createView(String componentName, ThemedReactContext context) { + void createView(String componentName, ReactStylesDiffMap props, ThemedReactContext context) { ClearableSynchronizedPool viewPool = getViewPoolForComponent(componentName); ViewManager viewManager = mViewManagerRegistry.get(componentName); // TODO: T31905686 Integrate / re-implement jsResponder - View view = viewManager.createView(context, null); + View view = viewManager.createViewWithProps(context, props, null); viewPool.release(view); } @UiThread - View getOrCreateView(String componentName, ThemedReactContext context) { + View getOrCreateView(String componentName, ReactStylesDiffMap props, ThemedReactContext context) { ClearableSynchronizedPool viewPool = getViewPoolForComponent(componentName); View view = viewPool.acquire(); if (view == null) { - createView(componentName, context); + createView(componentName, props, context); view = viewPool.acquire(); } return view; diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java index d42e865913b..a764e1ed7f0 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java @@ -248,7 +248,7 @@ public class NativeViewHierarchyManager { try { ViewManager viewManager = mViewManagers.get(className); - View view = viewManager.createView(themedContext, mJSResponderHandler); + View view = viewManager.createViewWithProps(themedContext, null, mJSResponderHandler); mTagsToViews.put(tag, view); mTagsToViewManagers.put(tag, viewManager); diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java index d3724a5e154..4a8c8ad0177 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java @@ -42,10 +42,20 @@ public abstract class ViewManager /** * Creates a view and installs event emitters on it. */ - public final @Nonnull T createView( + private final @Nonnull T createView( @Nonnull ThemedReactContext reactContext, JSResponderHandler jsResponderHandler) { - T view = createViewInstance(reactContext); + return this.createViewWithProps(reactContext, null, jsResponderHandler); + } + + /** + * Creates a view with knowledge of props. + */ + public @Nonnull T createViewWithProps( + @Nonnull ThemedReactContext reactContext, + ReactStylesDiffMap props, + JSResponderHandler jsResponderHandler) { + T view = createViewInstanceWithProps(reactContext, props); addEventEmitters(reactContext, view); if (view instanceof ReactInterceptingViewGroup) { ((ReactInterceptingViewGroup) view).setOnInterceptTouchEventListener(jsResponderHandler); @@ -53,6 +63,7 @@ public abstract class ViewManager return view; } + /** * @return the name of this view manager. This will be the name used to reference this view * manager from JavaScript in createReactNativeComponentClass. @@ -90,6 +101,20 @@ public abstract class ViewManager */ protected abstract @Nonnull T createViewInstance(@Nonnull ThemedReactContext reactContext); + /** + * Subclasses should return a new View instance of the proper type. + * This is an optional method that will call createViewInstance for you. + * Override it if you need props upon creation of the view. + * @param reactContext + */ + protected @Nonnull T createViewInstanceWithProps(@Nonnull ThemedReactContext reactContext, ReactStylesDiffMap initialProps) { + T view = createViewInstance(reactContext); + if (initialProps != null) { + updateProperties(view, initialProps); + } + return view; + } + /** * Called when view is detached from view hierarchy and allows for some additional cleanup by * the {@link ViewManager} subclass. diff --git a/ReactAndroid/src/test/java/com/facebook/react/uimanager/SimpleViewPropertyTest.java b/ReactAndroid/src/test/java/com/facebook/react/uimanager/SimpleViewPropertyTest.java index 95cc4d52767..e054da2e808 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/uimanager/SimpleViewPropertyTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/uimanager/SimpleViewPropertyTest.java @@ -83,7 +83,7 @@ public class SimpleViewPropertyTest { @Test public void testOpacity() { - View view = mManager.createView(mThemedContext, new JSResponderHandler()); + View view = mManager.createViewWithProps(mThemedContext, buildStyles(), new JSResponderHandler()); mManager.updateProperties(view, buildStyles()); assertThat(view.getAlpha()).isEqualTo(1.0f); @@ -97,7 +97,7 @@ public class SimpleViewPropertyTest { @Test public void testBackgroundColor() { - View view = mManager.createView(mThemedContext, new JSResponderHandler()); + View view = mManager.createViewWithProps(mThemedContext, buildStyles(), new JSResponderHandler()); mManager.updateProperties(view, buildStyles()); assertThat(view.getBackground()).isEqualTo(null);