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

Commit a7908183 authored by Chris Li's avatar Chris Li
Browse files

Cleanup InsetsSourceControl leash 

When WindowStateInsetsControlChangeItem doesn't execute, cleanup the
SurfaceControl

Bug: 339380439
Test: atest FrameworksCoreTests:ClientTransactionItemTest
Flag: com.android.window.flags.insets_control_changed_item
Change-Id: Icc04788959ae5b918fcc21dd251833563d60bf9a
parent e03a3175

core/java/android/app/servertransaction/WindowStateInsetsControlChangeItem.java

+7 −1
Original line number Diff line number Diff line
@@ -27,6 +27,8 @@ import android.view.IWindow; 
import android.view.InsetsSourceControl;

import android.view.InsetsState;



import com.android.internal.annotations.VisibleForTesting;



import java.util.Objects;



/**
@@ -38,7 +40,9 @@ public class WindowStateInsetsControlChangeItem extends WindowStateTransactionIt 
    private static final String TAG = "WindowStateInsetsControlChangeItem";



    private InsetsState mInsetsState;

    private InsetsSourceControl.Array mActiveControls;



    @VisibleForTesting

    public InsetsSourceControl.Array mActiveControls;



    @Override

    public void execute(@NonNull ClientTransactionHandler client, @NonNull IWindow window,
@@ -51,6 +55,8 @@ public class WindowStateInsetsControlChangeItem extends WindowStateTransactionIt 
            // An exception could happen if the process is restarted. It is safe to ignore since

            // the window should no longer exist.

            Log.w(TAG, "The original window no longer exists in the new process", e);

            // Prevent leak

            mActiveControls.release();

        }

        Trace.traceEnd(Trace.TRACE_TAG_WINDOW_MANAGER);

    }

core/java/android/view/InsetsSourceControl.java

+12 −0
Original line number Diff line number Diff line
@@ -305,6 +305,18 @@ public class InsetsSourceControl implements Parcelable { 
            return mControls;

        }



        /** Cleanup {@link SurfaceControl} stored in controls to prevent leak. */

        public void release() {

            if (mControls == null) {

                return;

            }

            for (InsetsSourceControl control : mControls) {

                if (control != null) {

                    control.release(SurfaceControl::release);

                }

            }

        }



        /** Sets the given flags to all controls. */

        public void setParcelableFlags(int parcelableFlags) {

            if (mControls == null) {

core/java/android/view/ViewRootImpl.java

+5 −6
Original line number Diff line number Diff line
@@ -2294,12 +2294,8 @@ public final class ViewRootImpl implements ViewParent, 
        mInsetsController.onStateChanged(insetsState);

        if (mAdded) {

            mInsetsController.onControlsChanged(controls);

        } else if (controls != null) {

            for (InsetsSourceControl control : controls) {

                if (control != null) {

                    control.release(SurfaceControl::release);

                }

            }

        } else {

            activeControls.release();

        }

    }










@@ -11306,6 +11302,9 @@ public final class ViewRootImpl implements ViewParent,













            mIsFromTransactionItem = false;















            final ViewRootImpl viewAncestor = mViewAncestor.get();















            if (viewAncestor == null) {













                if (isFromInsetsControlChangeItem) {













                    activeControls.release();













                }















                return;















            }















            if (insetsState.isSourceOrDefaultVisible(ID_IME, Type.ime())) {

































core/tests/coretests/src/android/app/servertransaction/ClientTransactionItemTest.java



+15
−3




























Original line number
Diff line number
Diff line








@@ -23,6 +23,8 @@ import static org.junit.Assert.assertTrue;













import static org.mockito.ArgumentMatchers.any;















import static org.mockito.ArgumentMatchers.eq;















import static org.mockito.Mockito.doReturn;













import static org.mockito.Mockito.doThrow;













import static org.mockito.Mockito.spy;















import static org.mockito.Mockito.verify;































import android.app.Activity;









@@ -39,7 +41,6 @@ import android.view.InsetsSourceControl;













import android.view.InsetsState;















import android.window.ActivityWindowInfo;















import android.window.ClientWindowFrames;













import android.window.WindowContext;















import android.window.WindowContextInfo;































import androidx.test.filters.SmallTest;










@@ -73,8 +74,6 @@ public class ClientTransactionItemTest {













    @Mock















    private IBinder mWindowClientToken;















    @Mock













    private WindowContext mWindowContext;













    @Mock















    private IWindow mWindow;































    // Can't mock final class.










@@ -176,4 +175,17 @@ public class ClientTransactionItemTest {





























        verify(mWindow).insetsControlChanged(mInsetsState, mActiveControls);















    }



























    @Test













    public void testWindowStateInsetsControlChangeItem_executeError() throws RemoteException {













        doThrow(new RemoteException()).when(mWindow).insetsControlChanged(any(), any());



























        mActiveControls = spy(mActiveControls);













        final WindowStateInsetsControlChangeItem item = WindowStateInsetsControlChangeItem.obtain(













                mWindow, mInsetsState, mActiveControls);













        item.mActiveControls = mActiveControls;













        item.execute(mHandler, mPendingActions);



























        verify(mActiveControls).release();













    }















}