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

Commit 5db377ff authored by Antony Sargent's avatar Antony Sargent
Browse files

Fix how we send flag changes to mDwpcHelper in relayoutWindow 

We were accidentally conflating the exclusive OR bitmask of which
flags have changed, and the flag values themselves. This was causing
two problems:

(1) if a flag like FLAG_SECURE is added early in an Activity lifecycle
such as in onCreate, the first layout happens with the flag present
and therefore it wouldn't be included in the exclusive OR bitmask
value sent to DisplayWindowPolicyController (which was expecting the
flag value itself).

(2) If a flag happened to have been removed, it would end up in the
exclusive OR bitmask and get passed to DisplayWindowPolicyController,
which would think that flag is present, not removed.

Fixes: 261991662
Test: atest WmTests:WindowManagerServiceTests
Change-Id: I18c359665264e689fcecea6428e3b45dc6337983
parent 0c71d059

services/core/java/com/android/server/wm/DisplayWindowPolicyControllerHelper.java

+2 −2
Original line number Diff line number Diff line
@@ -114,7 +114,7 @@ class DisplayWindowPolicyControllerHelper { 
     * @see DisplayWindowPolicyController#keepActivityOnWindowFlagsChanged(ActivityInfo, int, int)

     */

    boolean keepActivityOnWindowFlagsChanged(ActivityInfo aInfo, int flagChanges,

            int privateFlagChanges) {

            int privateFlagChanges, int flagValues, int privateFlagValues) {

        if (mDisplayWindowPolicyController == null) {

            return true;

        }
@@ -125,7 +125,7 @@ class DisplayWindowPolicyControllerHelper { 
        }



        return mDisplayWindowPolicyController.keepActivityOnWindowFlagsChanged(

                aInfo, flagChanges, privateFlagChanges);

                aInfo, flagValues, privateFlagValues);

    }



    /** Update the top activity and the uids of non-finishing activity */

services/core/java/com/android/server/wm/WindowManagerService.java

+18 −8
Original line number Diff line number Diff line
@@ -2290,9 +2290,18 @@ public class WindowManagerService extends IWindowManager.Stub 
                    winAnimator.setColorSpaceAgnosticLocked((win.mAttrs.privateFlags

                            & WindowManager.LayoutParams.PRIVATE_FLAG_COLOR_SPACE_AGNOSTIC) != 0);

                }

                if (win.mActivityRecord != null

                        && !displayContent.mDwpcHelper.keepActivityOnWindowFlagsChanged(

                                win.mActivityRecord.info, flagChanges, privateFlagChanges)) {

                // See if the DisplayWindowPolicyController wants to keep the activity on the window

                if (displayContent.mDwpcHelper.hasController()

                        && win.mActivityRecord != null && (!win.mRelayoutCalled || flagChanges != 0

                        || privateFlagChanges != 0)) {

                    int newOrChangedFlags = !win.mRelayoutCalled ? win.mAttrs.flags : flagChanges;

                    int newOrChangedPrivateFlags =

                            !win.mRelayoutCalled ? win.mAttrs.privateFlags : privateFlagChanges;



                    if (!displayContent.mDwpcHelper.keepActivityOnWindowFlagsChanged(

                            win.mActivityRecord.info, newOrChangedFlags, newOrChangedPrivateFlags,

                            win.mAttrs.flags,

                            win.mAttrs.privateFlags)) {

                        mH.sendMessage(mH.obtainMessage(H.REPARENT_TASK_TO_DEFAULT_DISPLAY,

                                win.mActivityRecord.getTask()));

                        Slog.w(TAG_WM, "Activity " + win.mActivityRecord + " window flag changed,"
@@ -2300,6 +2309,7 @@ public class WindowManagerService extends IWindowManager.Stub 
                        return 0;

                    }

                }

            }



            if (DEBUG_LAYOUT) Slog.v(TAG_WM, "Relayout " + win + ": viewVisibility=" + viewVisibility

                    + " req=" + requestedWidth + "x" + requestedHeight + " " + win.mAttrs);

services/tests/wmtests/src/com/android/server/wm/WindowManagerServiceTests.java

+122 −0
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ import static android.hardware.display.DisplayManager.VIRTUAL_DISPLAY_FLAG_TRUST 
import static android.view.Display.DEFAULT_DISPLAY;

import static android.view.Display.FLAG_OWN_FOCUS;

import static android.view.WindowManager.LayoutParams.FLAG_NOT_FOCUSABLE;

import static android.view.WindowManager.LayoutParams.FLAG_SECURE;

import static android.view.WindowManager.LayoutParams.INPUT_FEATURE_SPY;

import static android.view.WindowManager.LayoutParams.INVALID_WINDOW_TYPE;

import static android.view.WindowManager.LayoutParams.PRIVATE_FLAG_TRUSTED_OVERLAY;
@@ -65,6 +66,7 @@ import static org.mockito.Mockito.times; 
import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;



import android.content.pm.ActivityInfo;

import android.content.res.Resources;

import android.graphics.Point;

import android.graphics.Rect;
@@ -89,6 +91,7 @@ import android.view.SurfaceControl; 
import android.view.View;

import android.view.WindowInsets;

import android.view.WindowManager;

import android.view.WindowManager.LayoutParams;

import android.window.ClientWindowFrames;

import android.window.ScreenCapture;

import android.window.WindowContainerToken;
@@ -101,6 +104,7 @@ import com.android.compatibility.common.util.AdoptShellPermissionsRule; 
import org.junit.Rule;

import org.junit.Test;

import org.junit.runner.RunWith;

import org.mockito.ArgumentCaptor;



/**

 * Build/Install/Run:
@@ -271,6 +275,124 @@ public class WindowManagerServiceTests extends WindowTestsBase { 
                outConfig.getMergedConfiguration().densityDpi);

    }



    @Test

    public void testRelayout_firstLayout_dwpcHelperCalledWithCorrectFlags() {

        // When doing the first layout, the initial flags should be reported as changed to

        // keepActivityOnWindowFlagsChanged.

        testRelayoutFlagChanges(

                /*firstRelayout=*/ true,

                /*startFlags=*/ FLAG_SECURE,

                /*startPrivateFlags=*/ PRIVATE_FLAG_TRUSTED_OVERLAY,

                /*newFlags=*/ FLAG_SECURE,

                /*newPrivateFlags=*/ PRIVATE_FLAG_TRUSTED_OVERLAY,

                /*expectedChangedFlags=*/ FLAG_SECURE,

                /*expectedChangedPrivateFlags=*/ PRIVATE_FLAG_TRUSTED_OVERLAY,

                /*expectedFlagsValue=*/ FLAG_SECURE,

                /*expectedPrivateFlagsValue=*/ PRIVATE_FLAG_TRUSTED_OVERLAY);

    }



    @Test

    public void testRelayout_secondLayoutFlagAdded_dwpcHelperCalledWithCorrectFlags() {

        testRelayoutFlagChanges(

                /*firstRelayout=*/ false,

                /*startFlags=*/ 0,

                /*startPrivateFlags=*/ 0,

                /*newFlags=*/ FLAG_SECURE,

                /*newPrivateFlags=*/ 0,

                /*expectedChangedFlags=*/ FLAG_SECURE,

                /*expectedChangedPrivateFlags=*/ 0,

                /*expectedFlagsValue=*/ FLAG_SECURE,

                /*expectedPrivateFlagsValue=*/ 0);

    }



    @Test

    public void testRelayout_secondLayoutMultipleFlagsAddOne_dwpcHelperCalledWithCorrectFlags() {

        testRelayoutFlagChanges(

                /*firstRelayout=*/ false,

                /*startFlags=*/ FLAG_NOT_FOCUSABLE,

                /*startPrivateFlags=*/ 0,

                /*newFlags=*/ FLAG_SECURE | FLAG_NOT_FOCUSABLE,

                /*newPrivateFlags=*/ 0,

                /*expectedChangedFlags=*/ FLAG_SECURE,

                /*expectedChangedPrivateFlags=*/ 0,

                /*expectedFlagsValue=*/ FLAG_SECURE | FLAG_NOT_FOCUSABLE,

                /*expectedPrivateFlagsValue=*/ 0);

    }



    @Test

    public void testRelayout_secondLayoutPrivateFlagAdded_dwpcHelperCalledWithCorrectFlags() {

        testRelayoutFlagChanges(

                /*firstRelayout=*/ false,

                /*startFlags=*/ 0,

                /*startPrivateFlags=*/ 0,

                /*newFlags=*/ 0,

                /*newPrivateFlags=*/ PRIVATE_FLAG_TRUSTED_OVERLAY,

                /*expectedChangedFlags=*/ 0,

                /*expectedChangedPrivateFlags=*/ PRIVATE_FLAG_TRUSTED_OVERLAY,

                /*expectedFlagsValue=*/ 0,

                /*expectedPrivateFlagsValue=*/ PRIVATE_FLAG_TRUSTED_OVERLAY);

    }



    @Test

    public void testRelayout_secondLayoutFlagsRemoved_dwpcHelperCalledWithCorrectFlags() {

        testRelayoutFlagChanges(

                /*firstRelayout=*/ false,

                /*startFlags=*/ FLAG_SECURE,

                /*startPrivateFlags=*/ PRIVATE_FLAG_TRUSTED_OVERLAY,

                /*newFlags=*/ 0,

                /*newPrivateFlags=*/ 0,

                /*expectedChangedFlags=*/ FLAG_SECURE,

                /*expectedChangedPrivateFlags=*/ PRIVATE_FLAG_TRUSTED_OVERLAY,

                /*expectedFlagsValue=*/ 0,

                /*expectedPrivateFlagsValue=*/ 0);

    }



    // Helper method to test relayout of a window, either for the initial layout, or a subsequent

    // one, and makes sure that the flags and private flags changes and final values are properly

    // reported to mDwpcHelper.keepActivityOnWindowFlagsChanged.

    private void testRelayoutFlagChanges(boolean firstRelayout, int startFlags,

            int startPrivateFlags, int newFlags, int newPrivateFlags, int expectedChangedFlags,

            int expectedChangedPrivateFlags, int expectedFlagsValue,

            int expectedPrivateFlagsValue) {

        final WindowState win = createWindow(null, TYPE_BASE_APPLICATION, "appWin");

        win.mRelayoutCalled = !firstRelayout;

        mWm.mWindowMap.put(win.mClient.asBinder(), win);

        spyOn(mDisplayContent.mDwpcHelper);

        when(mDisplayContent.mDwpcHelper.hasController()).thenReturn(true);



        win.mAttrs.flags = startFlags;

        win.mAttrs.privateFlags = startPrivateFlags;



        LayoutParams newParams = new LayoutParams();

        newParams.copyFrom(win.mAttrs);

        newParams.flags = newFlags;

        newParams.privateFlags = newPrivateFlags;



        int seq = 1;

        if (!firstRelayout) {

            win.mRelayoutSeq = 1;

            seq = 2;

        }

        mWm.relayoutWindow(win.mSession, win.mClient, newParams, 100, 200, View.VISIBLE, 0, seq,

                0, new ClientWindowFrames(), new MergedConfiguration(),

                new SurfaceControl(), new InsetsState(), new InsetsSourceControl.Array(),

                new Bundle());



        ArgumentCaptor<Integer> changedFlags = ArgumentCaptor.forClass(Integer.class);

        ArgumentCaptor<Integer> changedPrivateFlags = ArgumentCaptor.forClass(Integer.class);

        ArgumentCaptor<Integer> flagsValue = ArgumentCaptor.forClass(Integer.class);

        ArgumentCaptor<Integer> privateFlagsValue = ArgumentCaptor.forClass(Integer.class);



        verify(mDisplayContent.mDwpcHelper).keepActivityOnWindowFlagsChanged(

                any(ActivityInfo.class), changedFlags.capture(), changedPrivateFlags.capture(),

                flagsValue.capture(), privateFlagsValue.capture());



        assertThat(changedFlags.getValue()).isEqualTo(expectedChangedFlags);

        assertThat(changedPrivateFlags.getValue()).isEqualTo(expectedChangedPrivateFlags);

        assertThat(flagsValue.getValue()).isEqualTo(expectedFlagsValue);

        assertThat(privateFlagsValue.getValue()).isEqualTo(expectedPrivateFlagsValue);

    }



    @Test

    public void testMoveWindowTokenToDisplay_NullToken_DoNothing() {

        mWm.moveWindowTokenToDisplay(null, mDisplayContent.getDisplayId());