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

Commit 793cef95 authored by Garfield Tan's avatar Garfield Tan
Browse files

Remove activity leak.

mState can be used across activities on configuration changes. This is
because even though we put it into Bundle, the bundle never leaves the
process so when we take State object, it's still the old one.

Test: No more activity leak after fixing. Auto tests pass.
Bug: 62147171
Change-Id: Ie805a85a56dc5b009dd0903e094fc5b8976f6b75
(cherry picked from commit 1071cea6)
parent ca4b65c1
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -244,6 +244,7 @@ public abstract class BaseActivity
    protected void onDestroy() {
        mRootsMonitor.stop();
        mPreferencesMonitor.stop();
        mSortController.destroy();
        super.onDestroy();
    }

+8 −1
Original line number Diff line number Diff line
@@ -44,6 +44,7 @@ public final class DropdownSortWidgetController implements WidgetController {
    private final TextView mDimensionButton;
    private final PopupMenu mMenu;
    private final ImageView mArrow;
    private final SortModel.UpdateListener mListener;

    public DropdownSortWidgetController(SortModel model, View widget) {
        mModel = model;
@@ -61,7 +62,8 @@ public final class DropdownSortWidgetController implements WidgetController {
        populateMenuItems();
        onModelUpdate(mModel, SortModel.UPDATE_TYPE_UNSPECIFIED);

        mModel.addListener(this::onModelUpdate);
        mListener = this::onModelUpdate;
        mModel.addListener(mListener);
    }

    @Override
@@ -69,6 +71,11 @@ public final class DropdownSortWidgetController implements WidgetController {
        mWidget.setVisibility(visibility);
    }

    @Override
    public void destroy() {
        mModel.removeListener(mListener);
    }

    private void populateMenuItems() {
        Menu menu = mMenu.getMenu();
        menu.clear();
+8 −0
Original line number Diff line number Diff line
@@ -65,6 +65,13 @@ public final class SortController {
        }
    }

    public void destroy() {
        mDropdownController.destroy();
        if (mTableHeaderController != null) {
            mTableHeaderController.destroy();
        }
    }

    public static SortController create(
            Activity activity,
            @ViewMode int initialMode,
@@ -98,5 +105,6 @@ public final class SortController {

    public interface WidgetController {
        void setVisibility(int visibility);
        void destroy();
    }
}
+7 −1
Original line number Diff line number Diff line
@@ -37,6 +37,7 @@ public final class TableHeaderController implements SortController.WidgetControl
    // We assign this here porque each method reference creates a new object
    // instance (which is wasteful).
    private final View.OnClickListener mOnCellClickListener = this::onCellClicked;
    private final SortModel.UpdateListener mModelListener = this::onModelUpdate;

    private final SortModel mModel;

@@ -54,7 +55,7 @@ public final class TableHeaderController implements SortController.WidgetControl

        onModelUpdate(mModel, SortModel.UPDATE_TYPE_UNSPECIFIED);

        mModel.addListener(this::onModelUpdate);
        mModel.addListener(mModelListener);
    }

    private void onModelUpdate(SortModel model, int updateTypeUnspecified) {
@@ -69,6 +70,11 @@ public final class TableHeaderController implements SortController.WidgetControl
        mTableHeader.setVisibility(visibility);
    }

    @Override
    public void destroy() {
        mModel.removeListener(mModelListener);
    }

    private void bindCell(HeaderCell cell, @SortDimensionId int id) {
        assert(cell != null);
        SortDimension dimension = mModel.getDimensionById(id);
+40 −9
Original line number Diff line number Diff line
@@ -32,37 +32,64 @@ import org.junit.runner.RunWith;
@SmallTest
public class SortControllerTest {

    // only created when in horizontal tablet layout.
    private final TestWidget mTableHeader = null;
    private TestWidget mTableHeader;
    private TestWidget mDropHeader;
    private SortController mController;

    @Before
    public void setUp() {
        mDropHeader = new TestWidget();
        mController = new SortController(mDropHeader, mTableHeader);
    }

    @Test
    public void testGridMode_ShowsDrop() {
        createWidget(true);
        mController.onViewModeChanged(State.MODE_GRID);
        mDropHeader.assertVisible();
        mTableHeader.assertGone();
    }

    @Test
    public void testListMode_ShowsTable() {
    public void testListMode_ShowsDrop_NoHeader() {
        createWidget(false);
        mController.onViewModeChanged(State.MODE_LIST);
        mDropHeader.assertVisible();
    }

    @Test
    public void testListMode_ShowsTable() {
        createWidget(true);
        mController.onViewModeChanged(State.MODE_LIST);
        mDropHeader.assertGone();
        mTableHeader.assertVisible();
    }

    @Test
    public void testDestroysWidgets() {
        createWidget(true);
        mController.destroy();

        mDropHeader.assertDestroyed();
        mTableHeader.assertDestroyed();
    }

    private void createWidget(boolean hasTableHeader) {
        mDropHeader = new TestWidget();
        if (hasTableHeader) {
            mTableHeader = new TestWidget();
        }
        mController = new SortController(mDropHeader, mTableHeader);
    }

    static class TestWidget implements SortController.WidgetController {
        private int mVisibility;
        private boolean mDestroyed;

        @Override
        public void setVisibility(int visibility) {
            mVisibility = visibility;
        }

        @Override
        public void destroy() {
            mDestroyed = true;
        }

        void assertVisible() {
            assertTrue(
                    "Expected mode VISIBLE, but was " + mVisibility,
@@ -74,5 +101,9 @@ public class SortControllerTest {
                    "Expected mode GONE, but was " + mVisibility,
                    mVisibility == View.GONE);
        }

        void assertDestroyed() {
            assertTrue("Widget is not destroyed.", mDestroyed);
        }
    }
}