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

Commit aa6eda44 authored by Jorge Gil's avatar Jorge Gil
Browse files

Close the decoration before removing it from the cache 

This is a speculative fix for a NullPointerException triggered
in DesktopModeTouchEventListener#onTouch where obtaining a decoration
returns null unexpectedly.

A task closing while the caption window still has a touch target ends
up dispatching one final MotionEvent (cancel) to the caption window
touch listener, who expects a non-null decoration to be found in the
decoration cache. This dispatching is triggered by
WindowDecoration#close, which is called by #destroyWindowDecoration.
However, right before #close is called, the decoration was cleaned up
from the decoration cache, guaranteeing a NPE when the motion event is
handled.

This change moves the clean up until after WindowDecoration#close is
called, to allow the touch handler to still have a valid reference to
the closing decoration when it receives the final motion event.

Fix: 327664694
Fix: 323347594
Test: m
Change-Id: I1a0e8370453f59d81735f024093579a0fbe3324c
parent cb5c34af

libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/DesktopModeWindowDecorViewModel.java

+11 −6
Original line number Diff line number Diff line
@@ -129,8 +129,7 @@ public class DesktopModeWindowDecorViewModel implements WindowDecorViewModel { 
    private final ExclusionRegionListener mExclusionRegionListener =

            new ExclusionRegionListenerImpl();



    private final SparseArray<DesktopModeWindowDecoration> mWindowDecorByTaskId =

            new SparseArray<>();

    private final SparseArray<DesktopModeWindowDecoration> mWindowDecorByTaskId;

    private final DragStartListenerImpl mDragStartListener = new DragStartListenerImpl();

    private final InputMonitorFactory mInputMonitorFactory;

    private TaskOperations mTaskOperations;
@@ -197,7 +196,8 @@ public class DesktopModeWindowDecorViewModel implements WindowDecorViewModel { 
                new DesktopModeWindowDecoration.Factory(),

                new InputMonitorFactory(),

                SurfaceControl.Transaction::new,

                rootTaskDisplayAreaOrganizer);

                rootTaskDisplayAreaOrganizer,

                new SparseArray<>());

    }



    @VisibleForTesting
@@ -219,7 +219,8 @@ public class DesktopModeWindowDecorViewModel implements WindowDecorViewModel { 
            DesktopModeWindowDecoration.Factory desktopModeWindowDecorFactory,

            InputMonitorFactory inputMonitorFactory,

            Supplier<SurfaceControl.Transaction> transactionFactory,

            RootTaskDisplayAreaOrganizer rootTaskDisplayAreaOrganizer) {

            RootTaskDisplayAreaOrganizer rootTaskDisplayAreaOrganizer,

            SparseArray<DesktopModeWindowDecoration> windowDecorByTaskId) {

        mContext = context;

        mMainExecutor = shellExecutor;

        mMainHandler = mainHandler;
@@ -239,6 +240,7 @@ public class DesktopModeWindowDecorViewModel implements WindowDecorViewModel { 
        mTransactionFactory = transactionFactory;

        mRootTaskDisplayAreaOrganizer = rootTaskDisplayAreaOrganizer;

        mInputManager = mContext.getSystemService(InputManager.class);

        mWindowDecorByTaskId = windowDecorByTaskId;



        shellInit.addInitCallback(this::onInit, this);

    }
@@ -340,8 +342,7 @@ public class DesktopModeWindowDecorViewModel implements WindowDecorViewModel { 


    @Override

    public void destroyWindowDecoration(RunningTaskInfo taskInfo) {

        final DesktopModeWindowDecoration decoration =

                mWindowDecorByTaskId.removeReturnOld(taskInfo.taskId);

        final DesktopModeWindowDecoration decoration = mWindowDecorByTaskId.get(taskInfo.taskId);

        if (decoration == null) return;



        decoration.close();
@@ -349,6 +350,10 @@ public class DesktopModeWindowDecorViewModel implements WindowDecorViewModel { 
        if (mEventReceiversByDisplay.contains(displayId)) {

            removeTaskFromEventReceiver(displayId);

        }

        // Remove the decoration from the cache last because WindowDecoration#close could still

        // issue CANCEL MotionEvents to touch listeners before its view host is released.

        // See b/327664694.

        mWindowDecorByTaskId.remove(taskInfo.taskId);

    }



    private class DesktopModeTouchEventListener extends GestureDetector.SimpleOnGestureListener

libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/DesktopModeWindowDecorViewModelTests.kt

+20 −1
Original line number Diff line number Diff line
@@ -29,6 +29,7 @@ import android.hardware.display.VirtualDisplay 
import android.os.Handler

import android.testing.AndroidTestingRunner

import android.testing.TestableLooper.RunWithLooper

import android.util.SparseArray

import android.view.Choreographer

import android.view.Display.DEFAULT_DISPLAY

import android.view.IWindowManager
@@ -72,6 +73,8 @@ import org.mockito.kotlin.eq 
import org.mockito.kotlin.whenever

import java.util.Optional

import java.util.function.Supplier

import org.mockito.Mockito

import org.mockito.kotlin.spy





/** Tests of [DesktopModeWindowDecorViewModel]  */
@@ -102,6 +105,7 @@ class DesktopModeWindowDecorViewModelTests : ShellTestCase() { 
    private val transactionFactory = Supplier<SurfaceControl.Transaction> {

        SurfaceControl.Transaction()

    }

    private val windowDecorByTaskIdSpy = spy(SparseArray<DesktopModeWindowDecoration>())



    private lateinit var shellInit: ShellInit

    private lateinit var desktopModeOnInsetsChangedListener: DesktopModeOnInsetsChangedListener
@@ -110,6 +114,7 @@ class DesktopModeWindowDecorViewModelTests : ShellTestCase() { 
    @Before

    fun setUp() {

        shellInit = ShellInit(mockShellExecutor)

        windowDecorByTaskIdSpy.clear()

        desktopModeWindowDecorViewModel = DesktopModeWindowDecorViewModel(

                mContext,

                mockShellExecutor,
@@ -128,7 +133,8 @@ class DesktopModeWindowDecorViewModelTests : ShellTestCase() { 
                mockDesktopModeWindowDecorFactory,

                mockInputMonitorFactory,

                transactionFactory,

                mockRootTaskDisplayAreaOrganizer

                mockRootTaskDisplayAreaOrganizer,

            windowDecorByTaskIdSpy

        )



        whenever(mockDisplayController.getDisplayLayout(any())).thenReturn(mockDisplayLayout)
@@ -332,6 +338,19 @@ class DesktopModeWindowDecorViewModelTests : ShellTestCase() { 
        verify(decoration, times(1)).relayout(task)

    }



    @Test

    fun testDestroyWindowDecoration_closesBeforeCleanup() {

        val task = createTask(windowingMode = WINDOWING_MODE_FREEFORM)

        val decoration = setUpMockDecorationForTask(task)

        val inOrder = Mockito.inOrder(decoration, windowDecorByTaskIdSpy)



        onTaskOpening(task)

        desktopModeWindowDecorViewModel.destroyWindowDecoration(task)



        inOrder.verify(decoration).close()

        inOrder.verify(windowDecorByTaskIdSpy).remove(task.taskId)

    }



    private fun onTaskOpening(task: RunningTaskInfo, leash: SurfaceControl = SurfaceControl()) {

        desktopModeWindowDecorViewModel.onTaskOpening(

                task,