From ff4ef2f633db05105cc10080d7f0514b37814fea 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 be9c60688a8..e58f8d9c7ba 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; @@ -248,6 +255,7 @@ import com.facebook.systrace.Systrace; // TODO(9577825): remove this ApplicationHolder.setApplication((Application) applicationContext.getApplicationContext()); + setDisplayMetrics(applicationContext); mApplicationContext = applicationContext; mJSBundleFile = jsBundleFile; @@ -286,6 +294,38 @@ import com.facebook.systrace.Systrace; 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 3450f883aa5..e6d35687609 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 4aab528e553..9742fc041c2 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 476a2673bf2..8981d4354d2 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 bce7c37361b..ffee1cea45d 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 f3473a05f67..6cbb5894e1c 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 2f6eea4b745..641e299920c 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 5d757eb2ffd..2652422e983 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 26db0858723..6752ba06290 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 890d0c0beee..d710ffbd871 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 56d79f8bfae..59871f217a1 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,