Ensure consistent attach/detach callback on transient view

- Not allow to add as transient view if it still has a parent.
  (The normal usage of addTransientView should be after removeView)
- If a view container is not attached, its transient view should
  not be attached.
- If a transient view is already detached from parent, no need
  to detach again when removing the transient view.

Bug: 109814657
Test: atest FrameworksCoreTests:ViewGroupTransientViewTest
Change-Id: I243cc292e4aaba987d48eeb202559de866a8c208

2 files changed, 176 insertions, 4 deletions

diff --git a/core/java/android/view/ViewGroup.java b/core/java/android/view/ViewGroup.java
index 794431966443..d0539ae30719 100644
--- a/core/java/android/view/ViewGroup.java
+++ b/core/java/android/view/ViewGroup.java
@@ -4652,7 +4652,7 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
      * which is added in order to fade it out in its old location should be removed
      * once the animation is complete.</p>
      *
-     * @param view The view to be added
+     * @param view The view to be added. The view must not have a parent.
      * @param index The index at which this view should be drawn, must be >= 0.
      * This value is relative to the {@link #getChildAt(int) index} values in the normal
      * child list of this container, where any transient view at a particular index will
@@ -4661,9 +4661,14 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
      * @hide
      */
     public void addTransientView(View view, int index) {
-        if (index < 0) {
+        if (index < 0 || view == null) {
             return;
         }
+        if (view.mParent != null) {
+            throw new IllegalStateException("The specified view already has a parent "
+                    + view.mParent);
+        }
+
         if (mTransientIndices == null) {
             mTransientIndices = new ArrayList<Integer>();
             mTransientViews = new ArrayList<View>();
@@ -4683,7 +4688,9 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
             mTransientViews.add(view);
         }
         view.mParent = this;
-        view.dispatchAttachedToWindow(mAttachInfo, (mViewFlags&VISIBILITY_MASK));
+        if (mAttachInfo != null) {
+            view.dispatchAttachedToWindow(mAttachInfo, (mViewFlags & VISIBILITY_MASK));
+        }
         invalidate(true);
     }
 
@@ -4705,7 +4712,9 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
                 mTransientViews.remove(i);
                 mTransientIndices.remove(i);
                 view.mParent = null;
-                view.dispatchDetachedFromWindow();
+                if (view.mAttachInfo != null) {
+                    view.dispatchDetachedFromWindow();
+                }
                 invalidate(true);
                 return;
             }
diff --git a/core/tests/coretests/src/android/view/ViewGroupTransientViewTest.java b/core/tests/coretests/src/android/view/ViewGroupTransientViewTest.java
new file mode 100644
index 000000000000..93ad41f08b06
--- /dev/null
+++ b/core/tests/coretests/src/android/view/ViewGroupTransientViewTest.java
@@ -0,0 +1,163 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.view;
+
+import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.fail;
+
+import android.app.Activity;
+import android.content.Context;
+import android.graphics.Canvas;
+import android.support.test.annotation.UiThreadTest;
+import android.support.test.filters.MediumTest;
+import android.support.test.rule.ActivityTestRule;
+import android.support.test.runner.AndroidJUnit4;
+import android.widget.FrameLayout;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+@MediumTest
+@RunWith(AndroidJUnit4.class)
+public class ViewGroupTransientViewTest {
+
+    @Rule
+    public ActivityTestRule<Activity> mActivityRule = new ActivityTestRule<>(Activity.class);
+
+    private FrameLayout mBasePanel;
+    private ViewGroup mTestViewGroup;
+    private TestView mTestView;
+
+    @Before
+    public void setUp() {
+        final Activity activity = mActivityRule.getActivity();
+        mBasePanel = new FrameLayout(activity);
+        mTestViewGroup = new FrameLayout(activity);
+        mTestView = new TestView(activity);
+        activity.runOnUiThread(() -> activity.setContentView(mBasePanel));
+    }
+
+    @UiThreadTest
+    @Test
+    public void addAndRemove_inNonAttachedViewGroup_shouldNotAttachAndDetach() {
+        mTestViewGroup.addTransientView(mTestView, 0);
+        assertEquals(0, mTestView.mAttachedCount);
+
+        mTestViewGroup.removeTransientView(mTestView);
+        assertEquals(0, mTestView.mDetachedCount);
+    }
+
+    @UiThreadTest
+    @Test
+    public void addAndRemove_inAttachedViewGroup_shouldAttachAndDetachOnce() {
+        mBasePanel.addView(mTestViewGroup);
+        mTestViewGroup.addTransientView(mTestView, 0);
+        assertEquals(mTestView, mTestViewGroup.getTransientView(0));
+        assertEquals(1, mTestViewGroup.getTransientViewCount());
+        assertEquals(1, mTestView.mAttachedCount);
+
+        mBasePanel.removeView(mTestViewGroup);
+        mTestViewGroup.removeTransientView(mTestView);
+        assertEquals(null, mTestViewGroup.getTransientView(0));
+        assertEquals(0, mTestViewGroup.getTransientViewCount());
+        assertEquals(1, mTestView.mDetachedCount);
+    }
+
+    @UiThreadTest
+    @Test
+    public void addRemoveAdd_noException() {
+        mBasePanel.addView(mTestViewGroup);
+        mTestViewGroup.addTransientView(mTestView, 1);
+        mTestViewGroup.removeTransientView(mTestView);
+        mTestViewGroup.addTransientView(mTestView, 2);
+    }
+
+    @UiThreadTest
+    @Test
+    public void reAddBeforeRemove_shouldThrowException() {
+        mTestViewGroup.addView(mTestView);
+
+        try {
+            mTestViewGroup.addTransientView(mTestView, 0);
+            fail("Not allow to add as transient view before removing it");
+        } catch (IllegalStateException e) {
+            // Expected
+        }
+
+        mTestViewGroup.removeView(mTestView);
+        mTestViewGroup.addTransientView(mTestView, 0);
+        try {
+            mTestViewGroup.addTransientView(mTestView, 1);
+            fail("Not allow to add the same transient view again");
+        } catch (IllegalStateException e) {
+            // Expected
+        }
+    }
+
+    @Test
+    public void drawTransientView() throws Exception {
+        // For view can be drawn if keyguard is active.
+        mActivityRule.getActivity().setShowWhenLocked(true);
+
+        final CountDownLatch latch = new CountDownLatch(1);
+        mTestView.mOnDraw = () -> latch.countDown();
+
+        mActivityRule.getActivity().runOnUiThread(() -> {
+            mBasePanel.addView(mTestViewGroup);
+            mTestViewGroup.addTransientView(mTestView, 0);
+        });
+
+        if (!latch.await(3, TimeUnit.SECONDS)) {
+            fail("Transient view does not draw");
+        }
+    }
+
+    private static class TestView extends View {
+        int mAttachedCount;
+        int mDetachedCount;
+        Runnable mOnDraw;
+
+        TestView(Context context) {
+            super(context);
+        }
+
+        @Override
+        protected void onAttachedToWindow() {
+            super.onAttachedToWindow();
+            mAttachedCount++;
+        }
+
+        @Override
+        protected void onDetachedFromWindow() {
+            super.onDetachedFromWindow();
+            mDetachedCount++;
+        }
+
+        @Override
+        protected void onDraw(Canvas canvas) {
+            if (mOnDraw != null) {
+                mOnDraw.run();
+                mOnDraw = null;
+            }
+        }
+    }
+}