diff --git a/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java b/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java index 2cc3bae09c1..a69340c2923 100644 --- a/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java +++ b/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java @@ -38,7 +38,10 @@ import android.os.StatsDimensionsValue; import android.os.UserHandle; import android.os.UserManager; import android.provider.Settings; + import androidx.annotation.VisibleForTesting; +import androidx.annotation.GuardedBy; + import android.util.Log; import android.util.Pair; @@ -65,10 +68,13 @@ public class AnomalyDetectionJobService extends JobService { static final int UID_NULL = -1; @VisibleForTesting static final int STATSD_UID_FILED = 1; - @VisibleForTesting static final long MAX_DELAY_MS = TimeUnit.MINUTES.toMillis(30); + private final Object mLock = new Object(); + @GuardedBy("mLock") + private boolean mIsJobCanceled = false; + public static void scheduleAnomalyDetection(Context context, Intent intent) { final JobScheduler jobScheduler = context.getSystemService(JobScheduler.class); final ComponentName component = new ComponentName(context, @@ -102,14 +108,14 @@ public class AnomalyDetectionJobService extends JobService { .getFactory(this).getMetricsFeatureProvider(); batteryUtils.initBatteryStatsHelper(batteryStatsHelper, null /* bundle */, userManager); - for (JobWorkItem item = params.dequeueWork(); item != null; - item = params.dequeueWork()) { + for (JobWorkItem item = dequeueWork(params); item != null; item = dequeueWork(params)) { saveAnomalyToDatabase(context, batteryStatsHelper, userManager, batteryDatabaseManager, batteryUtils, policy, powerWhitelistBackend, contentResolver, powerUsageFeatureProvider, metricsFeatureProvider, item.getIntent().getExtras()); + + completeWork(params, item); } - jobFinished(params, false /* wantsReschedule */); }); return true; @@ -117,7 +123,10 @@ public class AnomalyDetectionJobService extends JobService { @Override public boolean onStopJob(JobParameters jobParameters) { - return false; + synchronized (mLock) { + mIsJobCanceled = true; + } + return true; // Need to reschedule } @VisibleForTesting @@ -229,4 +238,26 @@ public class AnomalyDetectionJobService extends JobService { return anomalyInfo.anomalyType == StatsManagerConfig.AnomalyType.EXCESSIVE_BACKGROUND_SERVICE; } + + @VisibleForTesting + JobWorkItem dequeueWork(JobParameters parameters) { + synchronized (mLock) { + if (mIsJobCanceled) { + return null; + } + + return parameters.dequeueWork(); + } + } + + @VisibleForTesting + void completeWork(JobParameters parameters, JobWorkItem item) { + synchronized (mLock) { + if (mIsJobCanceled) { + return; + } + + parameters.completeWork(item); + } + } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobServiceTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobServiceTest.java index d6fad34e5dd..f159e7970e6 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobServiceTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobServiceTest.java @@ -23,11 +23,14 @@ import static android.os.StatsDimensionsValue.TUPLE_VALUE_TYPE; import static com.google.common.truth.Truth.assertThat; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyLong; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; @@ -37,7 +40,9 @@ import static org.robolectric.RuntimeEnvironment.application; import android.app.StatsManager; import android.app.job.JobInfo; +import android.app.job.JobParameters; import android.app.job.JobScheduler; +import android.app.job.JobWorkItem; import android.content.Context; import android.content.Intent; import android.os.Bundle; @@ -55,6 +60,7 @@ import com.android.settings.fuelgauge.BatteryUtils; import com.android.settings.overlay.FeatureFactory; import com.android.settings.testutils.FakeFeatureFactory; import com.android.settings.testutils.SettingsRobolectricTestRunner; +import com.android.settings.testutils.shadow.ShadowConnectivityManager; import com.android.settingslib.fuelgauge.PowerWhitelistBackend; import org.junit.Before; @@ -62,8 +68,11 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.robolectric.Robolectric; import org.robolectric.RuntimeEnvironment; import org.robolectric.Shadows; +import org.robolectric.android.controller.ServiceController; +import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowJobScheduler; import java.util.ArrayList; @@ -71,6 +80,7 @@ import java.util.List; import java.util.concurrent.TimeUnit; @RunWith(SettingsRobolectricTestRunner.class) +@Config(shadows = ShadowConnectivityManager.class) public class AnomalyDetectionJobServiceTest { private static final int UID = 12345; private static final String SYSTEM_PACKAGE = "com.android.system"; @@ -92,6 +102,10 @@ public class AnomalyDetectionJobServiceTest { private PowerWhitelistBackend mPowerWhitelistBackend; @Mock private StatsDimensionsValue mStatsDimensionsValue; + @Mock + private JobParameters mJobParameters; + @Mock + private JobWorkItem mJobWorkItem; private BatteryTipPolicy mPolicy; private Bundle mBundle; @@ -110,11 +124,14 @@ public class AnomalyDetectionJobServiceTest { mFeatureFactory = FakeFeatureFactory.setupForTest(); when(mBatteryUtils.getAppLongVersionCode(any())).thenReturn(VERSION_CODE); - mAnomalyDetectionJobService = spy(new AnomalyDetectionJobService()); + final ServiceController controller = + Robolectric.buildService(AnomalyDetectionJobService.class); + mAnomalyDetectionJobService = spy(controller.get()); + doNothing().when(mAnomalyDetectionJobService).jobFinished(any(), anyBoolean()); } @Test - public void testScheduleCleanUp() { + public void scheduleCleanUp() { AnomalyDetectionJobService.scheduleAnomalyDetection(application, new Intent()); ShadowJobScheduler shadowJobScheduler = @@ -129,7 +146,7 @@ public class AnomalyDetectionJobServiceTest { } @Test - public void testSaveAnomalyToDatabase_systemWhitelisted_doNotSave() { + public void saveAnomalyToDatabase_systemWhitelisted_doNotSave() { doReturn(UID).when(mAnomalyDetectionJobService).extractUidFromStatsDimensionsValue(any()); doReturn(true).when(mPowerWhitelistBackend).isSysWhitelistedExceptIdle(any(String[].class)); @@ -144,7 +161,7 @@ public class AnomalyDetectionJobServiceTest { } @Test - public void testSaveAnomalyToDatabase_systemApp_doNotSaveButLog() { + public void saveAnomalyToDatabase_systemApp_doNotSaveButLog() { final ArrayList cookies = new ArrayList<>(); cookies.add(SUBSCRIBER_COOKIES_AUTO_RESTRICTION); mBundle.putStringArrayList(StatsManager.EXTRA_STATS_BROADCAST_SUBSCRIBER_COOKIES, cookies); @@ -170,7 +187,7 @@ public class AnomalyDetectionJobServiceTest { } @Test - public void testSaveAnomalyToDatabase_systemUid_doNotSave() { + public void saveAnomalyToDatabase_systemUid_doNotSave() { doReturn(Process.SYSTEM_UID).when( mAnomalyDetectionJobService).extractUidFromStatsDimensionsValue(any()); @@ -185,7 +202,7 @@ public class AnomalyDetectionJobServiceTest { } @Test - public void testSaveAnomalyToDatabase_uidNull_doNotSave() { + public void saveAnomalyToDatabase_uidNull_doNotSave() { doReturn(AnomalyDetectionJobService.UID_NULL).when( mAnomalyDetectionJobService).extractUidFromStatsDimensionsValue(any()); @@ -200,7 +217,7 @@ public class AnomalyDetectionJobServiceTest { } @Test - public void testSaveAnomalyToDatabase_normalAppWithAutoRestriction_save() { + public void saveAnomalyToDatabase_normalAppWithAutoRestriction_save() { final ArrayList cookies = new ArrayList<>(); cookies.add(SUBSCRIBER_COOKIES_AUTO_RESTRICTION); mBundle.putStringArrayList(StatsManager.EXTRA_STATS_BROADCAST_SUBSCRIBER_COOKIES, cookies); @@ -224,9 +241,8 @@ public class AnomalyDetectionJobServiceTest { Pair.create(MetricsProto.MetricsEvent.FIELD_APP_VERSION_CODE, VERSION_CODE)); } - @Test - public void testSaveAnomalyToDatabase_normalAppWithoutAutoRestriction_save() { + public void saveAnomalyToDatabase_normalAppWithoutAutoRestriction_save() { final ArrayList cookies = new ArrayList<>(); cookies.add(SUBSCRIBER_COOKIES_NOT_AUTO_RESTRICTION); mBundle.putStringArrayList(StatsManager.EXTRA_STATS_BROADCAST_SUBSCRIBER_COOKIES, cookies); @@ -251,7 +267,7 @@ public class AnomalyDetectionJobServiceTest { } @Test - public void testExtractUidFromStatsDimensionsValue_extractCorrectUid() { + public void extractUidFromStatsDimensionsValue_extractCorrectUid() { // Build an integer dimensions value. final StatsDimensionsValue intValue = mock(StatsDimensionsValue.class); when(intValue.isValueType(INT_VALUE_TYPE)).thenReturn(true); @@ -270,7 +286,7 @@ public class AnomalyDetectionJobServiceTest { } @Test - public void testExtractUidFromStatsDimensionsValue_wrongFormat_returnNull() { + public void extractUidFromStatsDimensionsValue_wrongFormat_returnNull() { // Build a float dimensions value final StatsDimensionsValue floatValue = mock(StatsDimensionsValue.class); when(floatValue.isValueType(FLOAT_VALUE_TYPE)).thenReturn(true); @@ -280,4 +296,24 @@ public class AnomalyDetectionJobServiceTest { assertThat(mAnomalyDetectionJobService.extractUidFromStatsDimensionsValue( floatValue)).isEqualTo(AnomalyDetectionJobService.UID_NULL); } + + @Test + public void stopJobWhileDequeuingWork_shouldNotCrash() { + when(mJobParameters.dequeueWork()).thenThrow(new SecurityException()); + + mAnomalyDetectionJobService.onStopJob(mJobParameters); + + // Should not crash even job is stopped + mAnomalyDetectionJobService.dequeueWork(mJobParameters); + } + + @Test + public void stopJobWhileCompletingWork_shouldNotCrash() { + doThrow(new SecurityException()).when(mJobParameters).completeWork(any()); + + mAnomalyDetectionJobService.onStopJob(mJobParameters); + + // Should not crash even job is stopped + mAnomalyDetectionJobService.completeWork(mJobParameters, mJobWorkItem); + } }