Merge "[Notif] Rename/reset blocking helper stats file" into pi-dev
diff --git a/src/android/ext/services/notification/Assistant.java b/src/android/ext/services/notification/Assistant.java
index d34fec3..f878822 100644
--- a/src/android/ext/services/notification/Assistant.java
+++ b/src/android/ext/services/notification/Assistant.java
@@ -17,16 +17,20 @@
package android.ext.services.notification;
import static android.app.NotificationManager.IMPORTANCE_MIN;
-import static android.service.notification.NotificationListenerService.Ranking
- .USER_SENTIMENT_NEGATIVE;
+import static android.service.notification.NotificationListenerService.Ranking.USER_SENTIMENT_NEGATIVE;
import android.app.INotificationManager;
+import android.content.ContentResolver;
import android.content.Context;
+import android.database.ContentObserver;
import android.ext.services.R;
+import android.net.Uri;
import android.os.AsyncTask;
import android.os.Bundle;
import android.os.Environment;
+import android.os.Handler;
import android.os.storage.StorageManager;
+import android.provider.Settings;
import android.service.notification.Adjustment;
import android.service.notification.NotificationAssistantService;
import android.service.notification.NotificationStats;
@@ -74,9 +78,12 @@
PREJUDICAL_DISMISSALS.add(REASON_LISTENER_CANCEL);
}
+ private float mDismissToViewRatioLimit;
+ private int mStreakLimit;
+
// key : impressions tracker
// TODO: prune deleted channels and apps
- ArrayMap<String, ChannelImpressions> mkeyToImpressions = new ArrayMap<>();
+ final ArrayMap<String, ChannelImpressions> mkeyToImpressions = new ArrayMap<>();
// SBN key : channel id
ArrayMap<String, String> mLiveNotifications = new ArrayMap<>();
@@ -86,6 +93,14 @@
public Assistant() {
}
+ @Override
+ public void onCreate() {
+ super.onCreate();
+ // Contexts are correctly hooked up by the creation step, which is required for the observer
+ // to be hooked up/initialized.
+ new SettingsObserver(mHandler);
+ }
+
private void loadFile() {
if (DEBUG) Slog.d(TAG, "loadFile");
AsyncTask.execute(() -> {
@@ -120,7 +135,7 @@
continue;
}
String key = parser.getAttributeValue(null, ATT_KEY);
- ChannelImpressions ci = new ChannelImpressions();
+ ChannelImpressions ci = createChannelImpressionsWithThresholds();
ci.populateFromXml(parser);
synchronized (mkeyToImpressions) {
ci.append(mkeyToImpressions.get(key));
@@ -184,7 +199,7 @@
String key = getKey(
sbn.getPackageName(), sbn.getUserId(), ranking.getChannel().getId());
ChannelImpressions ci = mkeyToImpressions.getOrDefault(key,
- new ChannelImpressions());
+ createChannelImpressionsWithThresholds());
if (ranking.getImportance() > IMPORTANCE_MIN && ci.shouldTriggerBlock()) {
adjustNotification(createNegativeAdjustment(
sbn.getPackageName(), sbn.getKey(), sbn.getUserId()));
@@ -206,7 +221,7 @@
String key = getKey(sbn.getPackageName(), sbn.getUserId(), channelId);
synchronized (mkeyToImpressions) {
ChannelImpressions ci = mkeyToImpressions.getOrDefault(key,
- new ChannelImpressions());
+ createChannelImpressionsWithThresholds());
if (stats.hasSeen()) {
ci.incrementViews();
updatedImpressions = true;
@@ -310,4 +325,58 @@
mkeyToImpressions.put(key, ci);
}
}
+
+ private ChannelImpressions createChannelImpressionsWithThresholds() {
+ ChannelImpressions impressions = new ChannelImpressions();
+ impressions.updateThresholds(mDismissToViewRatioLimit, mStreakLimit);
+ return impressions;
+ }
+
+ /**
+ * Observer for updates on blocking helper threshold values.
+ */
+ private final class SettingsObserver extends ContentObserver {
+ private final Uri STREAK_LIMIT_URI =
+ Settings.Global.getUriFor(Settings.Global.BLOCKING_HELPER_STREAK_LIMIT);
+ private final Uri DISMISS_TO_VIEW_RATIO_LIMIT_URI =
+ Settings.Global.getUriFor(
+ Settings.Global.BLOCKING_HELPER_DISMISS_TO_VIEW_RATIO_LIMIT);
+
+ public SettingsObserver(Handler handler) {
+ super(handler);
+ ContentResolver resolver = getApplicationContext().getContentResolver();
+ resolver.registerContentObserver(
+ DISMISS_TO_VIEW_RATIO_LIMIT_URI, false, this, getUserId());
+ resolver.registerContentObserver(STREAK_LIMIT_URI, false, this, getUserId());
+
+ // Update all uris on creation.
+ update(null);
+ }
+
+ @Override
+ public void onChange(boolean selfChange, Uri uri) {
+ update(uri);
+ }
+
+ private void update(Uri uri) {
+ ContentResolver resolver = getApplicationContext().getContentResolver();
+ if (uri == null || DISMISS_TO_VIEW_RATIO_LIMIT_URI.equals(uri)) {
+ mDismissToViewRatioLimit = Settings.Global.getFloat(
+ resolver, Settings.Global.BLOCKING_HELPER_DISMISS_TO_VIEW_RATIO_LIMIT,
+ ChannelImpressions.DEFAULT_DISMISS_TO_VIEW_RATIO_LIMIT);
+ }
+ if (uri == null || STREAK_LIMIT_URI.equals(uri)) {
+ mStreakLimit = Settings.Global.getInt(
+ resolver, Settings.Global.BLOCKING_HELPER_STREAK_LIMIT,
+ ChannelImpressions.DEFAULT_STREAK_LIMIT);
+ }
+
+ // Update all existing channel impression objects with any new limits/thresholds.
+ synchronized (mkeyToImpressions) {
+ for (ChannelImpressions channelImpressions: mkeyToImpressions.values()) {
+ channelImpressions.updateThresholds(mDismissToViewRatioLimit, mStreakLimit);
+ }
+ }
+ }
+ }
}
\ No newline at end of file
diff --git a/src/android/ext/services/notification/ChannelImpressions.java b/src/android/ext/services/notification/ChannelImpressions.java
index de2659f..29ee920 100644
--- a/src/android/ext/services/notification/ChannelImpressions.java
+++ b/src/android/ext/services/notification/ChannelImpressions.java
@@ -21,6 +21,8 @@
import android.text.TextUtils;
import android.util.Log;
+import com.android.internal.annotations.VisibleForTesting;
+
import org.xmlpull.v1.XmlPullParser;
import org.xmlpull.v1.XmlSerializer;
@@ -30,8 +32,8 @@
private static final String TAG = "ExtAssistant.CI";
private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG);
- static final double DISMISS_TO_VIEW_RATIO_LIMIT = .4;
- static final int STREAK_LIMIT = 2;
+ static final float DEFAULT_DISMISS_TO_VIEW_RATIO_LIMIT = .8f;
+ static final int DEFAULT_STREAK_LIMIT = 2;
static final String ATT_DISMISSALS = "dismisses";
static final String ATT_VIEWS = "views";
static final String ATT_STREAK = "streak";
@@ -40,18 +42,20 @@
private int mViews = 0;
private int mStreak = 0;
- public ChannelImpressions() {
- }
+ private float mDismissToViewRatioLimit;
+ private int mStreakLimit;
- public ChannelImpressions(int dismissals, int views) {
- mDismissals = dismissals;
- mViews = views;
+ public ChannelImpressions() {
+ mDismissToViewRatioLimit = DEFAULT_DISMISS_TO_VIEW_RATIO_LIMIT;
+ mStreakLimit = DEFAULT_STREAK_LIMIT;
}
protected ChannelImpressions(Parcel in) {
mDismissals = in.readInt();
mViews = in.readInt();
mStreak = in.readInt();
+ mDismissToViewRatioLimit = in.readFloat();
+ mStreakLimit = in.readInt();
}
public int getStreak() {
@@ -71,6 +75,21 @@
mStreak++;
}
+ void updateThresholds(float dismissToViewRatioLimit, int streakLimit) {
+ mDismissToViewRatioLimit = dismissToViewRatioLimit;
+ mStreakLimit = streakLimit;
+ }
+
+ @VisibleForTesting
+ float getDismissToViewRatioLimit() {
+ return mDismissToViewRatioLimit;
+ }
+
+ @VisibleForTesting
+ int getStreakLimit() {
+ return mStreakLimit;
+ }
+
public void append(ChannelImpressions additionalImpressions) {
if (additionalImpressions != null) {
mViews += additionalImpressions.getViews();
@@ -94,8 +113,8 @@
if (DEBUG) {
Log.d(TAG, "should trigger? " + getDismissals() + " " + getViews() + " " + getStreak());
}
- return ((double) getDismissals() / getViews()) > DISMISS_TO_VIEW_RATIO_LIMIT
- && getStreak() > STREAK_LIMIT;
+ return ((float) getDismissals() / getViews()) > mDismissToViewRatioLimit
+ && getStreak() > mStreakLimit;
}
@Override
@@ -103,6 +122,8 @@
dest.writeInt(mDismissals);
dest.writeInt(mViews);
dest.writeInt(mStreak);
+ dest.writeFloat(mDismissToViewRatioLimit);
+ dest.writeInt(mStreakLimit);
}
@Override
@@ -148,7 +169,9 @@
sb.append("mDismissals=").append(mDismissals);
sb.append(", mViews=").append(mViews);
sb.append(", mStreak=").append(mStreak);
- sb.append('}');
+ sb.append(", thresholds=(").append(mDismissToViewRatioLimit);
+ sb.append(",").append(mStreakLimit);
+ sb.append(")}");
return sb.toString();
}
diff --git a/tests/AndroidManifest.xml b/tests/AndroidManifest.xml
index e6c7b97..ddf725b 100644
--- a/tests/AndroidManifest.xml
+++ b/tests/AndroidManifest.xml
@@ -17,6 +17,8 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="android.ext.services.tests.unit">
+ <uses-permission android:name="android.permission.INTERACT_ACROSS_USERS_FULL" />
+
<application>
<uses-library android:name="android.test.runner" />
</application>
diff --git a/tests/src/android/ext/services/notification/AssistantTest.java b/tests/src/android/ext/services/notification/AssistantTest.java
index db48f61..a6b6a6b 100644
--- a/tests/src/android/ext/services/notification/AssistantTest.java
+++ b/tests/src/android/ext/services/notification/AssistantTest.java
@@ -20,6 +20,7 @@
import static android.app.NotificationManager.IMPORTANCE_LOW;
import static android.app.NotificationManager.IMPORTANCE_MIN;
+import static junit.framework.Assert.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
@@ -27,11 +28,15 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
+import android.app.Application;
import android.app.INotificationManager;
import android.app.Notification;
import android.app.NotificationChannel;
+import android.content.ContentResolver;
+import android.content.IContentProvider;
import android.content.Intent;
import android.os.UserHandle;
+import android.provider.Settings;
import android.service.notification.Adjustment;
import android.service.notification.NotificationListenerService;
import android.service.notification.NotificationListenerService.Ranking;
@@ -78,10 +83,10 @@
new NotificationChannel("one", "", IMPORTANCE_LOW);
@Mock INotificationManager mNoMan;
- @Mock
- AtomicFile mFile;
+ @Mock AtomicFile mFile;
Assistant mAssistant;
+ Application mApplication;
@Rule
public final TestableContext mContext =
@@ -98,6 +103,16 @@
Intent startIntent =
new Intent("android.service.notification.NotificationAssistantService");
startIntent.setPackage("android.ext.services");
+
+ // To bypass real calls to global settings values, set the Settings values here.
+ Settings.Global.putFloat(mContext.getContentResolver(),
+ Settings.Global.BLOCKING_HELPER_DISMISS_TO_VIEW_RATIO_LIMIT, 0.8f);
+ Settings.Global.putInt(mContext.getContentResolver(),
+ Settings.Global.BLOCKING_HELPER_STREAK_LIMIT, 2);
+ mApplication = (Application) InstrumentationRegistry.getInstrumentation().
+ getTargetContext().getApplicationContext();
+ // Force the test to use the correct application instead of trying to use a mock application
+ setApplication(mApplication);
bindService(startIntent);
mAssistant = getService();
mAssistant.setNoMan(mNoMan);
@@ -128,7 +143,7 @@
}
private void almostBlockChannel(String pkg, int uid, NotificationChannel channel) {
- for (int i = 0; i < ChannelImpressions.STREAK_LIMIT; i++) {
+ for (int i = 0; i < ChannelImpressions.DEFAULT_STREAK_LIMIT; i++) {
dismissBadNotification(pkg, uid, channel, String.valueOf(i));
}
}
@@ -358,7 +373,7 @@
@Test
public void testRoundTripXml() throws Exception {
String key1 = mAssistant.getKey("pkg1", 1, "channel1");
- ChannelImpressions ci1 = new ChannelImpressions(9, 10);
+ ChannelImpressions ci1 = new ChannelImpressions();
String key2 = mAssistant.getKey("pkg1", 1, "channel2");
ChannelImpressions ci2 = new ChannelImpressions();
for (int i = 0; i < 3; i++) {
@@ -391,4 +406,43 @@
assertEquals(ci3, assistant.getImpressions(key3));
}
+ @Test
+ public void testSettingsProviderUpdate() {
+ ContentResolver resolver = mApplication.getContentResolver();
+
+ // Set up channels
+ String key = mAssistant.getKey("pkg1", 1, "channel1");
+ ChannelImpressions ci = new ChannelImpressions();
+ for (int i = 0; i < 3; i++) {
+ ci.incrementViews();
+ if (i % 2 == 0) {
+ ci.incrementDismissals();
+ }
+ }
+
+ mAssistant.insertImpressions(key, ci);
+
+ // With default values, the blocking helper shouldn't be triggered.
+ assertEquals(false, ci.shouldTriggerBlock());
+
+ // Update settings values.
+ float newDismissToViewRatioLimit = 0f;
+ int newStreakLimit = 0;
+ Settings.Global.putFloat(resolver,
+ Settings.Global.BLOCKING_HELPER_DISMISS_TO_VIEW_RATIO_LIMIT,
+ newDismissToViewRatioLimit);
+ Settings.Global.putInt(resolver,
+ Settings.Global.BLOCKING_HELPER_STREAK_LIMIT, newStreakLimit);
+
+ // Notify for the settings values we updated.
+ resolver.notifyChange(
+ Settings.Global.getUriFor(Settings.Global.BLOCKING_HELPER_STREAK_LIMIT), null);
+ resolver.notifyChange(
+ Settings.Global.getUriFor(
+ Settings.Global.BLOCKING_HELPER_DISMISS_TO_VIEW_RATIO_LIMIT),
+ null);
+
+ // With the new threshold, the blocking helper should be triggered.
+ assertEquals(true, ci.shouldTriggerBlock());
+ }
}
diff --git a/tests/src/android/ext/services/notification/ChannelImpressionsTest.java b/tests/src/android/ext/services/notification/ChannelImpressionsTest.java
index d28e2ac..3253802 100644
--- a/tests/src/android/ext/services/notification/ChannelImpressionsTest.java
+++ b/tests/src/android/ext/services/notification/ChannelImpressionsTest.java
@@ -16,7 +16,8 @@
package android.ext.services.notification;
-import static android.ext.services.notification.ChannelImpressions.STREAK_LIMIT;
+import static android.ext.services.notification.ChannelImpressions.DEFAULT_DISMISS_TO_VIEW_RATIO_LIMIT;
+import static android.ext.services.notification.ChannelImpressions.DEFAULT_STREAK_LIMIT;
import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertTrue;
@@ -37,7 +38,7 @@
public void testNoStreakNoBlock() {
ChannelImpressions ci = new ChannelImpressions();
- for (int i = 0; i < STREAK_LIMIT - 1; i++) {
+ for (int i = 0; i < DEFAULT_STREAK_LIMIT - 1; i++) {
ci.incrementViews();
ci.incrementDismissals();
}
@@ -49,10 +50,10 @@
public void testNoStreakNoBlock_breakStreak() {
ChannelImpressions ci = new ChannelImpressions();
- for (int i = 0; i < STREAK_LIMIT; i++) {
+ for (int i = 0; i < DEFAULT_STREAK_LIMIT; i++) {
ci.incrementViews();
ci.incrementDismissals();
- if (i == STREAK_LIMIT - 1) {
+ if (i == DEFAULT_STREAK_LIMIT - 1) {
ci.resetStreak();
}
}
@@ -64,7 +65,7 @@
public void testStreakBlock() {
ChannelImpressions ci = new ChannelImpressions();
- for (int i = 0; i <= STREAK_LIMIT; i++) {
+ for (int i = 0; i <= DEFAULT_STREAK_LIMIT; i++) {
ci.incrementViews();
ci.incrementDismissals();
}
@@ -76,7 +77,7 @@
public void testRatio_NoBlockEvenWithStreak() {
ChannelImpressions ci = new ChannelImpressions();
- for (int i = 0; i < STREAK_LIMIT; i++) {
+ for (int i = 0; i < DEFAULT_STREAK_LIMIT; i++) {
ci.incrementViews();
ci.incrementDismissals();
ci.incrementViews();
@@ -108,4 +109,53 @@
// no crash
ci.append(null);
}
+
+ @Test
+ public void testUpdateThresholds_streakLimitsCorrectlyApplied() {
+ int updatedStreakLimit = DEFAULT_STREAK_LIMIT + 3;
+ ChannelImpressions ci = new ChannelImpressions();
+ ci.updateThresholds(DEFAULT_DISMISS_TO_VIEW_RATIO_LIMIT, updatedStreakLimit);
+
+ for (int i = 0; i <= updatedStreakLimit; i++) {
+ ci.incrementViews();
+ ci.incrementDismissals();
+ }
+
+ ChannelImpressions ci2 = new ChannelImpressions();
+ ci2.updateThresholds(DEFAULT_DISMISS_TO_VIEW_RATIO_LIMIT, updatedStreakLimit);
+
+ for (int i = 0; i < updatedStreakLimit; i++) {
+ ci2.incrementViews();
+ ci2.incrementDismissals();
+ }
+
+ assertTrue(ci.shouldTriggerBlock());
+ assertFalse(ci2.shouldTriggerBlock());
+ }
+
+ @Test
+ public void testUpdateThresholds_ratioLimitsCorrectlyApplied() {
+ float updatedDismissRatio = .99f;
+ ChannelImpressions ci = new ChannelImpressions();
+ ci.updateThresholds(updatedDismissRatio, DEFAULT_STREAK_LIMIT);
+
+ // N views, N-1 dismissals, which doesn't satisfy the ratio = 1 criteria.
+ for (int i = 0; i <= DEFAULT_STREAK_LIMIT; i++) {
+ ci.incrementViews();
+ if (i != DEFAULT_STREAK_LIMIT) {
+ ci.incrementDismissals();
+ }
+ }
+
+ ChannelImpressions ci2 = new ChannelImpressions();
+ ci2.updateThresholds(updatedDismissRatio, DEFAULT_STREAK_LIMIT);
+
+ for (int i = 0; i <= DEFAULT_STREAK_LIMIT; i++) {
+ ci2.incrementViews();
+ ci2.incrementDismissals();
+ }
+
+ assertFalse(ci.shouldTriggerBlock());
+ assertTrue(ci2.shouldTriggerBlock());
+ }
}