From 710d5fc886729b28afbf77927888d6454ce2a52f Mon Sep 17 00:00:00 2001 From: Lei Yu Date: Tue, 17 Apr 2018 11:07:47 -0700 Subject: [PATCH] Force update all suggestions but app restriction When there is configuration change(icicle is not null), still force update all suggestion except app restriction. App restriction is not stateless: state HANDLED only happens when there is anomaly and it disappear in next cycle. So we should only update it when necessary. Change-Id: Ifb7a1c477962a0c78b5455a5fbc078590fd408f2 Fixes: 77973093 Test: RunSettingsRoboTests --- .../settings/fuelgauge/PowerUsageSummary.java | 10 ++++- .../BatteryTipPreferenceController.java | 7 ++++ .../fuelgauge/batterytip/tips/BatteryTip.java | 11 +++++ .../batterytip/tips/RestrictAppTip.java | 2 + .../fuelgauge/PowerUsageSummaryTest.java | 41 +++++++++++++------ .../batterytip/tips/BatteryTipTest.java | 1 + 6 files changed, 57 insertions(+), 15 deletions(-) diff --git a/src/com/android/settings/fuelgauge/PowerUsageSummary.java b/src/com/android/settings/fuelgauge/PowerUsageSummary.java index 2c8f9611bcd..4c4b6e926ca 100644 --- a/src/com/android/settings/fuelgauge/PowerUsageSummary.java +++ b/src/com/android/settings/fuelgauge/PowerUsageSummary.java @@ -107,7 +107,8 @@ public class PowerUsageSummary extends PowerUsageBase implements OnLongClickList BatteryHeaderPreferenceController mBatteryHeaderPreferenceController; @VisibleForTesting boolean mNeedUpdateBatteryTip; - private BatteryTipPreferenceController mBatteryTipPreferenceController; + @VisibleForTesting + BatteryTipPreferenceController mBatteryTipPreferenceController; private int mStatsType = BatteryStats.STATS_SINCE_CHARGED; @VisibleForTesting @@ -213,8 +214,8 @@ public class PowerUsageSummary extends PowerUsageBase implements OnLongClickList mAnomalySparseArray = new SparseArray<>(); restartBatteryInfoLoader(); - mNeedUpdateBatteryTip = icicle == null; mBatteryTipPreferenceController.restoreInstanceState(icicle); + updateBatteryTipFlag(icicle); } @Override @@ -382,6 +383,11 @@ public class PowerUsageSummary extends PowerUsageBase implements OnLongClickList } } + @VisibleForTesting + void updateBatteryTipFlag(Bundle icicle) { + mNeedUpdateBatteryTip = icicle == null || mBatteryTipPreferenceController.needUpdate(); + } + @Override public boolean onLongClick(View view) { showBothEstimates(); diff --git a/src/com/android/settings/fuelgauge/batterytip/BatteryTipPreferenceController.java b/src/com/android/settings/fuelgauge/batterytip/BatteryTipPreferenceController.java index 249bf9b3f57..784c54cfc1f 100644 --- a/src/com/android/settings/fuelgauge/batterytip/BatteryTipPreferenceController.java +++ b/src/com/android/settings/fuelgauge/batterytip/BatteryTipPreferenceController.java @@ -52,6 +52,7 @@ public class BatteryTipPreferenceController extends BasePreferenceController { private Map mBatteryTipMap; private SettingsActivity mSettingsActivity; private MetricsFeatureProvider mMetricsFeatureProvider; + private boolean mNeedUpdate; @VisibleForTesting PreferenceGroup mPreferenceGroup; @VisibleForTesting @@ -71,6 +72,7 @@ public class BatteryTipPreferenceController extends BasePreferenceController { mFragment = fragment; mSettingsActivity = settingsActivity; mMetricsFeatureProvider = FeatureFactory.getFactory(context).getMetricsFeatureProvider(); + mNeedUpdate = true; } @Override @@ -111,6 +113,7 @@ public class BatteryTipPreferenceController extends BasePreferenceController { mBatteryTipMap.put(preference.getKey(), batteryTip); mPreferenceGroup.addPreference(preference); batteryTip.log(mContext, mMetricsFeatureProvider); + mNeedUpdate = batteryTip.needUpdate(); break; } } @@ -153,6 +156,10 @@ public class BatteryTipPreferenceController extends BasePreferenceController { outState.putParcelableList(KEY_BATTERY_TIPS, mBatteryTips); } + public boolean needUpdate() { + return mNeedUpdate; + } + /** * Listener to give the control back to target fragment */ diff --git a/src/com/android/settings/fuelgauge/batterytip/tips/BatteryTip.java b/src/com/android/settings/fuelgauge/batterytip/tips/BatteryTip.java index 3c3a5c06992..f02dd7296b2 100644 --- a/src/com/android/settings/fuelgauge/batterytip/tips/BatteryTip.java +++ b/src/com/android/settings/fuelgauge/batterytip/tips/BatteryTip.java @@ -86,17 +86,23 @@ public abstract class BatteryTip implements Comparable, Parcelable { protected int mType; protected int mState; protected boolean mShowDialog; + /** + * Whether we need to update battery tip when configuration change + */ + protected boolean mNeedUpdate; BatteryTip(Parcel in) { mType = in.readInt(); mState = in.readInt(); mShowDialog = in.readBoolean(); + mNeedUpdate = in.readBoolean(); } BatteryTip(int type, int state, boolean showDialog) { mType = type; mState = state; mShowDialog = showDialog; + mNeedUpdate = true; } @Override @@ -109,6 +115,7 @@ public abstract class BatteryTip implements Comparable, Parcelable { dest.writeInt(mType); dest.writeInt(mState); dest.writeBoolean(mShowDialog); + dest.writeBoolean(mNeedUpdate); } public abstract CharSequence getTitle(Context context); @@ -144,6 +151,10 @@ public abstract class BatteryTip implements Comparable, Parcelable { return mShowDialog; } + public boolean needUpdate() { + return mNeedUpdate; + } + public String getKey() { return KEY_PREFIX + mType; } diff --git a/src/com/android/settings/fuelgauge/batterytip/tips/RestrictAppTip.java b/src/com/android/settings/fuelgauge/batterytip/tips/RestrictAppTip.java index 8b161661b55..9aa8363f415 100644 --- a/src/com/android/settings/fuelgauge/batterytip/tips/RestrictAppTip.java +++ b/src/com/android/settings/fuelgauge/batterytip/tips/RestrictAppTip.java @@ -42,12 +42,14 @@ public class RestrictAppTip extends BatteryTip { public RestrictAppTip(@StateType int state, List restrictApps) { super(TipType.APP_RESTRICTION, state, state == StateType.NEW /* showDialog */); mRestrictAppList = restrictApps; + mNeedUpdate = false; } public RestrictAppTip(@StateType int state, AppInfo appInfo) { super(TipType.APP_RESTRICTION, state, state == StateType.NEW /* showDialog */); mRestrictAppList = new ArrayList<>(); mRestrictAppList.add(appInfo); + mNeedUpdate = false; } @VisibleForTesting diff --git a/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageSummaryTest.java b/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageSummaryTest.java index 989c0332371..b20cf168261 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageSummaryTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageSummaryTest.java @@ -49,6 +49,7 @@ import com.android.settings.R; import com.android.settings.SettingsActivity; import com.android.settings.applications.LayoutPreference; import com.android.settings.fuelgauge.anomaly.Anomaly; +import com.android.settings.fuelgauge.batterytip.BatteryTipPreferenceController; import com.android.settings.testutils.FakeFeatureFactory; import com.android.settings.testutils.SettingsRobolectricTestRunner; import com.android.settings.testutils.XmlTestUtils; @@ -187,7 +188,7 @@ public class PowerUsageSummaryTest { } @Test - public void testUpdateLastFullChargePreference_noAverageTime_showLastFullChargeSummary() { + public void updateLastFullChargePreference_noAverageTime_showLastFullChargeSummary() { mFragment.mBatteryInfo = null; when(mFragment.getContext()).thenReturn(mRealContext); doReturn(TIME_SINCE_LAST_FULL_CHARGE_MS).when( @@ -200,7 +201,7 @@ public class PowerUsageSummaryTest { } @Test - public void testUpdateLastFullChargePreference_hasAverageTime_showFullChargeLastSummary() { + public void updateLastFullChargePreference_hasAverageTime_showFullChargeLastSummary() { mFragment.mBatteryInfo = mBatteryInfo; mBatteryInfo.averageTimeToDischarge = TIME_SINCE_LAST_FULL_CHARGE_MS; when(mFragment.getContext()).thenReturn(mRealContext); @@ -212,7 +213,7 @@ public class PowerUsageSummaryTest { } @Test - public void testNonIndexableKeys_MatchPreferenceKeys() { + public void nonIndexableKeys_MatchPreferenceKeys() { final Context context = RuntimeEnvironment.application; final List niks = PowerUsageSummary.SEARCH_INDEX_DATA_PROVIDER.getNonIndexableKeys(context); @@ -224,7 +225,7 @@ public class PowerUsageSummaryTest { } @Test - public void testPreferenceControllers_getPreferenceKeys_existInPreferenceScreen() { + public void preferenceControllers_getPreferenceKeys_existInPreferenceScreen() { final Context context = RuntimeEnvironment.application; final PowerUsageSummary fragment = new PowerUsageSummary(); final List preferenceScreenKeys = @@ -239,7 +240,7 @@ public class PowerUsageSummaryTest { } @Test - public void testUpdateAnomalySparseArray() { + public void updateAnomalySparseArray() { mFragment.mAnomalySparseArray = new SparseArray<>(); final List anomalies = new ArrayList<>(); final Anomaly anomaly1 = new Anomaly.Builder().setUid(UID).build(); @@ -256,7 +257,7 @@ public class PowerUsageSummaryTest { } @Test - public void testRestartBatteryTipLoader() { + public void restartBatteryTipLoader() { //TODO: add policy logic here when BatteryTipPolicy is implemented doReturn(mLoaderManager).when(mFragment).getLoaderManager(); @@ -267,7 +268,7 @@ public class PowerUsageSummaryTest { } @Test - public void testShowBothEstimates_summariesAreBothModified() { + public void showBothEstimates_summariesAreBothModified() { when(mFeatureFactory.powerUsageFeatureProvider.isEnhancedBatteryPredictionEnabled(any())) .thenReturn(true); doAnswer(new Answer() { @@ -296,7 +297,7 @@ public class PowerUsageSummaryTest { } @Test - public void testDebugMode() { + public void debugMode() { doReturn(true).when(mFeatureFactory.powerUsageFeatureProvider).isEstimateDebugEnabled(); doReturn(new TextView(mRealContext)).when(mBatteryLayoutPref).findViewById(R.id.summary2); @@ -315,7 +316,7 @@ public class PowerUsageSummaryTest { } @Test - public void testRestartBatteryStatsLoader_notClearHeader_quickUpdateNotInvoked() { + public void restartBatteryStatsLoader_notClearHeader_quickUpdateNotInvoked() { mFragment.mBatteryHeaderPreferenceController = mBatteryHeaderPreferenceController; mFragment.restartBatteryStatsLoader(false /* clearHeader */); @@ -324,7 +325,7 @@ public class PowerUsageSummaryTest { } @Test - public void testOptionsMenu_advancedPageEnabled() { + public void optionsMenu_advancedPageEnabled() { when(mFeatureFactory.powerUsageFeatureProvider.isPowerAccountingToggleEnabled()) .thenReturn(true); @@ -335,7 +336,7 @@ public class PowerUsageSummaryTest { } @Test - public void testOptionsMenu_clickAdvancedPage_fireIntent() { + public void optionsMenu_clickAdvancedPage_fireIntent() { final ArgumentCaptor captor = ArgumentCaptor.forClass(Intent.class); doAnswer(invocation -> { // Get the intent in which it has the app info bundle @@ -353,13 +354,27 @@ public class PowerUsageSummaryTest { } @Test - public void testRefreshUi_deviceRotate_doNotUpdateBatteryTip() { - mFragment.mNeedUpdateBatteryTip = false; + public void refreshUi_deviceRotate_doNotUpdateBatteryTip() { + mFragment.mBatteryTipPreferenceController = mock(BatteryTipPreferenceController.class); + when(mFragment.mBatteryTipPreferenceController.needUpdate()).thenReturn(false); + mFragment.updateBatteryTipFlag(new Bundle()); + mFragment.refreshUi(); verify(mFragment, never()).restartBatteryTipLoader(); } + @Test + public void refreshUi_tipNeedUpdate_updateBatteryTip() { + mFragment.mBatteryTipPreferenceController = mock(BatteryTipPreferenceController.class); + when(mFragment.mBatteryTipPreferenceController.needUpdate()).thenReturn(true); + mFragment.updateBatteryTipFlag(new Bundle()); + + mFragment.refreshUi(); + + verify(mFragment).restartBatteryTipLoader(); + } + public static class TestFragment extends PowerUsageSummary { private Context mContext; diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/BatteryTipTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/BatteryTipTest.java index cee647ec004..405e761c030 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/BatteryTipTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/BatteryTipTest.java @@ -74,6 +74,7 @@ public class BatteryTipTest { assertThat(parcelTip.getTitle(mContext)).isEqualTo(TITLE); assertThat(parcelTip.getSummary(mContext)).isEqualTo(SUMMARY); assertThat(parcelTip.getIconId()).isEqualTo(ICON_ID); + assertThat(parcelTip.needUpdate()).isTrue(); } @Test