Fixing async inflation for nested RemoteViews

> Fixing isRootNamespace check
> Updating ViewTree when ViewStub is inflated
> Applying ViewGroupAction on previously found views instead of finding it again
  as the viewTree might have changed.

Test: am instrument -w -e class android.widget.RemoteViewsTest com.android.frameworks.coretests/android.support.test.runner.AndroidJUnitRunner
Bug: 32639592

Change-Id: I0815fb3a981efbc04fb0d080b81949985c2d9bc3

6 files changed, 322 insertions, 38 deletions

diff --git a/core/java/android/view/ViewStub.java b/core/java/android/view/ViewStub.java
index ec852e88b17e..85d10f199218 100644
--- a/core/java/android/view/ViewStub.java
+++ b/core/java/android/view/ViewStub.java
@@ -142,11 +142,17 @@ public final class ViewStub extends View {
      * @see #getInflatedId()
      * @attr ref android.R.styleable#ViewStub_inflatedId
      */
-    @android.view.RemotableViewMethod
+    @android.view.RemotableViewMethod(asyncImpl = "setInflatedIdAsync")
     public void setInflatedId(@IdRes int inflatedId) {
         mInflatedId = inflatedId;
     }
 
+    /** @hide **/
+    public Runnable setInflatedIdAsync(@IdRes int inflatedId) {
+        mInflatedId = inflatedId;
+        return null;
+    }
+
     /**
      * Returns the layout resource that will be used by {@link #setVisibility(int)} or
      * {@link #inflate()} to replace this StubbedView
@@ -176,11 +182,17 @@ public final class ViewStub extends View {
      * @see #inflate()
      * @attr ref android.R.styleable#ViewStub_layout
      */
-    @android.view.RemotableViewMethod
+    @android.view.RemotableViewMethod(asyncImpl = "setLayoutResourceAsync")
     public void setLayoutResource(@LayoutRes int layoutResource) {
         mLayoutResource = layoutResource;
     }
 
+    /** @hide **/
+    public Runnable setLayoutResourceAsync(@LayoutRes int layoutResource) {
+        mLayoutResource = layoutResource;
+        return null;
+    }
+
     /**
      * Set {@link LayoutInflater} to use in {@link #inflate()}, or {@code null}
      * to use the default.
@@ -220,7 +232,7 @@ public final class ViewStub extends View {
      * @see #inflate() 
      */
     @Override
-    @android.view.RemotableViewMethod
+    @android.view.RemotableViewMethod(asyncImpl = "setVisibilityAsync")
     public void setVisibility(int visibility) {
         if (mInflatedViewRef != null) {
             View view = mInflatedViewRef.get();
@@ -237,6 +249,43 @@ public final class ViewStub extends View {
         }
     }
 
+    /** @hide **/
+    public Runnable setVisibilityAsync(int visibility) {
+        if (visibility == VISIBLE || visibility == INVISIBLE) {
+            ViewGroup parent = (ViewGroup) getParent();
+            return new ViewReplaceRunnable(inflateViewNoAdd(parent));
+        } else {
+            return null;
+        }
+    }
+
+    private View inflateViewNoAdd(ViewGroup parent) {
+        final LayoutInflater factory;
+        if (mInflater != null) {
+            factory = mInflater;
+        } else {
+            factory = LayoutInflater.from(mContext);
+        }
+        final View view = factory.inflate(mLayoutResource, parent, false);
+
+        if (mInflatedId != NO_ID) {
+            view.setId(mInflatedId);
+        }
+        return view;
+    }
+
+    private void replaceSelfWithView(View view, ViewGroup parent) {
+        final int index = parent.indexOfChild(this);
+        parent.removeViewInLayout(this);
+
+        final ViewGroup.LayoutParams layoutParams = getLayoutParams();
+        if (layoutParams != null) {
+            parent.addView(view, index, layoutParams);
+        } else {
+            parent.addView(view, index);
+        }
+    }
+
     /**
      * Inflates the layout resource identified by {@link #getLayoutResource()}
      * and replaces this StubbedView in its parent by the inflated layout resource.
@@ -250,31 +299,10 @@ public final class ViewStub extends View {
         if (viewParent != null && viewParent instanceof ViewGroup) {
             if (mLayoutResource != 0) {
                 final ViewGroup parent = (ViewGroup) viewParent;
-                final LayoutInflater factory;
-                if (mInflater != null) {
-                    factory = mInflater;
-                } else {
-                    factory = LayoutInflater.from(mContext);
-                }
-                final View view = factory.inflate(mLayoutResource, parent,
-                        false);
-
-                if (mInflatedId != NO_ID) {
-                    view.setId(mInflatedId);
-                }
-
-                final int index = parent.indexOfChild(this);
-                parent.removeViewInLayout(this);
-
-                final ViewGroup.LayoutParams layoutParams = getLayoutParams();
-                if (layoutParams != null) {
-                    parent.addView(view, index, layoutParams);
-                } else {
-                    parent.addView(view, index);
-                }
-
-                mInflatedViewRef = new WeakReference<View>(view);
+                final View view = inflateViewNoAdd(parent);
+                replaceSelfWithView(view, parent);
 
+                mInflatedViewRef = new WeakReference<>(view);
                 if (mInflateListener != null) {
                     mInflateListener.onInflate(this, view);
                 }
@@ -317,4 +345,18 @@ public final class ViewStub extends View {
          */
         void onInflate(ViewStub stub, View inflated);
     }
+
+    /** @hide **/
+    public class ViewReplaceRunnable implements Runnable {
+        public final View view;
+
+        ViewReplaceRunnable(View view) {
+            this.view = view;
+        }
+
+        @Override
+        public void run() {
+            replaceSelfWithView(view, (ViewGroup) getParent());
+        }
+    }
 }
diff --git a/core/java/android/widget/RemoteViews.java b/core/java/android/widget/RemoteViews.java
index b2a77d0051b3..18ce260e7136 100644
--- a/core/java/android/widget/RemoteViews.java
+++ b/core/java/android/widget/RemoteViews.java
@@ -58,6 +58,7 @@ import android.view.RemotableViewMethod;
 import android.view.View;
 import android.view.View.OnClickListener;
 import android.view.ViewGroup;
+import android.view.ViewStub;
 import android.widget.AdapterView.OnItemClickListener;
 
 import com.android.internal.R;
@@ -1456,6 +1457,13 @@ public class RemoteViews implements Parcelable, Filter {
                     if (endAction == null) {
                         return ACTION_NOOP;
                     } else {
+                        // Special case view stub
+                        if (endAction instanceof ViewStub.ViewReplaceRunnable) {
+                            root.createTree();
+                            // Replace child tree
+                            root.findViewTreeById(viewId).replaceView(
+                                    ((ViewStub.ViewReplaceRunnable) endAction).view);
+                        }
                         return new RunnableAction(endAction);
                     }
                 }
@@ -1581,16 +1589,26 @@ public class RemoteViews implements Parcelable, Filter {
             if ((target == null) || !(target.mRoot instanceof ViewGroup)) {
                 return ACTION_NOOP;
             }
+            final ViewGroup targetVg = (ViewGroup) target.mRoot;
             if (nestedViews == null) {
                 // Clear all children when nested views omitted
                 target.mChildren = null;
-                return this;
+                return new RuntimeAction() {
+                    @Override
+                    public void apply(View root, ViewGroup rootParent, OnClickHandler handler)
+                            throws ActionException {
+                        targetVg.removeAllViews();
+                    }
+                };
             } else {
                 // Inflate nested views and perform all the async tasks for the child remoteView.
                 final Context context = root.mRoot.getContext();
                 final AsyncApplyTask task = nestedViews.getAsyncApplyTask(
-                        context, (ViewGroup) target.mRoot, null, handler);
+                        context, targetVg, null, handler);
                 final ViewTree tree = task.doInBackground();
+                if (tree == null) {
+                    throw new ActionException(task.mError);
+                }
 
                 // Update the global view tree, so that next call to findViewTreeById
                 // goes through the subtree as well.
@@ -1600,10 +1618,8 @@ public class RemoteViews implements Parcelable, Filter {
 
                     @Override
                     public void apply(View root, ViewGroup rootParent, OnClickHandler handler) throws ActionException {
-                        // This view will exist as we have already made sure
-                        final ViewGroup target = (ViewGroup) root.findViewById(viewId);
                         task.onPostExecute(tree);
-                        target.addView(task.mResult);
+                        targetVg.addView(task.mResult);
                     }
                 };
             }
@@ -3360,7 +3376,7 @@ public class RemoteViews implements Parcelable, Filter {
                     int count = mRV.mActions.size();
                     mActions = new Action[count];
                     for (int i = 0; i < count && !isCancelled(); i++) {
-                        // TODO: check if isCanclled in nested views.
+                        // TODO: check if isCancelled in nested views.
                         mActions[i] = mRV.mActions.get(i).initActionAsync(mTree, mParent, mHandler);
                     }
                 } else {
@@ -3629,7 +3645,7 @@ public class RemoteViews implements Parcelable, Filter {
      * and can be searched.
      */
     private static class ViewTree {
-        private final View mRoot;
+        private View mRoot;
 
         private ArrayList<ViewTree> mChildren;
 
@@ -3643,7 +3659,7 @@ public class RemoteViews implements Parcelable, Filter {
             }
 
             mChildren = new ArrayList<>();
-            if (mRoot instanceof ViewGroup && mRoot.isRootNamespace()) {
+            if (mRoot instanceof ViewGroup) {
                 ViewGroup vg = (ViewGroup) mRoot;
                 int count = vg.getChildCount();
                 for (int i = 0; i < count; i++) {
@@ -3668,6 +3684,12 @@ public class RemoteViews implements Parcelable, Filter {
             return null;
         }
 
+        public void replaceView(View v) {
+            mRoot = v;
+            mChildren = null;
+            createTree();
+        }
+
         public View findViewById(int id) {
             if (mChildren == null) {
                 return mRoot.findViewById(id);
@@ -3685,6 +3707,12 @@ public class RemoteViews implements Parcelable, Filter {
         }
 
         private void addViewChild(View v) {
+            // ViewTree only contains Views which can be found using findViewById.
+            // If isRootNamespace is true, this view is skipped.
+            // @see ViewGroup#findViewTraversal(int)
+            if (v.isRootNamespace()) {
+                return;
+            }
             final ViewTree target;
 
             // If the view has a valid id, i.e., if can be found using findViewById, add it to the
@@ -3697,7 +3725,7 @@ public class RemoteViews implements Parcelable, Filter {
                 target = this;
             }
 
-            if (v instanceof ViewGroup && v.isRootNamespace()) {
+            if (v instanceof ViewGroup) {
                 if (target.mChildren == null) {
                     target.mChildren = new ArrayList<>();
                     ViewGroup vg = (ViewGroup) v;
diff --git a/core/tests/coretests/res/layout/remote_view_host.xml b/core/tests/coretests/res/layout/remote_view_host.xml
index 19d0a738decc..68095081a9d9 100644
--- a/core/tests/coretests/res/layout/remote_view_host.xml
+++ b/core/tests/coretests/res/layout/remote_view_host.xml
@@ -19,7 +19,7 @@
 -->
 
 <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
+    android:id="@+id/container"
     android:orientation="vertical"
     android:layout_width="match_parent"
-    android:layout_height="match_parent">
-</LinearLayout>
+    android:layout_height="match_parent" />
diff --git a/core/tests/coretests/res/layout/remote_views_text.xml b/core/tests/coretests/res/layout/remote_views_text.xml
new file mode 100644
index 000000000000..a265d2e4b21e
--- /dev/null
+++ b/core/tests/coretests/res/layout/remote_views_text.xml
@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+  ~ Copyright (C) 2016 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
+  -->
+
+<TextView xmlns:android="http://schemas.android.com/apk/res/android"
+        android:id="@+id/text"
+        android:layout_width="match_parent"
+        android:layout_height="match_parent" />
diff --git a/core/tests/coretests/res/layout/remote_views_viewstub.xml b/core/tests/coretests/res/layout/remote_views_viewstub.xml
new file mode 100644
index 000000000000..d5327491ac5d
--- /dev/null
+++ b/core/tests/coretests/res/layout/remote_views_viewstub.xml
@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+  ~ Copyright (C) 2016 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
+  -->
+
+<FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"
+             android:layout_width="match_parent"
+             android:layout_height="0dp"
+             android:layout_weight="1">
+
+    <ViewStub android:id="@+id/viewStub"
+              android:inflatedId="@+id/stub_inflated"
+              android:layout_width="match_parent"
+              android:layout_height="match_parent" />
+</FrameLayout>
diff --git a/core/tests/coretests/src/android/widget/RemoteViewsTest.java b/core/tests/coretests/src/android/widget/RemoteViewsTest.java
index 7ba46bed8b34..b6b0e68c7b89 100644
--- a/core/tests/coretests/src/android/widget/RemoteViewsTest.java
+++ b/core/tests/coretests/src/android/widget/RemoteViewsTest.java
@@ -20,11 +20,13 @@ import android.content.Context;
 import android.graphics.Bitmap;
 import android.graphics.drawable.BitmapDrawable;
 import android.graphics.drawable.Drawable;
+import android.os.AsyncTask;
 import android.os.Parcel;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.SmallTest;
 import android.support.test.runner.AndroidJUnit4;
 import android.view.View;
+import android.view.ViewGroup;
 
 import com.android.frameworks.coretests.R;
 
@@ -38,6 +40,10 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.concurrent.CountDownLatch;
+
 /**
  * Tests for RemoteViews.
  */
@@ -166,4 +172,164 @@ public class RemoteViewsTest {
         parcel.recycle();
         return size;
     }
+
+    @Test
+    public void asyncApply_fail() throws Exception {
+        RemoteViews views = new RemoteViews(mPackage, R.layout.remote_view_test_bad_1);
+        ViewAppliedListener listener = new ViewAppliedListener();
+        views.applyAsync(mContext, mContainer, AsyncTask.THREAD_POOL_EXECUTOR, listener);
+
+        boolean exceptionThrown = false;
+        try {
+            listener.waitAndGetView();
+        } catch (Exception e) {
+            exceptionThrown = true;
+        }
+        assertTrue(exceptionThrown);
+    }
+
+    @Test
+    public void asyncApply() throws Exception {
+        RemoteViews views = new RemoteViews(mPackage, R.layout.remote_views_test);
+        views.setTextViewText(R.id.text, "Dummy");
+
+        View syncView = views.apply(mContext, mContainer);
+
+        ViewAppliedListener listener = new ViewAppliedListener();
+        views.applyAsync(mContext, mContainer, AsyncTask.THREAD_POOL_EXECUTOR, listener);
+        View asyncView = listener.waitAndGetView();
+
+        verifyViewTree(syncView, asyncView, "Dummy");
+    }
+
+    @Test
+    public void asyncApply_viewStub() throws Exception {
+        RemoteViews views = new RemoteViews(mPackage, R.layout.remote_views_viewstub);
+        views.setInt(R.id.viewStub, "setLayoutResource", R.layout.remote_views_text);
+        // This will cause the view to be inflated
+        views.setViewVisibility(R.id.viewStub, View.INVISIBLE);
+        views.setTextViewText(R.id.stub_inflated, "Dummy");
+
+        View syncView = views.apply(mContext, mContainer);
+
+        ViewAppliedListener listener = new ViewAppliedListener();
+        views.applyAsync(mContext, mContainer, AsyncTask.THREAD_POOL_EXECUTOR, listener);
+        View asyncView = listener.waitAndGetView();
+
+        verifyViewTree(syncView, asyncView, "Dummy");
+    }
+
+    @Test
+    public void asyncApply_nestedViews() throws Exception {
+        RemoteViews views = new RemoteViews(mPackage, R.layout.remote_view_host);
+        views.removeAllViews(R.id.container);
+        views.addView(R.id.container, createViewChained(1, "row1-c1", "row1-c2", "row1-c3"));
+        views.addView(R.id.container, createViewChained(5, "row2-c1", "row2-c2"));
+        views.addView(R.id.container, createViewChained(2, "row3-c1", "row3-c2"));
+
+        View syncView = views.apply(mContext, mContainer);
+
+        ViewAppliedListener listener = new ViewAppliedListener();
+        views.applyAsync(mContext, mContainer, AsyncTask.THREAD_POOL_EXECUTOR, listener);
+        View asyncView = listener.waitAndGetView();
+
+        verifyViewTree(syncView, asyncView,
+                "row1-c1", "row1-c2", "row1-c3", "row2-c1", "row2-c2", "row3-c1", "row3-c2");
+    }
+
+    @Test
+    public void asyncApply_viewstub_nestedViews() throws Exception {
+        RemoteViews viewstub = new RemoteViews(mPackage, R.layout.remote_views_viewstub);
+        viewstub.setInt(R.id.viewStub, "setLayoutResource", R.layout.remote_view_host);
+        // This will cause the view to be inflated
+        viewstub.setViewVisibility(R.id.viewStub, View.INVISIBLE);
+        viewstub.addView(R.id.stub_inflated, createViewChained(1, "row1-c1", "row1-c2", "row1-c3"));
+
+        RemoteViews views = new RemoteViews(mPackage, R.layout.remote_view_host);
+        views.removeAllViews(R.id.container);
+        views.addView(R.id.container, viewstub);
+        views.addView(R.id.container, createViewChained(5, "row2-c1", "row2-c2"));
+
+        View syncView = views.apply(mContext, mContainer);
+
+        ViewAppliedListener listener = new ViewAppliedListener();
+        views.applyAsync(mContext, mContainer, AsyncTask.THREAD_POOL_EXECUTOR, listener);
+        View asyncView = listener.waitAndGetView();
+
+        verifyViewTree(syncView, asyncView, "row1-c1", "row1-c2", "row1-c3", "row2-c1", "row2-c2");
+    }
+
+    private RemoteViews createViewChained(int depth, String... texts) {
+        RemoteViews result = new RemoteViews(mPackage, R.layout.remote_view_host);
+
+        // Create depth
+        RemoteViews parent = result;
+        while(depth > 0) {
+            depth--;
+            RemoteViews child = new RemoteViews(mPackage, R.layout.remote_view_host);
+            parent.addView(R.id.container, child);
+            parent = child;
+        }
+
+        // Add texts
+        for (String text : texts) {
+            RemoteViews child = new RemoteViews(mPackage, R.layout.remote_views_text);
+            child.setTextViewText(R.id.text, text);
+            parent.addView(R.id.container, child);
+        }
+        return result;
+    }
+
+    private void verifyViewTree(View v1, View v2, String... texts) {
+        ArrayList<String> expectedTexts = new ArrayList<>(Arrays.asList(texts));
+        verifyViewTreeRecur(v1, v2, expectedTexts);
+        // Verify that all expected texts were found
+        assertEquals(0, expectedTexts.size());
+    }
+
+    private void verifyViewTreeRecur(View v1, View v2, ArrayList<String> expectedTexts) {
+        assertEquals(v1.getClass(), v2.getClass());
+
+        if (v1 instanceof TextView) {
+            String text = ((TextView) v1).getText().toString();
+            assertEquals(text, ((TextView) v2).getText().toString());
+            // Verify that the text was one of the expected texts and remove it from the list
+            assertTrue(expectedTexts.remove(text));
+        } else if (v1 instanceof ViewGroup) {
+            ViewGroup vg1 = (ViewGroup) v1;
+            ViewGroup vg2 = (ViewGroup) v2;
+            assertEquals(vg1.getChildCount(), vg2.getChildCount());
+            for (int i = vg1.getChildCount() - 1; i >= 0; i--) {
+                verifyViewTreeRecur(vg1.getChildAt(i), vg2.getChildAt(i), expectedTexts);
+            }
+        }
+    }
+
+    private class ViewAppliedListener implements RemoteViews.OnViewAppliedListener {
+
+        private final CountDownLatch mLatch = new CountDownLatch(1);
+        private View mView;
+        private Exception mError;
+
+        @Override
+        public void onViewApplied(View v) {
+            mView = v;
+            mLatch.countDown();
+        }
+
+        @Override
+        public void onError(Exception e) {
+            mError = e;
+            mLatch.countDown();
+        }
+
+        public View waitAndGetView() throws Exception {
+            mLatch.await();
+
+            if (mError != null) {
+                throw new Exception(mError);
+            }
+            return mView;
+        }
+    }
 }