Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5828b413 authored by Riddle Hsu's avatar Riddle Hsu
Browse files

Invoke setWindowStopped on the thread that created the view 

The windows belong to a activity will be set to stopped state
when the activity is stopped. But the associated thread of
view root could be different from activity, that results the
inconsistent calling thread of surface callback, and maybe
some potential races.

Since the surface could be destroyed during traversal, also
add a check for callback notifySurfaceDestroyed to prevent
duplicated calls if the surface is released.

Bug: 130553315
Test: atest ViewRootSurfaceCallbackTest
Change-Id: Iea2cf8ff51118c0fa8c00a35c1d3e53c959ea754
parent c3c30eef

core/java/android/view/ViewRootImpl.java

+2 −1
Original line number Diff line number Diff line
@@ -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

+19 −1
Original line number Diff line number Diff line
@@ -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.

                    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 → 100644
+105 −0
Original line number Diff line number Diff line 
/*

 * 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) {

    }

}