From efb65e566535052a79a7124b8ddeec9e409307bd Mon Sep 17 00:00:00 2001
From: Ahmed El-Helw <ahmedre@fb.com>
Date: Tue, 24 May 2016 14:48:23 -0700
Subject: [PATCH] Fix leak in Nodes due to removeClippedSubviews

Summary:
The removeClippedSubviews optimization often detaches views while
maintaining strong references to them (so they can be attached again later
on). However, when removing the parent view, any detached views end up not
being cleaned up or removed, thus leaking memory. This fixes this by
explicitly dropping detached views when the parent is removed.

Reviewed By: astreet

Differential Revision: D3337513
---
 .../flat/FlatNativeViewHierarchyManager.java  | 26 +++++++++++++++++++
 .../facebook/react/flat/FlatViewGroup.java    | 23 ++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatNativeViewHierarchyManager.java b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatNativeViewHierarchyManager.java
index 0403cc29a..4db2d180e 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatNativeViewHierarchyManager.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatNativeViewHierarchyManager.java
@@ -11,6 +11,8 @@ package com.facebook.react.flat;
 
 import javax.annotation.Nullable;
 
+import java.util.Collection;
+
 import android.view.View;
 import android.view.View.MeasureSpec;
 import android.view.ViewGroup;
@@ -134,6 +136,30 @@ import com.facebook.react.uimanager.ViewManagerRegistry;
     }
   }
 
+  @Override
+  protected void dropView(View view) {
+    super.dropView(view);
+
+    // As a result of removeClippedSubviews, some views have strong references but are not attached
+    // to a parent. consequently, when the parent gets removed, these Views don't get cleaned up,
+    // because they aren't children (they also aren't removed from mTagsToViews, thus causing a
+    // leak). To solve this, we ask for said detached views and explicitly drop them.
+    if (view instanceof FlatViewGroup) {
+      FlatViewGroup flatViewGroup = (FlatViewGroup) view;
+      if (flatViewGroup.getRemoveClippedSubviews()) {
+        Collection<FlatViewGroup> detachedViews = flatViewGroup.getDetachedViews();
+        for (FlatViewGroup detachedChild : detachedViews) {
+          // we can do super here because removeClippedSubviews is currently not recursive. if/when
+          // we become recursive one day, this should call vanilla dropView to be recursive as well.
+          super.dropView(detachedChild);
+          // trigger onDetachedFromWindow - this is currently needed due to using attach/detach
+          // instead of add/remove. if we move to add/remove in the future, we can remove this.
+          flatViewGroup.removeDetachedView(detachedChild);
+        }
+      }
+    }
+  }
+
   /* package */ void detachAllChildrenFromViews(int[] viewsToDetachAllChildrenFrom) {
     for (int viewTag : viewsToDetachAllChildrenFrom) {
       View view = resolveView(viewTag);
diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatViewGroup.java b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatViewGroup.java
index ed23daade..e7811bbae 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatViewGroup.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatViewGroup.java
@@ -13,6 +13,7 @@ import javax.annotation.Nullable;
 
 import java.lang.ref.WeakReference;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -407,6 +408,28 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper;
     invalidate();
   }
 
+  /**
+   * Return a list of FlatViewGroups that are detached (due to being clipped) but that we have a
+   * strong reference to. This is used by the FlatNativeViewHierarchyManager to explicitly clean up
+   * those views when removing this parent.
+   *
+   * @return a Collection of FlatViewGroups to clean up
+   */
+  Collection<FlatViewGroup> getDetachedViews() {
+    return mClippedSubviews.values();
+  }
+
+  /**
+   * Remove the detached view from the parent
+   * This is used during cleanup to trigger onDetachedFromWindow on any views that were in a
+   * temporary detached state due to them being clipped. This is called for cleanup of said views
+   * by FlatNativeViewHierarchyManager.
+   * @param flatViewGroup the detached FlatViewGroup to remove
+   */
+  void removeDetachedView(FlatViewGroup flatViewGroup) {
+    removeDetachedView(flatViewGroup, false);
+  }
+
   /* package */ void mountAttachDetachListeners(AttachDetachListener[] listeners) {
     if (mIsAttached) {
       // Ordering of the following 2 statements is very important. While logically it makes sense to