From 013d3f634274b7a7f920acc7d9c90925fd352d9b Mon Sep 17 00:00:00 2001 From: jackqdyulei Date: Mon, 22 May 2017 14:23:37 -0700 Subject: [PATCH] Use BatteryStatsLoader in InstalledAppDetails InstalledAppDetails uses AsyncTask to update foreground mBatteryStatsHelper in background thread, which is dangerous. This cl make InstalledAppDetails use BatteryStatsLoader to update batteryStatsHelper and only assign it once in UI thread. Bug: 38497555 Test: RunSettingsRoboTests Change-Id: I3078b60a2dae36995ae5f925b4d49e36e79bddfd --- .../applications/InstalledAppDetails.java | 72 +++++++++++-------- .../applications/InstalledAppDetailsTest.java | 21 ++++++ 2 files changed, 63 insertions(+), 30 deletions(-) diff --git a/src/com/android/settings/applications/InstalledAppDetails.java b/src/com/android/settings/applications/InstalledAppDetails.java index e9b7481d0e4..ab9f3588c10 100755 --- a/src/com/android/settings/applications/InstalledAppDetails.java +++ b/src/com/android/settings/applications/InstalledAppDetails.java @@ -93,6 +93,7 @@ import com.android.settings.datausage.DataUsageList; import com.android.settings.datausage.DataUsageSummary; import com.android.settings.fuelgauge.AdvancedPowerUsageDetail; import com.android.settings.fuelgauge.BatteryEntry; +import com.android.settings.fuelgauge.BatteryStatsHelperLoader; import com.android.settings.fuelgauge.BatteryUtils; import com.android.settings.notification.AppNotificationSettings; import com.android.settings.notification.NotificationBackend; @@ -143,6 +144,7 @@ public class InstalledAppDetails extends AppInfoBase private static final int LOADER_CHART_DATA = 2; private static final int LOADER_STORAGE = 3; + private static final int LOADER_BATTERY = 4; private static final int DLG_FORCE_STOP = DLG_BASE + 1; private static final int DLG_DISABLE = DLG_BASE + 2; @@ -204,6 +206,31 @@ public class InstalledAppDetails extends AppInfoBase private String mBatteryPercent; private BatteryUtils mBatteryUtils; + private final LoaderCallbacks mBatteryCallbacks = + new LoaderCallbacks() { + + @Override + public Loader onCreateLoader(int id, Bundle args) { + return new BatteryStatsHelperLoader(getContext(), args); + } + + @Override + public void onLoadFinished(Loader loader, + BatteryStatsHelper batteryHelper) { + mBatteryHelper = batteryHelper; + if (mPackageInfo != null) { + mSipper = findTargetSipper(batteryHelper, mPackageInfo.applicationInfo.uid); + if (getActivity() != null) { + updateBattery(); + } + } + } + + @Override + public void onLoaderReset(Loader loader) { + } + }; + private boolean handleDisableable(Button button) { boolean disableable = false; // Try to prevent the user from bricking their phone @@ -362,7 +389,6 @@ public class InstalledAppDetails extends AppInfoBase } else { removePreference(KEY_DATA); } - mBatteryHelper = new BatteryStatsHelper(getActivity(), true); mBatteryUtils = BatteryUtils.getInstance(getContext()); } @@ -386,7 +412,7 @@ public class InstalledAppDetails extends AppInfoBase mDataCallbacks); loaderManager.restartLoader(LOADER_STORAGE, Bundle.EMPTY, this); } - new BatteryUpdater().execute(); + getLoaderManager().initLoader(LOADER_BATTERY, Bundle.EMPTY, mBatteryCallbacks); new MemoryUpdater().execute(); updateDynamicPrefs(); } @@ -625,6 +651,19 @@ public class InstalledAppDetails extends AppInfoBase return showIt; } + @VisibleForTesting + BatterySipper findTargetSipper(BatteryStatsHelper batteryHelper, int uid) { + List usageList = batteryHelper.getUsageList(); + for (int i = 0, size = usageList.size(); i < size; i++) { + BatterySipper sipper = usageList.get(i); + if (sipper.getUid() == uid) { + return sipper; + } + } + + return null; + } + private boolean signaturesMatch(String pkg1, String pkg2) { if (pkg1 != null && pkg2 != null) { try { @@ -719,7 +758,7 @@ public class InstalledAppDetails extends AppInfoBase } private void updateBattery() { - if (mSipper != null) { + if (mSipper != null && mBatteryHelper != null) { mBatteryPreference.setEnabled(true); final int dischargeAmount = mBatteryHelper.getStats().getDischargeAmount( BatteryStats.STATS_SINCE_CHARGED); @@ -1343,33 +1382,6 @@ public class InstalledAppDetails extends AppInfoBase } - private class BatteryUpdater extends AsyncTask { - @Override - protected Void doInBackground(Void... params) { - mBatteryHelper.create((Bundle) null); - mBatteryHelper.refreshStats(BatteryStats.STATS_SINCE_CHARGED, - mUserManager.getUserProfiles()); - List usageList = mBatteryHelper.getUsageList(); - final int N = usageList.size(); - for (int i = 0; i < N; i++) { - BatterySipper sipper = usageList.get(i); - if (sipper.getUid() == mPackageInfo.applicationInfo.uid) { - mSipper = sipper; - break; - } - } - return null; - } - - @Override - protected void onPostExecute(Void result) { - if (getActivity() == null) { - return; - } - refreshUi(); - } - } - /** * Elicit this class for testing. Test cannot be done in robolectric because it * invokes the new API. diff --git a/tests/robotests/src/com/android/settings/applications/InstalledAppDetailsTest.java b/tests/robotests/src/com/android/settings/applications/InstalledAppDetailsTest.java index 17910bf37c5..a09aeec1dad 100644 --- a/tests/robotests/src/com/android/settings/applications/InstalledAppDetailsTest.java +++ b/tests/robotests/src/com/android/settings/applications/InstalledAppDetailsTest.java @@ -18,6 +18,7 @@ package com.android.settings.applications; import static com.google.common.truth.Truth.assertThat; + import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.doReturn; @@ -65,12 +66,17 @@ import org.robolectric.RuntimeEnvironment; import org.robolectric.annotation.Config; import org.robolectric.util.ReflectionHelpers; +import java.util.ArrayList; +import java.util.List; + @RunWith(SettingsRobolectricTestRunner.class) @Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) public final class InstalledAppDetailsTest { private static final String PACKAGE_NAME = "test_package_name"; + private static final int TARGET_UID = 111; + private static final int OTHER_UID = 222; @Mock(answer = Answers.RETURNS_DEEP_STUBS) private Context mContext; @@ -87,6 +93,8 @@ public final class InstalledAppDetailsTest { @Mock private BatterySipper mBatterySipper; @Mock + private BatterySipper mOtherBatterySipper; + @Mock private BatteryStatsHelper mBatteryStatsHelper; @Mock private BatteryStats.Uid mUid; @@ -105,6 +113,8 @@ public final class InstalledAppDetailsTest { mBatterySipper.drainType = BatterySipper.DrainType.IDLE; mBatterySipper.uidObj = mUid; + doReturn(TARGET_UID).when(mBatterySipper).getUid(); + doReturn(OTHER_UID).when(mOtherBatterySipper).getUid(); doReturn(mActivity).when(mAppDetail).getActivity(); doReturn(mShadowContext).when(mAppDetail).getContext(); doReturn(mPackageManager).when(mActivity).getPackageManager(); @@ -388,4 +398,15 @@ public final class InstalledAppDetailsTest { verify(mActivity).invalidateOptionsMenu(); } + + @Test + public void findTargetSipper_findCorrectSipper() { + List usageList = new ArrayList<>(); + usageList.add(mBatterySipper); + usageList.add(mOtherBatterySipper); + doReturn(usageList).when(mBatteryStatsHelper).getUsageList(); + + assertThat(mAppDetail.findTargetSipper(mBatteryStatsHelper, TARGET_UID)).isEqualTo( + mBatterySipper); + } }