Merge "Make a WeakReference to LifecycleOwner in LifecycleRegistry" into oc-mr1-support-27.0-dev
am: df71bd9c4d
Change-Id: Id4e0fba3094d3ca9b1c0cce42d46b86302aa9d35
diff --git a/lifecycle/runtime/src/main/java/android/arch/lifecycle/LifecycleRegistry.java b/lifecycle/runtime/src/main/java/android/arch/lifecycle/LifecycleRegistry.java
index 7936aee..83609ac 100644
--- a/lifecycle/runtime/src/main/java/android/arch/lifecycle/LifecycleRegistry.java
+++ b/lifecycle/runtime/src/main/java/android/arch/lifecycle/LifecycleRegistry.java
@@ -31,7 +31,9 @@
import android.arch.core.internal.FastSafeIterableMap;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
+import android.util.Log;
+import java.lang.ref.WeakReference;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.Map.Entry;
@@ -44,6 +46,8 @@
*/
public class LifecycleRegistry extends Lifecycle {
+ private static final String LOG_TAG = "LifecycleRegistry";
+
/**
* Custom list that keeps observers and can handle removals / additions during traversal.
*
@@ -59,8 +63,12 @@
private State mState;
/**
* The provider that owns this Lifecycle.
+ * Only WeakReference on LifecycleOwner is kept, so if somebody leaks Lifecycle, they won't leak
+ * the whole Fragment / Activity. However, to leak Lifecycle object isn't great idea neither,
+ * because it keeps strong references on all other listeners, so you'll leak all of them as
+ * well.
*/
- private final LifecycleOwner mLifecycleOwner;
+ private final WeakReference<LifecycleOwner> mLifecycleOwner;
private int mAddingObserverCounter = 0;
@@ -86,7 +94,7 @@
* @param provider The owner LifecycleOwner
*/
public LifecycleRegistry(@NonNull LifecycleOwner provider) {
- mLifecycleOwner = provider;
+ mLifecycleOwner = new WeakReference<>(provider);
mState = INITIALIZED;
}
@@ -148,15 +156,19 @@
if (previous != null) {
return;
}
+ LifecycleOwner lifecycleOwner = mLifecycleOwner.get();
+ if (lifecycleOwner == null) {
+ // it is null we should be destroyed. Fallback quickly
+ return;
+ }
boolean isReentrance = mAddingObserverCounter != 0 || mHandlingEvent;
-
State targetState = calculateTargetState(observer);
mAddingObserverCounter++;
while ((statefulObserver.mState.compareTo(targetState) < 0
&& mObserverMap.contains(observer))) {
pushParentState(statefulObserver.mState);
- statefulObserver.dispatchEvent(mLifecycleOwner, upEvent(statefulObserver.mState));
+ statefulObserver.dispatchEvent(lifecycleOwner, upEvent(statefulObserver.mState));
popParentState();
// mState / subling may have been changed recalculate
targetState = calculateTargetState(observer);
@@ -258,7 +270,7 @@
throw new IllegalArgumentException("Unexpected state value " + state);
}
- private void forwardPass() {
+ private void forwardPass(LifecycleOwner lifecycleOwner) {
Iterator<Entry<LifecycleObserver, ObserverWithState>> ascendingIterator =
mObserverMap.iteratorWithAdditions();
while (ascendingIterator.hasNext() && !mNewEventOccurred) {
@@ -267,13 +279,13 @@
while ((observer.mState.compareTo(mState) < 0 && !mNewEventOccurred
&& mObserverMap.contains(entry.getKey()))) {
pushParentState(observer.mState);
- observer.dispatchEvent(mLifecycleOwner, upEvent(observer.mState));
+ observer.dispatchEvent(lifecycleOwner, upEvent(observer.mState));
popParentState();
}
}
}
- private void backwardPass() {
+ private void backwardPass(LifecycleOwner lifecycleOwner) {
Iterator<Entry<LifecycleObserver, ObserverWithState>> descendingIterator =
mObserverMap.descendingIterator();
while (descendingIterator.hasNext() && !mNewEventOccurred) {
@@ -283,7 +295,7 @@
&& mObserverMap.contains(entry.getKey()))) {
Event event = downEvent(observer.mState);
pushParentState(getStateAfter(event));
- observer.dispatchEvent(mLifecycleOwner, event);
+ observer.dispatchEvent(lifecycleOwner, event);
popParentState();
}
}
@@ -292,16 +304,22 @@
// happens only on the top of stack (never in reentrance),
// so it doesn't have to take in account parents
private void sync() {
+ LifecycleOwner lifecycleOwner = mLifecycleOwner.get();
+ if (lifecycleOwner == null) {
+ Log.w(LOG_TAG, "LifecycleOwner is garbage collected, you shouldn't try dispatch "
+ + "new events from it.");
+ return;
+ }
while (!isSynced()) {
mNewEventOccurred = false;
// no need to check eldest for nullability, because isSynced does it for us.
if (mState.compareTo(mObserverMap.eldest().getValue().mState) < 0) {
- backwardPass();
+ backwardPass(lifecycleOwner);
}
Entry<LifecycleObserver, ObserverWithState> newest = mObserverMap.newest();
if (!mNewEventOccurred && newest != null
&& mState.compareTo(newest.getValue().mState) > 0) {
- forwardPass();
+ forwardPass(lifecycleOwner);
}
}
mNewEventOccurred = false;
diff --git a/lifecycle/runtime/src/test/java/android/arch/lifecycle/LifecycleRegistryTest.java b/lifecycle/runtime/src/test/java/android/arch/lifecycle/LifecycleRegistryTest.java
index 6506454..2a7bbad 100644
--- a/lifecycle/runtime/src/test/java/android/arch/lifecycle/LifecycleRegistryTest.java
+++ b/lifecycle/runtime/src/test/java/android/arch/lifecycle/LifecycleRegistryTest.java
@@ -566,6 +566,25 @@
verify(observer).onCreate();
}
+ private static void forceGc() {
+ Runtime.getRuntime().gc();
+ Runtime.getRuntime().runFinalization();
+ Runtime.getRuntime().gc();
+ Runtime.getRuntime().runFinalization();
+ }
+
+ @Test
+ public void goneLifecycleOwner() {
+ fullyInitializeRegistry();
+ mLifecycleOwner = null;
+ forceGc();
+ TestObserver observer = mock(TestObserver.class);
+ mRegistry.addObserver(observer);
+ verify(observer, never()).onCreate();
+ verify(observer, never()).onStart();
+ verify(observer, never()).onResume();
+ }
+
private void dispatchEvent(Lifecycle.Event event) {
when(mLifecycle.getCurrentState()).thenReturn(LifecycleRegistry.getStateAfter(event));
mRegistry.handleLifecycleEvent(event);