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

Commit e3d37b50 authored by Garfield Tan's avatar Garfield Tan Committed by Wale Ogunwale
Browse files

Adopt upstream Mockito's solution to mem leak. 

The upstream solution essentially disallows sharing mock between tests,
so the mock of PowerManagerWrapper is moved to SystemServicesTestRule so
that TestWindowManagerPolicy can use that to create window. It also
makes the life of PowerManagerWrapper mock the same as the
TestWindowManagerPolicy, which is used in both ActivityTestBase and
WindowTestBase but PowerManagerWrapper was only available in
WindowTestBase.

Also, Track mocks created in ActivityStarterTests#testStartActivityPreconditions
and clear them in the middle of execution because the memory usage could be
~0.8G just to run ActivityStarterTests only because of that.

Bug: 123984854
Test: "tradefed.sh run commandAndExit WmTests --include-annotation
android.platform.test.annotations.Presubmit --exclude-annotation
androidx.test.filters.FlakyTest" suffers from a lot fewer OOME.

Change-Id: I274d711fb52084d4c67547374360e373ad1e28d8
parent 8a1860a9

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

+15 −1
Original line number Diff line number Diff line
@@ -79,6 +79,7 @@ import android.view.Gravity; 
import androidx.test.filters.SmallTest;



import com.android.server.wm.LaunchParamsController.LaunchParamsModifier;

import com.android.server.wm.utils.MockTracker;



import org.junit.Before;

import org.junit.Test;
@@ -186,6 +187,19 @@ public class ActivityStarterTests extends ActivityTestsBase { 
        verifyStartActivityPreconditions(preconditions, 0 /*launchFlags*/, expectedResult);

    }



    private void verifyStartActivityPreconditions(int preconditions, int launchFlags,

            int expectedResult) {

        // We track mocks created here because this is used in a single test

        // (testStartActivityPreconditions) as a specific case, and mocks created inside it won't be

        // used for other cases. To avoid extensive memory usage, we clean up all used mocks after

        // each case. This is necessary because usually we only clean up mocks after a test

        // finishes, but this test creates too many mocks that the intermediate memory usage can be

        // ~0.8 GiB and thus very susceptible to OutOfMemoryException.

        try (MockTracker tracker = new MockTracker()) {

            verifyStartActivityPreconditionsUntracked(preconditions, launchFlags, expectedResult);

        }

    }



    /**

     * Excercises how the {@link ActivityStarter} reacts to various preconditions. The caller

     * provides a bitmask of all the set conditions (such as {@link #PRECONDITION_NO_CALLER_APP})
@@ -197,7 +211,7 @@ public class ActivityStarterTests extends ActivityTestsBase { 
     * @param launchFlags The launch flags to be provided by the launch {@link Intent}.

     * @param expectedResult The expected result from the launch.

     */

    private void verifyStartActivityPreconditions(int preconditions, int launchFlags,

    private void verifyStartActivityPreconditionsUntracked(int preconditions, int launchFlags,

            int expectedResult) {

        final ActivityTaskManagerService service = mService;

        final IPackageManager packageManager = mock(IPackageManager.class);

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

+0 −12
Original line number Diff line number Diff line
@@ -45,9 +45,7 @@ import android.testing.DexmakerShareClassLoaderRule; 
import android.view.DisplayInfo;



import com.android.server.AttributeCache;

import com.android.server.wm.utils.MockTracker;



import org.junit.After;

import org.junit.Before;

import org.junit.BeforeClass;

import org.junit.Rule;
@@ -70,8 +68,6 @@ class ActivityTestsBase { 
    RootActivityContainer mRootActivityContainer;

    ActivityStackSupervisor mSupervisor;



    private MockTracker mMockTracker;



    // Default package name

    static final String DEFAULT_COMPONENT_PACKAGE_NAME = "com.foo";
@@ -85,19 +81,11 @@ class ActivityTestsBase { 


    @Before

    public void setUpBase() {

        mMockTracker = new MockTracker();



        mService = mSystemServicesTestRule.getActivityTaskManagerService();

        mSupervisor = mService.mStackSupervisor;

        mRootActivityContainer = mService.mRootActivityContainer;

    }



    @After

    public void tearDownBase() {

        mMockTracker.close();

        mMockTracker = null;

    }



    /** Creates a {@link TestActivityDisplay}. */

    TestActivityDisplay createNewActivityDisplay() {

        return TestActivityDisplay.create(mSupervisor);

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

+23 −31
Original line number Diff line number Diff line
@@ -75,11 +75,11 @@ import com.android.server.policy.PermissionPolicyInternal; 
import com.android.server.policy.WindowManagerPolicy;

import com.android.server.statusbar.StatusBarManagerInternal;

import com.android.server.uri.UriGrantsManagerInternal;

import com.android.server.wm.utils.MockTracker;



import org.junit.rules.TestRule;

import org.junit.runner.Description;

import org.junit.runners.model.Statement;

import org.mockito.Mockito;

import org.mockito.quality.Strictness;



import java.io.File;
@@ -99,25 +99,19 @@ public class SystemServicesTestRule implements TestRule { 
    private final AtomicBoolean mCurrentMessagesProcessed = new AtomicBoolean(false);



    private Context mContext;

    private MockTracker mMockTracker;

    private StaticMockitoSession mMockitoSession;

    ServiceThread mHandlerThread;

    private ActivityManagerService mAmService;

    private ActivityTaskManagerService mAtmService;

    private WindowManagerService mWmService;

    private TestWindowManagerPolicy mWMPolicy;

    private WindowState.PowerManagerWrapper mPowerManagerWrapper;

    private InputManagerService mImService;

    /**

     * Spied {@link SurfaceControl.Transaction} class than can be used to verify calls.

     */

    SurfaceControl.Transaction mTransaction;



    /** {@link MockTracker} to track mocks created by {@link SystemServicesTestRule}. */

    private static class Tracker extends MockTracker {

        // This empty extended class is necessary since Mockito distinguishes a listener by it

        // class.

    }



    @Override

    public Statement apply(Statement base, Description description) {

        return new Statement() {
@@ -134,9 +128,6 @@ public class SystemServicesTestRule implements TestRule { 
    }



    private void setUp() {

        try {

            mMockTracker = new Tracker();



        mMockitoSession = mockitoSession()

                .spyStatic(LocalServices.class)

                .mockStatic(LockGuard.class)
@@ -148,9 +139,6 @@ public class SystemServicesTestRule implements TestRule { 
        setUpLocalServices();

        setUpActivityTaskManagerService();

        setUpWindowManagerService();

        } finally {

            mMockTracker.stopTracking();

        }

    }



    private void setUpSystemCore() {
@@ -260,7 +248,9 @@ public class SystemServicesTestRule implements TestRule { 
    }



    private void setUpWindowManagerService() {

        mWMPolicy = new TestWindowManagerPolicy(this::getWindowManagerService);

        mPowerManagerWrapper = mock(WindowState.PowerManagerWrapper.class);

        mWMPolicy = new TestWindowManagerPolicy(this::getWindowManagerService,

                mPowerManagerWrapper);

        mWmService = WindowManagerService.main(

                mContext, mImService, false, false, mWMPolicy, mAtmService, StubTransaction::new);

        spyOn(mWmService);
@@ -305,9 +295,12 @@ public class SystemServicesTestRule implements TestRule { 
        DeviceConfig.removeOnPropertiesChangedListener(mWmService.mPropertiesChangedListener);

        mWmService = null;

        mWMPolicy = null;

        mPowerManagerWrapper = null;



        tearDownLocalServices();

        tearDownSystemCore();



        Mockito.framework().clearInlineMocks();

    }



    private void tearDownSystemCore() {
@@ -316,11 +309,6 @@ public class SystemServicesTestRule implements TestRule { 
            mMockitoSession = null;

        }



        if (mMockTracker != null) {

            mMockTracker.close();

            mMockTracker = null;

        }



        if (mHandlerThread != null) {

            // Make sure there are no running messages and then quit the thread so the next test

            // won't be affected.
@@ -352,6 +340,10 @@ public class SystemServicesTestRule implements TestRule { 
        return mAtmService;

    }



    WindowState.PowerManagerWrapper getPowerManagerWrapper() {

        return mPowerManagerWrapper;

    }



    void cleanupWindowManagerHandlers() {

        final WindowManagerService wm = getWindowManagerService();

        if (wm == null) {

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

+6 −2
Original line number Diff line number Diff line
@@ -40,12 +40,14 @@ import android.view.animation.Animation; 
import com.android.internal.policy.IKeyguardDismissCallback;

import com.android.internal.policy.IShortcutService;

import com.android.server.policy.WindowManagerPolicy;

import com.android.server.wm.WindowState.PowerManagerWrapper;



import java.io.PrintWriter;

import java.util.function.Supplier;



class TestWindowManagerPolicy implements WindowManagerPolicy {

    private final Supplier<WindowManagerService> mWmSupplier;

    private final PowerManagerWrapper mPowerManagerWrapper;



    int mRotationToReport = 0;

    boolean mKeyguardShowingAndNotOccluded = false;
@@ -53,8 +55,10 @@ class TestWindowManagerPolicy implements WindowManagerPolicy { 


    private Runnable mRunnableWhenAddingSplashScreen;



    TestWindowManagerPolicy(Supplier<WindowManagerService> wmSupplier) {

    TestWindowManagerPolicy(Supplier<WindowManagerService> wmSupplier,

            PowerManagerWrapper powerManagerWrapper) {

        mWmSupplier = wmSupplier;

        mPowerManagerWrapper = powerManagerWrapper;

    }



    @Override
@@ -121,7 +125,7 @@ class TestWindowManagerPolicy implements WindowManagerPolicy { 
            doReturn(mock(IBinder.class)).when(iWindow).asBinder();

            window = WindowTestsBase.createWindow(null, TYPE_APPLICATION_STARTING, atoken,

                    "Starting window", 0 /* ownerId */, false /* internalWindows */, wm,

                    mock(Session.class), iWindow);

                    mock(Session.class), iWindow, mPowerManagerWrapper);

            atoken.startingWindow = window;

        }

        if (mRunnableWhenAddingSplashScreen != null) {

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

+11 −7
Original line number Diff line number Diff line
@@ -346,24 +346,28 @@ public class WindowStateTests extends WindowTestsBase { 
        firstWindow.mAttrs.flags |= WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON;

        secondWindow.mAttrs.flags |= WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON;



        reset(sPowerManagerWrapper);

        final WindowState.PowerManagerWrapper powerManagerWrapper =

                mSystemServicesTestRule.getPowerManagerWrapper();

        reset(powerManagerWrapper);

        firstWindow.prepareWindowToDisplayDuringRelayout(false /*wasVisible*/);

        verify(sPowerManagerWrapper).wakeUp(anyLong(), anyInt(), anyString());

        verify(powerManagerWrapper).wakeUp(anyLong(), anyInt(), anyString());



        reset(sPowerManagerWrapper);

        reset(powerManagerWrapper);

        secondWindow.prepareWindowToDisplayDuringRelayout(false /*wasVisible*/);

        verify(sPowerManagerWrapper).wakeUp(anyLong(), anyInt(), anyString());

        verify(powerManagerWrapper).wakeUp(anyLong(), anyInt(), anyString());

    }



    private void testPrepareWindowToDisplayDuringRelayout(WindowState appWindow,

            boolean expectedWakeupCalled, boolean expectedCurrentLaunchCanTurnScreenOn) {

        reset(sPowerManagerWrapper);

        final WindowState.PowerManagerWrapper powerManagerWrapper =

                mSystemServicesTestRule.getPowerManagerWrapper();

        reset(powerManagerWrapper);

        appWindow.prepareWindowToDisplayDuringRelayout(false /* wasVisible */);



        if (expectedWakeupCalled) {

            verify(sPowerManagerWrapper).wakeUp(anyLong(), anyInt(), anyString());

            verify(powerManagerWrapper).wakeUp(anyLong(), anyInt(), anyString());

        } else {

            verify(sPowerManagerWrapper, never()).wakeUp(anyLong(), anyInt(), anyString());

            verify(powerManagerWrapper, never()).wakeUp(anyLong(), anyInt(), anyString());

        }

        // If wakeup is expected to be called, the currentLaunchCanTurnScreenOn should be false

        // because the state will be consumed.