From cfd8662c1231d38a9739afb1b8b4b0ca3d146d2c Mon Sep 17 00:00:00 2001
From: "pyricau@users.noreply.github.com" <pyricau@users.noreply.github.com>
Date: Tue, 30 Jun 2020 09:44:17 -0700
Subject: [PATCH] Fix ApplicationWrapper activity weakRef leak (#1301)

Summary:
Fix for https://github.com/facebook/flipper/issues/1300

The weak refs were not being cleared in two cases:

- On config changes, isFinishing() would be false in onPause()
- When calling finish() from Activity.onCreate(), onPause() isn't guaranteed to be called.

## Changelog
Pull Request resolved: https://github.com/facebook/flipper/pull/1301

Reviewed By: mweststrate

Differential Revision: D22286182

Pulled By: passy

fbshipit-source-id: 948d1d9b2145b6526c0030cf537330409ff7f8c4
---
 .../plugins/inspector/ApplicationWrapper.java | 25 ++++++++-----------
 .../inspector/ApplicationWrapperTest.java     | 15 +++++++++++
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/android/src/main/java/com/facebook/flipper/plugins/inspector/ApplicationWrapper.java b/android/src/main/java/com/facebook/flipper/plugins/inspector/ApplicationWrapper.java
index 2044657c1..35f2f382f 100644
--- a/android/src/main/java/com/facebook/flipper/plugins/inspector/ApplicationWrapper.java
+++ b/android/src/main/java/com/facebook/flipper/plugins/inspector/ApplicationWrapper.java
@@ -54,19 +54,7 @@ public class ApplicationWrapper implements Application.ActivityLifecycleCallback
   public void onActivityResumed(Activity activity) {}
 
   @Override
-  public void onActivityPaused(Activity activity) {
-    if (activity.isFinishing()) {
-      final Iterator<WeakReference<Activity>> activityIterator = mActivities.iterator();
-
-      while (activityIterator.hasNext()) {
-        if (activityIterator.next().get() == activity) {
-          activityIterator.remove();
-        }
-      }
-
-      notifyListener();
-    }
-  }
+  public void onActivityPaused(Activity activity) {}
 
   @Override
   public void onActivityStopped(Activity activity) {}
@@ -75,7 +63,16 @@ public class ApplicationWrapper implements Application.ActivityLifecycleCallback
   public void onActivitySaveInstanceState(Activity activity, Bundle outState) {}
 
   @Override
-  public void onActivityDestroyed(Activity activity) {}
+  public void onActivityDestroyed(Activity activity) {
+    final Iterator<WeakReference<Activity>> activityIterator = mActivities.iterator();
+
+    while (activityIterator.hasNext()) {
+      if (activityIterator.next().get() == activity) {
+        activityIterator.remove();
+      }
+    }
+    notifyListener();
+  }
 
   private void notifyListener() {
     if (mListener != null) {
diff --git a/android/src/test/java/com/facebook/flipper/plugins/inspector/ApplicationWrapperTest.java b/android/src/test/java/com/facebook/flipper/plugins/inspector/ApplicationWrapperTest.java
index 48295d721..6d18f2edb 100644
--- a/android/src/test/java/com/facebook/flipper/plugins/inspector/ApplicationWrapperTest.java
+++ b/android/src/test/java/com/facebook/flipper/plugins/inspector/ApplicationWrapperTest.java
@@ -85,6 +85,21 @@ public class ApplicationWrapperTest {
     Mockito.when(activity2.isFinishing()).thenReturn(true);
     mCallbacks.onActivityPaused(activity2);
 
+    assertThat(mWrapper.getActivityStack().size(), equalTo(2));
+    assertThat(mWrapper.getActivityStack().get(0), equalTo(activity1));
+  }
+
+  @Test
+  public void testActivityDestroyed() {
+    final Activity activity1 = Mockito.mock(Activity.class);
+    mCallbacks.onActivityCreated(activity1, Mockito.mock(Bundle.class));
+
+    final Activity activity2 = Mockito.mock(Activity.class);
+    mCallbacks.onActivityCreated(activity2, Mockito.mock(Bundle.class));
+
+    Mockito.when(activity2.isFinishing()).thenReturn(true);
+    mCallbacks.onActivityDestroyed(activity2);
+
     assertThat(mWrapper.getActivityStack().size(), equalTo(1));
     assertThat(mWrapper.getActivityStack().get(0), equalTo(activity1));
   }