From 4254e8a0a9a9b2cfbaed6037de360420e6c268e4 Mon Sep 17 00:00:00 2001 From: Andrei Coman Date: Mon, 8 Feb 2016 11:45:30 -0800 Subject: [PATCH] Fix DisplayMetrics KeyboardListener dependency Summary: public KeyboardListener needs DisplayMetrics to be initialized when it is attached. At the moment, this breaks easily whenever we change these components, since DisplayMetrics are intialized in a module and KeyboardListener is created eagerly in ReactRootView, whereas ReactRootView can exist without the instance. This changes to create DisplayMetrics as soon as possible, when the react instance is built. The KeyboardListener is created and attached after the ReactRootView is attached to an existing instance, point at which DisplayMetrics have to be initialized. Reviewed By: dmmiller Differential Revision: D2911351 fb-gh-sync-id: 64d1805c5d5b2f6876adb694b565a2df059b381d --- .../react/ReactInstanceManagerImpl.java | 40 ++++++++++++ .../com/facebook/react/ReactRootView.java | 23 +++++-- .../react/uimanager/UIManagerModule.java | 63 +------------------ .../uimanager/UIManagerModuleConstants.java | 4 +- .../UIManagerModuleConstantsHelper.java | 9 +-- .../textinput/ReactTextInputManager.java | 3 +- .../uimanager/ReactPropConstantsTest.java | 3 + .../UIManagerModuleConstantsTest.java | 5 ++ .../react/uimanager/UIManagerModuleTest.java | 5 ++ .../react/views/text/ReactTextTest.java | 4 ++ .../react/views/textinput/TextInputTest.java | 4 ++ 11 files changed, 86 insertions(+), 77 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java index 3c570a061..a92b151e1 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java @@ -11,6 +11,8 @@ package com.facebook.react; import javax.annotation.Nullable; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -21,8 +23,12 @@ import android.app.Application; import android.content.Context; import android.content.Intent; import android.os.AsyncTask; +import android.os.Build; import android.os.Bundle; +import android.util.DisplayMetrics; +import android.view.Display; import android.view.View; +import android.view.WindowManager; import com.facebook.common.logging.FLog; import com.facebook.infer.annotation.Assertions; @@ -58,6 +64,7 @@ import com.facebook.react.devsupport.ReactInstanceDevCommandsHandler; import com.facebook.react.modules.core.DefaultHardwareBackBtnHandler; import com.facebook.react.modules.core.DeviceEventManagerModule; import com.facebook.react.uimanager.AppRegistry; +import com.facebook.react.uimanager.DisplayMetricsHolder; import com.facebook.react.uimanager.UIImplementationProvider; import com.facebook.react.uimanager.UIManagerModule; import com.facebook.react.uimanager.ViewManager; @@ -261,6 +268,7 @@ import static com.facebook.react.bridge.ReactMarkerConstants.*; // TODO(9577825): remove this ApplicationHolder.setApplication((Application) applicationContext.getApplicationContext()); + setDisplayMetrics(applicationContext); mApplicationContext = applicationContext; mJSBundleFile = jsBundleFile; @@ -299,6 +307,38 @@ import static com.facebook.react.bridge.ReactMarkerConstants.*; SoLoader.init(applicationContext, /* native exopackage */ false); } + private static void setDisplayMetrics(Context context) { + DisplayMetrics displayMetrics = new DisplayMetrics(); + displayMetrics.setTo(context.getResources().getDisplayMetrics()); + WindowManager wm = (WindowManager) context.getSystemService(Context.WINDOW_SERVICE); + Display display = wm.getDefaultDisplay(); + + // Get the real display metrics if we are using API level 17 or higher. + // The real metrics include system decor elements (e.g. soft menu bar). + // + // See: http://developer.android.com/reference/android/view/Display.html#getRealMetrics(android.util.DisplayMetrics) + if (Build.VERSION.SDK_INT >= 17){ + display.getRealMetrics(displayMetrics); + + } else { + // For 14 <= API level <= 16, we need to invoke getRawHeight and getRawWidth to get the real dimensions. + // Since react-native only supports API level 16+ we don't have to worry about other cases. + // + // Reflection exceptions are rethrown at runtime. + // + // See: http://stackoverflow.com/questions/14341041/how-to-get-real-screen-height-and-width/23861333#23861333 + try { + Method mGetRawH = Display.class.getMethod("getRawHeight"); + Method mGetRawW = Display.class.getMethod("getRawWidth"); + displayMetrics.widthPixels = (Integer) mGetRawW.invoke(display); + displayMetrics.heightPixels = (Integer) mGetRawH.invoke(display); + } catch (InvocationTargetException | IllegalAccessException | NoSuchMethodException e) { + throw new RuntimeException("Error getting real dimensions for API level < 17", e); + } + } + DisplayMetricsHolder.setDisplayMetrics(displayMetrics); + } + /** * Trigger react context initialization asynchronously in a background async task. This enables * applications to pre-load the application JS, and execute global code before diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java b/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java index 3450f883a..e6d356876 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java @@ -55,11 +55,10 @@ import com.facebook.react.uimanager.events.TouchEventType; */ public class ReactRootView extends SizeMonitoringFrameLayout implements RootView { - private final KeyboardListener mKeyboardListener = new KeyboardListener(); - private @Nullable ReactInstanceManager mReactInstanceManager; private @Nullable String mJSModuleName; private @Nullable Bundle mLaunchOptions; + private @Nullable KeyboardListener mKeyboardListener; private int mTargetTag = -1; private final float[] mTargetCoordinates = new float[2]; private boolean mChildIsHandlingNativeGesture = false; @@ -107,7 +106,7 @@ public class ReactRootView extends SizeMonitoringFrameLayout implements RootView Assertions.assertNotNull(mReactInstanceManager) .attachMeasuredRootView(ReactRootView.this); mIsAttachedToInstance = true; - getViewTreeObserver().addOnGlobalLayoutListener(mKeyboardListener); + getViewTreeObserver().addOnGlobalLayoutListener(getKeyboardListener()); } }); } @@ -298,7 +297,7 @@ public class ReactRootView extends SizeMonitoringFrameLayout implements RootView if (mReactInstanceManager != null && !mAttachScheduled) { mReactInstanceManager.detachRootView(this); mIsAttachedToInstance = false; - getViewTreeObserver().removeOnGlobalLayoutListener(mKeyboardListener); + getViewTreeObserver().removeOnGlobalLayoutListener(getKeyboardListener()); } } @@ -356,7 +355,7 @@ public class ReactRootView extends SizeMonitoringFrameLayout implements RootView if (mWasMeasured && mIsAttachedToWindow) { mReactInstanceManager.attachMeasuredRootView(this); mIsAttachedToInstance = true; - getViewTreeObserver().addOnGlobalLayoutListener(mKeyboardListener); + getViewTreeObserver().addOnGlobalLayoutListener(getKeyboardListener()); } else { mAttachScheduled = true; } @@ -381,9 +380,21 @@ public class ReactRootView extends SizeMonitoringFrameLayout implements RootView mWasMeasured = true; } + private KeyboardListener getKeyboardListener() { + if (mKeyboardListener == null) { + mKeyboardListener = new KeyboardListener(); + } + return mKeyboardListener; + } + private class KeyboardListener implements ViewTreeObserver.OnGlobalLayoutListener { + private final Rect mVisibleViewArea; + private int mKeyboardHeight = 0; - private final Rect mVisibleViewArea = new Rect(); + + /* package */ KeyboardListener() { + mVisibleViewArea = new Rect(); + } @Override public void onGlobalLayout() { diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java index 4aab528e5..9742fc041 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java @@ -11,22 +11,10 @@ package com.facebook.react.uimanager; import javax.annotation.Nullable; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; -import java.util.Arrays; import java.util.List; import java.util.Map; -import android.content.Context; -import android.os.Build; -import android.util.DisplayMetrics; -import android.view.Display; -import android.view.WindowManager; - -import com.facebook.csslayout.CSSLayoutContext; -import com.facebook.infer.annotation.Assertions; import com.facebook.react.animation.Animation; -import com.facebook.react.bridge.Arguments; import com.facebook.react.bridge.Callback; import com.facebook.react.bridge.LifecycleEventListener; import com.facebook.react.bridge.OnBatchCompleteListener; @@ -35,7 +23,6 @@ import com.facebook.react.bridge.ReactContextBaseJavaModule; import com.facebook.react.bridge.ReactMethod; import com.facebook.react.bridge.ReadableArray; import com.facebook.react.bridge.ReadableMap; -import com.facebook.react.bridge.WritableArray; import com.facebook.react.uimanager.debug.NotThreadSafeViewHierarchyUpdateDebugListener; import com.facebook.react.uimanager.events.EventDispatcher; import com.facebook.systrace.Systrace; @@ -90,11 +77,7 @@ public class UIManagerModule extends ReactContextBaseJavaModule implements UIImplementation uiImplementation) { super(reactContext); mEventDispatcher = new EventDispatcher(reactContext); - - DisplayMetrics displayMetrics = getDisplayMetrics(); - - DisplayMetricsHolder.setDisplayMetrics(displayMetrics); - mModuleConstants = createConstants(displayMetrics, viewManagerList); + mModuleConstants = createConstants(viewManagerList); mUIImplementation = uiImplementation; reactContext.addLifecycleEventListener(this); @@ -131,14 +114,10 @@ public class UIManagerModule extends ReactContextBaseJavaModule implements mEventDispatcher.onCatalystInstanceDestroyed(); } - private static Map createConstants( - DisplayMetrics displayMetrics, - List viewManagerList) { + private static Map createConstants(List viewManagerList) { Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "CreateUIManagerConstants"); try { - return UIManagerModuleConstantsHelper.createConstants( - displayMetrics, - viewManagerList); + return UIManagerModuleConstantsHelper.createConstants(viewManagerList); } finally { Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); } @@ -460,40 +439,4 @@ public class UIManagerModule extends ReactContextBaseJavaModule implements public void sendAccessibilityEvent(int tag, int eventType) { mUIImplementation.sendAccessibilityEvent(tag, eventType); } - - private DisplayMetrics getDisplayMetrics() { - Context context = getReactApplicationContext(); - - DisplayMetrics displayMetrics = new DisplayMetrics(); - displayMetrics.setTo(context.getResources().getDisplayMetrics()); - WindowManager wm = (WindowManager) context.getSystemService(Context.WINDOW_SERVICE); - Display display = wm.getDefaultDisplay(); - - // Get the real display metrics if we are using API level 17 or higher. - // The real metrics include system decor elements (e.g. soft menu bar). - // - // See: http://developer.android.com/reference/android/view/Display.html#getRealMetrics(android.util.DisplayMetrics) - if (Build.VERSION.SDK_INT >= 17){ - display.getRealMetrics(displayMetrics); - - } else { - // For 14 <= API level <= 16, we need to invoke getRawHeight and getRawWidth to get the real dimensions. - // Since react-native only supports API level 16+ we don't have to worry about other cases. - // - // Reflection exceptions are rethrown at runtime. - // - // See: http://stackoverflow.com/questions/14341041/how-to-get-real-screen-height-and-width/23861333#23861333 - try { - Method mGetRawH = Display.class.getMethod("getRawHeight"); - Method mGetRawW = Display.class.getMethod("getRawWidth"); - displayMetrics.widthPixels = (Integer) mGetRawW.invoke(display); - displayMetrics.heightPixels = (Integer) mGetRawH.invoke(display); - } catch (InvocationTargetException | IllegalAccessException | NoSuchMethodException e) { - throw new RuntimeException("Error getting real dimensions for API level < 17", e); - } - } - - return displayMetrics; - } - } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleConstants.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleConstants.java index 476a2673b..8981d4354 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleConstants.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleConstants.java @@ -12,7 +12,6 @@ package com.facebook.react.uimanager; import java.util.HashMap; import java.util.Map; -import android.text.InputType; import android.util.DisplayMetrics; import android.view.accessibility.AccessibilityEvent; import android.widget.ImageView; @@ -80,7 +79,7 @@ import com.facebook.react.uimanager.events.TouchEventType; .build(); } - public static Map getConstants(DisplayMetrics displayMetrics) { + public static Map getConstants() { HashMap constants = new HashMap(); constants.put( "UIView", @@ -92,6 +91,7 @@ import com.facebook.react.uimanager.events.TouchEventType; "ScaleAspectFill", ImageView.ScaleType.CENTER_CROP.ordinal()))); + DisplayMetrics displayMetrics = DisplayMetricsHolder.getDisplayMetrics(); constants.put( "Dimensions", MapBuilder.of( diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleConstantsHelper.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleConstantsHelper.java index bce7c3736..ffee1cea4 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleConstantsHelper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleConstantsHelper.java @@ -9,12 +9,9 @@ package com.facebook.react.uimanager; -import java.util.HashMap; import java.util.List; import java.util.Map; -import android.util.DisplayMetrics; - import com.facebook.react.common.MapBuilder; /** @@ -39,10 +36,8 @@ import com.facebook.react.common.MapBuilder; * {@link UIManagerModuleConstants}. * TODO(6845124): Create a test for this */ - /* package */ static Map createConstants( - DisplayMetrics displayMetrics, - List viewManagers) { - Map constants = UIManagerModuleConstants.getConstants(displayMetrics); + /* package */ static Map createConstants(List viewManagers) { + Map constants = UIManagerModuleConstants.getConstants(); Map bubblingEventTypesConstants = UIManagerModuleConstants.getBubblingEventTypeConstants(); Map directEventTypesConstants = UIManagerModuleConstants.getDirectEventTypeConstants(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java index f3473a05f..6cbb5894e 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java @@ -29,18 +29,17 @@ import android.view.inputmethod.EditorInfo; import android.widget.TextView; import com.facebook.infer.annotation.Assertions; -import com.facebook.react.bridge.JSApplicationCausedNativeException; import com.facebook.react.bridge.JSApplicationIllegalArgumentException; import com.facebook.react.bridge.ReactContext; import com.facebook.react.bridge.ReadableArray; import com.facebook.react.common.MapBuilder; import com.facebook.react.uimanager.BaseViewManager; import com.facebook.react.uimanager.PixelUtil; -import com.facebook.react.uimanager.annotations.ReactProp; import com.facebook.react.uimanager.ThemedReactContext; import com.facebook.react.uimanager.UIManagerModule; import com.facebook.react.uimanager.ViewDefaults; import com.facebook.react.uimanager.ViewProps; +import com.facebook.react.uimanager.annotations.ReactProp; import com.facebook.react.uimanager.events.EventDispatcher; import com.facebook.react.views.text.DefaultStyleValuesUtil; import com.facebook.react.views.text.ReactTextUpdate; diff --git a/ReactAndroid/src/test/java/com/facebook/react/uimanager/ReactPropConstantsTest.java b/ReactAndroid/src/test/java/com/facebook/react/uimanager/ReactPropConstantsTest.java index 2f6eea4b7..641e29992 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/uimanager/ReactPropConstantsTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/uimanager/ReactPropConstantsTest.java @@ -13,6 +13,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import android.util.DisplayMetrics; import android.view.View; import com.facebook.react.bridge.ReactApplicationContext; @@ -145,6 +146,8 @@ public class ReactPropConstantsTest { public void testNativePropsIncludeCorrectTypes() { List viewManagers = Arrays.asList(new ViewManagerUnderTest()); ReactApplicationContext reactContext = new ReactApplicationContext(RuntimeEnvironment.application); + DisplayMetrics displayMetrics = reactContext.getResources().getDisplayMetrics(); + DisplayMetricsHolder.setDisplayMetrics(displayMetrics); UIManagerModule uiManagerModule = new UIManagerModule( reactContext, viewManagers, diff --git a/ReactAndroid/src/test/java/com/facebook/react/uimanager/UIManagerModuleConstantsTest.java b/ReactAndroid/src/test/java/com/facebook/react/uimanager/UIManagerModuleConstantsTest.java index 5d757eb2f..2652422e9 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/uimanager/UIManagerModuleConstantsTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/uimanager/UIManagerModuleConstantsTest.java @@ -13,6 +13,8 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import android.util.DisplayMetrics; + import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.common.MapBuilder; @@ -56,6 +58,9 @@ public class UIManagerModuleConstantsTest { public void setUp() { mReactContext = new ReactApplicationContext(RuntimeEnvironment.application); mUIImplementation = mock(UIImplementation.class); + + DisplayMetrics displayMetrics = mReactContext.getResources().getDisplayMetrics(); + DisplayMetricsHolder.setDisplayMetrics(displayMetrics); } @Test diff --git a/ReactAndroid/src/test/java/com/facebook/react/uimanager/UIManagerModuleTest.java b/ReactAndroid/src/test/java/com/facebook/react/uimanager/UIManagerModuleTest.java index 26db08587..6752ba062 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/uimanager/UIManagerModuleTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/uimanager/UIManagerModuleTest.java @@ -14,6 +14,7 @@ import java.util.Arrays; import java.util.List; import android.graphics.Color; +import android.util.DisplayMetrics; import android.view.Choreographer; import android.view.View; import android.view.ViewGroup; @@ -107,9 +108,13 @@ public class UIManagerModuleTest { mReactContext = new ReactApplicationContext(RuntimeEnvironment.application); mReactContext.initializeWithInstance(mCatalystInstanceMock); + DisplayMetrics displayMetrics = mReactContext.getResources().getDisplayMetrics(); + DisplayMetricsHolder.setDisplayMetrics(displayMetrics); + UIManagerModule uiManagerModuleMock = mock(UIManagerModule.class); when(mCatalystInstanceMock.getNativeModule(UIManagerModule.class)) .thenReturn(uiManagerModuleMock); + } @Test diff --git a/ReactAndroid/src/test/java/com/facebook/react/views/text/ReactTextTest.java b/ReactAndroid/src/test/java/com/facebook/react/views/text/ReactTextTest.java index 890d0c0be..d710ffbd8 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/views/text/ReactTextTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/views/text/ReactTextTest.java @@ -21,6 +21,7 @@ import android.graphics.drawable.Drawable; import android.os.Build; import android.text.Spanned; import android.text.TextUtils; +import android.util.DisplayMetrics; import android.text.style.AbsoluteSizeSpan; import android.view.Choreographer; import android.widget.TextView; @@ -31,6 +32,7 @@ import com.facebook.react.bridge.ReactTestHelper; import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.SimpleArray; import com.facebook.react.bridge.SimpleMap; +import com.facebook.react.uimanager.DisplayMetricsHolder; import com.facebook.react.uimanager.ReactChoreographer; import com.facebook.react.uimanager.UIImplementation; import com.facebook.react.uimanager.UIManagerModule; @@ -371,6 +373,8 @@ public class ReactTextTest { public UIManagerModule getUIManagerModule() { ReactApplicationContext reactContext = ReactTestHelper.createCatalystContextForTest(); + DisplayMetrics displayMetrics = reactContext.getResources().getDisplayMetrics(); + DisplayMetricsHolder.setDisplayMetrics(displayMetrics); List viewManagers = Arrays.asList( new ViewManager[] { new ReactTextViewManager(), diff --git a/ReactAndroid/src/test/java/com/facebook/react/views/textinput/TextInputTest.java b/ReactAndroid/src/test/java/com/facebook/react/views/textinput/TextInputTest.java index 56d79f8bf..59871f217 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/views/textinput/TextInputTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/views/textinput/TextInputTest.java @@ -13,6 +13,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import android.util.DisplayMetrics; import android.view.Choreographer; import android.widget.EditText; @@ -22,6 +23,7 @@ import com.facebook.react.bridge.ReactTestHelper; import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.SimpleArray; import com.facebook.react.bridge.SimpleMap; +import com.facebook.react.uimanager.DisplayMetricsHolder; import com.facebook.react.uimanager.ReactChoreographer; import com.facebook.react.uimanager.UIImplementation; import com.facebook.react.uimanager.UIManagerModule; @@ -181,6 +183,8 @@ public class TextInputTest { new ViewManager[] { new ReactTextInputManager(), }); + DisplayMetrics displayMetrics = reactContext.getResources().getDisplayMetrics(); + DisplayMetricsHolder.setDisplayMetrics(displayMetrics); UIManagerModule uiManagerModule = new UIManagerModule( reactContext, viewManagers,