Merge "Invoke setWindowStopped on the thread that created the view" into qt-dev

core/java/android/view/ViewRootImpl.java

@@ -1522,6 +1522,7 @@ public final class ViewRootImpl implements ViewParent,
     }
 
     void setWindowStopped(boolean stopped) {
+        checkThread();
         if (mStopped != stopped) {
             mStopped = stopped;
             final ThreadedRenderer renderer = mAttachInfo.mThreadedRenderer;
@@ -1543,7 +1544,7 @@ public final class ViewRootImpl implements ViewParent,
             }
 
             if (mStopped) {
-                if (mSurfaceHolder != null) {
+                if (mSurfaceHolder != null && mSurface.isValid()) {
                     notifySurfaceDestroyed();
                 }
                 destroySurface();

core/java/android/view/WindowManagerGlobal.java

@@ -636,13 +636,21 @@ public final class WindowManagerGlobal {
     }
 
     public void setStoppedState(IBinder token, boolean stopped) {
+        ArrayList<ViewRootImpl> nonCurrentThreadRoots = null;
         synchronized (mLock) {
             int count = mViews.size();
             for (int i = count - 1; i >= 0; i--) {
                 if (token == null || mParams.get(i).token == token) {
                     ViewRootImpl root = mRoots.get(i);
                     // Client might remove the view by "stopped" event.
-                    root.setWindowStopped(stopped);
+                    if (root.mThread == Thread.currentThread()) {
+                        root.setWindowStopped(stopped);
+                    } else {
+                        if (nonCurrentThreadRoots == null) {
+                            nonCurrentThreadRoots = new ArrayList<>();
+                        }
+                        nonCurrentThreadRoots.add(root);
+                    }
                     // Recursively forward stopped state to View's attached
                     // to this Window rather than the root application token,
                     // e.g. PopupWindow's.
@@ -650,6 +658,16 @@ public final class WindowManagerGlobal {
                 }
             }
         }
+
+        // Update the stopped state synchronously to ensure the surface won't be used after server
+        // side has destroyed it. This operation should be outside the lock to avoid any potential
+        // paths from setWindowStopped to WindowManagerGlobal which may cause deadlocks.
+        if (nonCurrentThreadRoots != null) {
+            for (int i = nonCurrentThreadRoots.size() - 1; i >= 0; i--) {
+                ViewRootImpl root = nonCurrentThreadRoots.get(i);
+                root.mHandler.runWithScissors(() -> root.setWindowStopped(stopped), 0);
+            }
+        }
     }
 
     public void reportNewConfiguration(Configuration config) {

core/tests/coretests/src/android/view/ViewRootSurfaceCallbackTest.java

@@ -0,0 +1,105 @@
+/*
+ * Copyright (C) 2019 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 android.os.Process.myTid;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+
+import android.app.Activity;
+import android.app.AlertDialog;
+import android.os.HandlerThread;
+
+import androidx.test.filters.SmallTest;
+import androidx.test.platform.app.InstrumentationRegistry;
+import androidx.test.rule.ActivityTestRule;
+
+import org.junit.Rule;
+import org.junit.Test;
+
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+
+@SmallTest
+public class ViewRootSurfaceCallbackTest implements SurfaceHolder.Callback2 {
+    private static final String TAG = ViewRootSurfaceCallbackTest.class.getSimpleName();
+    private static final long TIMEOUT_MS = TimeUnit.SECONDS.toMillis(3);
+
+    @Rule
+    public ActivityTestRule<Activity> mActivityRule = new ActivityTestRule<>(Activity.class);
+
+    private final CompletableFuture<Integer> mTidOfSurfaceCreated = new CompletableFuture<>();
+    private final CompletableFuture<Integer> mTidOfSurfaceDestroyed = new CompletableFuture<>();
+
+    private boolean mDuplicatedSurfaceDestroyed;
+
+    /**
+     * Verifies that the calling thread of {@link SurfaceHolder.Callback2} should be the same as the
+     * thread that created the view root.
+     */
+    @Test
+    public void testCallingTidOfSurfaceCallback() throws Exception {
+        final Activity activity = mActivityRule.getActivity();
+        activity.setTurnScreenOn(true);
+        activity.setShowWhenLocked(true);
+
+        // Create a dialog that runs on another thread and let it handle surface by itself.
+        final HandlerThread thread = new HandlerThread(TAG);
+        thread.start();
+        thread.getThreadHandler().runWithScissors(() -> {
+            final AlertDialog dialog = new AlertDialog.Builder(activity)
+                    .setTitle(TAG).setMessage(TAG).create();
+            dialog.getWindow().takeSurface(this);
+            dialog.show();
+        }, TIMEOUT_MS);
+        final int attachedTid = thread.getThreadId();
+
+        assertEquals(attachedTid,
+                mTidOfSurfaceCreated.get(TIMEOUT_MS, TimeUnit.MILLISECONDS).intValue());
+
+        // Make the activity invisible.
+        activity.moveTaskToBack(true /* nonRoot */);
+        InstrumentationRegistry.getInstrumentation().waitForIdleSync();
+
+        assertEquals(attachedTid,
+                mTidOfSurfaceDestroyed.get(TIMEOUT_MS, TimeUnit.MILLISECONDS).intValue());
+        assertFalse("surfaceDestroyed should not be called twice", mDuplicatedSurfaceDestroyed);
+    }
+
+    @Override
+    public void surfaceCreated(SurfaceHolder holder) {
+        mTidOfSurfaceCreated.complete(myTid());
+    }
+
+    @Override
+    public void surfaceDestroyed(SurfaceHolder holder) {
+        if  (!mTidOfSurfaceDestroyed.isDone()) {
+            mTidOfSurfaceDestroyed.complete(myTid());
+        } else {
+            mDuplicatedSurfaceDestroyed = true;
+        }
+    }
+
+    @Override
+    public void surfaceChanged(SurfaceHolder holder, int format, int width, int height) {
+    }
+
+    @Override
+    public void surfaceRedrawNeeded(SurfaceHolder holder) {
+    }
+}