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

Commit 4854d8f9 authored by Chris Li's avatar Chris Li Committed by Android (Google) Code Review
Browse files

Merge "Refactor for ClientTransaction to return boolean for RemoteException" into main

parents 298ab122 86ad6dfa

core/java/android/app/servertransaction/ClientTransaction.java

+10 −2
Original line number Diff line number Diff line
@@ -236,9 +236,17 @@ public class ClientTransaction implements Parcelable { 
     * 2. The transaction message is scheduled.

     * 3. The client calls {@link TransactionExecutor#execute(ClientTransaction)}, which executes

     *    all callbacks and necessary lifecycle transitions.

     *

     * @return {@link RemoteException} if the transaction failed.

     */

    public void schedule() throws RemoteException {

    @Nullable

    public RemoteException schedule() {

        try {

            mClient.scheduleTransaction(this);

            return null;

        } catch (RemoteException e) {

            return e;

        }

    }



    // Parcelable implementation

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

+61 −41
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import android.app.servertransaction.LaunchActivityItem; 
import android.compat.annotation.ChangeId;

import android.compat.annotation.EnabledSince;

import android.os.Build;

import android.os.DeadObjectException;

import android.os.IBinder;

import android.os.RemoteException;

import android.os.Trace;
@@ -66,64 +67,80 @@ class ClientLifecycleManager { 
    /**

     * Schedules a transaction, which may consist of multiple callbacks and a lifecycle request.

     * @param transaction A sequence of client transaction items.

     * @throws RemoteException

     *

     * @return {@code false} if the transaction failed because of {@link RemoteException}.

     * @see ClientTransaction

     */

    @VisibleForTesting

    void scheduleTransaction(@NonNull ClientTransaction transaction) throws RemoteException {

        final IApplicationThread client = transaction.getClient();

        try {

            transaction.schedule();

        } catch (RemoteException e) {

            Slog.w(TAG, "Failed to deliver transaction for " + client

                            + "\ntransaction=" + transaction);

            throw e;

    boolean scheduleTransaction(@NonNull ClientTransaction transaction) {

        final RemoteException e = transaction.schedule();

        if (e != null) {

            final WindowProcessController wpc = mWms.mAtmService.getProcessController(

                    transaction.getClient());

            Slog.w(TAG, "Failed to deliver transaction for " + wpc + "\ntransaction=" + this, e);

            return false;

        }

        return true;

    }



    /**

     * Similar to {@link #scheduleTransactionItem}, but it sends the transaction immediately and

     * it can be called without WM lock.

     *

     * @return {@code false} if the transaction failed because of {@link RemoteException}.

     * @see WindowProcessController#setReportedProcState(int)

     */

    void scheduleTransactionItemNow(@NonNull IApplicationThread client,

    boolean scheduleTransactionItemNow(@NonNull IApplicationThread client,

            @NonNull ClientTransactionItem transactionItem) throws RemoteException {

        final ClientTransaction clientTransaction = new ClientTransaction(client);

        clientTransaction.addTransactionItem(transactionItem);

        scheduleTransaction(clientTransaction);

        final boolean res = scheduleTransaction(clientTransaction);

        if (!com.android.window.flags.Flags.cleanupDispatchPendingTransactionsRemoteException()

                && !res) {

            throw new DeadObjectException();

        }

        return res;

    }



    /**

     * Schedules a transaction with the given item, delivery to client application.

     * Schedules a transaction with the given item, delivery to client application, which may be

     * queued to dispatched later.

     *

     * @throws RemoteException

     * @return {@code false} if the transaction was dispatched immediately, but failed because of

     *         {@link RemoteException}. If the transaction is queued, any failure will be ignored.

     * @see ClientTransactionItem

     */

    void scheduleTransactionItem(@NonNull IApplicationThread client,

    boolean scheduleTransactionItem(@NonNull IApplicationThread client,

            @NonNull ClientTransactionItem item) throws RemoteException {

        // Wait until RootWindowContainer#performSurfacePlacementNoTrace to dispatch all pending

        // transactions at once.

        final ClientTransaction clientTransaction = getOrCreatePendingTransaction(client);

        clientTransaction.addTransactionItem(item);



        onClientTransactionItemScheduled(clientTransaction, false /* shouldDispatchImmediately */);

        final boolean res = onClientTransactionItemScheduled(clientTransaction,

                false /* shouldDispatchImmediately */);

        if (!com.android.window.flags.Flags.cleanupDispatchPendingTransactionsRemoteException()

                && !res) {

            throw new DeadObjectException();

        }

        return res;

    }



    /**

     * Schedules a transaction with the given items, delivery to client application.

     * Schedules a transaction with the given items, delivery to client application, which may be

     * queued to dispatched later.

     *

     * @throws RemoteException

     * @return {@code false} if the transaction was dispatched immediately, but failed because of

     *         {@link RemoteException}. If the transaction is queued, any failure will be ignored.

     * @see ClientTransactionItem

     */

    void scheduleTransactionItems(@NonNull IApplicationThread client,

    boolean scheduleTransactionItems(@NonNull IApplicationThread client,

            @NonNull ClientTransactionItem... items) throws RemoteException {

        scheduleTransactionItems(client, false /* shouldDispatchImmediately */, items);

        return scheduleTransactionItems(client, false /* shouldDispatchImmediately */, items);

    }



    /**

     * Schedules a transaction with the given items, delivery to client application.

     * Schedules a transaction with the given items, delivery to client application, which may be

     * queued to dispatched later.

     *

     * @param shouldDispatchImmediately whether or not to dispatch the transaction immediately. This

     *                                  should only be {@code true} when it is important to know the
@@ -131,11 +148,11 @@ class ClientLifecycleManager { 
     *                                  launches an app, the server needs to know if the transaction

     *                                  is dispatched successfully, and may restart the process if

     *                                  not.

     *

     * @throws RemoteException

     * @return {@code false} if the transaction was dispatched immediately, but failed because of

     *         {@link RemoteException}. If the transaction is queued, any failure will be ignored.

     * @see ClientTransactionItem

     */

    void scheduleTransactionItems(@NonNull IApplicationThread client,

    boolean scheduleTransactionItems(@NonNull IApplicationThread client,

            boolean shouldDispatchImmediately,

            @NonNull ClientTransactionItem... items) throws RemoteException {

        // Wait until RootWindowContainer#performSurfacePlacementNoTrace to dispatch all pending
@@ -147,7 +164,13 @@ class ClientLifecycleManager { 
            clientTransaction.addTransactionItem(items[i]);

        }



        onClientTransactionItemScheduled(clientTransaction, shouldDispatchImmediately);

        final boolean res = onClientTransactionItemScheduled(clientTransaction,

                shouldDispatchImmediately);

        if (!com.android.window.flags.Flags.cleanupDispatchPendingTransactionsRemoteException()

                && !res) {

            throw new DeadObjectException();

        }

        return res;

    }



    /** Executes all the pending transactions. */
@@ -159,12 +182,7 @@ class ClientLifecycleManager { 
        final int size = mPendingTransactions.size();

        for (int i = 0; i < size; i++) {

            final ClientTransaction transaction = mPendingTransactions.valueAt(i);

            try {

            scheduleTransaction(transaction);

            } catch (RemoteException e) {

                Slog.e(TAG, "Failed to deliver pending transaction", e);

                // TODO(b/323801078): apply cleanup for individual transaction item if needed.

            }

        }

        mPendingTransactions.clear();

        Trace.traceEnd(Trace.TRACE_TAG_WINDOW_MANAGER);
@@ -174,12 +192,7 @@ class ClientLifecycleManager { 
    void dispatchPendingTransaction(@NonNull IApplicationThread client) {

        final ClientTransaction pendingTransaction = mPendingTransactions.remove(client.asBinder());

        if (pendingTransaction != null) {

            try {

            scheduleTransaction(pendingTransaction);

            } catch (RemoteException e) {

                Slog.e(TAG, "Failed to deliver pending transaction", e);

                // TODO(b/323801078): apply cleanup for individual transaction item if needed.

            }

        }

    }
@@ -211,15 +224,22 @@ class ClientLifecycleManager { 
        return transaction;

    }



    /** Must only be called with WM lock. */

    private void onClientTransactionItemScheduled(

    /**

     * Must only be called with WM lock.

     * If the transaction should not be queued, it will be dispatched immediately.

     *

     * @return {@code false} if the transaction was dispatched immediately, but failed because of

     *         {@link RemoteException}.

     */

    private boolean onClientTransactionItemScheduled(

            @NonNull ClientTransaction clientTransaction,

            boolean shouldDispatchImmediately) throws RemoteException {

            boolean shouldDispatchImmediately) {

        if (shouldDispatchImmediately || shouldDispatchPendingTransactionsImmediately()) {

            // Dispatch the pending transaction immediately.

            mPendingTransactions.remove(clientTransaction.getClient().asBinder());

            scheduleTransaction(clientTransaction);

            return scheduleTransaction(clientTransaction);

        }

        return true;

    }



    /** Must only be called with WM lock. */

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

+3 −4
Original line number Diff line number Diff line
@@ -41,8 +41,8 @@ import static android.content.pm.ActivityInfo.SCREEN_ORIENTATION_PORTRAIT; 
import static android.content.pm.ActivityInfo.SCREEN_ORIENTATION_REVERSE_LANDSCAPE;

import static android.content.pm.ActivityInfo.SCREEN_ORIENTATION_UNSET;

import static android.content.pm.ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED;

import static android.content.pm.ApplicationInfo.CATEGORY_SOCIAL;

import static android.content.pm.ApplicationInfo.CATEGORY_GAME;

import static android.content.pm.ApplicationInfo.CATEGORY_SOCIAL;

import static android.content.res.Configuration.ORIENTATION_LANDSCAPE;

import static android.content.res.Configuration.ORIENTATION_PORTRAIT;

import static android.content.res.Configuration.UI_MODE_TYPE_DESK;
@@ -66,7 +66,6 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.anyBoolean; 
import static com.android.dx.mockito.inline.extended.ExtendedMockito.atLeast;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.doAnswer;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.doCallRealMethod;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.doNothing;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.eq;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.mock;
@@ -196,7 +195,7 @@ public class ActivityRecordTests extends WindowTestsBase { 
        // Because the booted state is set, avoid starting real home if there is no task.

        doReturn(false).when(mRootWindowContainer).resumeHomeActivity(any(), anyString(), any());

        // Do not execute the transaction, because we can't verify the parameter after it recycles.

        doNothing().when(mClientLifecycleManager).scheduleTransaction(any());

        doReturn(true).when(mClientLifecycleManager).scheduleTransaction(any());

    }



    private TestStartingWindowOrganizer registerTestStartingWindowOrganizer() {
@@ -266,7 +265,7 @@ public class ActivityRecordTests extends WindowTestsBase { 
                    break;

                }

            }

            return null;

            return true;

        }).when(mClientLifecycleManager).scheduleTransaction(any());



        activity.setState(STOPPED, "testPausingWhenVisibleFromStopped");

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

+54 −22
Original line number Diff line number Diff line
@@ -16,11 +16,13 @@ 


package com.android.server.wm;



import static com.android.dx.mockito.inline.extended.ExtendedMockito.doThrow;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.spy;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify;



import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertNull;

import static org.junit.Assert.assertTrue;

import static org.mockito.ArgumentMatchers.any;
@@ -32,17 +34,19 @@ import android.app.IApplicationThread; 
import android.app.servertransaction.ActivityLifecycleItem;

import android.app.servertransaction.ClientTransaction;

import android.app.servertransaction.ClientTransactionItem;

import android.os.DeadObjectException;

import android.os.IBinder;

import android.os.RemoteException;

import android.platform.test.annotations.EnableFlags;

import android.platform.test.annotations.Presubmit;



import androidx.test.filters.SmallTest;



import com.android.window.flags.Flags;



import org.junit.Before;

import org.junit.Test;

import org.junit.runner.RunWith;

import org.mockito.ArgumentCaptor;

import org.mockito.Captor;

import org.mockito.Mock;

import org.mockito.MockitoAnnotations;
@@ -60,15 +64,11 @@ public class ClientLifecycleManagerTests extends SystemServiceTestsBase { 
    @Mock

    private IApplicationThread mClient;

    @Mock

    private IApplicationThread.Stub mNonBinderClient;

    @Mock

    private ClientTransaction mTransaction;

    @Mock

    private ClientTransactionItem mTransactionItem;

    @Mock

    private ActivityLifecycleItem mLifecycleItem;

    @Captor

    private ArgumentCaptor<ClientTransaction> mTransactionCaptor;



    private WindowManagerService mWms;

    private ClientLifecycleManager mLifecycleManager;
@@ -83,7 +83,6 @@ public class ClientLifecycleManagerTests extends SystemServiceTestsBase { 


        doReturn(true).when(mLifecycleItem).isActivityLifecycleItem();

        doReturn(mClientBinder).when(mClient).asBinder();

        doReturn(mNonBinderClient).when(mNonBinderClient).asBinder();

    }



    @Test
@@ -91,13 +90,11 @@ public class ClientLifecycleManagerTests extends SystemServiceTestsBase { 
        spyOn(mWms.mWindowPlacerLocked);

        doReturn(true).when(mWms.mWindowPlacerLocked).isTraversalScheduled();



        // Use non binder client to get non-recycled ClientTransaction.

        mLifecycleManager.scheduleTransactionItem(mNonBinderClient, mTransactionItem);

        mLifecycleManager.scheduleTransactionItem(mClient, mTransactionItem);



        // When there is traversal scheduled, add transaction items to pending.

        assertEquals(1, mLifecycleManager.mPendingTransactions.size());

        ClientTransaction transaction =

                mLifecycleManager.mPendingTransactions.get(mNonBinderClient);

        ClientTransaction transaction = mLifecycleManager.mPendingTransactions.get(mClientBinder);

        assertEquals(1, transaction.getTransactionItems().size());

        assertEquals(mTransactionItem, transaction.getTransactionItems().get(0));

        // TODO(b/324203798): cleanup after remove UnsupportedAppUsage
@@ -108,10 +105,10 @@ public class ClientLifecycleManagerTests extends SystemServiceTestsBase { 


        // Add new transaction item to the existing pending.

        clearInvocations(mLifecycleManager);

        mLifecycleManager.scheduleTransactionItem(mNonBinderClient, mLifecycleItem);

        mLifecycleManager.scheduleTransactionItem(mClient, mLifecycleItem);



        assertEquals(1, mLifecycleManager.mPendingTransactions.size());

        transaction = mLifecycleManager.mPendingTransactions.get(mNonBinderClient);

        transaction = mLifecycleManager.mPendingTransactions.get(mClientBinder);

        assertEquals(2, transaction.getTransactionItems().size());

        assertEquals(mTransactionItem, transaction.getTransactionItems().get(0));

        assertEquals(mLifecycleItem, transaction.getTransactionItems().get(1));
@@ -124,8 +121,7 @@ public class ClientLifecycleManagerTests extends SystemServiceTestsBase { 


    @Test

    public void testScheduleTransactionItemNow() throws RemoteException {

        // Use non binder client to get non-recycled ClientTransaction.

        mLifecycleManager.scheduleTransactionItemNow(mNonBinderClient, mTransactionItem);

        mLifecycleManager.scheduleTransactionItemNow(mClient, mTransactionItem);



        // Dispatch immediately.

        assertTrue(mLifecycleManager.mPendingTransactions.isEmpty());
@@ -137,13 +133,11 @@ public class ClientLifecycleManagerTests extends SystemServiceTestsBase { 
        spyOn(mWms.mWindowPlacerLocked);

        doReturn(true).when(mWms.mWindowPlacerLocked).isTraversalScheduled();



        // Use non binder client to get non-recycled ClientTransaction.

        mLifecycleManager.scheduleTransactionItems(mNonBinderClient, mTransactionItem,

                mLifecycleItem);

        mLifecycleManager.scheduleTransactionItems(mClient, mTransactionItem, mLifecycleItem);



        assertEquals(1, mLifecycleManager.mPendingTransactions.size());

        final ClientTransaction transaction =

                mLifecycleManager.mPendingTransactions.get(mNonBinderClient);

                mLifecycleManager.mPendingTransactions.get(mClientBinder);

        assertEquals(2, transaction.getTransactionItems().size());

        assertEquals(mTransactionItem, transaction.getTransactionItems().get(0));

        assertEquals(mLifecycleItem, transaction.getTransactionItems().get(1));
@@ -160,8 +154,8 @@ public class ClientLifecycleManagerTests extends SystemServiceTestsBase { 
        spyOn(mWms.mWindowPlacerLocked);

        doReturn(true).when(mWms.mWindowPlacerLocked).isTraversalScheduled();



        // Use non binder client to get non-recycled ClientTransaction.

        mLifecycleManager.scheduleTransactionItems(mNonBinderClient,

        mLifecycleManager.scheduleTransactionItems(

                mClient,

                true /* shouldDispatchImmediately */,

                mTransactionItem, mLifecycleItem);
@@ -187,7 +181,7 @@ public class ClientLifecycleManagerTests extends SystemServiceTestsBase { 
        doReturn(true).when(mWms.mWindowPlacerLocked).isLayoutDeferred();



        // Queue transactions during layout deferred.

        mLifecycleManager.scheduleTransactionItem(mNonBinderClient, mLifecycleItem);

        mLifecycleManager.scheduleTransactionItem(mClient, mLifecycleItem);



        verify(mLifecycleManager, never()).scheduleTransaction(any());
@@ -203,4 +197,42 @@ public class ClientLifecycleManagerTests extends SystemServiceTestsBase { 


        verify(mLifecycleManager).scheduleTransaction(any());

    }



    @EnableFlags(Flags.FLAG_CLEANUP_DISPATCH_PENDING_TRANSACTIONS_REMOTE_EXCEPTION)

    @Test

    public void testOnRemoteException_returnTrueOnSuccess() throws RemoteException {

        final boolean res = mLifecycleManager.scheduleTransactionItemNow(mClient, mTransactionItem);



        assertTrue(res);

    }



    @EnableFlags(Flags.FLAG_CLEANUP_DISPATCH_PENDING_TRANSACTIONS_REMOTE_EXCEPTION)

    @Test

    public void testOnRemoteException_returnFalseOnFailure() throws RemoteException {

        final DeadObjectException e = new DeadObjectException();

        doThrow(e).when(mClient).scheduleTransaction(any());



        // No exception when flag enabled.

        final boolean res = mLifecycleManager.scheduleTransactionItemNow(mClient, mTransactionItem);



        assertFalse(res);

    }



    @EnableFlags(Flags.FLAG_CLEANUP_DISPATCH_PENDING_TRANSACTIONS_REMOTE_EXCEPTION)

    @Test

    public void testOnRemoteException_returnTrueForQueueing() throws RemoteException {

        spyOn(mWms.mWindowPlacerLocked);

        doReturn(true).when(mWms.mWindowPlacerLocked).isLayoutDeferred();

        final DeadObjectException e = new DeadObjectException();

        doThrow(e).when(mClient).scheduleTransaction(any());



        final boolean res = mLifecycleManager.scheduleTransactionItem(mClient, mTransactionItem);



        assertTrue(res);



        doReturn(false).when(mWms.mWindowPlacerLocked).isLayoutDeferred();

        mLifecycleManager.onLayoutContinued();



        verify(mLifecycleManager).scheduleTransaction(any());

    }

}

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

+1 −2
Original line number Diff line number Diff line
@@ -24,7 +24,6 @@ import static android.content.res.Configuration.GRAMMATICAL_GENDER_NOT_SPECIFIED 
import static android.content.res.Configuration.ORIENTATION_LANDSCAPE;

import static android.content.res.Configuration.ORIENTATION_PORTRAIT;



import static com.android.dx.mockito.inline.extended.ExtendedMockito.doNothing;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.never;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify;
@@ -295,7 +294,7 @@ public class WindowProcessControllerTests extends WindowTestsBase { 


    @Test

    public void testCachedStateConfigurationChange() throws RemoteException {

        doNothing().when(mClientLifecycleManager).scheduleTransactionItemNow(any(), any());

        doReturn(true).when(mClientLifecycleManager).scheduleTransactionItemNow(any(), any());

        final IApplicationThread thread = mWpc.getThread();

        final Configuration newConfig = new Configuration(mWpc.getConfiguration());

        newConfig.densityDpi += 100;