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

Commit 3af92031 authored by Fabián Kozynski's avatar Fabián Kozynski
Browse files

Move QSPanelControllerBase callback to init/destroy 

Before this change, attaching and dettaching QSFragment would trigger
tiles re-fetching and recreating of callbacks and views. In order to
prepare for scenes (where we may want to dettach and re-attach QSImpl),
this does the following two changes:

* The callback to QSHost is added in `init` and removed in `destroy`.
* If the tiles collection is exactly the same as the current one (same
  QSTile objects, in same order), do not re-create the TileRecord and
  views.

Bug: 280887232
Test: manual
Test: atest QSPanelControllerBaseTest
Test: heapdump, no leftover QSImpl after recreating QSFragmentLegacy
Change-Id: Ie27d23c34cbb4c8d645d6cd3df5d0812ab6ff70a
parent 86b02274

packages/SystemUI/src/com/android/systemui/qs/QSImpl.java

+2 −0
Original line number Diff line number Diff line
@@ -323,6 +323,8 @@ public class QSImpl implements QS, CommandQueue.Callbacks, StatusBarStateControl 
    public void onDestroy() {

        mCommandQueue.removeCallback(this);

        mStatusBarStateController.removeCallback(this);

        mQSPanelController.destroy();

        mQuickQSPanelController.destroy();

        if (mListening) {

            setListening(false);

        }

packages/SystemUI/src/com/android/systemui/qs/QSPanelControllerBase.java

+36 −14
Original line number Diff line number Diff line
@@ -158,6 +158,7 @@ public abstract class QSPanelControllerBase<T extends QSPanel> extends ViewContr 
    protected void onInit() {

        mView.initialize(mQSLogger);

        mQSLogger.logAllTilesChangeListening(mView.isListening(), mView.getDumpableTag(), "");

        mHost.addCallback(mQSHostCallback);

    }



    /**
@@ -171,6 +172,18 @@ public abstract class QSPanelControllerBase<T extends QSPanel> extends ViewContr 
        mView.setSquishinessFraction(squishinessFraction);

    }



    @Override

    public void destroy() {

        super.destroy();

        mHost.removeCallback(mQSHostCallback);



        for (TileRecord record : mRecords) {

            record.tile.removeCallback(record.callback);

            mView.removeTile(record);

        }

        mRecords.clear();

    }



    @Override

    protected void onViewAttached() {

        mQsTileRevealController = createTileRevealController();
@@ -180,7 +193,6 @@ public abstract class QSPanelControllerBase<T extends QSPanel> extends ViewContr 


        mMediaHost.addVisibilityChangeListener(mMediaHostVisibilityListener);

        mView.addOnConfigurationChangedListener(mOnConfigurationChangedListener);

        mHost.addCallback(mQSHostCallback);

        setTiles();

        mLastOrientation = getResources().getConfiguration().orientation;

        mQSLogger.logOnViewAttached(mLastOrientation, mView.getDumpableTag());
@@ -193,16 +205,11 @@ public abstract class QSPanelControllerBase<T extends QSPanel> extends ViewContr 
    protected void onViewDetached() {

        mQSLogger.logOnViewDetached(mLastOrientation, mView.getDumpableTag());

        mView.removeOnConfigurationChangedListener(mOnConfigurationChangedListener);

        mHost.removeCallback(mQSHostCallback);



        mView.getTileLayout().setListening(false, mUiEventLogger);



        mMediaHost.removeVisibilityChangeListener(mMediaHostVisibilityListener);



        for (TileRecord record : mRecords) {

            record.tile.removeCallback(record.callback);

        }

        mRecords.clear();

        mDumpManager.unregisterDumpable(mView.getDumpableTag());

    }
@@ -222,7 +229,21 @@ public abstract class QSPanelControllerBase<T extends QSPanel> extends ViewContr 
        if (!collapsedView && mQsTileRevealController != null) {

            mQsTileRevealController.updateRevealedTiles(tiles);

        }

        boolean shouldChange = false;

        if (tiles.size() == mRecords.size()) {

            int i = 0;

            for (QSTile tile : tiles) {

                if (tile != mRecords.get(i).tile) {

                    shouldChange = true;

                    break;

                }

                i++;

            }

        } else {

            shouldChange = true;

        }



        if (shouldChange) {

            for (QSPanelControllerBase.TileRecord record : mRecords) {

                mView.removeTile(record);

                record.tile.removeCallback(record.callback);
@@ -233,6 +254,7 @@ public abstract class QSPanelControllerBase<T extends QSPanel> extends ViewContr 
                addTile(tile, collapsedView);

            }

        }

    }



    /** */

    public void refreshAllTiles() {

packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelControllerBaseTest.java

+85 −3
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ import static org.mockito.ArgumentMatchers.any; 
import static org.mockito.ArgumentMatchers.anyBoolean;

import static org.mockito.ArgumentMatchers.anyFloat;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.clearInvocations;

import static org.mockito.Mockito.doAnswer;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.reset;
@@ -42,7 +43,6 @@ import com.android.internal.logging.MetricsLogger; 
import com.android.internal.logging.UiEventLogger;

import com.android.internal.logging.nano.MetricsProto.MetricsEvent;

import com.android.internal.logging.testing.UiEventLoggerFake;

import com.android.systemui.res.R;

import com.android.systemui.SysuiTestCase;

import com.android.systemui.dump.DumpManager;

import com.android.systemui.media.controls.ui.MediaHost;
@@ -50,6 +50,7 @@ import com.android.systemui.plugins.qs.QSTile; 
import com.android.systemui.qs.customize.QSCustomizerController;

import com.android.systemui.qs.logging.QSLogger;

import com.android.systemui.qs.tileimpl.QSTileImpl;

import com.android.systemui.res.R;

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

import com.android.systemui.util.animation.DisappearParameters;
@@ -341,11 +342,92 @@ public class QSPanelControllerBaseTest extends SysuiTestCase { 
    }



    @Test

    public void onViewDetached_removesJustTheAssociatedCallback() {

    public void onDestroy_removesJustTheAssociatedCallback() {

        QSPanelControllerBase.TileRecord record = mController.mRecords.get(0);



        mController.onViewDetached();

        mController.destroy();

        verify(mQSTile).removeCallback(record.callback);

        verify(mQSTile, never()).removeCallbacks();



        assertThat(mController.mRecords).isEmpty();

    }



    @Test

    public void onViewDettached_callbackNotRemoved() {

        QSPanelControllerBase.TileRecord record = mController.mRecords.get(0);



        mController.onViewDetached();

        verify(mQSTile, never()).removeCallback(record.callback);

        verify(mQSTile, never()).removeCallbacks();

    }



    @Test

    public void onInit_qsHostCallbackAdded() {

        verify(mQSHost).addCallback(any());

    }



    @Test

    public void onViewDettached_qsHostCallbackNotRemoved() {

        mController.onViewDetached();

        verify(mQSHost, never()).removeCallback(any());

    }



    @Test

    public void onDestroy_qsHostCallbackRemoved() {

        mController.destroy();

        verify(mQSHost).removeCallback(any());

    }



    @Test

    public void setTiles_sameTiles_doesntRemoveAndReaddViews() {

        when(mQSHost.getTiles()).thenReturn(List.of(mQSTile, mOtherTile));

        mController.setTiles();



        clearInvocations(mQSPanel);



        mController.setTiles();

        verify(mQSPanel, never()).removeTile(any());

        verify(mQSPanel, never()).addTile(any());

    }



    @Test

    public void setTiles_differentTiles_allTilesRemovedAndNewTilesAdded() {

        when(mQSHost.getTiles()).thenReturn(List.of(mQSTile, mOtherTile));

        mController.setTiles();



        clearInvocations(mQSPanel);



        when(mQSHost.getTiles()).thenReturn(List.of(mQSTile));

        mController.setTiles();



        verify(mQSPanel, times(2)).removeTile(any());

        verify(mQSPanel).addTile(any());

    }



    @Test

    public void detachAndReattach_sameTiles_doesntRemoveAndReAddViews() {

        when(mQSHost.getTiles()).thenReturn(List.of(mQSTile, mOtherTile));

        mController.setTiles();



        clearInvocations(mQSPanel);



        mController.onViewDetached();

        mController.onViewAttached();

        verify(mQSPanel, never()).removeTile(any());

        verify(mQSPanel, never()).addTile(any());

    }



    @Test

    public void setTiles_sameTilesDifferentOrder_removesAndReadds() {

        when(mQSHost.getTiles()).thenReturn(List.of(mQSTile, mOtherTile));

        mController.setTiles();



        clearInvocations(mQSPanel);



        when(mQSHost.getTiles()).thenReturn(List.of(mOtherTile, mQSTile));

        mController.setTiles();



        verify(mQSPanel, times(2)).removeTile(any());

        verify(mQSPanel, times(2)).addTile(any());

    }

}