Fix missing time-in-foreground and time-in-background for some apps
The gist of the issue is that many apps have two UIDs associated with them: a regular, "real" UID, e.g. 10123, and a shared group GID, e.g. 50123, which is used for multiuser support. Prior to this fix, the code in BatteryAppListPreferenceController, would go over the list of all UidBatteryConsumers and would randomly encounter either the "real" UID or the shared GID for each app first. The UidBatteryConsumer for a shared GID does not have all of the properties of the real UID, so some information, such as time-in-foreground and time-in-background would be lost with a high probability. After this fix, we process "real" UIDs before shared GIDs ensuring that time-in-* and other properties such as package names are obtained for the real UID. When we later encounter a shared GID for the same app, we just add the consumed power and time-in-* durations to the real UID's BatteryEntry. Bug: 188656360 Test: make RunSettingsRoboTests Test: make RunSettingsGoogleRoboTests Change-Id: I4bfea813ac5eb8f866804b2c4a9153eb877fb325
This commit is contained in:
@@ -118,7 +118,8 @@ public class AppBatteryPreferenceController extends BasePreferenceController
|
||||
final UserManager userManager =
|
||||
(UserManager) mContext.getSystemService(Context.USER_SERVICE);
|
||||
final BatteryEntry entry = new BatteryEntry(mContext, /* handler */null, userManager,
|
||||
mUidBatteryConsumer, /* isHidden */ false, /* packages */ null, mPackageName);
|
||||
mUidBatteryConsumer, /* isHidden */ false,
|
||||
mUidBatteryConsumer.getUid(), /* packages */ null, mPackageName);
|
||||
AdvancedPowerUsageDetail.startBatteryDetailPage(mParent.getActivity(), mParent,
|
||||
entry, mBatteryPercent);
|
||||
} else {
|
||||
|
@@ -55,6 +55,7 @@ import com.android.settingslib.core.lifecycle.events.OnPause;
|
||||
import com.android.settingslib.utils.StringUtil;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Comparator;
|
||||
import java.util.List;
|
||||
|
||||
/**
|
||||
@@ -66,6 +67,7 @@ public class BatteryAppListPreferenceController extends AbstractPreferenceContro
|
||||
static final boolean USE_FAKE_DATA = false;
|
||||
private static final int MAX_ITEMS_TO_LIST = USE_FAKE_DATA ? 30 : 20;
|
||||
private static final int MIN_AVERAGE_POWER_THRESHOLD_MILLI_AMP = 10;
|
||||
private static final String MEDIASERVER_PACKAGE_NAME = "mediaserver";
|
||||
|
||||
private final String mPreferenceKey;
|
||||
@VisibleForTesting
|
||||
@@ -303,27 +305,17 @@ public class BatteryAppListPreferenceController extends AbstractPreferenceContro
|
||||
final ArrayList<BatteryEntry> results = new ArrayList<>();
|
||||
final List<UidBatteryConsumer> uidBatteryConsumers =
|
||||
mBatteryUsageStats.getUidBatteryConsumers();
|
||||
|
||||
// Sort to have all apps with "real" UIDs first, followed by apps that are supposed
|
||||
// to be combined with the real ones.
|
||||
uidBatteryConsumers.sort(Comparator.comparingInt(
|
||||
consumer -> consumer.getUid() == getRealUid(consumer) ? 0 : 1));
|
||||
|
||||
for (int i = 0, size = uidBatteryConsumers.size(); i < size; i++) {
|
||||
final UidBatteryConsumer consumer = uidBatteryConsumers.get(i);
|
||||
int realUid = consumer.getUid();
|
||||
final int uid = getRealUid(consumer);
|
||||
|
||||
// Check if this UID is a shared GID. If so, we combine it with the OWNER's
|
||||
// actual app UID.
|
||||
if (isSharedGid(consumer.getUid())) {
|
||||
realUid = UserHandle.getUid(UserHandle.USER_SYSTEM,
|
||||
UserHandle.getAppIdFromSharedAppGid(consumer.getUid()));
|
||||
}
|
||||
|
||||
// Check if this UID is a system UID (mediaserver, logd, nfc, drm, etc).
|
||||
if (isSystemUid(realUid)
|
||||
&& !"mediaserver".equals(consumer.getPackageWithHighestDrain())) {
|
||||
// Use the system UID for all UIDs running in their own sandbox that
|
||||
// are not apps. We exclude mediaserver because we already are expected to
|
||||
// report that as a separate item.
|
||||
realUid = Process.SYSTEM_UID;
|
||||
}
|
||||
|
||||
final String[] packages = mPackageManager.getPackagesForUid(consumer.getUid());
|
||||
final String[] packages = mPackageManager.getPackagesForUid(uid);
|
||||
if (mBatteryUtils.shouldHideUidBatteryConsumerUnconditionally(consumer, packages)) {
|
||||
continue;
|
||||
}
|
||||
@@ -333,11 +325,11 @@ public class BatteryAppListPreferenceController extends AbstractPreferenceContro
|
||||
continue;
|
||||
}
|
||||
|
||||
final int index = batteryEntryList.indexOfKey(realUid);
|
||||
final int index = batteryEntryList.indexOfKey(uid);
|
||||
if (index < 0) {
|
||||
// New entry.
|
||||
batteryEntryList.put(realUid, new BatteryEntry(mContext, mHandler, mUserManager,
|
||||
consumer, isHidden, packages, null, loadDataInBackground));
|
||||
batteryEntryList.put(uid, new BatteryEntry(mContext, mHandler, mUserManager,
|
||||
consumer, isHidden, uid, packages, null, loadDataInBackground));
|
||||
} else {
|
||||
// Combine BatterySippers if we already have one with this UID.
|
||||
final BatteryEntry existingSipper = batteryEntryList.valueAt(index);
|
||||
@@ -385,7 +377,8 @@ public class BatteryAppListPreferenceController extends AbstractPreferenceContro
|
||||
for (int i = 0, size = userBatteryConsumers.size(); i < size; i++) {
|
||||
final UserBatteryConsumer consumer = userBatteryConsumers.get(i);
|
||||
results.add(new BatteryEntry(mContext, mHandler, mUserManager,
|
||||
consumer, /* isHidden */ true, null, null, loadDataInBackground));
|
||||
consumer, /* isHidden */ true, Process.INVALID_UID, null, null,
|
||||
loadDataInBackground));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -400,6 +393,27 @@ public class BatteryAppListPreferenceController extends AbstractPreferenceContro
|
||||
return results;
|
||||
}
|
||||
|
||||
private int getRealUid(UidBatteryConsumer consumer) {
|
||||
int realUid = consumer.getUid();
|
||||
|
||||
// Check if this UID is a shared GID. If so, we combine it with the OWNER's
|
||||
// actual app UID.
|
||||
if (isSharedGid(consumer.getUid())) {
|
||||
realUid = UserHandle.getUid(UserHandle.USER_SYSTEM,
|
||||
UserHandle.getAppIdFromSharedAppGid(consumer.getUid()));
|
||||
}
|
||||
|
||||
// Check if this UID is a system UID (mediaserver, logd, nfc, drm, etc).
|
||||
if (isSystemUid(realUid)
|
||||
&& !MEDIASERVER_PACKAGE_NAME.equals(consumer.getPackageWithHighestDrain())) {
|
||||
// Use the system UID for all UIDs running in their own sandbox that
|
||||
// are not apps. We exclude mediaserver because we already are expected to
|
||||
// report that as a separate item.
|
||||
realUid = Process.SYSTEM_UID;
|
||||
}
|
||||
return realUid;
|
||||
}
|
||||
|
||||
@VisibleForTesting
|
||||
void setUsageSummary(Preference preference, BatteryEntry entry) {
|
||||
// Only show summary when usage time is longer than one minute
|
||||
|
@@ -159,12 +159,15 @@ public class BatteryEntry {
|
||||
|
||||
private final Context mContext;
|
||||
private final BatteryConsumer mBatteryConsumer;
|
||||
private final int mUid;
|
||||
private final boolean mIsHidden;
|
||||
@ConvertUtils.ConsumerType
|
||||
private final int mConsumerType;
|
||||
@BatteryConsumer.PowerComponent
|
||||
private final int mPowerComponentId;
|
||||
private long mUsageDurationMs;
|
||||
private long mTimeInForegroundMs;
|
||||
private long mTimeInBackgroundMs;
|
||||
|
||||
public String name;
|
||||
public Drawable icon;
|
||||
@@ -180,13 +183,13 @@ public class BatteryEntry {
|
||||
}
|
||||
|
||||
public BatteryEntry(Context context, Handler handler, UserManager um,
|
||||
@NonNull BatteryConsumer batteryConsumer, boolean isHidden, String[] packages,
|
||||
@NonNull BatteryConsumer batteryConsumer, boolean isHidden, int uid, String[] packages,
|
||||
String packageName) {
|
||||
this(context, handler, um, batteryConsumer, isHidden, packages, packageName, true);
|
||||
this(context, handler, um, batteryConsumer, isHidden, uid, packages, packageName, true);
|
||||
}
|
||||
|
||||
public BatteryEntry(Context context, Handler handler, UserManager um,
|
||||
@NonNull BatteryConsumer batteryConsumer, boolean isHidden, String[] packages,
|
||||
@NonNull BatteryConsumer batteryConsumer, boolean isHidden, int uid, String[] packages,
|
||||
String packageName, boolean loadDataInBackground) {
|
||||
sHandler = handler;
|
||||
mContext = context;
|
||||
@@ -196,11 +199,11 @@ public class BatteryEntry {
|
||||
mPowerComponentId = -1;
|
||||
|
||||
if (batteryConsumer instanceof UidBatteryConsumer) {
|
||||
mUid = uid;
|
||||
mConsumerType = ConvertUtils.CONSUMER_TYPE_UID_BATTERY;
|
||||
mConsumedPower = batteryConsumer.getConsumedPower();
|
||||
|
||||
UidBatteryConsumer uidBatteryConsumer = (UidBatteryConsumer) batteryConsumer;
|
||||
int uid = uidBatteryConsumer.getUid();
|
||||
if (mDefaultPackageName == null) {
|
||||
// Apps should only have one package
|
||||
if (packages != null && packages.length == 1) {
|
||||
@@ -222,7 +225,12 @@ public class BatteryEntry {
|
||||
}
|
||||
}
|
||||
getQuickNameIconForUid(uid, packages, loadDataInBackground);
|
||||
mTimeInForegroundMs =
|
||||
uidBatteryConsumer.getTimeInStateMs(UidBatteryConsumer.STATE_FOREGROUND);
|
||||
mTimeInBackgroundMs =
|
||||
uidBatteryConsumer.getTimeInStateMs(UidBatteryConsumer.STATE_BACKGROUND);
|
||||
} else if (batteryConsumer instanceof UserBatteryConsumer) {
|
||||
mUid = Process.INVALID_UID;
|
||||
mConsumerType = ConvertUtils.CONSUMER_TYPE_USER_BATTERY;
|
||||
mConsumedPower = batteryConsumer.getConsumedPower();
|
||||
final NameAndIcon nameAndIcon = getNameAndIconFromUserId(
|
||||
@@ -239,6 +247,7 @@ public class BatteryEntry {
|
||||
double appsPowerMah, long usageDurationMs) {
|
||||
mContext = context;
|
||||
mBatteryConsumer = null;
|
||||
mUid = Process.INVALID_UID;
|
||||
mIsHidden = false;
|
||||
mPowerComponentId = powerComponentId;
|
||||
mConsumedPower =
|
||||
@@ -261,6 +270,7 @@ public class BatteryEntry {
|
||||
double devicePowerMah, double appsPowerMah) {
|
||||
mContext = context;
|
||||
mBatteryConsumer = null;
|
||||
mUid = Process.INVALID_UID;
|
||||
mIsHidden = false;
|
||||
mPowerComponentId = powerComponentId;
|
||||
|
||||
@@ -438,7 +448,7 @@ public class BatteryEntry {
|
||||
*/
|
||||
public String getKey() {
|
||||
if (mBatteryConsumer instanceof UidBatteryConsumer) {
|
||||
return Integer.toString(((UidBatteryConsumer) mBatteryConsumer).getUid());
|
||||
return Integer.toString(mUid);
|
||||
} else if (mBatteryConsumer instanceof UserBatteryConsumer) {
|
||||
return "U|" + ((UserBatteryConsumer) mBatteryConsumer).getUserId();
|
||||
} else {
|
||||
@@ -482,11 +492,7 @@ public class BatteryEntry {
|
||||
* Returns the UID of the app described by this entry.
|
||||
*/
|
||||
public int getUid() {
|
||||
if (mBatteryConsumer instanceof UidBatteryConsumer) {
|
||||
return ((UidBatteryConsumer) mBatteryConsumer).getUid();
|
||||
} else {
|
||||
return Process.INVALID_UID;
|
||||
}
|
||||
return mUid;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -494,8 +500,7 @@ public class BatteryEntry {
|
||||
*/
|
||||
public long getTimeInForegroundMs() {
|
||||
if (mBatteryConsumer instanceof UidBatteryConsumer) {
|
||||
return ((UidBatteryConsumer) mBatteryConsumer).getTimeInStateMs(
|
||||
UidBatteryConsumer.STATE_FOREGROUND);
|
||||
return mTimeInForegroundMs;
|
||||
} else {
|
||||
return mUsageDurationMs;
|
||||
}
|
||||
@@ -506,8 +511,7 @@ public class BatteryEntry {
|
||||
*/
|
||||
public long getTimeInBackgroundMs() {
|
||||
if (mBatteryConsumer instanceof UidBatteryConsumer) {
|
||||
return ((UidBatteryConsumer) mBatteryConsumer).getTimeInStateMs(
|
||||
UidBatteryConsumer.STATE_BACKGROUND);
|
||||
return mTimeInBackgroundMs;
|
||||
} else {
|
||||
return 0;
|
||||
}
|
||||
@@ -526,9 +530,15 @@ public class BatteryEntry {
|
||||
*/
|
||||
public void add(BatteryConsumer batteryConsumer) {
|
||||
mConsumedPower += batteryConsumer.getConsumedPower();
|
||||
if (mDefaultPackageName == null && batteryConsumer instanceof UidBatteryConsumer) {
|
||||
mDefaultPackageName =
|
||||
((UidBatteryConsumer) batteryConsumer).getPackageWithHighestDrain();
|
||||
if (batteryConsumer instanceof UidBatteryConsumer) {
|
||||
UidBatteryConsumer uidBatteryConsumer = (UidBatteryConsumer) batteryConsumer;
|
||||
mTimeInForegroundMs += uidBatteryConsumer.getTimeInStateMs(
|
||||
UidBatteryConsumer.STATE_FOREGROUND);
|
||||
mTimeInBackgroundMs += uidBatteryConsumer.getTimeInStateMs(
|
||||
UidBatteryConsumer.STATE_BACKGROUND);
|
||||
if (mDefaultPackageName == null) {
|
||||
mDefaultPackageName = uidBatteryConsumer.getPackageWithHighestDrain();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
@@ -93,7 +93,7 @@ public class BatteryEntryTest {
|
||||
when(consumer.getUid()).thenReturn(APP_UID);
|
||||
when(consumer.getPackageWithHighestDrain()).thenReturn(highDrainPackage);
|
||||
return new BatteryEntry(mMockContext, mockHandler, mockUserManager,
|
||||
consumer, false, packages, packageName);
|
||||
consumer, false, APP_UID, packages, packageName);
|
||||
}
|
||||
|
||||
private BatteryEntry createAggregateBatteryEntry(int powerComponentId) {
|
||||
@@ -108,7 +108,7 @@ public class BatteryEntryTest {
|
||||
UserBatteryConsumer consumer = mock(UserBatteryConsumer.class);
|
||||
when(consumer.getUserId()).thenReturn(userId);
|
||||
return new BatteryEntry(mMockContext, mockHandler, mockUserManager,
|
||||
consumer, false, null, null);
|
||||
consumer, false, 0, null, null);
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -169,12 +169,12 @@ public class BatteryEntryTest {
|
||||
|
||||
@Test
|
||||
public void getTimeInForegroundMs_app() {
|
||||
final BatteryEntry entry = new BatteryEntry(RuntimeEnvironment.application, mockHandler,
|
||||
mockUserManager, mUidBatteryConsumer, false, null, null);
|
||||
|
||||
when(mUidBatteryConsumer.getTimeInStateMs(UidBatteryConsumer.STATE_FOREGROUND))
|
||||
.thenReturn(100L);
|
||||
|
||||
final BatteryEntry entry = new BatteryEntry(RuntimeEnvironment.application, mockHandler,
|
||||
mockUserManager, mUidBatteryConsumer, false, 0, null, null);
|
||||
|
||||
assertThat(entry.getTimeInForegroundMs()).isEqualTo(100L);
|
||||
}
|
||||
|
||||
@@ -188,12 +188,12 @@ public class BatteryEntryTest {
|
||||
|
||||
@Test
|
||||
public void getTimeInBackgroundMs_app() {
|
||||
final BatteryEntry entry = new BatteryEntry(RuntimeEnvironment.application, mockHandler,
|
||||
mockUserManager, mUidBatteryConsumer, false, null, null);
|
||||
|
||||
when(mUidBatteryConsumer.getTimeInStateMs(UidBatteryConsumer.STATE_BACKGROUND))
|
||||
.thenReturn(100L);
|
||||
|
||||
final BatteryEntry entry = new BatteryEntry(RuntimeEnvironment.application, mockHandler,
|
||||
mockUserManager, mUidBatteryConsumer, false, 0, null, null);
|
||||
|
||||
assertThat(entry.getTimeInBackgroundMs()).isEqualTo(100L);
|
||||
}
|
||||
|
||||
|
Reference in New Issue
Block a user