From c06efc0831b9156ee6261d7ddd2086ed87cbc0aa Mon Sep 17 00:00:00 2001 From: Felix Oghina Date: Wed, 2 Dec 2015 09:10:28 -0800 Subject: [PATCH] remove activities from module constructors Summary: Refactor modules that take activities (or activities that implement some interface) as constructor args to not do that. Expose `getCurrentActivity()` in `ReactContext` and use that wherever the activity is needed. public Reviewed By: astreet Differential Revision: D2680462 fb-gh-sync-id: f263b3fe5b422b7aab9fdadd051cef4e82797b0a --- .../facebook/react/ReactInstanceManager.java | 17 +++++++++++++++++ .../react/ReactInstanceManagerImpl.java | 15 ++++++++++++++- .../com/facebook/react/bridge/ReactContext.java | 11 ++++++++++- .../bridge/ReactContextBaseJavaModule.java | 17 +++++++++++++++++ 4 files changed, 58 insertions(+), 2 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java index 2449c28ea..bdffdf261 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java @@ -47,6 +47,18 @@ import com.facebook.react.uimanager.ViewManager; * have a static method, and so cannot (in Java < 8), be one. */ public abstract class ReactInstanceManager { + + /** + * Listener interface for react instance events. + */ + public interface ReactInstanceEventListener { + /** + * Called when the react context is initialized (all modules registered). Always called on the + * UI thread. + */ + void onReactContextInitialized(ReactContext context); + } + public abstract DevSupportManager getDevSupportManager(); /** @@ -118,6 +130,11 @@ public abstract class ReactInstanceManager { public abstract List createAllViewManagers( ReactApplicationContext catalystApplicationContext); + /** + * Add a listener to be notified of react instance events. + */ + public abstract void addReactInstanceEventListener(ReactInstanceEventListener listener); + @VisibleForTesting public abstract @Nullable ReactContext getCurrentReactContext(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java index c97a55b06..1eecb4032 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java @@ -12,10 +12,11 @@ package com.facebook.react; import javax.annotation.Nullable; import java.util.ArrayList; +import java.util.Collection; import java.util.List; +import java.util.concurrent.ConcurrentLinkedQueue; import android.app.Activity; -import android.app.Application; import android.content.Context; import android.content.Intent; import android.os.AsyncTask; @@ -95,6 +96,8 @@ import com.facebook.systrace.Systrace; private @Nullable DefaultHardwareBackBtnHandler mDefaultBackButtonImpl; private String mSourceUrl; private @Nullable Activity mCurrentActivity; + private final Collection mReactInstanceEventListeners = + new ConcurrentLinkedQueue<>(); private volatile boolean mHasStartedCreatingInitialContext = false; private final UIImplementationProvider mUIImplementationProvider; @@ -406,6 +409,7 @@ import com.facebook.systrace.Systrace; mCurrentReactContext = null; mHasStartedCreatingInitialContext = false; } + mCurrentActivity = null; } @Override @@ -482,6 +486,11 @@ import com.facebook.systrace.Systrace; } } + @Override + public void addReactInstanceEventListener(ReactInstanceEventListener listener) { + mReactInstanceEventListeners.add(listener); + } + @VisibleForTesting @Override public @Nullable ReactContext getCurrentReactContext() { @@ -535,6 +544,10 @@ import com.facebook.systrace.Systrace; for (ReactRootView rootView : mAttachedRootViews) { attachMeasuredRootViewToInstance(rootView, catalystInstance); } + + for (ReactInstanceEventListener listener : mReactInstanceEventListeners) { + listener.onReactContextInitialized(reactContext); + } } private void attachMeasuredRootViewToInstance( diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java index ca2d40c73..e1632401f 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java @@ -146,7 +146,6 @@ public class ReactContext extends ContextWrapper { */ public void onPause() { UiThreadUtil.assertOnUiThread(); - mCurrentActivity = null; for (LifecycleEventListener listener : mLifecycleEventListeners) { listener.onHostPause(); } @@ -163,6 +162,7 @@ public class ReactContext extends ContextWrapper { if (mCatalystInstance != null) { mCatalystInstance.destroy(); } + mCurrentActivity = null; } /** @@ -239,4 +239,13 @@ public class ReactContext extends ContextWrapper { mCurrentActivity.startActivityForResult(intent, code, bundle); return true; } + + /** + * Get the activity to which this context is currently attached, or {@code null} if not attached. + * DO NOT HOLD LONG-LIVED REFERENCES TO THE OBJECT RETURNED BY THIS METHOD, AS THIS WILL CAUSE + * MEMORY LEAKS. + */ + /* package */ @Nullable Activity getCurrentActivity() { + return mCurrentActivity; + } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContextBaseJavaModule.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContextBaseJavaModule.java index 4d0470f46..6a73f73cc 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContextBaseJavaModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContextBaseJavaModule.java @@ -9,6 +9,10 @@ package com.facebook.react.bridge; +import javax.annotation.Nullable; + +import android.app.Activity; + /** * Base class for Catalyst native modules that require access to the {@link ReactContext} * instance. @@ -27,4 +31,17 @@ public abstract class ReactContextBaseJavaModule extends BaseJavaModule { protected final ReactApplicationContext getReactApplicationContext() { return mReactApplicationContext; } + + /** + * Get the activity to which this context is currently attached, or {@code null} if not attached. + * + * DO NOT HOLD LONG-LIVED REFERENCES TO THE OBJECT RETURNED BY THIS METHOD, AS THIS WILL CAUSE + * MEMORY LEAKS. + * + * For example, never store the value returned by this method in a member variable. Instead, call + * this method whenever you actually need the Activity and make sure to check for {@code null}. + */ + protected @Nullable final Activity getCurrentActivity() { + return mReactApplicationContext.getCurrentActivity(); + } }