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

Commit 670412c1 authored by Lucas Silva's avatar Lucas Silva
Browse files

Update dream state controller to hold weak ref to callbacks. 

This avoids memory leaks caused by callers of this state controller not
properly removing callbacks.

This change also creates a WeakReferenceFactory util so that weak
references may be cleared in tests.

Bug: 309396474
Flag: NONE
Test: atest DreamOverlayStateControllerTest
Change-Id: If8f3331a8d943d18bb305c73c8bc29f4777cb97a
parent 3129fc5b

packages/SystemUI/src/com/android/systemui/complication/SmartSpaceComplication.java

+13 −11
Original line number Diff line number Diff line
@@ -71,6 +71,18 @@ public class SmartSpaceComplication implements Complication { 
        private final DreamOverlayStateController mDreamOverlayStateController;

        private final SmartSpaceComplication mComplication;

        private final FeatureFlags mFeatureFlags;

        private final DreamOverlayStateController.Callback mStateControllerCallback =

                new DreamOverlayStateController.Callback() {

                    @Override

                    public void onStateChanged() {

                        if (mDreamOverlayStateController.isOverlayActive()) {

                            mSmartSpaceController.addListener(mSmartspaceListener);

                        } else {

                            mSmartSpaceController.removeListener(mSmartspaceListener);

                            mDreamOverlayStateController.removeComplication(mComplication);

                        }

                    }

                };



        private final BcSmartspaceDataPlugin.SmartspaceTargetListener mSmartspaceListener =

                new BcSmartspaceDataPlugin.SmartspaceTargetListener() {
@@ -103,17 +115,7 @@ public class SmartSpaceComplication implements Complication { 
                return;

            }



            mDreamOverlayStateController.addCallback(new DreamOverlayStateController.Callback() {

                @Override

                public void onStateChanged() {

                    if (mDreamOverlayStateController.isOverlayActive()) {

                        mSmartSpaceController.addListener(mSmartspaceListener);

                    } else {

                        mSmartSpaceController.removeListener(mSmartspaceListener);

                        mDreamOverlayStateController.removeComplication(mComplication);

                    }

                }

            });

            mDreamOverlayStateController.addCallback(mStateControllerCallback);

        }

    }

packages/SystemUI/src/com/android/systemui/dagger/SystemUIModule.java

+2 −0
Original line number Diff line number Diff line
@@ -129,6 +129,7 @@ import com.android.systemui.user.domain.UserDomainLayerModule; 
import com.android.systemui.util.concurrency.SysUIConcurrencyModule;

import com.android.systemui.util.dagger.UtilModule;

import com.android.systemui.util.kotlin.CoroutinesModule;

import com.android.systemui.util.reference.ReferenceModule;

import com.android.systemui.util.sensors.SensorModule;

import com.android.systemui.util.settings.SettingsUtilModule;

import com.android.systemui.util.time.SystemClock;
@@ -199,6 +200,7 @@ import javax.inject.Named; 
        PrivacyModule.class,

        QRCodeScannerModule.class,

        QSFragmentStartableModule.class,

        ReferenceModule.class,

        RetailModeModule.class,

        ScreenshotModule.class,

        SensorModule.class,

packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayStateController.java

+40 −13
Original line number Diff line number Diff line
@@ -31,11 +31,15 @@ import com.android.systemui.flags.Flags; 
import com.android.systemui.log.LogBuffer;

import com.android.systemui.log.dagger.DreamLog;

import com.android.systemui.statusbar.policy.CallbackController;

import com.android.systemui.util.annotations.WeaklyReferencedCallback;

import com.android.systemui.util.reference.WeakReferenceFactory;



import java.lang.ref.WeakReference;

import java.util.ArrayList;

import java.util.Collection;

import java.util.Collections;

import java.util.HashSet;

import java.util.Iterator;

import java.util.Objects;

import java.util.concurrent.Executor;

import java.util.function.Consumer;
@@ -68,7 +72,10 @@ public class DreamOverlayStateController implements 


    /**

     * Callback for dream overlay events.

     * NOTE: Caller should maintain a strong reference to this themselves so the callback does

     * not get garbage collected.

     */

    @WeaklyReferencedCallback

    public interface Callback {

        /**

         * Called when the composition of complications changes.
@@ -97,7 +104,7 @@ public class DreamOverlayStateController implements 


    private final Executor mExecutor;

    private final boolean mOverlayEnabled;

    private final ArrayList<Callback> mCallbacks = new ArrayList<>();

    private final ArrayList<WeakReference<Callback>> mCallbacks = new ArrayList<>();



    @Complication.ComplicationType

    private int mAvailableComplicationTypes = Complication.COMPLICATION_TYPE_NONE;
@@ -107,6 +114,7 @@ public class DreamOverlayStateController implements 
    private final Collection<Complication> mComplications = new HashSet();



    private final FeatureFlags mFeatureFlags;

    private final WeakReferenceFactory mWeakReferenceFactory;



    private final int mSupportedTypes;
@@ -117,11 +125,13 @@ public class DreamOverlayStateController implements 
    public DreamOverlayStateController(@Main Executor executor,

            @Named(DREAM_OVERLAY_ENABLED) boolean overlayEnabled,

            FeatureFlags featureFlags,

            @DreamLog LogBuffer logBuffer) {

            @DreamLog LogBuffer logBuffer,

            WeakReferenceFactory weakReferenceFactory) {

        mExecutor = executor;

        mOverlayEnabled = overlayEnabled;

        mLogger = new DreamLogger(logBuffer, TAG);

        mFeatureFlags = featureFlags;

        mWeakReferenceFactory = weakReferenceFactory;

        if (mFeatureFlags.isEnabled(Flags.ALWAYS_SHOW_HOME_CONTROLS_ON_DREAMS)) {

            mSupportedTypes = Complication.COMPLICATION_TYPE_NONE

                    | Complication.COMPLICATION_TYPE_HOME_CONTROLS;
@@ -143,7 +153,7 @@ public class DreamOverlayStateController implements 
        mExecutor.execute(() -> {

            if (mComplications.add(complication)) {

                mLogger.logAddComplication(complication.toString());

                mCallbacks.stream().forEach(callback -> callback.onComplicationsChanged());

                notifyCallbacksLocked(Callback::onComplicationsChanged);

            }

        });

    }
@@ -160,7 +170,7 @@ public class DreamOverlayStateController implements 
        mExecutor.execute(() -> {

            if (mComplications.remove(complication)) {

                mLogger.logRemoveComplication(complication.toString());

                mCallbacks.stream().forEach(callback -> callback.onComplicationsChanged());

                notifyCallbacksLocked(Callback::onComplicationsChanged);

            }

        });

    }
@@ -199,22 +209,33 @@ public class DreamOverlayStateController implements 
    }



    private void notifyCallbacks(Consumer<Callback> callbackConsumer) {

        mExecutor.execute(() -> {

            for (Callback callback : mCallbacks) {

        mExecutor.execute(() -> notifyCallbacksLocked(callbackConsumer));

    }



    private void notifyCallbacksLocked(Consumer<Callback> callbackConsumer) {

        final Iterator<WeakReference<Callback>> iterator = mCallbacks.iterator();

        while (iterator.hasNext()) {

            final Callback callback = iterator.next().get();

            // Remove any callbacks which have been GC'd

            if (callback == null) {

                iterator.remove();

            } else {

                callbackConsumer.accept(callback);

            }

        });

        }

    }



    @Override

    public void addCallback(@NonNull Callback callback) {

        mExecutor.execute(() -> {

            Objects.requireNonNull(callback, "Callback must not be null. b/128895449");

            if (mCallbacks.contains(callback)) {

            final boolean containsCallback = mCallbacks.stream()

                    .anyMatch(reference -> reference.get() == callback);

            if (containsCallback) {

                return;

            }



            mCallbacks.add(callback);

            mCallbacks.add(mWeakReferenceFactory.create(callback));



            if (mComplications.isEmpty()) {

                return;
@@ -228,7 +249,13 @@ public class DreamOverlayStateController implements 
    public void removeCallback(@NonNull Callback callback) {

        mExecutor.execute(() -> {

            Objects.requireNonNull(callback, "Callback must not be null. b/128895449");

            mCallbacks.remove(callback);

            final Iterator<WeakReference<Callback>> iterator = mCallbacks.iterator();

            while (iterator.hasNext()) {

                final Callback cb = iterator.next().get();

                if (cb == null || cb == callback) {

                    iterator.remove();

                }

            }

        });

    }
@@ -318,7 +345,7 @@ public class DreamOverlayStateController implements 


        if (isLowLightActive() && !active) {

            // Notify that we're exiting low light only on the transition from active to not active.

            mCallbacks.forEach(Callback::onExitLowLight);

            notifyCallbacks(Callback::onExitLowLight);

        }

        modifyState(active ? OP_SET_STATE : OP_CLEAR_STATE, STATE_LOW_LIGHT_ACTIVE);

    }
@@ -375,7 +402,7 @@ public class DreamOverlayStateController implements 
        mExecutor.execute(() -> {

            mLogger.logAvailableComplicationTypes(types);

            mAvailableComplicationTypes = types;

            mCallbacks.forEach(Callback::onAvailableComplicationTypesChanged);

            notifyCallbacksLocked(Callback::onAvailableComplicationTypesChanged);

        });

    }
@@ -393,7 +420,7 @@ public class DreamOverlayStateController implements 
        mExecutor.execute(() -> {

            mLogger.logShouldShowComplications(shouldShowComplications);

            mShouldShowComplications = shouldShowComplications;

            mCallbacks.forEach(Callback::onAvailableComplicationTypesChanged);

            notifyCallbacksLocked(Callback::onAvailableComplicationTypesChanged);

        });

    }

}

packages/SystemUI/src/com/android/systemui/util/reference/ReferenceModule.kt

0 → 100644
+26 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2023 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.systemui.util.reference



import dagger.Binds

import dagger.Module



@Module

abstract class ReferenceModule {

    @Binds

    abstract fun bindWeakReferenceFactory(impl: WeakReferenceFactoryImpl): WeakReferenceFactory

}

packages/SystemUI/src/com/android/systemui/util/reference/WeakReferenceFactory.kt

0 → 100644
+30 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2023 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.systemui.util.reference



import java.lang.ref.WeakReference

import javax.inject.Inject



interface WeakReferenceFactory {

    fun <T> create(referent: T): WeakReference<T>

}



class WeakReferenceFactoryImpl @Inject constructor() : WeakReferenceFactory {

    override fun <T> create(referent: T): WeakReference<T> {

        return WeakReference(referent)

    }

}