From c29b984a98156b34be07cc9026a31d40a9a8656c Mon Sep 17 00:00:00 2001 From: Pavel Grafov Date: Fri, 20 Apr 2018 12:52:28 +0100 Subject: [PATCH 1/7] Pick default notification setting for work profile Managed profiles cannot completely hide notifications, so this setting should be treated as always "true" for them. Change-Id: I9808c1e9736d83efccb0e947d9097379bda59ebb Fixes: 78194020 Test: atest RedactionInterstitialTest --- .../notification/RedactionInterstitial.java | 22 ++- .../RedactionInterstitialTest.java | 175 ++++++++++++++++++ .../shadow/ShadowRestrictedLockUtils.java | 23 ++- .../testutils/shadow/ShadowUserManager.java | 17 +- 4 files changed, 226 insertions(+), 11 deletions(-) create mode 100644 tests/robotests/src/com/android/settings/notification/RedactionInterstitialTest.java diff --git a/src/com/android/settings/notification/RedactionInterstitial.java b/src/com/android/settings/notification/RedactionInterstitial.java index 12fc796550d..517c0410b31 100644 --- a/src/com/android/settings/notification/RedactionInterstitial.java +++ b/src/com/android/settings/notification/RedactionInterstitial.java @@ -18,6 +18,8 @@ package com.android.settings.notification; import static android.app.admin.DevicePolicyManager.KEYGUARD_DISABLE_SECURE_NOTIFICATIONS; import static android.app.admin.DevicePolicyManager.KEYGUARD_DISABLE_UNREDACTED_NOTIFICATIONS; +import static android.provider.Settings.Secure.LOCK_SCREEN_ALLOW_PRIVATE_NOTIFICATIONS; +import static android.provider.Settings.Secure.LOCK_SCREEN_SHOW_NOTIFICATIONS; import static com.android.settingslib.RestrictedLockUtils.EnforcedAdmin; @@ -164,15 +166,17 @@ public class RedactionInterstitial extends SettingsActivity { } private void loadFromSettings() { - final boolean managed = UserManager.get(getContext()).isManagedProfile(mUserId); - final boolean enabled = !managed || Settings.Secure.getIntForUser(getContentResolver(), - Settings.Secure.LOCK_SCREEN_SHOW_NOTIFICATIONS, 0, mUserId) != 0; - final boolean show = Settings.Secure.getIntForUser(getContentResolver(), - Settings.Secure.LOCK_SCREEN_ALLOW_PRIVATE_NOTIFICATIONS, 1, mUserId) != 0; + final boolean managedProfile = UserManager.get(getContext()).isManagedProfile(mUserId); + // Hiding all notifications is device-wide setting, managed profiles can only set + // whether their notifications are show in full or redacted. + final boolean showNotifications = managedProfile || Settings.Secure.getIntForUser( + getContentResolver(), LOCK_SCREEN_SHOW_NOTIFICATIONS, 0, mUserId) != 0; + final boolean showUnredacted = Settings.Secure.getIntForUser( + getContentResolver(), LOCK_SCREEN_ALLOW_PRIVATE_NOTIFICATIONS, 1, mUserId) != 0; int checkedButtonId = R.id.hide_all; - if (enabled) { - if (show && !mShowAllButton.isDisabledByAdmin()) { + if (showNotifications) { + if (showUnredacted && !mShowAllButton.isDisabledByAdmin()) { checkedButtonId = R.id.show_all; } else if (!mRedactSensitiveButton.isDisabledByAdmin()) { checkedButtonId = R.id.redact_sensitive; @@ -188,9 +192,9 @@ public class RedactionInterstitial extends SettingsActivity { final boolean enabled = (checkedId != R.id.hide_all); Settings.Secure.putIntForUser(getContentResolver(), - Settings.Secure.LOCK_SCREEN_ALLOW_PRIVATE_NOTIFICATIONS, show ? 1 : 0, mUserId); + LOCK_SCREEN_ALLOW_PRIVATE_NOTIFICATIONS, show ? 1 : 0, mUserId); Settings.Secure.putIntForUser(getContentResolver(), - Settings.Secure.LOCK_SCREEN_SHOW_NOTIFICATIONS, enabled ? 1 : 0, mUserId); + LOCK_SCREEN_SHOW_NOTIFICATIONS, enabled ? 1 : 0, mUserId); } } diff --git a/tests/robotests/src/com/android/settings/notification/RedactionInterstitialTest.java b/tests/robotests/src/com/android/settings/notification/RedactionInterstitialTest.java new file mode 100644 index 00000000000..f3633d3a9e2 --- /dev/null +++ b/tests/robotests/src/com/android/settings/notification/RedactionInterstitialTest.java @@ -0,0 +1,175 @@ +package com.android.settings.notification; + +import static android.app.admin.DevicePolicyManager.KEYGUARD_DISABLE_SECURE_NOTIFICATIONS; +import static android.app.admin.DevicePolicyManager.KEYGUARD_DISABLE_UNREDACTED_NOTIFICATIONS; +import static android.provider.Settings.Secure.LOCK_SCREEN_ALLOW_PRIVATE_NOTIFICATIONS; +import static android.provider.Settings.Secure.LOCK_SCREEN_SHOW_NOTIFICATIONS; + +import static com.google.common.truth.Truth.assertThat; + +import static org.robolectric.Robolectric.buildActivity; + +import android.content.ContentResolver; +import android.content.Intent; +import android.os.UserHandle; +import android.provider.Settings; +import android.view.View; +import android.widget.RadioButton; + +import com.android.settings.R; +import com.android.settings.RestrictedRadioButton; +import com.android.settings.notification.RedactionInterstitial.RedactionInterstitialFragment; +import com.android.settings.testutils.SettingsRobolectricTestRunner; +import com.android.settings.testutils.shadow.SettingsShadowResources; +import com.android.settings.testutils.shadow.SettingsShadowResourcesImpl; +import com.android.settings.testutils.shadow.ShadowRestrictedLockUtils; +import com.android.settings.testutils.shadow.ShadowUserManager; +import com.android.settings.testutils.shadow.ShadowUtils; + +import org.junit.After; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; + +@RunWith(SettingsRobolectricTestRunner.class) +@Config(shadows = { + SettingsShadowResources.class, + SettingsShadowResourcesImpl.class, + SettingsShadowResources.SettingsShadowTheme.class, + ShadowUtils.class, + ShadowRestrictedLockUtils.class, + ShadowUserManager.class, +}) +public class RedactionInterstitialTest { + private RedactionInterstitial mActivity; + private RedactionInterstitialFragment mFragment; + + @After + public void tearDown() { + ShadowUserManager.getShadow().reset(); + ShadowRestrictedLockUtils.reset(); + } + + @Test + public void primaryUserDefaultStateTest() { + setupSettings(1 /* show */, 1 /* showUnredacted */); + setupActivity(); + + assertHideAllVisible(true); + assertEnabledButtons(true /* all */, true /* redact */); + assertSelectedButton(R.id.show_all); + } + + @Test + public void primaryUserRedactSensitiveTest() { + setupSettings(1 /* show */, 0 /* showUnredacted */); + setupActivity(); + + assertHideAllVisible(true); + assertEnabledButtons(true /* all */, true /* redact */); + assertSelectedButton(R.id.redact_sensitive); + } + + @Test + public void primaryUserHideAllTest() { + setupSettings(0 /* show */, 0 /* showUnredacted */); + setupActivity(); + + assertHideAllVisible(true); + assertEnabledButtons(true /* all */, true /* redact */); + assertSelectedButton(R.id.hide_all); + } + + @Test + public void primaryUserUnredactedRestrictionTest() { + setupSettings(1 /* show */, 1 /* showUnredacted */); + ShadowRestrictedLockUtils.setKeyguardDisabledFeatures( + KEYGUARD_DISABLE_UNREDACTED_NOTIFICATIONS); + setupActivity(); + + assertHideAllVisible(true); + assertEnabledButtons(false /* all */, true /* redact */); + assertSelectedButton(R.id.redact_sensitive); + } + + @Test + public void primaryUserNotificationRestrictionTest() { + setupSettings(1 /* show */, 1 /* showUnredacted */); + ShadowRestrictedLockUtils.setKeyguardDisabledFeatures( + KEYGUARD_DISABLE_SECURE_NOTIFICATIONS); + setupActivity(); + + assertHideAllVisible(true); + assertEnabledButtons(false /* all */, false /* redact */); + assertSelectedButton(R.id.hide_all); + } + + @Test + public void managedProfileNoRestrictionsTest() { + setupSettings(1 /* show */, 1 /* showUnredacted */); + ShadowUserManager.getShadow().addManagedProfile(UserHandle.myUserId()); + setupActivity(); + + assertHideAllVisible(false); + assertEnabledButtons(true /* all */, true /* redact */); + assertSelectedButton(R.id.show_all); + } + + @Test + public void managedProfileUnredactedRestrictionTest() { + setupSettings(1 /* show */, 1 /* showUnredacted */); + ShadowUserManager.getShadow().addManagedProfile(UserHandle.myUserId()); + ShadowRestrictedLockUtils.setKeyguardDisabledFeatures( + KEYGUARD_DISABLE_UNREDACTED_NOTIFICATIONS); + setupActivity(); + + assertHideAllVisible(false); + assertEnabledButtons(false /* all */, true /* redact */); + assertSelectedButton(R.id.redact_sensitive); + } + + private void setupActivity() { + mActivity = buildActivity(RedactionInterstitial.class, new Intent()).setup().get(); + mFragment = (RedactionInterstitialFragment) + mActivity.getFragmentManager().findFragmentById(R.id.main_content); + assertThat(mActivity).isNotNull(); + assertThat(mFragment).isNotNull(); + } + + private void setupSettings(int show, int showUnredacted) { + final ContentResolver resolver = RuntimeEnvironment.application.getContentResolver(); + Settings.Secure.putIntForUser(resolver, + LOCK_SCREEN_SHOW_NOTIFICATIONS, show, UserHandle.myUserId()); + Settings.Secure.putIntForUser(resolver, + LOCK_SCREEN_ALLOW_PRIVATE_NOTIFICATIONS, showUnredacted, UserHandle.myUserId()); + } + + private void assertHideAllVisible(boolean visible) { + Assert.assertEquals(visible, getButton(R.id.hide_all).getVisibility() != View.GONE); + } + + private void assertEnabledButtons(boolean all, boolean redact) { + Assert.assertEquals(all, buttonEnabled(R.id.show_all)); + Assert.assertEquals(redact, buttonEnabled(R.id.redact_sensitive)); + } + + private void assertSelectedButton(int resId) { + Assert.assertEquals(resId == R.id.show_all, buttonChecked(R.id.show_all)); + Assert.assertEquals(resId == R.id.redact_sensitive, buttonChecked(R.id.redact_sensitive)); + Assert.assertEquals(resId == R.id.hide_all, buttonChecked(R.id.hide_all)); + } + + private boolean buttonChecked(int resource) { + return getButton(resource).isChecked(); + } + + private boolean buttonEnabled(int resource) { + return !((RestrictedRadioButton) getButton(resource)).isDisabledByAdmin(); + } + + private RadioButton getButton(int resource) { + return (RadioButton) mFragment.getView().findViewById(resource); + } +} diff --git a/tests/robotests/src/com/android/settings/testutils/shadow/ShadowRestrictedLockUtils.java b/tests/robotests/src/com/android/settings/testutils/shadow/ShadowRestrictedLockUtils.java index afede1a200e..c53f7712bf5 100644 --- a/tests/robotests/src/com/android/settings/testutils/shadow/ShadowRestrictedLockUtils.java +++ b/tests/robotests/src/com/android/settings/testutils/shadow/ShadowRestrictedLockUtils.java @@ -15,6 +15,7 @@ */ package com.android.settings.testutils.shadow; +import android.annotation.UserIdInt; import android.content.Context; import com.android.internal.util.ArrayUtils; @@ -23,15 +24,25 @@ import com.android.settingslib.RestrictedLockUtils.EnforcedAdmin; import org.robolectric.annotation.Implementation; import org.robolectric.annotation.Implements; +import org.robolectric.annotation.Resetter; @Implements(RestrictedLockUtils.class) public class ShadowRestrictedLockUtils { private static boolean isRestricted; private static String[] restrictedPkgs; private static boolean adminSupportDetailsIntentLaunched; + private static int keyguardDisabledFeatures; + + @Resetter + public static void reset() { + isRestricted = false; + restrictedPkgs = null; + adminSupportDetailsIntentLaunched = false; + keyguardDisabledFeatures = 0; + } @Implementation - public static RestrictedLockUtils.EnforcedAdmin checkIfMeteredDataRestricted(Context context, + public static EnforcedAdmin checkIfMeteredDataRestricted(Context context, String packageName, int userId) { if (isRestricted) { return new EnforcedAdmin(); @@ -47,6 +58,12 @@ public class ShadowRestrictedLockUtils { adminSupportDetailsIntentLaunched = true; } + @Implementation + public static EnforcedAdmin checkIfKeyguardFeaturesDisabled(Context context, + int features, final @UserIdInt int userId) { + return (keyguardDisabledFeatures & features) == 0 ? null : new EnforcedAdmin(); + } + public static boolean hasAdminSupportDetailsIntentLaunched() { return adminSupportDetailsIntentLaunched; } @@ -62,4 +79,8 @@ public class ShadowRestrictedLockUtils { public static void setRestrictedPkgs(String... pkgs) { restrictedPkgs = pkgs; } + + public static void setKeyguardDisabledFeatures(int features) { + keyguardDisabledFeatures = features; + } } diff --git a/tests/robotests/src/com/android/settings/testutils/shadow/ShadowUserManager.java b/tests/robotests/src/com/android/settings/testutils/shadow/ShadowUserManager.java index 9979ddbb2f6..f7fd12f58fe 100644 --- a/tests/robotests/src/com/android/settings/testutils/shadow/ShadowUserManager.java +++ b/tests/robotests/src/com/android/settings/testutils/shadow/ShadowUserManager.java @@ -31,8 +31,10 @@ import org.robolectric.shadow.api.Shadow; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; @Implements(value = UserManager.class, inheritImplementationMethods = true) public class ShadowUserManager extends org.robolectric.shadows.ShadowUserManager { @@ -40,12 +42,16 @@ public class ShadowUserManager extends org.robolectric.shadows.ShadowUserManager private SparseArray mUserInfos = new SparseArray<>(); private final List mRestrictions = new ArrayList<>(); private final Map> mRestrictionSources = new HashMap<>(); - private List mUserProfileInfos = new ArrayList<>(); + private final List mUserProfileInfos = new ArrayList<>(); + private final Set mManagedProfiles = new HashSet<>(); @Resetter public void reset() { + mUserInfos.clear(); mRestrictions.clear(); mUserProfileInfos.clear(); + mRestrictionSources.clear(); + mManagedProfiles.clear(); } public void setUserInfo(int userHandle, UserInfo userInfo) { @@ -95,4 +101,13 @@ public class ShadowUserManager extends org.robolectric.shadows.ShadowUserManager String restrictionKey, UserHandle userHandle, List enforcers) { mRestrictionSources.put(restrictionKey + userHandle.getIdentifier(), enforcers); } + + @Implementation + public boolean isManagedProfile(@UserIdInt int userId) { + return mManagedProfiles.contains(userId); + } + + public void addManagedProfile(int userId) { + mManagedProfiles.add(userId); + } } From 252d4e1fc0ba0b5a9a659cc792a86f928dd06fba Mon Sep 17 00:00:00 2001 From: Lei Yu Date: Tue, 17 Apr 2018 17:41:07 -0700 Subject: [PATCH 2/7] Fix bugs in auto restriction. 1. Dismiss this tip once user clicks it and goes to detail page. 2. In auto restriction, since apps are automatically restricted, we need to remove apps in list that unrestricted by user. 3. Refactor the code to remove the unnecessary parameter(i.e. SettingsActivity) Bug: 78187414 Test: RunSettingsRoboTests Change-Id: I1c950f7c55df35795641c2ea8579ce9e011dba28 --- .../RestrictAppPreferenceController.java | 9 +- .../fuelgauge/RestrictedAppDetails.java | 6 +- .../fuelgauge/SmartBatterySettings.java | 2 +- .../AnomalyDetectionJobService.java | 1 + .../fuelgauge/batterytip/BatteryTipUtils.java | 3 +- .../OpenRestrictAppFragmentAction.java | 25 ++--- .../detectors/RestrictAppDetector.java | 17 ++-- ...oPredicate.java => AppLabelPredicate.java} | 12 +-- .../tips/AppRestrictionPredicate.java | 43 ++++++++ .../RestrictAppPreferenceControllerTest.java | 8 +- .../fuelgauge/RestrictedAppDetailsTest.java | 7 +- .../OpenRestrictAppFragmentActionTest.java | 99 +++++++++++++++++++ .../detectors/RestrictAppDetectorTest.java | 34 ++++++- .../fuelgauge/batterytip/RestrictAppTest.java | 19 +++- 14 files changed, 234 insertions(+), 51 deletions(-) rename src/com/android/settings/fuelgauge/batterytip/tips/{AppInfoPredicate.java => AppLabelPredicate.java} (76%) create mode 100644 src/com/android/settings/fuelgauge/batterytip/tips/AppRestrictionPredicate.java create mode 100644 tests/robotests/src/com/android/settings/fuelgauge/batterytip/actions/OpenRestrictAppFragmentActionTest.java diff --git a/src/com/android/settings/fuelgauge/RestrictAppPreferenceController.java b/src/com/android/settings/fuelgauge/RestrictAppPreferenceController.java index 7fdf8294cb9..6acd2bafc25 100644 --- a/src/com/android/settings/fuelgauge/RestrictAppPreferenceController.java +++ b/src/com/android/settings/fuelgauge/RestrictAppPreferenceController.java @@ -43,7 +43,6 @@ public class RestrictAppPreferenceController extends BasePreferenceController { @VisibleForTesting List mAppInfos; private AppOpsManager mAppOpsManager; - private SettingsActivity mSettingsActivity; private InstrumentedPreferenceFragment mPreferenceFragment; private UserManager mUserManager; @@ -53,10 +52,8 @@ public class RestrictAppPreferenceController extends BasePreferenceController { mUserManager = context.getSystemService(UserManager.class); } - public RestrictAppPreferenceController(SettingsActivity settingsActivity, - InstrumentedPreferenceFragment preferenceFragment) { - this(settingsActivity.getApplicationContext()); - mSettingsActivity = settingsActivity; + public RestrictAppPreferenceController(InstrumentedPreferenceFragment preferenceFragment) { + this(preferenceFragment.getContext()); mPreferenceFragment = preferenceFragment; } @@ -83,7 +80,7 @@ public class RestrictAppPreferenceController extends BasePreferenceController { public boolean handlePreferenceTreeClick(Preference preference) { if (getPreferenceKey().equals(preference.getKey())) { // start fragment - RestrictedAppDetails.startRestrictedAppDetails(mSettingsActivity, mPreferenceFragment, + RestrictedAppDetails.startRestrictedAppDetails(mPreferenceFragment, mAppInfos); return true; } diff --git a/src/com/android/settings/fuelgauge/RestrictedAppDetails.java b/src/com/android/settings/fuelgauge/RestrictedAppDetails.java index e75112c68a5..9d30fd180dd 100644 --- a/src/com/android/settings/fuelgauge/RestrictedAppDetails.java +++ b/src/com/android/settings/fuelgauge/RestrictedAppDetails.java @@ -67,12 +67,12 @@ public class RestrictedAppDetails extends DashboardFragment { private final FooterPreferenceMixin mFooterPreferenceMixin = new FooterPreferenceMixin(this, getLifecycle()); - public static void startRestrictedAppDetails(SettingsActivity caller, - InstrumentedPreferenceFragment fragment, List appInfos) { + public static void startRestrictedAppDetails(InstrumentedPreferenceFragment fragment, + List appInfos) { final Bundle args = new Bundle(); args.putParcelableList(EXTRA_APP_INFO_LIST, appInfos); - new SubSettingLauncher(caller) + new SubSettingLauncher(fragment.getContext()) .setDestination(RestrictedAppDetails.class.getName()) .setArguments(args) .setTitle(R.string.restricted_app_title) diff --git a/src/com/android/settings/fuelgauge/SmartBatterySettings.java b/src/com/android/settings/fuelgauge/SmartBatterySettings.java index bcbac3f1c81..5e639769281 100644 --- a/src/com/android/settings/fuelgauge/SmartBatterySettings.java +++ b/src/com/android/settings/fuelgauge/SmartBatterySettings.java @@ -71,7 +71,7 @@ public class SmartBatterySettings extends DashboardFragment { controllers.add(new SmartBatteryPreferenceController(context)); if (settingsActivity != null && fragment != null) { controllers.add( - new RestrictAppPreferenceController(settingsActivity, fragment)); + new RestrictAppPreferenceController(fragment)); } else { controllers.add(new RestrictAppPreferenceController(context)); } diff --git a/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java b/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java index 518b633a480..7f6eb9b544f 100644 --- a/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java +++ b/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java @@ -100,6 +100,7 @@ public class AnomalyDetectionJobService extends JobService { .getFactory(this).getPowerUsageFeatureProvider(this); final MetricsFeatureProvider metricsFeatureProvider = FeatureFactory .getFactory(this).getMetricsFeatureProvider(); + batteryUtils.initBatteryStatsHelper(batteryStatsHelper, null /* bundle */, userManager); for (JobWorkItem item = params.dequeueWork(); item != null; item = params.dequeueWork()) { diff --git a/src/com/android/settings/fuelgauge/batterytip/BatteryTipUtils.java b/src/com/android/settings/fuelgauge/batterytip/BatteryTipUtils.java index 1f79e7cd756..be255189fb5 100644 --- a/src/com/android/settings/fuelgauge/batterytip/BatteryTipUtils.java +++ b/src/com/android/settings/fuelgauge/batterytip/BatteryTipUtils.java @@ -101,8 +101,7 @@ public class BatteryTipUtils { } case BatteryTip.TipType.APP_RESTRICTION: if (batteryTip.getState() == BatteryTip.StateType.HANDLED) { - return new OpenRestrictAppFragmentAction(settingsActivity, fragment, - (RestrictAppTip) batteryTip); + return new OpenRestrictAppFragmentAction(fragment, (RestrictAppTip) batteryTip); } else { return new RestrictAppAction(settingsActivity, (RestrictAppTip) batteryTip); } diff --git a/src/com/android/settings/fuelgauge/batterytip/actions/OpenRestrictAppFragmentAction.java b/src/com/android/settings/fuelgauge/batterytip/actions/OpenRestrictAppFragmentAction.java index afb80f22886..f9fb9498e9e 100644 --- a/src/com/android/settings/fuelgauge/batterytip/actions/OpenRestrictAppFragmentAction.java +++ b/src/com/android/settings/fuelgauge/batterytip/actions/OpenRestrictAppFragmentAction.java @@ -16,15 +16,16 @@ package com.android.settings.fuelgauge.batterytip.actions; -import android.app.Fragment; +import android.support.annotation.VisibleForTesting; import com.android.internal.logging.nano.MetricsProto; -import com.android.settings.SettingsActivity; import com.android.settings.core.InstrumentedPreferenceFragment; -import com.android.settings.fuelgauge.BatteryUtils; import com.android.settings.fuelgauge.RestrictedAppDetails; +import com.android.settings.fuelgauge.batterytip.AnomalyDatabaseHelper; import com.android.settings.fuelgauge.batterytip.AppInfo; +import com.android.settings.fuelgauge.batterytip.BatteryDatabaseManager; import com.android.settings.fuelgauge.batterytip.tips.RestrictAppTip; +import com.android.settingslib.utils.ThreadUtils; import java.util.List; @@ -33,17 +34,16 @@ import java.util.List; */ public class OpenRestrictAppFragmentAction extends BatteryTipAction { private final RestrictAppTip mRestrictAppTip; - private final BatteryUtils mBatteryUtils; - private final SettingsActivity mSettingsActivity; private final InstrumentedPreferenceFragment mFragment; + @VisibleForTesting + BatteryDatabaseManager mBatteryDatabaseManager; - public OpenRestrictAppFragmentAction(SettingsActivity settingsActivity, - InstrumentedPreferenceFragment fragment, RestrictAppTip tip) { + public OpenRestrictAppFragmentAction(InstrumentedPreferenceFragment fragment, + RestrictAppTip tip) { super(fragment.getContext()); - mSettingsActivity = settingsActivity; mFragment = fragment; mRestrictAppTip = tip; - mBatteryUtils = BatteryUtils.getInstance(mContext); + mBatteryDatabaseManager = BatteryDatabaseManager.getInstance(mContext); } /** @@ -54,7 +54,10 @@ public class OpenRestrictAppFragmentAction extends BatteryTipAction { mMetricsFeatureProvider.action(mContext, MetricsProto.MetricsEvent.ACTION_TIP_OPEN_APP_RESTRICTION_PAGE, metricsKey); final List mAppInfos = mRestrictAppTip.getRestrictAppList(); - RestrictedAppDetails.startRestrictedAppDetails(mSettingsActivity, mFragment, - mAppInfos); + RestrictedAppDetails.startRestrictedAppDetails(mFragment, mAppInfos); + + // Mark all the anomalies as handled, so it won't show up again. + ThreadUtils.postOnBackgroundThread(() -> mBatteryDatabaseManager.updateAnomalies(mAppInfos, + AnomalyDatabaseHelper.State.HANDLED)); } } diff --git a/src/com/android/settings/fuelgauge/batterytip/detectors/RestrictAppDetector.java b/src/com/android/settings/fuelgauge/batterytip/detectors/RestrictAppDetector.java index 5a640e56c4f..aacacc522cf 100644 --- a/src/com/android/settings/fuelgauge/batterytip/detectors/RestrictAppDetector.java +++ b/src/com/android/settings/fuelgauge/batterytip/detectors/RestrictAppDetector.java @@ -20,18 +20,17 @@ import android.content.Context; import android.support.annotation.VisibleForTesting; import android.text.format.DateUtils; -import com.android.settings.Utils; import com.android.settings.fuelgauge.batterytip.AnomalyDatabaseHelper; import com.android.settings.fuelgauge.batterytip.AppInfo; import com.android.settings.fuelgauge.batterytip.BatteryDatabaseManager; import com.android.settings.fuelgauge.batterytip.BatteryTipPolicy; -import com.android.settings.fuelgauge.batterytip.tips.AppInfoPredicate; +import com.android.settings.fuelgauge.batterytip.tips.AppLabelPredicate; +import com.android.settings.fuelgauge.batterytip.tips.AppRestrictionPredicate; import com.android.settings.fuelgauge.batterytip.tips.BatteryTip; import com.android.settings.fuelgauge.batterytip.tips.RestrictAppTip; import java.util.ArrayList; import java.util.List; -import java.util.function.Predicate; /** * Detector whether to show summary tip. This detector should be executed as the last @@ -45,13 +44,15 @@ public class RestrictAppDetector implements BatteryTipDetector { BatteryDatabaseManager mBatteryDatabaseManager; private Context mContext; - private AppInfoPredicate mAppInfoPredicate; + private AppRestrictionPredicate mAppRestrictionPredicate; + private AppLabelPredicate mAppLabelPredicate; public RestrictAppDetector(Context context, BatteryTipPolicy policy) { mContext = context; mPolicy = policy; mBatteryDatabaseManager = BatteryDatabaseManager.getInstance(context); - mAppInfoPredicate = new AppInfoPredicate(context); + mAppRestrictionPredicate = new AppRestrictionPredicate(context); + mAppLabelPredicate = new AppLabelPredicate(context); } @Override @@ -64,7 +65,8 @@ public class RestrictAppDetector implements BatteryTipDetector { final long oneDayBeforeMs = System.currentTimeMillis() - DateUtils.DAY_IN_MILLIS; final List highUsageApps = mBatteryDatabaseManager.queryAllAnomalies( oneDayBeforeMs, AnomalyDatabaseHelper.State.NEW); - highUsageApps.removeIf(mAppInfoPredicate); + // Remove it if it doesn't have label or been restricted + highUsageApps.removeIf(mAppLabelPredicate.or(mAppRestrictionPredicate)); if (!highUsageApps.isEmpty()) { // If there are new anomalies, show them return new RestrictAppTip(BatteryTip.StateType.NEW, highUsageApps); @@ -72,7 +74,8 @@ public class RestrictAppDetector implements BatteryTipDetector { // Otherwise, show auto-handled one if it exists final List autoHandledApps = mBatteryDatabaseManager.queryAllAnomalies( oneDayBeforeMs, AnomalyDatabaseHelper.State.AUTO_HANDLED); - autoHandledApps.removeIf(mAppInfoPredicate); + // Remove it if it doesn't have label or unrestricted + autoHandledApps.removeIf(mAppLabelPredicate.or(mAppRestrictionPredicate.negate())); return new RestrictAppTip(autoHandledApps.isEmpty() ? BatteryTip.StateType.INVISIBLE : BatteryTip.StateType.HANDLED, autoHandledApps); } diff --git a/src/com/android/settings/fuelgauge/batterytip/tips/AppInfoPredicate.java b/src/com/android/settings/fuelgauge/batterytip/tips/AppLabelPredicate.java similarity index 76% rename from src/com/android/settings/fuelgauge/batterytip/tips/AppInfoPredicate.java rename to src/com/android/settings/fuelgauge/batterytip/tips/AppLabelPredicate.java index df78caaf4a8..13a2452bb23 100644 --- a/src/com/android/settings/fuelgauge/batterytip/tips/AppInfoPredicate.java +++ b/src/com/android/settings/fuelgauge/batterytip/tips/AppLabelPredicate.java @@ -25,22 +25,20 @@ import com.android.settings.fuelgauge.batterytip.AppInfo; import java.util.function.Predicate; /** - * {@link Predicate} for {@link AppInfo} to check whether it has label or restricted. + * {@link Predicate} for {@link AppInfo} to check whether it has label */ -public class AppInfoPredicate implements Predicate { +public class AppLabelPredicate implements Predicate { private Context mContext; private AppOpsManager mAppOpsManager; - public AppInfoPredicate(Context context) { + public AppLabelPredicate(Context context) { mContext = context; mAppOpsManager = context.getSystemService(AppOpsManager.class); } @Override public boolean test(AppInfo appInfo) { - // Return true if app doesn't have label or already been restricted - return Utils.getApplicationLabel(mContext, appInfo.packageName) == null - || mAppOpsManager.checkOpNoThrow(AppOpsManager.OP_RUN_ANY_IN_BACKGROUND, - appInfo.uid, appInfo.packageName) == AppOpsManager.MODE_IGNORED; + // Return true if app doesn't have label + return Utils.getApplicationLabel(mContext, appInfo.packageName) == null; } } diff --git a/src/com/android/settings/fuelgauge/batterytip/tips/AppRestrictionPredicate.java b/src/com/android/settings/fuelgauge/batterytip/tips/AppRestrictionPredicate.java new file mode 100644 index 00000000000..21bdbf10816 --- /dev/null +++ b/src/com/android/settings/fuelgauge/batterytip/tips/AppRestrictionPredicate.java @@ -0,0 +1,43 @@ +/* + * 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.fuelgauge.batterytip.tips; + +import android.app.AppOpsManager; +import android.content.Context; + +import com.android.settings.Utils; +import com.android.settings.fuelgauge.batterytip.AppInfo; + +import java.util.function.Predicate; + +/** + * {@link Predicate} for {@link AppInfo} to check whether it is restricted. + */ +public class AppRestrictionPredicate implements Predicate { + private AppOpsManager mAppOpsManager; + + public AppRestrictionPredicate(Context context) { + mAppOpsManager = context.getSystemService(AppOpsManager.class); + } + + @Override + public boolean test(AppInfo appInfo) { + // Return true if app already been restricted + return mAppOpsManager.checkOpNoThrow(AppOpsManager.OP_RUN_ANY_IN_BACKGROUND, + appInfo.uid, appInfo.packageName) == AppOpsManager.MODE_IGNORED; + } +} diff --git a/tests/robotests/src/com/android/settings/fuelgauge/RestrictAppPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/fuelgauge/RestrictAppPreferenceControllerTest.java index 7d26755deec..bf1e8871ba9 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/RestrictAppPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/RestrictAppPreferenceControllerTest.java @@ -67,8 +67,6 @@ public class RestrictAppPreferenceControllerTest { @Mock private AppOpsManager.PackageOps mOtherUserPackageOps; @Mock - private SettingsActivity mSettingsActivity; - @Mock private InstrumentedPreferenceFragment mFragment; @Mock private UserManager mUserManager; @@ -102,9 +100,9 @@ public class RestrictAppPreferenceControllerTest { mContext = spy(RuntimeEnvironment.application); doReturn(mAppOpsManager).when(mContext).getSystemService(Context.APP_OPS_SERVICE); doReturn(mUserManager).when(mContext).getSystemService(UserManager.class); - doReturn(mContext).when(mSettingsActivity).getApplicationContext(); + doReturn(mContext).when(mFragment).getContext(); mRestrictAppPreferenceController = - new RestrictAppPreferenceController(mSettingsActivity, mFragment); + new RestrictAppPreferenceController(mFragment); mPackageOpsList = new ArrayList<>(); mPreference = new Preference(mContext); mPreference.setKey(mRestrictAppPreferenceController.getPreferenceKey()); @@ -171,7 +169,7 @@ public class RestrictAppPreferenceControllerTest { mRestrictAppPreferenceController.handlePreferenceTreeClick(mPreference); - verify(mSettingsActivity).startActivity(intent.capture()); + verify(mContext).startActivity(intent.capture()); assertThat(intent.getValue().getStringExtra(EXTRA_SHOW_FRAGMENT)) .isEqualTo(RestrictedAppDetails.class.getName()); assertThat(intent.getValue().getIntExtra(EXTRA_SHOW_FRAGMENT_TITLE_RESID, -1)) diff --git a/tests/robotests/src/com/android/settings/fuelgauge/RestrictedAppDetailsTest.java b/tests/robotests/src/com/android/settings/fuelgauge/RestrictedAppDetailsTest.java index 94a690326df..8875add59bc 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/RestrictedAppDetailsTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/RestrictedAppDetailsTest.java @@ -67,8 +67,6 @@ public class RestrictedAppDetailsTest { @Mock(answer = Answers.RETURNS_DEEP_STUBS) private PreferenceManager mPreferenceManager; @Mock - private SettingsActivity mSettingsActivity; - @Mock private InstrumentedPreferenceFragment mFragment; private RestrictedAppDetails mRestrictedAppDetails; private Context mContext; @@ -88,6 +86,7 @@ public class RestrictedAppDetailsTest { doReturn(mPreferenceManager).when(mRestrictedAppDetails).getPreferenceManager(); doReturn(mContext).when(mPreferenceManager).getContext(); + doReturn(mContext).when(mFragment).getContext(); mRestrictedAppDetails.mPackageManager = mPackageManager; mRestrictedAppDetails.mIconDrawableFactory = mIconDrawableFactory; mRestrictedAppDetails.mAppInfos = new ArrayList<>(); @@ -119,9 +118,9 @@ public class RestrictedAppDetailsTest { // Get the intent in which it has the app info bundle mIntent = captor.getValue(); return true; - }).when(mSettingsActivity).startActivity(captor.capture()); + }).when(mContext).startActivity(captor.capture()); - RestrictedAppDetails.startRestrictedAppDetails(mSettingsActivity, mFragment, + RestrictedAppDetails.startRestrictedAppDetails(mFragment, mRestrictedAppDetails.mAppInfos); final Bundle bundle = mIntent.getBundleExtra( diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/actions/OpenRestrictAppFragmentActionTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/actions/OpenRestrictAppFragmentActionTest.java new file mode 100644 index 00000000000..39555b2ba9d --- /dev/null +++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/actions/OpenRestrictAppFragmentActionTest.java @@ -0,0 +1,99 @@ +/* + * 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.fuelgauge.batterytip.actions; + +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.content.Context; + +import com.android.internal.logging.nano.MetricsProto; +import com.android.settings.core.InstrumentedPreferenceFragment; +import com.android.settings.fuelgauge.batterytip.AnomalyDatabaseHelper; +import com.android.settings.fuelgauge.batterytip.AppInfo; +import com.android.settings.fuelgauge.batterytip.BatteryDatabaseManager; +import com.android.settings.fuelgauge.batterytip.tips.BatteryTip; +import com.android.settings.fuelgauge.batterytip.tips.RestrictAppTip; +import com.android.settings.testutils.DatabaseTestUtils; +import com.android.settings.testutils.FakeFeatureFactory; +import com.android.settings.testutils.SettingsRobolectricTestRunner; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.robolectric.RuntimeEnvironment; + +import java.util.ArrayList; +import java.util.List; + +@RunWith(SettingsRobolectricTestRunner.class) +public class OpenRestrictAppFragmentActionTest { + + private static final String PACKAGE_NAME_1 = "com.android.app1"; + private static final String PACKAGE_NAME_2 = "com.android.app2"; + private static final int ANOMALY_WAKEUP = 0; + private static final int ANOMALY_BT = 1; + private static final int METRICS_KEY = 1; + + @Mock + private InstrumentedPreferenceFragment mFragment; + @Mock + private BatteryDatabaseManager mBatteryDatabaseManager; + private OpenRestrictAppFragmentAction mAction; + private FakeFeatureFactory mFeatureFactory; + private Context mContext; + private List mAppInfos; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + mContext = RuntimeEnvironment.application; + + mAppInfos = new ArrayList<>(); + mAppInfos.add(new AppInfo.Builder() + .setPackageName(PACKAGE_NAME_1) + .addAnomalyType(ANOMALY_BT) + .build()); + mAppInfos.add(new AppInfo.Builder() + .setPackageName(PACKAGE_NAME_2) + .addAnomalyType(ANOMALY_WAKEUP) + .build()); + mFeatureFactory = FakeFeatureFactory.setupForTest(); + when(mFragment.getContext()).thenReturn(mContext); + + mAction = new OpenRestrictAppFragmentAction(mFragment, + new RestrictAppTip(BatteryTip.StateType.HANDLED, mAppInfos)); + mAction.mBatteryDatabaseManager = mBatteryDatabaseManager; + } + + @After + public void cleanUp() { + DatabaseTestUtils.clearDb(mContext); + } + + @Test + public void testHandlePositiveAction() { + mAction.handlePositiveAction(METRICS_KEY); + + verify(mFeatureFactory.metricsFeatureProvider).action(mContext, + MetricsProto.MetricsEvent.ACTION_TIP_OPEN_APP_RESTRICTION_PAGE, METRICS_KEY); + verify(mBatteryDatabaseManager).updateAnomalies(mAppInfos, + AnomalyDatabaseHelper.State.HANDLED); + } +} diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/detectors/RestrictAppDetectorTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/detectors/RestrictAppDetectorTest.java index 76e2928cb9d..a8989a0962e 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/detectors/RestrictAppDetectorTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/detectors/RestrictAppDetectorTest.java @@ -55,9 +55,11 @@ import java.util.List; public class RestrictAppDetectorTest { private static final int RESTRICTED_UID = 222; + private static final int UNRESTRICTED_UID = 333; private static final String PACKAGE_NAME = "com.android.app"; private static final String UNINSTALLED_PACKAGE_NAME = "com.android.uninstalled"; private static final String RESTRICTED_PACKAGE_NAME = "com.android.restricted"; + private static final String UNRESTRICTED_PACKAGE_NAME = "com.android.unrestricted"; private Context mContext; private BatteryTipPolicy mPolicy; private RestrictAppDetector mRestrictAppDetector; @@ -88,9 +90,12 @@ public class RestrictAppDetectorTest { doReturn(mAppOpsManager).when(mContext).getSystemService(AppOpsManager.class); doReturn(AppOpsManager.MODE_IGNORED).when(mAppOpsManager).checkOpNoThrow( AppOpsManager.OP_RUN_ANY_IN_BACKGROUND, RESTRICTED_UID, RESTRICTED_PACKAGE_NAME); + doReturn(AppOpsManager.MODE_ALLOWED).when(mAppOpsManager).checkOpNoThrow( + AppOpsManager.OP_RUN_ANY_IN_BACKGROUND, UNRESTRICTED_UID, + UNRESTRICTED_PACKAGE_NAME); doReturn(mPackageManager).when(mContext).getPackageManager(); - doReturn(mApplicationInfo).when(mPackageManager).getApplicationInfo(eq(PACKAGE_NAME), + doReturn(mApplicationInfo).when(mPackageManager).getApplicationInfo(any(), anyInt()); doReturn(PACKAGE_NAME).when(mApplicationInfo).loadLabel(any()); doThrow(new PackageManager.NameNotFoundException()).when( @@ -116,6 +121,10 @@ public class RestrictAppDetectorTest { @Test public void testDetect_hasAutoHandledAnomaly_tipHandled() { + mAppInfoList.add(new AppInfo.Builder() + .setUid(RESTRICTED_UID) + .setPackageName(RESTRICTED_PACKAGE_NAME) + .build()); doReturn(new ArrayList()).when(mBatteryDatabaseManager) .queryAllAnomalies(anyLong(), eq(AnomalyDatabaseHelper.State.NEW)); doReturn(mAppInfoList).when(mBatteryDatabaseManager) @@ -126,7 +135,7 @@ public class RestrictAppDetectorTest { } @Test - public void testDetect_hasUninstalledAnomaly_removeIt() { + public void testDetect_typeNewHasUninstalledAnomaly_removeIt() { mAppInfoList.add(new AppInfo.Builder() .setPackageName(UNINSTALLED_PACKAGE_NAME) .build()); @@ -139,7 +148,7 @@ public class RestrictAppDetectorTest { } @Test - public void testDetect_hasRestrictedAnomaly_removeIt() throws + public void testDetect_typeNewHasRestrictedAnomaly_removeIt() throws PackageManager.NameNotFoundException { mAppInfoList.add(new AppInfo.Builder() .setUid(RESTRICTED_UID) @@ -155,6 +164,25 @@ public class RestrictAppDetectorTest { assertThat(restrictAppTip.getRestrictAppList()).containsExactly(mAppInfo); } + @Test + public void testDetect_typeHandledHasUnRestrictedAnomaly_removeIt() throws + PackageManager.NameNotFoundException { + mAppInfoList.clear(); + mAppInfoList.add(new AppInfo.Builder() + .setUid(UNRESTRICTED_UID) + .setPackageName(UNRESTRICTED_PACKAGE_NAME) + .build()); + doReturn(new ArrayList<>()).when(mBatteryDatabaseManager) + .queryAllAnomalies(anyLong(), eq(AnomalyDatabaseHelper.State.NEW)); + doReturn(mAppInfoList).when(mBatteryDatabaseManager) + .queryAllAnomalies(anyLong(), eq(AnomalyDatabaseHelper.State.AUTO_HANDLED)); + doReturn(mApplicationInfo).when(mPackageManager).getApplicationInfo( + eq(UNRESTRICTED_PACKAGE_NAME), anyInt()); + + final RestrictAppTip restrictAppTip = (RestrictAppTip) mRestrictAppDetector.detect(); + assertThat(restrictAppTip.getState()).isEqualTo(BatteryTip.StateType.INVISIBLE); + } + @Test public void testDetect_noAnomaly_tipInvisible() { doReturn(new ArrayList()).when(mBatteryDatabaseManager) diff --git a/tests/unit/src/com/android/settings/fuelgauge/batterytip/RestrictAppTest.java b/tests/unit/src/com/android/settings/fuelgauge/batterytip/RestrictAppTest.java index b3842d28162..5d68c76e641 100644 --- a/tests/unit/src/com/android/settings/fuelgauge/batterytip/RestrictAppTest.java +++ b/tests/unit/src/com/android/settings/fuelgauge/batterytip/RestrictAppTest.java @@ -55,7 +55,7 @@ public class RestrictAppTest { } @Test - public void testBatterySettings_hasOneAnomaly_showAnomaly() throws + public void batterySettings_hasOneAnomaly_showAnomaly() throws PackageManager.NameNotFoundException { mBatteryDatabaseManager.insertAnomaly(mPackageManager.getPackageUid(PACKAGE_SETTINGS, 0), PACKAGE_SETTINGS, 1, @@ -67,7 +67,7 @@ public class RestrictAppTest { } @Test - public void testBatterySettings_hasTwoAnomalies_showAnomalies() throws + public void batterySettings_hasTwoAnomalies_showAnomalies() throws PackageManager.NameNotFoundException { mBatteryDatabaseManager.insertAnomaly(mPackageManager.getPackageUid(PACKAGE_SETTINGS, 0), PACKAGE_SETTINGS, 1, @@ -80,4 +80,19 @@ public class RestrictAppTest { instrumentation.startActivitySync(new Intent(BATTERY_INTENT)); onView(withText("Restrict 2 apps")).check(matches(isDisplayed())); } + + @Test + public void batterySettings_hasAutoHandledAnomalies_showAutoHandled() throws + PackageManager.NameNotFoundException { + mBatteryDatabaseManager.insertAnomaly(mPackageManager.getPackageUid(PACKAGE_SETTINGS, 0), + PACKAGE_SETTINGS, 1, + AnomalyDatabaseHelper.State.AUTO_HANDLED, System.currentTimeMillis()); + mBatteryDatabaseManager.insertAnomaly(mPackageManager.getPackageUid(PACKAGE_SYSTEM_UI, 0), + PACKAGE_SYSTEM_UI, 1, + AnomalyDatabaseHelper.State.AUTO_HANDLED, System.currentTimeMillis()); + + Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation(); + instrumentation.startActivitySync(new Intent(BATTERY_INTENT)); + onView(withText("2 apps recently restricted")).check(matches(isDisplayed())); + } } From 4c94926ba5a742e44a3fff684b0c9bee1330cba5 Mon Sep 17 00:00:00 2001 From: Lei Yu Date: Fri, 20 Apr 2018 16:26:21 -0700 Subject: [PATCH 3/7] Fix the bug using wrong TimeUnit It should use DAYS, not HOURS. Change-Id: I26784822c86e58cad93d35b6772ea54af7efb8f5 Fixes: 78362305 Test: RunSettingsRoboTests --- .../batterytip/AnomalyCleanupJobService.java | 2 +- .../AnomalyCleanupJobServiceTest.java | 57 +++++++++++++++++-- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobService.java b/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobService.java index 9e57433bcb7..d6021946d4c 100644 --- a/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobService.java +++ b/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobService.java @@ -64,7 +64,7 @@ public class AnomalyCleanupJobService extends JobService { final BatteryTipPolicy policy = new BatteryTipPolicy(this); ThreadUtils.postOnBackgroundThread(() -> { batteryDatabaseManager.deleteAllAnomaliesBeforeTimeStamp( - System.currentTimeMillis() - TimeUnit.HOURS.toMillis( + System.currentTimeMillis() - TimeUnit.DAYS.toMillis( policy.dataHistoryRetainDay)); jobFinished(params, false /* wantsReschedule */); }); diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobServiceTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobServiceTest.java index a39276df44f..f600d41352d 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobServiceTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobServiceTest.java @@ -17,36 +17,55 @@ package com.android.settings.fuelgauge.batterytip; import static com.google.common.truth.Truth.assertThat; + import static org.junit.Assert.assertEquals; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.robolectric.RuntimeEnvironment.application; import android.app.job.JobInfo; +import android.app.job.JobParameters; import android.app.job.JobScheduler; import android.content.Context; import com.android.settings.R; +import com.android.settings.testutils.DatabaseTestUtils; import com.android.settings.testutils.SettingsRobolectricTestRunner; +import com.android.settings.testutils.shadow.ShadowThreadUtils; +import org.junit.After; 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.RuntimeEnvironment; import org.robolectric.Shadows; +import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowJobScheduler; import java.util.List; import java.util.concurrent.TimeUnit; @RunWith(SettingsRobolectricTestRunner.class) +@Config(shadows = ShadowThreadUtils.class) public class AnomalyCleanupJobServiceTest { + private static final int UID = 1234; + private static final String PACKAGE_NAME = "com.android.package"; + private static final String PACKAGE_NAME_OLD = "com.android.package.old"; + private static final int ANOMALY_TYPE = 1; + private static final long TIMESTAMP_NOW = System.currentTimeMillis(); + private static final long TIMESTAMP_31_DAYS_BEFORE = TIMESTAMP_NOW - TimeUnit.DAYS.toMillis(31); + private Context mContext; private JobScheduler mJobScheduler; + @Mock + private JobParameters mParams; @Before public void setUp() { @@ -57,12 +76,17 @@ public class AnomalyCleanupJobServiceTest { when(mContext.getSystemService(JobScheduler.class)).thenReturn(mJobScheduler); } + @After + public void cleanUp() { + DatabaseTestUtils.clearDb(mContext); + } + @Test - public void testScheduleCleanUp() { + public void scheduleCleanUp() { AnomalyCleanupJobService.scheduleCleanUp(mContext); ShadowJobScheduler shadowJobScheduler = - Shadows.shadowOf(mContext.getSystemService(JobScheduler.class)); + Shadows.shadowOf(mContext.getSystemService(JobScheduler.class)); List pendingJobs = shadowJobScheduler.getAllPendingJobs(); assertEquals(1, pendingJobs.size()); JobInfo pendingJob = pendingJobs.get(0); @@ -74,10 +98,35 @@ public class AnomalyCleanupJobServiceTest { } @Test - public void testScheduleCleanUp_invokeTwice_onlyScheduleOnce() { + public void scheduleCleanUp_invokeTwice_onlyScheduleOnce() { AnomalyCleanupJobService.scheduleCleanUp(mContext); AnomalyCleanupJobService.scheduleCleanUp(mContext); verify(mJobScheduler, times(1)).schedule(any()); } + + @Test + public void onStartJob_cleanUpDataBefore30days() { + final BatteryDatabaseManager databaseManager = BatteryDatabaseManager.getInstance(mContext); + final AnomalyCleanupJobService service = spy(Robolectric.setupService( + AnomalyCleanupJobService.class)); + doNothing().when(service).jobFinished(any(), anyBoolean()); + + // Insert two records, one is current and the other one is 31 days before + databaseManager.insertAnomaly(UID, PACKAGE_NAME, ANOMALY_TYPE, + AnomalyDatabaseHelper.State.NEW, TIMESTAMP_NOW); + databaseManager.insertAnomaly(UID, PACKAGE_NAME_OLD, ANOMALY_TYPE, + AnomalyDatabaseHelper.State.NEW, TIMESTAMP_31_DAYS_BEFORE); + + service.onStartJob(mParams); + + // In database, it only contains the current record + final List appInfos = databaseManager.queryAllAnomalies(0, + AnomalyDatabaseHelper.State.NEW); + assertThat(appInfos).containsExactly(new AppInfo.Builder() + .setUid(UID) + .setPackageName(PACKAGE_NAME) + .addAnomalyType(ANOMALY_TYPE) + .build()); + } } From 35b76437e092a164dbe2b001e8779b0e163269a7 Mon Sep 17 00:00:00 2001 From: Salvador Martinez Date: Mon, 23 Apr 2018 10:50:22 -0700 Subject: [PATCH 4/7] Disable hidden network spinner on existing networks We only want this to be modifiable if you are adding a network manually, so this CL disables it for existing networks. You can still see what the spinner is set to though. Test: robotests Bug: 78436456 Change-Id: If660e432eca2dabf5bd16881368657ee89fe5a57 --- .../android/settings/wifi/WifiConfigController.java | 3 +++ .../settings/wifi/WifiConfigControllerTest.java | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/src/com/android/settings/wifi/WifiConfigController.java b/src/com/android/settings/wifi/WifiConfigController.java index db657f90149..e995c31efc3 100644 --- a/src/com/android/settings/wifi/WifiConfigController.java +++ b/src/com/android/settings/wifi/WifiConfigController.java @@ -220,6 +220,7 @@ public class WifiConfigController implements TextWatcher, mMeteredSettingsSpinner = mView.findViewById(R.id.metered_settings); mHiddenSettingsSpinner = mView.findViewById(R.id.hidden_settings); mHiddenSettingsSpinner.setOnItemSelectedListener(this); + mHiddenSettingsSpinner.setEnabled(false); mHiddenWarningView = mView.findViewById(R.id.hidden_settings_warning); mHiddenWarningView.setVisibility( mHiddenSettingsSpinner.getSelectedItemPosition() == NOT_HIDDEN_NETWORK @@ -238,6 +239,8 @@ public class WifiConfigController implements TextWatcher, showIpConfigFields(); showProxyFields(); mView.findViewById(R.id.wifi_advanced_toggle).setVisibility(View.VISIBLE); + // Hidden option can be changed only when the user adds a network manually. + mHiddenSettingsSpinner.setEnabled(true); ((CheckBox) mView.findViewById(R.id.wifi_advanced_togglebox)) .setOnCheckedChangeListener(this); diff --git a/tests/robotests/src/com/android/settings/wifi/WifiConfigControllerTest.java b/tests/robotests/src/com/android/settings/wifi/WifiConfigControllerTest.java index 3b41d3852b4..d445c0df15f 100644 --- a/tests/robotests/src/com/android/settings/wifi/WifiConfigControllerTest.java +++ b/tests/robotests/src/com/android/settings/wifi/WifiConfigControllerTest.java @@ -258,6 +258,16 @@ public class WifiConfigControllerTest { assertThat(warningView.getVisibility()).isEqualTo(View.GONE); } + @Test + public void hiddenView_isDisabledWhenAppropriate() { + View hiddenSpinner = mView.findViewById(R.id.hidden_settings); + assertThat(hiddenSpinner.isEnabled()).isFalse(); + + mController = new TestWifiConfigController(mConfigUiBase, mView, null /* accessPoint */, + WifiConfigUiBase.MODE_CONNECT); + assertThat(hiddenSpinner.isEnabled()).isTrue(); + } + public class TestWifiConfigController extends WifiConfigController { private TestWifiConfigController( From 0fcddc77c7803cfde7876cf25fca2704233d82f3 Mon Sep 17 00:00:00 2001 From: Lei Yu Date: Thu, 19 Apr 2018 17:37:18 -0700 Subject: [PATCH 5/7] Add primary key for anomaly database. In this case it won't insert the duplicate data. We don't do migration since anomaly database only contains transient data. Bug: 77968649 Test: test still pass Change-Id: I638564d89ead008ec184b9a4db137436d47df5bc --- .../fuelgauge/batterytip/AnomalyDatabaseHelper.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/com/android/settings/fuelgauge/batterytip/AnomalyDatabaseHelper.java b/src/com/android/settings/fuelgauge/batterytip/AnomalyDatabaseHelper.java index 10858c60010..0b217472ee0 100644 --- a/src/com/android/settings/fuelgauge/batterytip/AnomalyDatabaseHelper.java +++ b/src/com/android/settings/fuelgauge/batterytip/AnomalyDatabaseHelper.java @@ -34,7 +34,7 @@ public class AnomalyDatabaseHelper extends SQLiteOpenHelper { private static final String TAG = "BatteryDatabaseHelper"; private static final String DATABASE_NAME = "battery_settings.db"; - private static final int DATABASE_VERSION = 3; + private static final int DATABASE_VERSION = 4; @Retention(RetentionPolicy.SOURCE) @IntDef({State.NEW, @@ -79,15 +79,18 @@ public class AnomalyDatabaseHelper extends SQLiteOpenHelper { "CREATE TABLE " + Tables.TABLE_ANOMALY + "(" + AnomalyColumns.UID + - " INTEGER, " + + " INTEGER NOT NULL, " + AnomalyColumns.PACKAGE_NAME + " TEXT, " + AnomalyColumns.ANOMALY_TYPE + - " INTEGER, " + + " INTEGER NOT NULL, " + AnomalyColumns.ANOMALY_STATE + - " INTEGER, " + + " INTEGER NOT NULL, " + AnomalyColumns.TIME_STAMP_MS + - " INTEGER)"; + " INTEGER NOT NULL, " + + " PRIMARY KEY (" + AnomalyColumns.UID + "," + AnomalyColumns.ANOMALY_TYPE + "," + + AnomalyColumns.ANOMALY_STATE + "," + AnomalyColumns.TIME_STAMP_MS + ")" + + ")"; private static AnomalyDatabaseHelper sSingleton; From 4bfbaa6047aa30a124282579a4f16573ca8a2682 Mon Sep 17 00:00:00 2001 From: Lei Yu Date: Fri, 20 Apr 2018 10:51:33 -0700 Subject: [PATCH 6/7] Update insert method for battery database 1. Add conflict code to ignore the confliction and return -1 when insert fails. 2. Add boolean return value to show whether insert sucessfully 3. Add unit test. We don't write robo test because robolectirc use "update" to shadow "insert" operation, which returns the wrong data. Bug: 77968649 Test: SettingsUnitTest Change-Id: Ibd3b53cdb1796d74ea4a2ca1f067e2b302b939e9 --- .../batterytip/BatteryDatabaseManager.java | 9 +++-- .../fuelgauge/batterytip/RestrictAppTest.java | 33 +++++++++++++++++-- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/com/android/settings/fuelgauge/batterytip/BatteryDatabaseManager.java b/src/com/android/settings/fuelgauge/batterytip/BatteryDatabaseManager.java index 58e9d03ef5c..772660a69d3 100644 --- a/src/com/android/settings/fuelgauge/batterytip/BatteryDatabaseManager.java +++ b/src/com/android/settings/fuelgauge/batterytip/BatteryDatabaseManager.java @@ -16,6 +16,8 @@ package com.android.settings.fuelgauge.batterytip; +import static android.database.sqlite.SQLiteDatabase.CONFLICT_IGNORE; + import static com.android.settings.fuelgauge.batterytip.AnomalyDatabaseHelper.AnomalyColumns .ANOMALY_STATE; import static com.android.settings.fuelgauge.batterytip.AnomalyDatabaseHelper.AnomalyColumns @@ -64,12 +66,15 @@ public class BatteryDatabaseManager { /** * Insert an anomaly log to database. * + * @param uid the uid of the app * @param packageName the package name of the app * @param type the type of the anomaly * @param anomalyState the state of the anomaly * @param timestampMs the time when it is happened + * @return {@code true} if insert operation succeed */ - public synchronized void insertAnomaly(int uid, String packageName, int type, int anomalyState, + public synchronized boolean insertAnomaly(int uid, String packageName, int type, + int anomalyState, long timestampMs) { try (SQLiteDatabase db = mDatabaseHelper.getWritableDatabase()) { ContentValues values = new ContentValues(); @@ -78,7 +83,7 @@ public class BatteryDatabaseManager { values.put(ANOMALY_TYPE, type); values.put(ANOMALY_STATE, anomalyState); values.put(TIME_STAMP_MS, timestampMs); - db.insert(TABLE_ANOMALY, null, values); + return db.insertWithOnConflict(TABLE_ANOMALY, null, values, CONFLICT_IGNORE) != -1; } } diff --git a/tests/unit/src/com/android/settings/fuelgauge/batterytip/RestrictAppTest.java b/tests/unit/src/com/android/settings/fuelgauge/batterytip/RestrictAppTest.java index 5d68c76e641..a24a17ae070 100644 --- a/tests/unit/src/com/android/settings/fuelgauge/batterytip/RestrictAppTest.java +++ b/tests/unit/src/com/android/settings/fuelgauge/batterytip/RestrictAppTest.java @@ -21,6 +21,8 @@ import static android.support.test.espresso.assertion.ViewAssertions.matches; import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed; import static android.support.test.espresso.matcher.ViewMatchers.withText; +import static com.google.common.truth.Truth.assertThat; + import android.app.Instrumentation; import android.content.Context; import android.content.Intent; @@ -33,6 +35,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import java.util.List; import java.util.concurrent.TimeUnit; @RunWith(AndroidJUnit4.class) @@ -41,6 +44,8 @@ public class RestrictAppTest { private static final String BATTERY_INTENT = "android.intent.action.POWER_USAGE_SUMMARY"; private static final String PACKAGE_SETTINGS = "com.android.settings"; private static final String PACKAGE_SYSTEM_UI = "com.android.systemui"; + private static final int ANOMALY_TYPE = + StatsManagerConfig.AnomalyType.EXCESSIVE_WAKELOCK_ALL_SCREEN_OFF; private BatteryDatabaseManager mBatteryDatabaseManager; private PackageManager mPackageManager; @@ -58,7 +63,7 @@ public class RestrictAppTest { public void batterySettings_hasOneAnomaly_showAnomaly() throws PackageManager.NameNotFoundException { mBatteryDatabaseManager.insertAnomaly(mPackageManager.getPackageUid(PACKAGE_SETTINGS, 0), - PACKAGE_SETTINGS, 1, + PACKAGE_SETTINGS, ANOMALY_TYPE, AnomalyDatabaseHelper.State.NEW, System.currentTimeMillis()); Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation(); @@ -70,10 +75,10 @@ public class RestrictAppTest { public void batterySettings_hasTwoAnomalies_showAnomalies() throws PackageManager.NameNotFoundException { mBatteryDatabaseManager.insertAnomaly(mPackageManager.getPackageUid(PACKAGE_SETTINGS, 0), - PACKAGE_SETTINGS, 1, + PACKAGE_SETTINGS, ANOMALY_TYPE, AnomalyDatabaseHelper.State.NEW, System.currentTimeMillis()); mBatteryDatabaseManager.insertAnomaly(mPackageManager.getPackageUid(PACKAGE_SYSTEM_UI, 0), - PACKAGE_SYSTEM_UI, 1, + PACKAGE_SYSTEM_UI, ANOMALY_TYPE, AnomalyDatabaseHelper.State.NEW, System.currentTimeMillis()); Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation(); @@ -95,4 +100,26 @@ public class RestrictAppTest { instrumentation.startActivitySync(new Intent(BATTERY_INTENT)); onView(withText("2 apps recently restricted")).check(matches(isDisplayed())); } + + @Test + public void insertDuplicateAnomalies_onlyInsertOnce() throws + PackageManager.NameNotFoundException { + final int uid = mPackageManager.getPackageUid(PACKAGE_SETTINGS, 0); + final long now = System.currentTimeMillis(); + + // Insert same anomaly twice, it fails at the second time. + assertThat(mBatteryDatabaseManager.insertAnomaly(uid, PACKAGE_SETTINGS, ANOMALY_TYPE, + AnomalyDatabaseHelper.State.NEW, now)).isTrue(); + assertThat(mBatteryDatabaseManager.insertAnomaly(uid, PACKAGE_SETTINGS, ANOMALY_TYPE, + AnomalyDatabaseHelper.State.NEW, now)).isFalse(); + + // In database, only contains one row + List newAppInfos = mBatteryDatabaseManager.queryAllAnomalies(0, + AnomalyDatabaseHelper.State.NEW); + assertThat(newAppInfos).containsExactly(new AppInfo.Builder() + .setUid(uid) + .setPackageName(PACKAGE_SETTINGS) + .addAnomalyType(ANOMALY_TYPE) + .build()); + } } From b0877b390d8d96da30775dcd6161ecec59f006d0 Mon Sep 17 00:00:00 2001 From: Doris Ling Date: Fri, 20 Apr 2018 16:42:10 -0700 Subject: [PATCH 7/7] Fix footer text not translated in Magnification settings. - pass in the summary res id instead of the actual text when building the launch bundle for the magnification settings. - remove the title argument as we are passing the title res already, which takes precedence. Change-Id: I4ba624e1d9722aa980ea94c306df9a015a159555 Fixes: 78126057 Test: run i18nscreenshots --- .../settings/accessibility/AccessibilitySettings.java | 1 + .../MagnificationGesturesPreferenceController.java | 6 ++---- .../MagnificationNavbarPreferenceController.java | 7 ++----- .../accessibility/ToggleFeaturePreferenceFragment.java | 5 ++++- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/com/android/settings/accessibility/AccessibilitySettings.java b/src/com/android/settings/accessibility/AccessibilitySettings.java index 3f7bd2582c1..3f94af46664 100644 --- a/src/com/android/settings/accessibility/AccessibilitySettings.java +++ b/src/com/android/settings/accessibility/AccessibilitySettings.java @@ -129,6 +129,7 @@ public class AccessibilitySettings extends SettingsPreferenceFragment implements static final String EXTRA_TITLE_RES = "title_res"; static final String EXTRA_RESOLVE_INFO = "resolve_info"; static final String EXTRA_SUMMARY = "summary"; + static final String EXTRA_SUMMARY_RES = "summary_res"; static final String EXTRA_SETTINGS_TITLE = "settings_title"; static final String EXTRA_COMPONENT_NAME = "component_name"; static final String EXTRA_SETTINGS_COMPONENT_NAME = "settings_component_name"; diff --git a/src/com/android/settings/accessibility/MagnificationGesturesPreferenceController.java b/src/com/android/settings/accessibility/MagnificationGesturesPreferenceController.java index 98f6672b339..71901af1acc 100644 --- a/src/com/android/settings/accessibility/MagnificationGesturesPreferenceController.java +++ b/src/com/android/settings/accessibility/MagnificationGesturesPreferenceController.java @@ -65,12 +65,10 @@ public class MagnificationGesturesPreferenceController extends BasePreferenceCon static void populateMagnificationGesturesPreferenceExtras(Bundle extras, Context context) { extras.putString(AccessibilitySettings.EXTRA_PREFERENCE_KEY, Settings.Secure.ACCESSIBILITY_DISPLAY_MAGNIFICATION_ENABLED); - extras.putString(AccessibilitySettings.EXTRA_TITLE, context.getString( - R.string.accessibility_screen_magnification_gestures_title)); extras.putInt(AccessibilitySettings.EXTRA_TITLE_RES, R.string.accessibility_screen_magnification_gestures_title); - extras.putCharSequence(AccessibilitySettings.EXTRA_SUMMARY, context.getResources().getText( - R.string.accessibility_screen_magnification_summary)); + extras.putInt(AccessibilitySettings.EXTRA_SUMMARY_RES, + R.string.accessibility_screen_magnification_summary); extras.putBoolean(AccessibilitySettings.EXTRA_CHECKED, Settings.Secure.getInt(context.getContentResolver(), Settings.Secure.ACCESSIBILITY_DISPLAY_MAGNIFICATION_ENABLED, 0) == 1); diff --git a/src/com/android/settings/accessibility/MagnificationNavbarPreferenceController.java b/src/com/android/settings/accessibility/MagnificationNavbarPreferenceController.java index e0ea6f27c9f..947893d76cb 100644 --- a/src/com/android/settings/accessibility/MagnificationNavbarPreferenceController.java +++ b/src/com/android/settings/accessibility/MagnificationNavbarPreferenceController.java @@ -39,13 +39,10 @@ public class MagnificationNavbarPreferenceController extends BasePreferenceContr Bundle extras = preference.getExtras(); extras.putString(AccessibilitySettings.EXTRA_PREFERENCE_KEY, Settings.Secure.ACCESSIBILITY_DISPLAY_MAGNIFICATION_NAVBAR_ENABLED); - extras.putString(AccessibilitySettings.EXTRA_TITLE, mContext.getString( - R.string.accessibility_screen_magnification_navbar_title)); extras.putInt(AccessibilitySettings.EXTRA_TITLE_RES, R.string.accessibility_screen_magnification_navbar_title); - extras.putCharSequence(AccessibilitySettings.EXTRA_SUMMARY, - mContext.getResources().getText( - R.string.accessibility_screen_magnification_navbar_summary)); + extras.putInt(AccessibilitySettings.EXTRA_SUMMARY_RES, + R.string.accessibility_screen_magnification_navbar_summary); extras.putBoolean(AccessibilitySettings.EXTRA_CHECKED, Settings.Secure.getInt(mContext.getContentResolver(), Settings.Secure.ACCESSIBILITY_DISPLAY_MAGNIFICATION_NAVBAR_ENABLED, 0) diff --git a/src/com/android/settings/accessibility/ToggleFeaturePreferenceFragment.java b/src/com/android/settings/accessibility/ToggleFeaturePreferenceFragment.java index 7dacbb08684..208bb73c6f5 100644 --- a/src/com/android/settings/accessibility/ToggleFeaturePreferenceFragment.java +++ b/src/com/android/settings/accessibility/ToggleFeaturePreferenceFragment.java @@ -135,7 +135,10 @@ public abstract class ToggleFeaturePreferenceFragment extends SettingsPreference } // Summary. - if (arguments.containsKey(AccessibilitySettings.EXTRA_SUMMARY)) { + if (arguments.containsKey(AccessibilitySettings.EXTRA_SUMMARY_RES)) { + final int summary = arguments.getInt(AccessibilitySettings.EXTRA_SUMMARY_RES); + mFooterPreferenceMixin.createFooterPreference().setTitle(summary); + } else if (arguments.containsKey(AccessibilitySettings.EXTRA_SUMMARY)) { final CharSequence summary = arguments.getCharSequence( AccessibilitySettings.EXTRA_SUMMARY); mFooterPreferenceMixin.createFooterPreference().setTitle(summary);