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

Commit 72a89275 authored by Zhenwei Chen's avatar Zhenwei Chen Committed by Wesley Wang
Browse files

Fix LoaderCallback.onLoadFinished uncalled issue 



When two loaders started almost at the same time,
it is possible onLoadFinished is never called.

Bug: 260687359
Test: Unit tests passed and manual test on device
Merged-In: I41a041d5878f9930db44775408380d0d4588faba
Change-Id: I41a041d5878f9930db44775408380d0d4588faba
Signed-off-by: default avatarZhenwei Chen <zhenwec@google.com>
(cherry picked from commit 41ce8772)
parent 80095fa0

src/com/android/settings/fuelgauge/batteryusage/PowerUsageAdvanced.java

+1 −2
Original line number Diff line number Diff line
@@ -55,7 +55,6 @@ public class PowerUsageAdvanced extends PowerUsageBase { 
    private static final String KEY_REFRESH_TYPE = "refresh_type";

    private static final String KEY_BATTERY_GRAPH = "battery_graph";

    private static final String KEY_APP_LIST = "app_list";

    private static final int LOADER_BATTERY_USAGE_STATS = 2;



    @VisibleForTesting

    BatteryHistoryPreference mHistPref;
@@ -188,7 +187,7 @@ public class PowerUsageAdvanced extends PowerUsageBase { 
        // Uses customized battery history loader if chart design is enabled.

        if (mIsChartGraphEnabled && !mIsChartDataLoaded) {

            mIsChartDataLoaded = true;

            getLoaderManager().restartLoader(LOADER_BATTERY_USAGE_STATS, bundle,

            restartLoader(LoaderIndex.BATTERY_HISTORY_LOADER, bundle,

                    mBatteryHistoryLoaderCallbacks);

        } else if (!mIsChartGraphEnabled) {

            super.restartBatteryStatsLoader(refreshType);

src/com/android/settings/fuelgauge/batteryusage/PowerUsageBase.java

+42 −6
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ import android.os.Bundle; 
import android.os.UserManager;

import android.util.Log;



import androidx.annotation.IntDef;

import androidx.annotation.NonNull;

import androidx.annotation.VisibleForTesting;

import androidx.loader.app.LoaderManager;
@@ -33,17 +34,19 @@ import com.android.settings.dashboard.DashboardFragment; 
import com.android.settings.fuelgauge.BatteryBroadcastReceiver;

import com.android.settings.fuelgauge.BatteryUtils;



import java.lang.annotation.Retention;

import java.lang.annotation.RetentionPolicy;



/**

 * Common base class for things that need to show the battery usage graph.

 */

public abstract class PowerUsageBase extends DashboardFragment {



    private static final String TAG = "PowerUsageBase";

    private static final String KEY_REFRESH_TYPE = "refresh_type";

    private static final String KEY_INCLUDE_HISTORY = "include_history";



    private static final int LOADER_BATTERY_USAGE_STATS = 1;



    @VisibleForTesting

    static final String KEY_REFRESH_TYPE = "refresh_type";

    @VisibleForTesting

    static final String KEY_INCLUDE_HISTORY = "include_history";

    @VisibleForTesting

    BatteryUsageStats mBatteryUsageStats;
@@ -55,6 +58,21 @@ public abstract class PowerUsageBase extends DashboardFragment { 
    final BatteryUsageStatsLoaderCallbacks mBatteryUsageStatsLoaderCallbacks =

            new BatteryUsageStatsLoaderCallbacks();



    @Retention(RetentionPolicy.SOURCE)

    @IntDef({

            LoaderIndex.BATTERY_USAGE_STATS_LOADER,

            LoaderIndex.BATTERY_INFO_LOADER,

            LoaderIndex.BATTERY_TIP_LOADER,

            LoaderIndex.BATTERY_HISTORY_LOADER



    })

    public @interface LoaderIndex {

        int BATTERY_USAGE_STATS_LOADER = 0;

        int BATTERY_INFO_LOADER = 1;

        int BATTERY_TIP_LOADER = 2;

        int BATTERY_HISTORY_LOADER = 3;

    }



    @Override

    public void onAttach(Activity activity) {

        super.onAttach(activity);
@@ -91,10 +109,28 @@ public abstract class PowerUsageBase extends DashboardFragment { 
        final Bundle bundle = new Bundle();

        bundle.putInt(KEY_REFRESH_TYPE, refreshType);

        bundle.putBoolean(KEY_INCLUDE_HISTORY, isBatteryHistoryNeeded());

        getLoaderManager().restartLoader(LOADER_BATTERY_USAGE_STATS, bundle,

        restartLoader(LoaderIndex.BATTERY_USAGE_STATS_LOADER, bundle,

                mBatteryUsageStatsLoaderCallbacks);

    }



    protected LoaderManager getLoaderManagerForCurrentFragment() {

        return LoaderManager.getInstance(this);

    }



    protected void restartLoader(int loaderId, Bundle bundle,

            LoaderManager.LoaderCallbacks<?> loaderCallbacks) {

        LoaderManager loaderManager = getLoaderManagerForCurrentFragment();

        Loader<?> loader = loaderManager.getLoader(

                loaderId);

        if (loader != null && !loader.isReset()) {

            loaderManager.restartLoader(loaderId, bundle,

                    loaderCallbacks);

        } else {

            loaderManager.initLoader(loaderId, bundle,

                    loaderCallbacks);

        }

    }



    protected void onLoadFinished(@BatteryUpdateType int refreshType) {

        refreshUi(refreshType);

    }

src/com/android/settings/fuelgauge/batteryusage/PowerUsageSummary.java

+2 −8
Original line number Diff line number Diff line
@@ -64,11 +64,6 @@ public class PowerUsageSummary extends PowerUsageBase implements 
    @VisibleForTesting

    static final String KEY_BATTERY_USAGE = "battery_usage_summary";



    @VisibleForTesting

    static final int BATTERY_INFO_LOADER = 1;

    @VisibleForTesting

    static final int BATTERY_TIP_LOADER = 2;



    @VisibleForTesting

    PowerUsageFeatureProvider mPowerFeatureProvider;

    @VisibleForTesting
@@ -241,7 +236,7 @@ public class PowerUsageSummary extends PowerUsageBase implements 


    @VisibleForTesting

    void restartBatteryTipLoader() {

        getLoaderManager().restartLoader(BATTERY_TIP_LOADER, Bundle.EMPTY, mBatteryTipsCallbacks);

        restartLoader(LoaderIndex.BATTERY_TIP_LOADER, Bundle.EMPTY, mBatteryTipsCallbacks);

    }



    @VisibleForTesting
@@ -274,8 +269,7 @@ public class PowerUsageSummary extends PowerUsageBase implements 
        if (!mIsBatteryPresent) {

            return;

        }

        getLoaderManager().restartLoader(BATTERY_INFO_LOADER, Bundle.EMPTY,

                mBatteryInfoLoaderCallbacks);

        restartLoader(LoaderIndex.BATTERY_INFO_LOADER, Bundle.EMPTY, mBatteryInfoLoaderCallbacks);

    }



    @VisibleForTesting

tests/robotests/src/com/android/settings/fuelgauge/batteryusage/PowerUsageBaseTest.java

+72 −3
Original line number Diff line number Diff line
@@ -15,18 +15,26 @@ 
 */

package com.android.settings.fuelgauge.batteryusage;



import static com.android.settings.fuelgauge.batteryusage.PowerUsageBase.KEY_INCLUDE_HISTORY;

import static com.android.settings.fuelgauge.batteryusage.PowerUsageBase.KEY_REFRESH_TYPE;



import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.ArgumentMatchers.refEq;

import static org.mockito.Mockito.doReturn;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.spy;

import static org.mockito.Mockito.verify;



import android.content.Context;

import android.os.BatteryUsageStats;

import android.os.Bundle;



import androidx.loader.app.LoaderManager;

import androidx.loader.content.Loader;



import com.android.settings.fuelgauge.BatteryBroadcastReceiver;

import com.android.settings.testutils.shadow.ShadowDashboardFragment;

import com.android.settingslib.core.AbstractPreferenceController;
@@ -46,14 +54,15 @@ public class PowerUsageBaseTest { 


    @Mock

    private LoaderManager mLoaderManager;

    @Mock

    private Loader<BatteryUsageStats> mBatteryUsageStatsLoader;

    private TestFragment mFragment;



    @Before

    public void setUp() {

        MockitoAnnotations.initMocks(this);



        mFragment = spy(new TestFragment());

        doReturn(mLoaderManager).when(mFragment).getLoaderManager();

        mFragment = spy(new TestFragment(mLoaderManager));

    }



    @Test
@@ -63,7 +72,62 @@ public class PowerUsageBaseTest { 
        verify(mLoaderManager, never()).initLoader(anyInt(), any(Bundle.class), any());

    }



    public static class TestFragment extends PowerUsageBase {

    @Test

    public void restartBatteryInfoLoader() {

        final Bundle bundle = new Bundle();

        bundle.putInt(KEY_REFRESH_TYPE, BatteryBroadcastReceiver.BatteryUpdateType.BATTERY_STATUS);

        bundle.putBoolean(KEY_INCLUDE_HISTORY, false);

        doReturn(mBatteryUsageStatsLoader).when(mLoaderManager).getLoader(

                PowerUsageBase.LoaderIndex.BATTERY_USAGE_STATS_LOADER);

        doReturn(false).when(mBatteryUsageStatsLoader).isReset();



        mFragment.restartBatteryStatsLoader(

                BatteryBroadcastReceiver.BatteryUpdateType.BATTERY_STATUS);



        verify(mLoaderManager)

                .restartLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_USAGE_STATS_LOADER),

                        refEq(bundle), any());

    }



    @Test

    public void restartBatteryInfoLoader_loaderReset_initLoader() {

        final Bundle bundle = new Bundle();

        bundle.putInt(KEY_REFRESH_TYPE, BatteryBroadcastReceiver.BatteryUpdateType.BATTERY_STATUS);

        bundle.putBoolean(KEY_INCLUDE_HISTORY, false);

        doReturn(mBatteryUsageStatsLoader).when(mLoaderManager).getLoader(

                PowerUsageBase.LoaderIndex.BATTERY_USAGE_STATS_LOADER);

        doReturn(true).when(mBatteryUsageStatsLoader).isReset();



        mFragment.restartBatteryStatsLoader(

                BatteryBroadcastReceiver.BatteryUpdateType.BATTERY_STATUS);



        verify(mLoaderManager)

                .initLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_USAGE_STATS_LOADER),

                        refEq(bundle), any());

    }



    @Test

    public void restartBatteryInfoLoader_nullLoader_initLoader() {

        final Bundle bundle = new Bundle();

        bundle.putInt(KEY_REFRESH_TYPE, BatteryBroadcastReceiver.BatteryUpdateType.BATTERY_STATUS);

        bundle.putBoolean(KEY_INCLUDE_HISTORY, false);

        doReturn(null).when(mLoaderManager).getLoader(

                PowerUsageBase.LoaderIndex.BATTERY_USAGE_STATS_LOADER);



        mFragment.restartBatteryStatsLoader(

                BatteryBroadcastReceiver.BatteryUpdateType.BATTERY_STATUS);



        verify(mLoaderManager).initLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_USAGE_STATS_LOADER),

                refEq(bundle), any());

    }



    private static class TestFragment extends PowerUsageBase {



        private LoaderManager mLoaderManager;



        TestFragment(LoaderManager loaderManager) {

            mLoaderManager = loaderManager;

        }



        @Override

        public int getMetricsCategory() {
@@ -94,5 +158,10 @@ public class PowerUsageBaseTest { 
        protected List<AbstractPreferenceController> createPreferenceControllers(Context context) {

            return null;

        }



        @Override

        protected LoaderManager getLoaderManagerForCurrentFragment() {

            return mLoaderManager;

        }

    }

}

tests/robotests/src/com/android/settings/fuelgauge/batteryusage/PowerUsageSummaryTest.java

+127 −13
Original line number Diff line number Diff line
@@ -15,7 +15,6 @@ 
 */

package com.android.settings.fuelgauge.batteryusage;



import static com.android.settings.fuelgauge.batteryusage.PowerUsageSummary.BATTERY_INFO_LOADER;

import static com.android.settings.fuelgauge.batteryusage.PowerUsageSummary.KEY_BATTERY_ERROR;

import static com.android.settings.fuelgauge.batteryusage.PowerUsageSummary.KEY_BATTERY_USAGE;
@@ -39,14 +38,17 @@ import android.os.Bundle; 
import android.provider.Settings;



import androidx.loader.app.LoaderManager;

import androidx.loader.content.Loader;

import androidx.preference.Preference;

import androidx.preference.PreferenceScreen;



import com.android.settings.R;

import com.android.settings.SettingsActivity;

import com.android.settings.fuelgauge.BatteryBroadcastReceiver;

import com.android.settings.fuelgauge.BatteryInfo;

import com.android.settings.fuelgauge.BatteryUtils;

import com.android.settings.fuelgauge.batterytip.BatteryTipPreferenceController;

import com.android.settings.fuelgauge.batterytip.tips.BatteryTip;

import com.android.settings.testutils.FakeFeatureFactory;

import com.android.settings.testutils.XmlTestUtils;

import com.android.settings.testutils.shadow.ShadowUtils;
@@ -69,8 +71,6 @@ import java.util.List; 
// TODO: Improve this test class so that it starts up the real activity and fragment.

@RunWith(RobolectricTestRunner.class)

public class PowerUsageSummaryTest {



    private static final long TIME_SINCE_LAST_FULL_CHARGE_MS = 120 * 60 * 1000;

    private static Intent sAdditionalBatteryInfoIntent;



    @BeforeClass
@@ -83,6 +83,10 @@ public class PowerUsageSummaryTest { 
    @Mock

    private LoaderManager mLoaderManager;

    @Mock

    private Loader<BatteryTip> mBatteryTipLoader;

    @Mock

    private Loader<BatteryInfo> mBatteryInfoLoader;

    @Mock

    private ContentResolver mContentResolver;

    @Mock

    private BatteryBroadcastReceiver mBatteryBroadcastReceiver;
@@ -105,11 +109,9 @@ public class PowerUsageSummaryTest { 


        mRealContext = spy(RuntimeEnvironment.application);

        mFeatureFactory = FakeFeatureFactory.setupForTest();

        mFragment = spy(new TestFragment(mRealContext));

        mFragment = spy(new TestFragment(mRealContext, mLoaderManager));

        mFragment.initFeatureProvider();

        doNothing().when(mFragment).restartBatteryStatsLoader(anyInt());

        doReturn(mock(LoaderManager.class)).when(mFragment).getLoaderManager();



        when(mFragment.getActivity()).thenReturn(mSettingsActivity);

        when(mFeatureFactory.powerUsageFeatureProvider.getAdditionalBatteryInfoIntent())

                .thenReturn(sAdditionalBatteryInfoIntent);
@@ -155,12 +157,58 @@ public class PowerUsageSummaryTest { 
    @Test

    public void restartBatteryTipLoader() {

        //TODO: add policy logic here when BatteryTipPolicy is implemented

        doReturn(mLoaderManager).when(mFragment).getLoaderManager();

        doReturn(mBatteryTipLoader).when(mLoaderManager).getLoader(

                PowerUsageBase.LoaderIndex.BATTERY_TIP_LOADER);

        doReturn(false).when(mBatteryTipLoader).isReset();



        mFragment.restartBatteryTipLoader();



        verify(mLoaderManager).restartLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_TIP_LOADER),

                eq(Bundle.EMPTY), any());

    }



    @Test

    public void restartBatteryTipLoader_nullLoader_initLoader() {

        doReturn(null).when(mLoaderManager).getLoader(

                PowerUsageBase.LoaderIndex.BATTERY_TIP_LOADER);



        mFragment.restartBatteryTipLoader();



        verify(mLoaderManager).initLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_TIP_LOADER),

                eq(Bundle.EMPTY), any());

    }



    @Test

    public void restartBatteryTipLoader_loaderReset_initLoader() {

        doReturn(mBatteryTipLoader).when(mLoaderManager).getLoader(

                PowerUsageBase.LoaderIndex.BATTERY_TIP_LOADER);

        doReturn(true).when(mBatteryTipLoader).isReset();



        mFragment.restartBatteryTipLoader();



        verify(mLoaderManager)

                .restartLoader(eq(PowerUsageSummary.BATTERY_TIP_LOADER), eq(Bundle.EMPTY), any());



        verify(mLoaderManager).initLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_TIP_LOADER),

                eq(Bundle.EMPTY), any());

    }





    @Test

    public void refreshUi_contextNull_doNothing() {

        doReturn(null).when(mFragment).getContext();



        mFragment.refreshUi(BatteryBroadcastReceiver.BatteryUpdateType.MANUAL);



        verify(mFragment, never()).restartBatteryTipLoader();

        verify(mFragment, never()).restartBatteryInfoLoader();

    }



    @Test

    public void refreshUi_batteryNotPresent_doNothing() {

        mFragment.setIsBatteryPresent(false);

        mFragment.refreshUi(BatteryBroadcastReceiver.BatteryUpdateType.MANUAL);



        verify(mFragment, never()).restartBatteryTipLoader();

        verify(mFragment, never()).restartBatteryInfoLoader();

    }



    @Test
@@ -168,10 +216,12 @@ public class PowerUsageSummaryTest { 
        mFragment.mBatteryTipPreferenceController = mock(BatteryTipPreferenceController.class);

        when(mFragment.mBatteryTipPreferenceController.needUpdate()).thenReturn(false);

        mFragment.updateBatteryTipFlag(new Bundle());

        doNothing().when(mFragment).restartBatteryInfoLoader();



        mFragment.refreshUi(BatteryBroadcastReceiver.BatteryUpdateType.MANUAL);



        verify(mFragment, never()).restartBatteryTipLoader();

        verify(mFragment).restartBatteryInfoLoader();

    }



    @Test
@@ -179,10 +229,12 @@ public class PowerUsageSummaryTest { 
        mFragment.mBatteryTipPreferenceController = mock(BatteryTipPreferenceController.class);

        when(mFragment.mBatteryTipPreferenceController.needUpdate()).thenReturn(true);

        mFragment.updateBatteryTipFlag(new Bundle());

        doNothing().when(mFragment).restartBatteryInfoLoader();



        mFragment.refreshUi(BatteryBroadcastReceiver.BatteryUpdateType.BATTERY_LEVEL);



        verify(mFragment, never()).restartBatteryTipLoader();

        verify(mFragment).restartBatteryInfoLoader();

    }



    @Test
@@ -190,10 +242,13 @@ public class PowerUsageSummaryTest { 
        mFragment.mBatteryTipPreferenceController = mock(BatteryTipPreferenceController.class);

        when(mFragment.mBatteryTipPreferenceController.needUpdate()).thenReturn(true);

        mFragment.updateBatteryTipFlag(new Bundle());

        doNothing().when(mFragment).restartBatteryInfoLoader();

        doNothing().when(mFragment).restartBatteryTipLoader();



        mFragment.refreshUi(BatteryBroadcastReceiver.BatteryUpdateType.MANUAL);



        verify(mFragment).restartBatteryTipLoader();

        verify(mFragment).restartBatteryInfoLoader();

    }



    @Test
@@ -217,19 +272,68 @@ public class PowerUsageSummaryTest { 
    @Test

    public void restartBatteryInfoLoader_contextNull_doNothing() {

        when(mFragment.getContext()).thenReturn(null);

        when(mFragment.getLoaderManager()).thenReturn(mLoaderManager);



        mFragment.restartBatteryInfoLoader();



        verify(mLoaderManager, never()).restartLoader(BATTERY_INFO_LOADER, Bundle.EMPTY,

        verify(mLoaderManager, never()).restartLoader(

                PowerUsageBase.LoaderIndex.BATTERY_INFO_LOADER, Bundle.EMPTY,

                mFragment.mBatteryInfoLoaderCallbacks);

    }



    public static class TestFragment extends PowerUsageSummary {

    @Test

    public void restartBatteryInfoLoader_batteryIsNotPresent_doNothing() {

        mFragment.setIsBatteryPresent(false);



        mFragment.restartBatteryInfoLoader();



        verify(mLoaderManager, never()).restartLoader(

                PowerUsageBase.LoaderIndex.BATTERY_INFO_LOADER, Bundle.EMPTY,

                mFragment.mBatteryInfoLoaderCallbacks);

    }



    @Test

    public void restartBatteryInfoLoader() {

        doReturn(mBatteryInfoLoader).when(mLoaderManager).getLoader(

                PowerUsageBase.LoaderIndex.BATTERY_INFO_LOADER);

        doReturn(false).when(mBatteryTipLoader).isReset();



        mFragment.restartBatteryInfoLoader();



        verify(mLoaderManager).restartLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_INFO_LOADER),

                eq(Bundle.EMPTY), any());

    }



    @Test

    public void restartBatteryInfoLoader_nullLoader_initLoader() {

        doReturn(null).when(mLoaderManager).getLoader(

                PowerUsageBase.LoaderIndex.BATTERY_INFO_LOADER);



        mFragment.restartBatteryInfoLoader();



        verify(mLoaderManager).initLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_INFO_LOADER),

                eq(Bundle.EMPTY), any());

    }



    @Test

    public void restartBatteryInfoLoader_loaderReset_initLoader() {

        mFragment.setIsBatteryPresent(true);

        doReturn(mBatteryInfoLoader).when(mLoaderManager).getLoader(

                PowerUsageBase.LoaderIndex.BATTERY_INFO_LOADER);

        doReturn(true).when(mBatteryInfoLoader).isReset();



        mFragment.restartBatteryInfoLoader();



        verify(mLoaderManager).initLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_INFO_LOADER),

                eq(Bundle.EMPTY), any());

    }



    private static class TestFragment extends PowerUsageSummary {

        private Context mContext;

        private LoaderManager mLoaderManager;



        public TestFragment(Context context) {

        TestFragment(Context context, LoaderManager loaderManager) {

            mContext = context;

            mLoaderManager = loaderManager;

        }



        @Override
@@ -242,5 +346,15 @@ public class PowerUsageSummaryTest { 
            // Override it so we can access this method in test

            return super.getContentResolver();

        }



        public void setIsBatteryPresent(boolean isBatteryPresent) {

            mIsBatteryPresent = isBatteryPresent;

        }



        @Override

        protected LoaderManager getLoaderManagerForCurrentFragment() {

            return mLoaderManager;

        }

    }



}