Make detachViewFromParent more robust

Calling detachViewFromParent() without calling remove or attach in the
same drawing frame could, in some situations, cause a crash in the native
DisplayList code. detach/attach are intended to be very lightweight and do
not manage the native DisplayList content the same way that add/remove do.
Nor do they cause an invalidate() or requestLayout(), which would cause the
native structures to get recreated appropriately.

This fix makes this process more robust in two ways:
- DisplayLists should not get finalized (therefore destroying their native
structures) when there are still parent DisplayLists referring to them
(each DisplayList keeps references to its child DisplayLists). This will
prevent the native crash associated with unmatched detach*() calls.
- The docs for detach/attach have been enhanced to make it easier for
developers to understand how to use these methods more correctly and
successfully.

Issue #7064818 detachViewFromParent() should be more robust

Change-Id: I53befc04d5d58c225060f397725566d470488c9b

3 files changed, 60 insertions, 22 deletions

diff --git a/core/java/android/view/GLES20DisplayList.java b/core/java/android/view/GLES20DisplayList.java
index 2d2e8e48e16d..3bdd5c054c3c 100644
--- a/core/java/android/view/GLES20DisplayList.java
+++ b/core/java/android/view/GLES20DisplayList.java
@@ -25,9 +25,12 @@ import java.util.ArrayList;
  * An implementation of display list for OpenGL ES 2.0.
  */
 class GLES20DisplayList extends DisplayList {
-    // These lists ensure that any Bitmaps recorded by a DisplayList are kept alive as long
-    // as the DisplayList is alive.  The Bitmaps are populated by the GLES20RecordingCanvas.
+    // These lists ensure that any Bitmaps and DisplayLists recorded by a DisplayList are kept
+    // alive as long as the DisplayList is alive.  The Bitmap and DisplayList lists
+    // are populated by the GLES20RecordingCanvas during appropriate drawing calls and are
+    // cleared at the start of a new drawing frame or when the view is detached from the window.
     final ArrayList<Bitmap> mBitmaps = new ArrayList<Bitmap>(5);
+    final ArrayList<DisplayList> mChildDisplayLists = new ArrayList<DisplayList>();
 
     private GLES20RecordingCanvas mCanvas;
     private boolean mValid;
@@ -79,6 +82,7 @@ class GLES20DisplayList extends DisplayList {
     public void clear() {
         if (!mValid) {
             mBitmaps.clear();
+            mChildDisplayLists.clear();
         }
     }
 
diff --git a/core/java/android/view/GLES20RecordingCanvas.java b/core/java/android/view/GLES20RecordingCanvas.java
index c9ba65f3ab40..ba8be6c34763 100644
--- a/core/java/android/view/GLES20RecordingCanvas.java
+++ b/core/java/android/view/GLES20RecordingCanvas.java
@@ -76,6 +76,7 @@ class GLES20RecordingCanvas extends GLES20Canvas implements Poolable<GLES20Recor
 
     void start() {
         mDisplayList.mBitmaps.clear();
+        mDisplayList.mChildDisplayLists.clear();
     }
 
     int end(int nativeDisplayList) {
@@ -156,6 +157,13 @@ class GLES20RecordingCanvas extends GLES20Canvas implements Poolable<GLES20Recor
     }
 
     @Override
+    public int drawDisplayList(DisplayList displayList, Rect dirty, int flags) {
+        int status = super.drawDisplayList(displayList, dirty, flags);
+        mDisplayList.mChildDisplayLists.add(displayList);
+        return status;
+    }
+
+    @Override
     public void drawLine(float startX, float startY, float stopX, float stopY, Paint paint) {
         super.drawLine(startX, startY, stopX, stopY, paint);
         recordShaderBitmap(paint);
diff --git a/core/java/android/view/ViewGroup.java b/core/java/android/view/ViewGroup.java
index 3ab0e9480561..62e138353328 100644
--- a/core/java/android/view/ViewGroup.java
+++ b/core/java/android/view/ViewGroup.java
@@ -3806,6 +3806,13 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
     /**
      * Finishes the removal of a detached view. This method will dispatch the detached from
      * window event and notify the hierarchy change listener.
+     * <p>
+     * This method is intended to be lightweight and makes no assumptions about whether the
+     * parent or child should be redrawn. Proper use of this method will include also making
+     * any appropriate {@link #requestLayout()} or {@link #invalidate()} calls.
+     * For example, callers can {@link #post(Runnable) post} a {@link Runnable}
+     * which performs a {@link #requestLayout()} on the next frame, after all detach/remove
+     * calls are finished, causing layout to be run prior to redrawing the view hierarchy.
      *
      * @param child the child to be definitely removed from the view hierarchy
      * @param animate if true and the view has an animation, the view is placed in the
@@ -3846,10 +3853,17 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
 
     /**
      * Attaches a view to this view group. Attaching a view assigns this group as the parent,
-     * sets the layout parameters and puts the view in the list of children so it can be retrieved
-     * by calling {@link #getChildAt(int)}.
-     *
-     * This method should be called only for view which were detached from their parent.
+     * sets the layout parameters and puts the view in the list of children so that
+     * it can be retrieved by calling {@link #getChildAt(int)}.
+     * <p>
+     * This method is intended to be lightweight and makes no assumptions about whether the
+     * parent or child should be redrawn. Proper use of this method will include also making
+     * any appropriate {@link #requestLayout()} or {@link #invalidate()} calls.
+     * For example, callers can {@link #post(Runnable) post} a {@link Runnable}
+     * which performs a {@link #requestLayout()} on the next frame, after all detach/attach
+     * calls are finished, causing layout to be run prior to redrawing the view hierarchy.
+     * <p>
+     * This method should be called only for views which were detached from their parent.
      *
      * @param child the child to attach
      * @param index the index at which the child should be attached
@@ -3881,10 +3895,13 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
     }
 
     /**
-     * Detaches a view from its parent. Detaching a view should be temporary and followed
-     * either by a call to {@link #attachViewToParent(View, int, android.view.ViewGroup.LayoutParams)}
-     * or a call to {@link #removeDetachedView(View, boolean)}. When a view is detached,
-     * its parent is null and cannot be retrieved by a call to {@link #getChildAt(int)}.
+     * Detaches a view from its parent. Detaching a view should be followed
+     * either by a call to
+     * {@link #attachViewToParent(View, int, android.view.ViewGroup.LayoutParams)}
+     * or a call to {@link #removeDetachedView(View, boolean)}. Detachment should only be
+     * temporary; reattachment or removal should happen within the same drawing cycle as
+     * detachment. When a view is detached, its parent is null and cannot be retrieved by a
+     * call to {@link #getChildAt(int)}.
      *
      * @param child the child to detach
      *
@@ -3899,10 +3916,13 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
     }
 
     /**
-     * Detaches a view from its parent. Detaching a view should be temporary and followed
-     * either by a call to {@link #attachViewToParent(View, int, android.view.ViewGroup.LayoutParams)}
-     * or a call to {@link #removeDetachedView(View, boolean)}. When a view is detached,
-     * its parent is null and cannot be retrieved by a call to {@link #getChildAt(int)}.
+     * Detaches a view from its parent. Detaching a view should be followed
+     * either by a call to
+     * {@link #attachViewToParent(View, int, android.view.ViewGroup.LayoutParams)}
+     * or a call to {@link #removeDetachedView(View, boolean)}. Detachment should only be
+     * temporary; reattachment or removal should happen within the same drawing cycle as
+     * detachment. When a view is detached, its parent is null and cannot be retrieved by a
+     * call to {@link #getChildAt(int)}.
      *
      * @param index the index of the child to detach
      *
@@ -3917,10 +3937,13 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
     }
 
     /**
-     * Detaches a range of view from their parent. Detaching a view should be temporary and followed
-     * either by a call to {@link #attachViewToParent(View, int, android.view.ViewGroup.LayoutParams)}
-     * or a call to {@link #removeDetachedView(View, boolean)}. When a view is detached, its
-     * parent is null and cannot be retrieved by a call to {@link #getChildAt(int)}.
+     * Detaches a range of views from their parents. Detaching a view should be followed
+     * either by a call to
+     * {@link #attachViewToParent(View, int, android.view.ViewGroup.LayoutParams)}
+     * or a call to {@link #removeDetachedView(View, boolean)}. Detachment should only be
+     * temporary; reattachment or removal should happen within the same drawing cycle as
+     * detachment. When a view is detached, its parent is null and cannot be retrieved by a
+     * call to {@link #getChildAt(int)}.
      *
      * @param start the first index of the childrend range to detach
      * @param count the number of children to detach
@@ -3936,10 +3959,13 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
     }
 
     /**
-     * Detaches all views from the parent. Detaching a view should be temporary and followed
-     * either by a call to {@link #attachViewToParent(View, int, android.view.ViewGroup.LayoutParams)}
-     * or a call to {@link #removeDetachedView(View, boolean)}. When a view is detached,
-     * its parent is null and cannot be retrieved by a call to {@link #getChildAt(int)}.
+     * Detaches all views from the parent. Detaching a view should be followed
+     * either by a call to
+     * {@link #attachViewToParent(View, int, android.view.ViewGroup.LayoutParams)}
+     * or a call to {@link #removeDetachedView(View, boolean)}. Detachment should only be
+     * temporary; reattachment or removal should happen within the same drawing cycle as
+     * detachment. When a view is detached, its parent is null and cannot be retrieved by a
+     * call to {@link #getChildAt(int)}.
      *
      * @see #detachViewFromParent(View)
      * @see #detachViewFromParent(int)