From aa625bd06ea21399caf68c3d6cbd1d51466c6969 Mon Sep 17 00:00:00 2001 From: Lei Yu Date: Mon, 29 Apr 2019 13:25:24 -0700 Subject: [PATCH] Fix a crash in battery settings page Regression from ag/7161923, in this case we should use onResume/onPause pair. Also sort the method to fit android lifecycle. Bug: 131615524 Test: RunSettingsRoboTests Change-Id: I299032bfeb119dac293039917c6673dd4c0ef4e0 --- .../settings/fuelgauge/PowerUsageSummary.java | 34 +++++------ .../fuelgauge/PowerUsageSummaryTest.java | 57 ++++++++++++++++--- 2 files changed, 68 insertions(+), 23 deletions(-) diff --git a/src/com/android/settings/fuelgauge/PowerUsageSummary.java b/src/com/android/settings/fuelgauge/PowerUsageSummary.java index 78391f271b1..85a08793b42 100644 --- a/src/com/android/settings/fuelgauge/PowerUsageSummary.java +++ b/src/com/android/settings/fuelgauge/PowerUsageSummary.java @@ -19,6 +19,7 @@ package com.android.settings.fuelgauge; import static com.android.settings.fuelgauge.BatteryBroadcastReceiver.BatteryUpdateType; import android.app.settings.SettingsEnums; +import android.content.ContentResolver; import android.content.Context; import android.database.ContentObserver; import android.net.Uri; @@ -107,7 +108,8 @@ public class PowerUsageSummary extends PowerUsageBase implements OnLongClickList BatteryTipPreferenceController mBatteryTipPreferenceController; private int mStatsType = BatteryStats.STATS_SINCE_CHARGED; - private final ContentObserver mSettingsObserver = new ContentObserver(new Handler()) { + @VisibleForTesting + final ContentObserver mSettingsObserver = new ContentObserver(new Handler()) { @Override public void onChange(boolean selfChange, Uri uri) { restartBatteryInfoLoader(); @@ -201,21 +203,6 @@ public class PowerUsageSummary extends PowerUsageBase implements OnLongClickList } }; - @Override - public void onStop() { - getContentResolver().unregisterContentObserver(mSettingsObserver); - super.onStop(); - } - - @Override - public void onResume() { - super.onResume(); - getContentResolver().registerContentObserver( - Global.getUriFor(Global.BATTERY_ESTIMATES_LAST_UPDATE_TIME), - false, - mSettingsObserver); - } - @Override public void onAttach(Context context) { super.onAttach(context); @@ -251,6 +238,21 @@ public class PowerUsageSummary extends PowerUsageBase implements OnLongClickList updateBatteryTipFlag(icicle); } + @Override + public void onResume() { + super.onResume(); + getContentResolver().registerContentObserver( + Global.getUriFor(Global.BATTERY_ESTIMATES_LAST_UPDATE_TIME), + false, + mSettingsObserver); + } + + @Override + public void onPause() { + getContentResolver().unregisterContentObserver(mSettingsObserver); + super.onPause(); + } + @Override public int getMetricsCategory() { return SettingsEnums.FUELGAUGE_POWER_USAGE_SUMMARY_V2; diff --git a/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageSummaryTest.java b/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageSummaryTest.java index 2ee6d0c9588..2dd0d9be71e 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageSummaryTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageSummaryTest.java @@ -33,16 +33,20 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import android.content.ContentResolver; import android.content.Context; import android.content.Intent; import android.os.Bundle; +import android.provider.Settings; import android.view.Menu; import android.view.MenuInflater; import android.view.MenuItem; import android.view.View; import android.widget.TextView; +import androidx.annotation.VisibleForTesting; import androidx.loader.app.LoaderManager; +import androidx.preference.PreferenceScreen; import com.android.internal.os.BatterySipper; import com.android.internal.os.BatteryStatsHelper; @@ -51,6 +55,7 @@ import com.android.settings.SettingsActivity; import com.android.settings.fuelgauge.batterytip.BatteryTipPreferenceController; import com.android.settings.testutils.FakeFeatureFactory; import com.android.settings.testutils.XmlTestUtils; +import com.android.settingslib.core.instrumentation.VisibilityLoggerMixin; import com.android.settingslib.widget.LayoutPreference; import org.junit.Before; @@ -66,6 +71,7 @@ import org.mockito.stubbing.Answer; import org.robolectric.Robolectric; import org.robolectric.RobolectricTestRunner; import org.robolectric.RuntimeEnvironment; +import org.robolectric.util.ReflectionHelpers; import java.util.ArrayList; import java.util.List; @@ -117,6 +123,14 @@ public class PowerUsageSummaryTest { private MenuItem mAdvancedPageMenu; @Mock private BatteryInfo mBatteryInfo; + @Mock + private ContentResolver mContentResolver; + @Mock + private BatteryBroadcastReceiver mBatteryBroadcastReceiver; + @Mock + private VisibilityLoggerMixin mVisibilityLoggerMixin; + @Mock + private PreferenceScreen mPreferenceScreen; private List mUsageList; private Context mRealContext; @@ -148,7 +162,7 @@ public class PowerUsageSummaryTest { .thenReturn(sAdditionalBatteryInfoIntent); when(mBatteryHelper.getTotalPower()).thenReturn(TOTAL_POWER); when(mBatteryHelper.getStats().computeBatteryRealtime(anyLong(), anyInt())) - .thenReturn(TIME_SINCE_LAST_FULL_CHARGE_US); + .thenReturn(TIME_SINCE_LAST_FULL_CHARGE_US); when(mNormalBatterySipper.getUid()).thenReturn(UID); mNormalBatterySipper.totalPowerMah = POWER_MAH; @@ -176,6 +190,11 @@ public class PowerUsageSummaryTest { mFragment.mScreenUsagePref = mScreenUsagePref; mFragment.mLastFullChargePref = mLastFullChargePref; mFragment.mBatteryUtils = spy(new BatteryUtils(mRealContext)); + ReflectionHelpers.setField(mFragment, "mVisibilityLoggerMixin", mVisibilityLoggerMixin); + ReflectionHelpers.setField(mFragment, "mBatteryBroadcastReceiver", + mBatteryBroadcastReceiver); + doReturn(mPreferenceScreen).when(mFragment).getPreferenceScreen(); + when(mFragment.getContentResolver()).thenReturn(mContentResolver); } @Test @@ -207,10 +226,10 @@ public class PowerUsageSummaryTest { public void nonIndexableKeys_MatchPreferenceKeys() { final Context context = RuntimeEnvironment.application; final List niks = - PowerUsageSummary.SEARCH_INDEX_DATA_PROVIDER.getNonIndexableKeys(context); + PowerUsageSummary.SEARCH_INDEX_DATA_PROVIDER.getNonIndexableKeys(context); final List keys = - XmlTestUtils.getKeysFromPreferenceXml(context, R.xml.power_usage_summary); + XmlTestUtils.getKeysFromPreferenceXml(context, R.xml.power_usage_summary); assertThat(keys).containsAllIn(niks); } @@ -223,25 +242,25 @@ public class PowerUsageSummaryTest { mFragment.restartBatteryTipLoader(); verify(mLoaderManager) - .restartLoader(eq(PowerUsageSummary.BATTERY_TIP_LOADER), eq(Bundle.EMPTY), any()); + .restartLoader(eq(PowerUsageSummary.BATTERY_TIP_LOADER), eq(Bundle.EMPTY), any()); } @Test public void showBothEstimates_summariesAreBothModified() { when(mFeatureFactory.powerUsageFeatureProvider.isEnhancedBatteryPredictionEnabled(any())) - .thenReturn(true); + .thenReturn(true); doAnswer(new Answer() { @Override public Object answer(InvocationOnMock invocation) { return mRealContext.getString( - R.string.power_usage_old_debug, invocation.getArguments()[0]); + R.string.power_usage_old_debug, invocation.getArguments()[0]); } }).when(mFeatureFactory.powerUsageFeatureProvider).getOldEstimateDebugString(any()); doAnswer(new Answer() { @Override public Object answer(InvocationOnMock invocation) { return mRealContext.getString( - R.string.power_usage_enhanced_debug, invocation.getArguments()[0]); + R.string.power_usage_enhanced_debug, invocation.getArguments()[0]); } }).when(mFeatureFactory.powerUsageFeatureProvider).getEnhancedEstimateDebugString(any()); @@ -336,6 +355,24 @@ public class PowerUsageSummaryTest { verify(mFragment).restartBatteryTipLoader(); } + @Test + public void onResume_registerContentObserver() { + mFragment.onResume(); + + verify(mContentResolver).registerContentObserver( + Settings.Global.getUriFor(Settings.Global.BATTERY_ESTIMATES_LAST_UPDATE_TIME), + false, + mFragment.mSettingsObserver); + } + + @Test + public void onPause_unregisterContentObserver() { + mFragment.onPause(); + + verify(mContentResolver).unregisterContentObserver( + mFragment.mSettingsObserver); + } + public static class TestFragment extends PowerUsageSummary { private Context mContext; @@ -348,6 +385,12 @@ public class PowerUsageSummaryTest { return mContext; } + @Override + protected ContentResolver getContentResolver() { + // Override it so we can access this method in test + return super.getContentResolver(); + } + @Override void showBothEstimates() { List fakeBatteryInfo = new ArrayList<>(2);