Merge "Invoke setWindowStopped on the thread that created the view" into qt-dev
diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java
index e835675..e9c856f 100644
--- a/core/java/android/view/ViewRootImpl.java
+++ b/core/java/android/view/ViewRootImpl.java
@@ -1522,6 +1522,7 @@
}
void setWindowStopped(boolean stopped) {
+ checkThread();
if (mStopped != stopped) {
mStopped = stopped;
final ThreadedRenderer renderer = mAttachInfo.mThreadedRenderer;
@@ -1543,7 +1544,7 @@
}
if (mStopped) {
- if (mSurfaceHolder != null) {
+ if (mSurfaceHolder != null && mSurface.isValid()) {
notifySurfaceDestroyed();
}
destroySurface();
diff --git a/core/java/android/view/WindowManagerGlobal.java b/core/java/android/view/WindowManagerGlobal.java
index 8a111cf..379acbe 100644
--- a/core/java/android/view/WindowManagerGlobal.java
+++ b/core/java/android/view/WindowManagerGlobal.java
@@ -636,13 +636,21 @@
}
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 @@
}
}
}
+
+ // 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) {
diff --git a/core/tests/coretests/src/android/view/ViewRootSurfaceCallbackTest.java b/core/tests/coretests/src/android/view/ViewRootSurfaceCallbackTest.java
new file mode 100644
index 0000000..94917cf
--- /dev/null
+++ b/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) {
+ }
+}
