From a00e39365c7c445f40aa50cc7eb1233187c201dc Mon Sep 17 00:00:00 2001 From: Lei Yu Date: Wed, 9 May 2018 19:12:34 -0700 Subject: [PATCH] Fix a bug in AnomalyDetectionJobService Even though onStopJob is called, the service may still exist, so we need to reset the mIsJobCanceled in onStartJob Also remove the batteryStats since we don't need it anymore. Change-Id: I6d48efecb750ad4e464569cac426d87014a8db90 Fixes: 77968649 Test: RunSettingsRoboTests --- .../AnomalyDetectionJobService.java | 14 +++++----- .../AnomalyDetectionJobServiceTest.java | 28 +++++++++++-------- .../shadow/ShadowPowerWhitelistBackend.java | 28 +++++++++++++++++++ 3 files changed, 51 insertions(+), 19 deletions(-) create mode 100644 tests/robotests/src/com/android/settings/testutils/shadow/ShadowPowerWhitelistBackend.java diff --git a/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java b/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java index 2fac9b6cef3..b6bcd54a692 100644 --- a/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java +++ b/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java @@ -71,7 +71,8 @@ public class AnomalyDetectionJobService extends JobService { private final Object mLock = new Object(); @GuardedBy("mLock") - private boolean mIsJobCanceled = false; + @VisibleForTesting + boolean mIsJobCanceled = false; public static void scheduleAnomalyDetection(Context context, Intent intent) { final JobScheduler jobScheduler = context.getSystemService(JobScheduler.class); @@ -89,6 +90,9 @@ public class AnomalyDetectionJobService extends JobService { @Override public boolean onStartJob(JobParameters params) { + synchronized (mLock) { + mIsJobCanceled = false; + } ThreadUtils.postOnBackgroundThread(() -> { final Context context = AnomalyDetectionJobService.this; final BatteryDatabaseManager batteryDatabaseManager = @@ -96,18 +100,15 @@ public class AnomalyDetectionJobService extends JobService { final BatteryTipPolicy policy = new BatteryTipPolicy(this); final BatteryUtils batteryUtils = BatteryUtils.getInstance(this); final ContentResolver contentResolver = getContentResolver(); - final BatteryStatsHelper batteryStatsHelper = new BatteryStatsHelper(this, - true /* collectBatteryBroadcast */); final UserManager userManager = getSystemService(UserManager.class); final PowerWhitelistBackend powerWhitelistBackend = PowerWhitelistBackend.getInstance(); final PowerUsageFeatureProvider powerUsageFeatureProvider = FeatureFactory .getFactory(this).getPowerUsageFeatureProvider(this); final MetricsFeatureProvider metricsFeatureProvider = FeatureFactory .getFactory(this).getMetricsFeatureProvider(); - batteryUtils.initBatteryStatsHelper(batteryStatsHelper, null /* bundle */, userManager); for (JobWorkItem item = dequeueWork(params); item != null; item = dequeueWork(params)) { - saveAnomalyToDatabase(context, batteryStatsHelper, userManager, + saveAnomalyToDatabase(context, userManager, batteryDatabaseManager, batteryUtils, policy, powerWhitelistBackend, contentResolver, powerUsageFeatureProvider, metricsFeatureProvider, item.getIntent().getExtras()); @@ -128,8 +129,7 @@ public class AnomalyDetectionJobService extends JobService { } @VisibleForTesting - void saveAnomalyToDatabase(Context context, BatteryStatsHelper batteryStatsHelper, - UserManager userManager, + void saveAnomalyToDatabase(Context context, UserManager userManager, BatteryDatabaseManager databaseManager, BatteryUtils batteryUtils, BatteryTipPolicy policy, PowerWhitelistBackend powerWhitelistBackend, ContentResolver contentResolver, PowerUsageFeatureProvider powerUsageFeatureProvider, 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 f159e7970e6..0f6af1d4770 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobServiceTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobServiceTest.java @@ -46,8 +46,6 @@ import android.app.job.JobWorkItem; import android.content.Context; import android.content.Intent; import android.os.Bundle; -import android.os.Parcel; -import android.os.Parcelable; import android.os.Process; import android.os.StatsDimensionsValue; import android.os.UserManager; @@ -57,11 +55,11 @@ import com.android.internal.logging.nano.MetricsProto; import com.android.internal.os.BatteryStatsHelper; import com.android.settings.R; 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 com.android.settings.testutils.shadow.ShadowPowerWhitelistBackend; import org.junit.Before; import org.junit.Test; @@ -80,7 +78,7 @@ import java.util.List; import java.util.concurrent.TimeUnit; @RunWith(SettingsRobolectricTestRunner.class) -@Config(shadows = ShadowConnectivityManager.class) +@Config(shadows = {ShadowConnectivityManager.class, ShadowPowerWhitelistBackend.class}) public class AnomalyDetectionJobServiceTest { private static final int UID = 12345; private static final String SYSTEM_PACKAGE = "com.android.system"; @@ -91,8 +89,6 @@ public class AnomalyDetectionJobServiceTest { private static final int ANOMALY_TYPE = 6; private static final long VERSION_CODE = 15; @Mock - private BatteryStatsHelper mBatteryStatsHelper; - @Mock private UserManager mUserManager; @Mock private BatteryDatabaseManager mBatteryDatabaseManager; @@ -150,7 +146,7 @@ public class AnomalyDetectionJobServiceTest { doReturn(UID).when(mAnomalyDetectionJobService).extractUidFromStatsDimensionsValue(any()); doReturn(true).when(mPowerWhitelistBackend).isSysWhitelistedExceptIdle(any(String[].class)); - mAnomalyDetectionJobService.saveAnomalyToDatabase(mContext, mBatteryStatsHelper, + mAnomalyDetectionJobService.saveAnomalyToDatabase(mContext, mUserManager, mBatteryDatabaseManager, mBatteryUtils, mPolicy, mPowerWhitelistBackend, mContext.getContentResolver(), mFeatureFactory.powerUsageFeatureProvider, @@ -171,7 +167,7 @@ public class AnomalyDetectionJobServiceTest { mAnomalyDetectionJobService).extractUidFromStatsDimensionsValue(any()); doReturn(true).when(mBatteryUtils).shouldHideAnomaly(any(), anyInt()); - mAnomalyDetectionJobService.saveAnomalyToDatabase(mContext, mBatteryStatsHelper, + mAnomalyDetectionJobService.saveAnomalyToDatabase(mContext, mUserManager, mBatteryDatabaseManager, mBatteryUtils, mPolicy, mPowerWhitelistBackend, mContext.getContentResolver(), mFeatureFactory.powerUsageFeatureProvider, @@ -191,7 +187,7 @@ public class AnomalyDetectionJobServiceTest { doReturn(Process.SYSTEM_UID).when( mAnomalyDetectionJobService).extractUidFromStatsDimensionsValue(any()); - mAnomalyDetectionJobService.saveAnomalyToDatabase(mContext, mBatteryStatsHelper, + mAnomalyDetectionJobService.saveAnomalyToDatabase(mContext, mUserManager, mBatteryDatabaseManager, mBatteryUtils, mPolicy, mPowerWhitelistBackend, mContext.getContentResolver(), mFeatureFactory.powerUsageFeatureProvider, mFeatureFactory.metricsFeatureProvider, @@ -206,7 +202,7 @@ public class AnomalyDetectionJobServiceTest { doReturn(AnomalyDetectionJobService.UID_NULL).when( mAnomalyDetectionJobService).extractUidFromStatsDimensionsValue(any()); - mAnomalyDetectionJobService.saveAnomalyToDatabase(mContext, mBatteryStatsHelper, + mAnomalyDetectionJobService.saveAnomalyToDatabase(mContext, mUserManager, mBatteryDatabaseManager, mBatteryUtils, mPolicy, mPowerWhitelistBackend, mContext.getContentResolver(), mFeatureFactory.powerUsageFeatureProvider, mFeatureFactory.metricsFeatureProvider, @@ -226,7 +222,7 @@ public class AnomalyDetectionJobServiceTest { doReturn(Process.FIRST_APPLICATION_UID).when( mAnomalyDetectionJobService).extractUidFromStatsDimensionsValue(any()); - mAnomalyDetectionJobService.saveAnomalyToDatabase(mContext, mBatteryStatsHelper, + mAnomalyDetectionJobService.saveAnomalyToDatabase(mContext, mUserManager, mBatteryDatabaseManager, mBatteryUtils, mPolicy, mPowerWhitelistBackend, mContext.getContentResolver(), mFeatureFactory.powerUsageFeatureProvider, mFeatureFactory.metricsFeatureProvider, @@ -251,7 +247,7 @@ public class AnomalyDetectionJobServiceTest { doReturn(Process.FIRST_APPLICATION_UID).when( mAnomalyDetectionJobService).extractUidFromStatsDimensionsValue(any()); - mAnomalyDetectionJobService.saveAnomalyToDatabase(mContext, mBatteryStatsHelper, + mAnomalyDetectionJobService.saveAnomalyToDatabase(mContext, mUserManager, mBatteryDatabaseManager, mBatteryUtils, mPolicy, mPowerWhitelistBackend, mContext.getContentResolver(), mFeatureFactory.powerUsageFeatureProvider, mFeatureFactory.metricsFeatureProvider, @@ -316,4 +312,12 @@ public class AnomalyDetectionJobServiceTest { // Should not crash even job is stopped mAnomalyDetectionJobService.completeWork(mJobParameters, mJobWorkItem); } + + @Test + public void restartWorkAfterBeenStopped_jobStarted() { + mAnomalyDetectionJobService.onStopJob(mJobParameters); + mAnomalyDetectionJobService.onStartJob(mJobParameters); + + assertThat(mAnomalyDetectionJobService.mIsJobCanceled).isFalse(); + } } diff --git a/tests/robotests/src/com/android/settings/testutils/shadow/ShadowPowerWhitelistBackend.java b/tests/robotests/src/com/android/settings/testutils/shadow/ShadowPowerWhitelistBackend.java new file mode 100644 index 00000000000..4500b23b2a4 --- /dev/null +++ b/tests/robotests/src/com/android/settings/testutils/shadow/ShadowPowerWhitelistBackend.java @@ -0,0 +1,28 @@ +/* + * Copyright (C) 2018 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.settings.testutils.shadow; + +import com.android.settingslib.fuelgauge.PowerWhitelistBackend; + +import org.robolectric.annotation.Implements; + +@Implements(PowerWhitelistBackend.class) +public class ShadowPowerWhitelistBackend { + public void __constructor__() { + // Do nothing + } +}