Reduce the timeframe which Deletion Helper queries for usage stats.
By querying only 1 year back instead of infinity, we can improve
get results slightly faster and avoid some issues w.r.t. querying
for all time (i.e. start time 0).
As a side-note, this should also fix the problem where apps which
should be ineligible were showing up as eligible. We will need
to monitor that situation in the dogfood population.
Bug: 34400961
Test: Robotests
Change-Id: I5cd3202ad25db5716150c857c7da2422c0fa741e
diff --git a/res/values/strings.xml b/res/values/strings.xml
index 1259e30..07f747e 100644
--- a/res/values/strings.xml
+++ b/res/values/strings.xml
@@ -24,8 +24,8 @@
<string name="deletion_helper_title">Choose items to remove</string>
<!-- Summary of how much storage an app is using and the number of days since last use. [CHAR LIMIT=NONE]-->
<string name="deletion_helper_app_summary"><xliff:g id="used" example="1.2GB">%1$s</xliff:g> • <xliff:g id="days" example="67">%2$d</xliff:g> days ago</string>
- <!-- Summary of how much storage an app is using when it has never been used before. [CHAR LIMIT=NONE]-->
- <string name="deletion_helper_app_summary_never_used"><xliff:g id="used" example="1.2GB">%1$s</xliff:g> • Never used before</string>
+ <!-- Summary of how much storage an app is using when it has not been used in the last year. [CHAR LIMIT=NONE]-->
+ <string name="deletion_helper_app_summary_never_used"><xliff:g id="used" example="1.2GB">%1$s</xliff:g> • Not used in last year</string>
<!-- Summary of how much storage an app is using when its last use is unknown. [CHAR LIMIT=NONE]-->
<string name="deletion_helper_app_summary_unknown_used"><xliff:g id="used" example="1.2GB">%1$s</xliff:g> • Not sure when last used</string>
<!-- Button which clears out storage in the deletion helper. [CHAR LIMIT=60]-->
diff --git a/robotests/src/com/android/storagemanager/automatic/AutomaticStorageManagementJobServiceTest.java b/robotests/src/com/android/storagemanager/automatic/AutomaticStorageManagementJobServiceTest.java
deleted file mode 100644
index c0e1cd4..0000000
--- a/robotests/src/com/android/storagemanager/automatic/AutomaticStorageManagementJobServiceTest.java
+++ /dev/null
@@ -1,248 +0,0 @@
-/*
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License"); you may not
- * use this file except in compliance with the License. You may obtain a copy of
- * the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
- * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
- * License for the specific language governing permissions and limitations under
- * the License.
- */
-
-package com.android.storagemanager.automatic;
-
-import android.app.NotificationManager;
-import android.app.job.JobParameters;
-import android.content.ContentResolver;
-import android.content.Context;
-import android.content.Intent;
-import android.os.BatteryManager;
-import android.os.storage.StorageManager;
-import android.os.storage.VolumeInfo;
-import android.provider.Settings;
-import com.android.settingslib.deviceinfo.StorageVolumeProvider;
-import com.android.storagemanager.overlay.FeatureFactory;
-import com.android.storagemanager.overlay.StorageManagementJobProvider;
-import com.android.storagemanager.testing.TestingConstants;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.MockitoAnnotations;
-import org.robolectric.Robolectric;
-import org.robolectric.RobolectricTestRunner;
-import org.robolectric.annotation.Config;
-import org.robolectric.shadows.ShadowApplication;
-import org.robolectric.util.ReflectionHelpers;
-
-import java.io.File;
-import java.util.ArrayList;
-import java.util.List;
-
-import static com.google.common.truth.Truth.assertThat;
-import static org.mockito.Mockito.any;
-import static org.mockito.Mockito.eq;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.spy;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.verifyNoMoreInteractions;
-import static org.mockito.Mockito.when;
-
-@RunWith(RobolectricTestRunner.class)
-@Config(manifest=TestingConstants.MANIFEST, sdk=TestingConstants.SDK_VERSION)
-public class AutomaticStorageManagementJobServiceTest {
- @Mock private BatteryManager mBatteryManager;
- @Mock private NotificationManager mNotificationManager;
- @Mock private VolumeInfo mVolumeInfo;
- @Mock private File mFile;
- @Mock private JobParameters mJobParameters;
- @Mock private StorageManagementJobProvider mStorageManagementJobProvider;
- @Mock private FeatureFactory mFeatureFactory;
- @Mock private StorageVolumeProvider mStorageVolumeProvider;
- private AutomaticStorageManagementJobService mJobService;
- private ShadowApplication mApplication;
- private List<VolumeInfo> mVolumes;
-
- @Before
- public void setUp() {
- MockitoAnnotations.initMocks(this);
-
- when(mJobParameters.getJobId()).thenReturn(0);
-
- // Let's set up our system services to act like a device that has the following conditions:
- // 1. We're plugged in and charging.
- // 2. We have a completely full device.
- // 3. ASM is disabled.
- when(mBatteryManager.isCharging()).thenReturn(true);
- mVolumes = new ArrayList<>();
- when(mVolumeInfo.getPath()).thenReturn(mFile);
- when(mVolumeInfo.getType()).thenReturn(VolumeInfo.TYPE_PRIVATE);
- when(mVolumeInfo.getFsUuid()).thenReturn(StorageManager.UUID_PRIMARY_PHYSICAL);
- when(mFile.getTotalSpace()).thenReturn(100L);
- when(mFile.getFreeSpace()).thenReturn(0L);
- mVolumes.add(mVolumeInfo);
- when(mStorageVolumeProvider.getPrimaryStorageSize()).thenReturn(100L);
- when(mStorageVolumeProvider.getVolumes()).thenReturn(mVolumes);
-
- mApplication = ShadowApplication.getInstance();
- mApplication.setSystemService(Context.BATTERY_SERVICE, mBatteryManager);
- mApplication.setSystemService(Context.NOTIFICATION_SERVICE, mNotificationManager);
-
- // This is a hack-y injection of our own FeatureFactory.
- // By default, the Storage Manager has a FeatureFactory which returns null for all features.
- // Using reflection, we can inject our own FeatureFactory which returns a mock for the
- // StorageManagementJobProvider feature. This lets us observe when the ASMJobService
- // actually tries to run the job.
- when(mFeatureFactory.getStorageManagementJobProvider())
- .thenReturn(mStorageManagementJobProvider);
- when(mStorageManagementJobProvider.onStartJob(any(Context.class),
- any(JobParameters.class), any(Integer.class))).thenReturn(false);
- ReflectionHelpers.setStaticField(FeatureFactory.class, "sFactory", mFeatureFactory);
-
- // And we can't forget to initialize the actual job service.
- mJobService = spy(Robolectric.setupService(AutomaticStorageManagementJobService.class));
- mJobService.setStorageVolumeProvider(mStorageVolumeProvider);
- }
-
- @Test
- public void testJobRequiresCharging() {
- when(mBatteryManager.isCharging()).thenReturn(false);
- assertThat(mJobService.onStartJob(mJobParameters)).isFalse();
- // The job should report that it needs to be retried, if not charging.
- assertJobFinished(true);
-
- when(mBatteryManager.isCharging()).thenReturn(true);
- assertThat(mJobService.onStartJob(mJobParameters)).isFalse();
- assertJobFinished(false);
- }
-
- @Test
- public void testStartJobTriesUpsellWhenASMDisabled() {
- assertThat(mJobService.onStartJob(mJobParameters)).isFalse();
- assertJobFinished(false);
- mApplication.runBackgroundTasks();
-
- List<Intent> broadcastedIntents = mApplication.getBroadcastIntents();
- assertThat(broadcastedIntents.size()).isEqualTo(1);
-
- Intent lastIntent = broadcastedIntents.get(0);
- assertThat(lastIntent.getAction())
- .isEqualTo(NotificationController.INTENT_ACTION_SHOW_NOTIFICATION);
- assertThat(lastIntent.getComponent().getClassName())
- .isEqualTo(NotificationController.class.getCanonicalName());
-
- assertStorageManagerJobDidNotRun();
- }
-
- @Test
- public void testASMJobRunsWithValidConditions() {
- activateASM();
- assertThat(mJobService.onStartJob(mJobParameters)).isFalse();
- assertStorageManagerJobRan();
- }
-
- @Test
- public void testJobDoesntRunIfStorageNotFull() {
- activateASM();
- when(mFile.getFreeSpace()).thenReturn(100L);
- assertThat(mJobService.onStartJob(mJobParameters)).isFalse();
- assertStorageManagerJobDidNotRun();
- }
-
- @Test
- public void testJobOnlyRunsIfFreeStorageIsUnder15Percent() {
- activateASM();
- when(mFile.getFreeSpace()).thenReturn(15L);
- assertThat(mJobService.onStartJob(mJobParameters)).isFalse();
- assertStorageManagerJobDidNotRun();
-
- when(mFile.getFreeSpace()).thenReturn(14L);
- assertThat(mJobService.onStartJob(mJobParameters)).isFalse();
- assertStorageManagerJobRan();
- }
-
- @Test
- public void testNonDefaultDaysToRetain() {
- ContentResolver resolver = mApplication.getApplicationContext().getContentResolver();
- Settings.Secure.putInt(resolver, Settings.Secure.AUTOMATIC_STORAGE_MANAGER_DAYS_TO_RETAIN,
- 30);
- activateASM();
- assertThat(mJobService.onStartJob(mJobParameters)).isFalse();
- assertStorageManagerJobRan(30);
- }
-
- @Test
- public void testNonPrivateDrivesIgnoredForFreeSpaceCalculation() {
- File notPrivate = mock(File.class);
- VolumeInfo nonPrivateVolume = mock(VolumeInfo.class);
- when(nonPrivateVolume.getPath()).thenReturn(notPrivate);
- when(nonPrivateVolume.getType()).thenReturn(VolumeInfo.TYPE_PUBLIC);
- when(notPrivate.getTotalSpace()).thenReturn(100L);
- when(notPrivate.getFreeSpace()).thenReturn(0L);
- mVolumes.add(nonPrivateVolume);
- activateASM();
- when(mFile.getFreeSpace()).thenReturn(15L);
-
- assertThat(mJobService.onStartJob(mJobParameters)).isFalse();
- assertStorageManagerJobDidNotRun();
- }
-
- @Test
- public void testMultiplePrivateVolumesCountedForASMActivationThrsehold() {
- File privateVolume = mock(File.class);
- VolumeInfo privateVolumeInfo = mock(VolumeInfo.class);
- when(privateVolumeInfo.getPath()).thenReturn(privateVolume);
- when(privateVolumeInfo.getType()).thenReturn(VolumeInfo.TYPE_PRIVATE);
- when(privateVolumeInfo.getFsUuid()).thenReturn(StorageManager.UUID_PRIVATE_INTERNAL);
- when(privateVolume.getTotalSpace()).thenReturn(100L);
- when(privateVolume.getFreeSpace()).thenReturn(0L);
- mVolumes.add(privateVolumeInfo);
- activateASM();
- when(mFile.getFreeSpace()).thenReturn(15L);
-
- assertThat(mJobService.onStartJob(mJobParameters)).isFalse();
- assertStorageManagerJobRan();
- }
-
- @Test
- public void testPrimaryPrivateStorageDefersToStorageManager() {
- // The Storage Manager has a more accurate calculation for the UUID_PRIVATE_INTERNAL
- // volume, so we don't use the getTotalSpace() method to find it.
- // If the job accidentally uses the getTotalSpace(), it will run the job accidentally.
- when(mVolumeInfo.getFsUuid()).thenReturn(StorageManager.UUID_PRIVATE_INTERNAL);
- when(mFile.getTotalSpace()).thenReturn(1000L);
- when(mFile.getFreeSpace()).thenReturn(15L);
- activateASM();
-
- assertThat(mJobService.onStartJob(mJobParameters)).isFalse();
- assertStorageManagerJobDidNotRun();
- }
-
- private void assertJobFinished(boolean retryNeeded) {
- verify(mJobService).jobFinished(any(JobParameters.class), eq(retryNeeded));
- }
-
- private void assertStorageManagerJobRan() {
- assertStorageManagerJobRan(
- Settings.Secure.AUTOMATIC_STORAGE_MANAGER_DAYS_TO_RETAIN_DEFAULT);
- }
-
- private void assertStorageManagerJobRan(int daysToRetain) {
- verify(mStorageManagementJobProvider).onStartJob(eq(mJobService), eq(mJobParameters),
- eq(daysToRetain));
- }
-
- private void assertStorageManagerJobDidNotRun() {
- verifyNoMoreInteractions(mStorageManagementJobProvider);
- }
-
- private void activateASM() {
- ContentResolver resolver = mApplication.getApplicationContext().getContentResolver();
- Settings.Secure.putInt(resolver, Settings.Secure.AUTOMATIC_STORAGE_MANAGER_ENABLED, 1);
- }
-}
diff --git a/src/com/android/storagemanager/deletionhelper/AppStateUsageStatsBridge.java b/src/com/android/storagemanager/deletionhelper/AppStateUsageStatsBridge.java
index c39eec8..07ca7c1 100644
--- a/src/com/android/storagemanager/deletionhelper/AppStateUsageStatsBridge.java
+++ b/src/com/android/storagemanager/deletionhelper/AppStateUsageStatsBridge.java
@@ -24,8 +24,8 @@
import android.content.pm.PackageManager;
import android.os.SystemProperties;
import android.os.UserHandle;
+import android.text.format.DateUtils;
import android.util.Log;
-import com.android.storagemanager.deletionhelper.AppStateBaseBridge;
import com.android.storagemanager.deletionhelper.AppStateBaseBridge.Callback;
import com.android.settingslib.applications.ApplicationsState;
import com.android.settingslib.applications.ApplicationsState.AppEntry;
@@ -42,12 +42,17 @@
private static final String TAG = "AppStateUsageStatsBridge";
private static final String DEBUG_APP_UNUSED_OVERRIDE = "debug.asm.app_unused_limit";
-
- private UsageStatsManager mUsageStatsManager;
- private PackageManager mPm;
public static final long NEVER_USED = Long.MAX_VALUE;
public static final long UNKNOWN_LAST_USE = -1;
public static final long UNUSED_DAYS_DELETION_THRESHOLD = 90;
+ private static final long DAYS_IN_A_TYPICAL_YEAR = 365;
+
+ private UsageStatsManager mUsageStatsManager;
+ private PackageManager mPm;
+ // This clock is used to provide the time. By default, it uses the system clock, but can be
+ // replaced for test purposes.
+ protected Clock mClock;
+
public AppStateUsageStatsBridge(Context context, ApplicationsState appState,
Callback callback) {
@@ -55,6 +60,7 @@
mUsageStatsManager =
(UsageStatsManager) context.getSystemService(Context.USAGE_STATS_SERVICE);
mPm = context.getPackageManager();
+ mClock = new Clock();
}
@Override
@@ -62,8 +68,7 @@
ArrayList<AppEntry> apps = mAppSession.getAllApps();
if (apps == null) return;
- final Map<String, UsageStats> map = mUsageStatsManager.queryAndAggregateUsageStats(0,
- System.currentTimeMillis());
+ final Map<String, UsageStats> map = getAggregatedUsageStats();
for (AppEntry entry : apps) {
UsageStats usageStats = map.get(entry.info.packageName);
entry.extraInfo = new UsageStatsState(getDaysSinceLastUse(usageStats),
@@ -74,8 +79,7 @@
@Override
protected void updateExtraInfo(AppEntry app, String pkg, int uid) {
- Map<String, UsageStats> map = mUsageStatsManager.queryAndAggregateUsageStats(0,
- System.currentTimeMillis());
+ Map<String, UsageStats> map = getAggregatedUsageStats();
UsageStats usageStats = map.get(app.info.packageName);
app.extraInfo = new UsageStatsState(getDaysSinceLastUse(usageStats),
getDaysSinceInstalled(app.info.packageName),
@@ -91,7 +95,13 @@
if (lastUsed <= 0) {
return UNKNOWN_LAST_USE;
}
- return TimeUnit.MILLISECONDS.toDays(System.currentTimeMillis() - lastUsed);
+
+ // Theoretically, this should be impossible, but UsageStatsService, uh, finds a way.
+ long days = (TimeUnit.MILLISECONDS.toDays(mClock.getCurrentTime() - lastUsed));
+ if (days > DAYS_IN_A_TYPICAL_YEAR) {
+ return NEVER_USED;
+ }
+ return days;
}
private long getDaysSinceInstalled(String packageName) {
@@ -105,8 +115,13 @@
if (pi == null) {
return UNKNOWN_LAST_USE;
}
+ return (TimeUnit.MILLISECONDS.toDays(mClock.getCurrentTime() - pi.firstInstallTime));
+ }
- return (TimeUnit.MILLISECONDS.toDays(System.currentTimeMillis() - pi.firstInstallTime));
+ private Map<String, UsageStats> getAggregatedUsageStats() {
+ long now = mClock.getCurrentTime();
+ long startTime = now - DateUtils.YEAR_IN_MILLIS;
+ return mUsageStatsManager.queryAndAggregateUsageStats(startTime, now);
}
/**
@@ -171,4 +186,13 @@
this.userId = userId;
}
}
+
+ /**
+ * Clock provides the current time.
+ */
+ static class Clock {
+ public long getCurrentTime() {
+ return System.currentTimeMillis();
+ }
+ }
}
