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

Commit 71712222 authored by Bryce Lee's avatar Bryce Lee
Browse files

Reapply "Fix DreamOverlayService memory leaks."

This changelist re-introduces fixes to address memory leaks in
DreamOverlayService by making sure components are not reused between
dream instances to prevent torn down components from being used again.
To address this, the components are recreated from the passed in
factories.

Test: atest DreamOverlayService#testComponentsRecreatedBetweenDreams
Bug: 361085016
Flag: EXEMPT bugfix

Change-Id: I46bb1596057a7b7bb9fde41dfeed6b64786feb91
parent 0d12496f
Loading
Loading
Loading
Loading
+34 −11
Original line number Diff line number Diff line
@@ -28,7 +28,9 @@ import android.os.RemoteException;
import android.util.Log;
import android.view.WindowManager;

import java.lang.ref.WeakReference;
import java.util.concurrent.Executor;
import java.util.function.Consumer;


/**
@@ -52,43 +54,51 @@ public abstract class DreamOverlayService extends Service {
    // An {@link IDreamOverlayClient} implementation that identifies itself when forwarding
    // requests to the {@link DreamOverlayService}
    private static class OverlayClient extends IDreamOverlayClient.Stub {
        private final DreamOverlayService mService;
        private final WeakReference<DreamOverlayService> mService;
        private boolean mShowComplications;
        private ComponentName mDreamComponent;
        IDreamOverlayCallback mDreamOverlayCallback;

        OverlayClient(DreamOverlayService service) {
        OverlayClient(WeakReference<DreamOverlayService> service) {
            mService = service;
        }

        private void applyToDream(Consumer<DreamOverlayService> consumer) {
            final DreamOverlayService service = mService.get();

            if (service != null) {
                consumer.accept(service);
            }
        }

        @Override
        public void startDream(WindowManager.LayoutParams params, IDreamOverlayCallback callback,
                String dreamComponent, boolean shouldShowComplications) throws RemoteException {
            mDreamComponent = ComponentName.unflattenFromString(dreamComponent);
            mShowComplications = shouldShowComplications;
            mDreamOverlayCallback = callback;
            mService.startDream(this, params);
            applyToDream(dreamOverlayService -> dreamOverlayService.startDream(this, params));
        }

        @Override
        public void wakeUp() {
            mService.wakeUp(this);
            applyToDream(dreamOverlayService -> dreamOverlayService.wakeUp(this));
        }

        @Override
        public void endDream() {
            mService.endDream(this);
            applyToDream(dreamOverlayService -> dreamOverlayService.endDream(this));
        }

        @Override
        public void comeToFront() {
            mService.comeToFront(this);
            applyToDream(dreamOverlayService -> dreamOverlayService.comeToFront(this));
        }

        @Override
        public void onWakeRequested() {
            if (Flags.dreamWakeRedirect()) {
                mService.onWakeRequested();
                applyToDream(DreamOverlayService::onWakeRequested);
            }
        }

@@ -161,17 +171,24 @@ public abstract class DreamOverlayService extends Service {
        });
    }

    private IDreamOverlay mDreamOverlay = new IDreamOverlay.Stub() {
    private static class DreamOverlay extends IDreamOverlay.Stub {
        private final WeakReference<DreamOverlayService> mService;

        DreamOverlay(DreamOverlayService service) {
            mService = new WeakReference<>(service);
        }

        @Override
        public void getClient(IDreamOverlayClientCallback callback) {
            try {
                callback.onDreamOverlayClient(
                        new OverlayClient(DreamOverlayService.this));
                callback.onDreamOverlayClient(new OverlayClient(mService));
            } catch (RemoteException e) {
                Log.e(TAG, "could not send client to callback", e);
            }
        }
    };
    }

    private final IDreamOverlay mDreamOverlay = new DreamOverlay(this);

    public DreamOverlayService() {
    }
@@ -195,6 +212,12 @@ public abstract class DreamOverlayService extends Service {
        }
    }

    @Override
    public void onDestroy() {
        mCurrentClient = null;
        super.onDestroy();
    }

    @Nullable
    @Override
    public final IBinder onBind(@NonNull Intent intent) {
+10 −0
Original line number Diff line number Diff line
@@ -62,6 +62,7 @@ import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatchers;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;

@@ -324,4 +325,13 @@ public class DreamOverlayContainerViewControllerTest extends SysuiTestCase {
        // enabled.
        mController.onViewAttached();
    }

    @Test
    public void destroy_cleansUpState() {
        mController.destroy();
        verify(mStateController).removeCallback(any());
        verify(mAmbientStatusBarViewController).destroy();
        verify(mComplicationHostViewController).destroy();
        verify(mLowLightTransitionCoordinator).setLowLightEnterListener(ArgumentMatchers.isNull());
    }
}
+117 −36
Original line number Diff line number Diff line
@@ -68,7 +68,6 @@ import com.android.systemui.navigationbar.gestural.domain.GestureInteractor
import com.android.systemui.testKosmos
import com.android.systemui.touch.TouchInsetManager
import com.android.systemui.util.concurrency.FakeExecutor
import com.android.systemui.util.mockito.whenever
import com.android.systemui.util.time.FakeSystemClock
import com.google.common.truth.Truth.assertThat
import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -89,7 +88,9 @@ import org.mockito.MockitoAnnotations
import org.mockito.kotlin.any
import org.mockito.kotlin.argumentCaptor
import org.mockito.kotlin.eq
import org.mockito.kotlin.mock
import org.mockito.kotlin.spy
import org.mockito.kotlin.whenever

@OptIn(ExperimentalCoroutinesApi::class)
@SmallTest
@@ -115,8 +116,6 @@ class DreamOverlayServiceTest : SysuiTestCase() {

    @Mock lateinit var mComplicationComponentFactory: ComplicationComponent.Factory

    @Mock lateinit var mComplicationComponent: ComplicationComponent

    @Mock lateinit var mComplicationHostViewController: ComplicationHostViewController

    @Mock lateinit var mComplicationVisibilityController: ComplicationLayoutEngine
@@ -125,20 +124,12 @@ class DreamOverlayServiceTest : SysuiTestCase() {
    lateinit var mDreamComplicationComponentFactory:
        com.android.systemui.dreams.complication.dagger.ComplicationComponent.Factory

    @Mock
    lateinit var mDreamComplicationComponent:
        com.android.systemui.dreams.complication.dagger.ComplicationComponent

    @Mock lateinit var mHideComplicationTouchHandler: HideComplicationTouchHandler

    @Mock lateinit var mDreamOverlayComponentFactory: DreamOverlayComponent.Factory

    @Mock lateinit var mDreamOverlayComponent: DreamOverlayComponent

    @Mock lateinit var mAmbientTouchComponentFactory: AmbientTouchComponent.Factory

    @Mock lateinit var mAmbientTouchComponent: AmbientTouchComponent

    @Mock lateinit var mDreamOverlayContainerView: DreamOverlayContainerView

    @Mock lateinit var mDreamOverlayContainerViewController: DreamOverlayContainerViewController
@@ -170,40 +161,100 @@ class DreamOverlayServiceTest : SysuiTestCase() {
    private lateinit var communalRepository: FakeCommunalSceneRepository
    private var viewCaptureSpy = spy(ViewCaptureFactory.getInstance(context))
    private lateinit var gestureInteractor: GestureInteractor
    private lateinit var environmentComponents: EnvironmentComponents

    @Captor var mViewCaptor: ArgumentCaptor<View>? = null
    private lateinit var mService: DreamOverlayService

    @Before
    fun setup() {
        MockitoAnnotations.initMocks(this)
    private class EnvironmentComponents(
        val dreamsComplicationComponent:
            com.android.systemui.dreams.complication.dagger.ComplicationComponent,
        val dreamOverlayComponent: DreamOverlayComponent,
        val complicationComponent: ComplicationComponent,
        val ambientTouchComponent: AmbientTouchComponent,
    ) {
        fun clearInvocations() {
            clearInvocations(
                dreamsComplicationComponent,
                dreamOverlayComponent,
                complicationComponent,
                ambientTouchComponent
            )
        }

        lifecycleRegistry = FakeLifecycleRegistry(mLifecycleOwner)
        bouncerRepository = kosmos.fakeKeyguardBouncerRepository
        communalRepository = kosmos.fakeCommunalSceneRepository
        gestureInteractor = spy(kosmos.gestureInteractor)
        fun verifyNoMoreInteractions() {
            Mockito.verifyNoMoreInteractions(
                dreamsComplicationComponent,
                dreamOverlayComponent,
                complicationComponent,
                ambientTouchComponent
            )
        }
    }

        whenever(mDreamOverlayComponent.getDreamOverlayContainerViewController())
    private fun setupComponentFactories(
        dreamComplicationComponentFactory:
            com.android.systemui.dreams.complication.dagger.ComplicationComponent.Factory,
        dreamOverlayComponentFactory: DreamOverlayComponent.Factory,
        complicationComponentFactory: ComplicationComponent.Factory,
        ambientTouchComponentFactory: AmbientTouchComponent.Factory
    ): EnvironmentComponents {
        val dreamOverlayComponent = mock<DreamOverlayComponent>()
        whenever(dreamOverlayComponent.getDreamOverlayContainerViewController())
            .thenReturn(mDreamOverlayContainerViewController)
        whenever(mComplicationComponent.getComplicationHostViewController())

        val complicationComponent = mock<ComplicationComponent>()
        whenever(complicationComponent.getComplicationHostViewController())
            .thenReturn(mComplicationHostViewController)
        whenever(mLifecycleOwner.registry).thenReturn(lifecycleRegistry)

        mCommunalInteractor = Mockito.spy(kosmos.communalInteractor)

        whenever(mComplicationComponentFactory.create(any(), any(), any(), any()))
            .thenReturn(mComplicationComponent)
        whenever(mComplicationComponent.getVisibilityController())
        whenever(complicationComponentFactory.create(any(), any(), any(), any()))
            .thenReturn(complicationComponent)
        whenever(complicationComponent.getVisibilityController())
            .thenReturn(mComplicationVisibilityController)
        whenever(mDreamComplicationComponent.getHideComplicationTouchHandler())

        val dreamComplicationComponent =
            mock<com.android.systemui.dreams.complication.dagger.ComplicationComponent>()
        whenever(dreamComplicationComponent.getHideComplicationTouchHandler())
            .thenReturn(mHideComplicationTouchHandler)
        whenever(mDreamComplicationComponentFactory.create(any(), any()))
            .thenReturn(mDreamComplicationComponent)
        whenever(mDreamOverlayComponentFactory.create(any(), any(), any()))
            .thenReturn(mDreamOverlayComponent)
        whenever(mAmbientTouchComponentFactory.create(any(), any()))
            .thenReturn(mAmbientTouchComponent)
        whenever(mAmbientTouchComponent.getTouchMonitor()).thenReturn(mTouchMonitor)
        whenever(dreamComplicationComponentFactory.create(any(), any()))
            .thenReturn(dreamComplicationComponent)

        whenever(dreamOverlayComponentFactory.create(any(), any(), any()))
            .thenReturn(dreamOverlayComponent)

        val ambientTouchComponent = mock<AmbientTouchComponent>()
        whenever(ambientTouchComponentFactory.create(any(), any()))
            .thenReturn(ambientTouchComponent)
        whenever(ambientTouchComponent.getTouchMonitor()).thenReturn(mTouchMonitor)

        return EnvironmentComponents(
            dreamComplicationComponent,
            dreamOverlayComponent,
            complicationComponent,
            ambientTouchComponent
        )
    }

    @Before
    fun setup() {
        MockitoAnnotations.initMocks(this)

        lifecycleRegistry = FakeLifecycleRegistry(mLifecycleOwner)
        bouncerRepository = kosmos.fakeKeyguardBouncerRepository
        communalRepository = kosmos.fakeCommunalSceneRepository
        gestureInteractor = spy(kosmos.gestureInteractor)

        environmentComponents =
            setupComponentFactories(
                mDreamComplicationComponentFactory,
                mDreamOverlayComponentFactory,
                mComplicationComponentFactory,
                mAmbientTouchComponentFactory
            )

        whenever(mDreamOverlayContainerViewController.containerView)
            .thenReturn(mDreamOverlayContainerView)
        whenever(mScrimManager.getCurrentController()).thenReturn(mScrimController)
@@ -570,9 +621,8 @@ class DreamOverlayServiceTest : SysuiTestCase() {

        // Assert that the overlay is not showing complications.
        assertThat(mService.shouldShowComplications()).isFalse()
        Mockito.clearInvocations(mDreamOverlayComponent)
        Mockito.clearInvocations(mAmbientTouchComponent)
        Mockito.clearInvocations(mWindowManager)
        environmentComponents.clearInvocations()
        clearInvocations(mWindowManager)

        // New dream starting with dream complications showing. Note that when a new dream is
        // binding to the dream overlay service, it receives the same instance of IBinder as the
@@ -594,8 +644,11 @@ class DreamOverlayServiceTest : SysuiTestCase() {

        // Verify that new instances of overlay container view controller and overlay touch monitor
        // are created.
        verify(mDreamOverlayComponent).getDreamOverlayContainerViewController()
        verify(mAmbientTouchComponent).getTouchMonitor()
        verify(environmentComponents.dreamOverlayComponent).getDreamOverlayContainerViewController()
        verify(environmentComponents.ambientTouchComponent).getTouchMonitor()

        // Verify DreamOverlayContainerViewController is destroyed.
        verify(mDreamOverlayContainerViewController).destroy()
    }

    @Test
@@ -1002,6 +1055,34 @@ class DreamOverlayServiceTest : SysuiTestCase() {
            .isEqualTo(ComponentName.unflattenFromString(DREAM_COMPONENT)?.packageName)
    }

    @Test
    fun testComponentsRecreatedBetweenDreams() {
        clearInvocations(
            mDreamComplicationComponentFactory,
            mDreamOverlayComponentFactory,
            mComplicationComponentFactory,
            mAmbientTouchComponentFactory
        )

        mService.onEndDream()

        setupComponentFactories(
            mDreamComplicationComponentFactory,
            mDreamOverlayComponentFactory,
            mComplicationComponentFactory,
            mAmbientTouchComponentFactory
        )

        client.startDream(
            mWindowParams,
            mDreamOverlayCallback,
            DREAM_COMPONENT,
            false /*shouldShowComplication*/
        )
        mMainExecutor.runAllReady()
        environmentComponents.verifyNoMoreInteractions()
    }

    internal class FakeLifecycleRegistry(provider: LifecycleOwner) : LifecycleRegistry(provider) {
        val mLifecycles: MutableList<State> = ArrayList()

+5 −0
Original line number Diff line number Diff line
@@ -122,4 +122,9 @@ public interface TouchHandler {
     * @param session
     */
    void onSessionStart(TouchSession session);

    /**
     * Called when the handler is being torn down.
     */
    default void onDestroy() {}
}
+4 −0
Original line number Diff line number Diff line
@@ -581,6 +581,10 @@ public class TouchMonitor {
            mBoundsFlow.cancel(new CancellationException());
        }

        for (TouchHandler handler : mHandlers) {
            handler.onDestroy();
        }

        mInitialized = false;
    }

Loading