From a69b8835373c9b1fcb7d31032d6f384a7b6f0478 Mon Sep 17 00:00:00 2001 From: Andy Street Date: Thu, 5 May 2016 05:06:38 -0700 Subject: [PATCH] Simplify ReactRootView Summary: I need to do some refactoring of what happens when a ReactRootView is attached/detached from the window. In doing so, I found ReactRootView is way more complicated than it needs to be and very hard to understand. This simplifies it while maintaining functionality. Basically: - When you start a react application, if the root view is measured, we attach it to the instance. - If it wasn't measured, we do nothing - In onMeasure, if we have a ReactInstanceManager, but are not attached to it, attach to it. None of this needs to depend on whether we're attached to the window or whether an attach is scheduled afaict. Reviewed By: dmmiller Differential Revision: D3260056 fb-gh-sync-id: 974c19ed75a07f83299472f75346bd6be7414128 fbshipit-source-id: 974c19ed75a07f83299472f75346bd6be7414128 --- .../com/facebook/react/ReactRootView.java | 57 +++++++------------ 1 file changed, 19 insertions(+), 38 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java b/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java index 1c2882958..2c4802e24 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java @@ -59,8 +59,6 @@ public class ReactRootView extends SizeMonitoringFrameLayout implements RootView private @Nullable KeyboardListener mKeyboardListener; private @Nullable OnGenericMotionListener mOnGenericMotionListener; private boolean mWasMeasured = false; - private boolean mAttachScheduled = false; - private boolean mIsAttachedToWindow = false; private boolean mIsAttachedToInstance = false; private final JSTouchDispatcher mJSTouchDispatcher = new JSTouchDispatcher(this); @@ -92,18 +90,13 @@ public class ReactRootView extends SizeMonitoringFrameLayout implements RootView MeasureSpec.getSize(heightMeasureSpec)); mWasMeasured = true; - if (mAttachScheduled && mReactInstanceManager != null && mIsAttachedToWindow) { - // Scheduled from {@link #startReactApplication} call in case when the view measurements are - // not available - mAttachScheduled = false; + // Check if we were waiting for onMeasure to attach the root view + if (mReactInstanceManager != null && !mIsAttachedToInstance) { // Enqueue it to UIThread not to block onMeasure waiting for the catalyst instance creation UiThreadUtil.runOnUiThread(new Runnable() { @Override public void run() { - Assertions.assertNotNull(mReactInstanceManager) - .attachMeasuredRootView(ReactRootView.this); - mIsAttachedToInstance = true; - getViewTreeObserver().addOnGlobalLayoutListener(getKeyboardListener()); + attachToReactInstanceManager(); } }); } @@ -176,28 +169,13 @@ public class ReactRootView extends SizeMonitoringFrameLayout implements RootView protected void onDetachedFromWindow() { super.onDetachedFromWindow(); - mIsAttachedToWindow = false; - - if (mReactInstanceManager != null && !mAttachScheduled) { + if (mReactInstanceManager != null && mIsAttachedToInstance) { mReactInstanceManager.detachRootView(this); mIsAttachedToInstance = false; getViewTreeObserver().removeOnGlobalLayoutListener(getKeyboardListener()); } } - @Override - protected void onAttachedToWindow() { - super.onAttachedToWindow(); - - mIsAttachedToWindow = true; - - // If the view re-attached and catalyst instance has been set before, we'd attach again to the - // catalyst instance (expecting measure to be called after {@link onAttachedToWindow}) - if (mReactInstanceManager != null) { - mAttachScheduled = true; - } - } - /** * {@see #startReactApplication(ReactInstanceManager, String, android.os.Bundle)} */ @@ -222,8 +200,7 @@ public class ReactRootView extends SizeMonitoringFrameLayout implements RootView // it in the case of re-creating the catalyst instance Assertions.assertCondition( mReactInstanceManager == null, - "This root view has already " + - "been attached to a catalyst instance manager"); + "This root view has already been attached to a catalyst instance manager"); mReactInstanceManager = reactInstanceManager; mJSModuleName = moduleName; @@ -233,15 +210,10 @@ public class ReactRootView extends SizeMonitoringFrameLayout implements RootView mReactInstanceManager.createReactContextInBackground(); } - // We need to wait for the initial onMeasure, if this view has not yet been measured, we set - // mAttachScheduled flag, which will make this view startReactApplication itself to instance - // manager once onMeasure is called. - if (mWasMeasured && mIsAttachedToWindow) { - mReactInstanceManager.attachMeasuredRootView(this); - mIsAttachedToInstance = true; - getViewTreeObserver().addOnGlobalLayoutListener(getKeyboardListener()); - } else { - mAttachScheduled = true; + // We need to wait for the initial onMeasure, if this view has not yet been measured, we set which + // will make this view startReactApplication itself to instance manager once onMeasure is called. + if (mWasMeasured) { + attachToReactInstanceManager(); } } @@ -259,7 +231,6 @@ public class ReactRootView extends SizeMonitoringFrameLayout implements RootView */ @VisibleForTesting /* package */ void simulateAttachForTesting() { - mIsAttachedToWindow = true; mIsAttachedToInstance = true; mWasMeasured = true; } @@ -271,6 +242,16 @@ public class ReactRootView extends SizeMonitoringFrameLayout implements RootView return mKeyboardListener; } + private void attachToReactInstanceManager() { + if (mIsAttachedToInstance) { + return; + } + + mIsAttachedToInstance = true; + Assertions.assertNotNull(mReactInstanceManager).attachMeasuredRootView(this); + getViewTreeObserver().addOnGlobalLayoutListener(getKeyboardListener()); + } + private class KeyboardListener implements ViewTreeObserver.OnGlobalLayoutListener { private final Rect mVisibleViewArea; private final int mMinKeyboardHeightDetected;