Merge "Lock state of NotificationComparator while sorting in RankingHelper" into udc-qpr-dev
This commit is contained in:
committed by
Android (Google) Code Review
commit
6e5d4c6524
@@ -16,6 +16,7 @@
|
||||
|
||||
package com.android.internal.util;
|
||||
|
||||
import android.annotation.Nullable;
|
||||
import android.app.Notification;
|
||||
import android.app.NotificationManager;
|
||||
import android.content.Context;
|
||||
@@ -39,10 +40,12 @@ public class NotificationMessagingUtil {
|
||||
|
||||
private static final String DEFAULT_SMS_APP_SETTING = Settings.Secure.SMS_DEFAULT_APPLICATION;
|
||||
private final Context mContext;
|
||||
private SparseArray<String> mDefaultSmsApp = new SparseArray<>();
|
||||
private final SparseArray<String> mDefaultSmsApp = new SparseArray<>();
|
||||
private final Object mStateLock;
|
||||
|
||||
public NotificationMessagingUtil(Context context) {
|
||||
public NotificationMessagingUtil(Context context, @Nullable Object stateLock) {
|
||||
mContext = context;
|
||||
mStateLock = stateLock != null ? stateLock : new Object();
|
||||
mContext.getContentResolver().registerContentObserver(
|
||||
Settings.Secure.getUriFor(DEFAULT_SMS_APP_SETTING), false, mSmsContentObserver);
|
||||
}
|
||||
@@ -63,16 +66,20 @@ public class NotificationMessagingUtil {
|
||||
private boolean isDefaultMessagingApp(StatusBarNotification sbn) {
|
||||
final int userId = sbn.getUserId();
|
||||
if (userId == UserHandle.USER_NULL || userId == UserHandle.USER_ALL) return false;
|
||||
if (mDefaultSmsApp.get(userId) == null) {
|
||||
cacheDefaultSmsApp(userId);
|
||||
synchronized (mStateLock) {
|
||||
if (mDefaultSmsApp.get(userId) == null) {
|
||||
cacheDefaultSmsApp(userId);
|
||||
}
|
||||
return Objects.equals(mDefaultSmsApp.get(userId), sbn.getPackageName());
|
||||
}
|
||||
return Objects.equals(mDefaultSmsApp.get(userId), sbn.getPackageName());
|
||||
}
|
||||
|
||||
private void cacheDefaultSmsApp(int userId) {
|
||||
mDefaultSmsApp.put(userId, Settings.Secure.getStringForUser(
|
||||
mContext.getContentResolver(),
|
||||
Settings.Secure.SMS_DEFAULT_APPLICATION, userId));
|
||||
String smsApp = Settings.Secure.getStringForUser(mContext.getContentResolver(),
|
||||
Settings.Secure.SMS_DEFAULT_APPLICATION, userId);
|
||||
synchronized (mStateLock) {
|
||||
mDefaultSmsApp.put(userId, smsApp);
|
||||
}
|
||||
}
|
||||
|
||||
private final ContentObserver mSmsContentObserver = new ContentObserver(
|
||||
|
||||
@@ -24,11 +24,11 @@ import com.android.internal.logging.UiEventLoggerImpl;
|
||||
import com.android.internal.util.NotificationMessagingUtil;
|
||||
import com.android.internal.widget.LockPatternUtils;
|
||||
|
||||
import javax.inject.Singleton;
|
||||
|
||||
import dagger.Module;
|
||||
import dagger.Provides;
|
||||
|
||||
import javax.inject.Singleton;
|
||||
|
||||
/**
|
||||
* Provides items imported from com.android.internal.
|
||||
*/
|
||||
@@ -51,7 +51,7 @@ public class AndroidInternalsModule {
|
||||
/** */
|
||||
@Provides
|
||||
public NotificationMessagingUtil provideNotificationMessagingUtil(Context context) {
|
||||
return new NotificationMessagingUtil(context);
|
||||
return new NotificationMessagingUtil(context, null);
|
||||
}
|
||||
|
||||
/** Provides an instance of {@link com.android.internal.logging.UiEventLogger} */
|
||||
|
||||
@@ -25,6 +25,7 @@ import android.content.Intent;
|
||||
import android.content.IntentFilter;
|
||||
import android.telecom.TelecomManager;
|
||||
|
||||
import com.android.internal.os.BackgroundThread;
|
||||
import com.android.internal.util.NotificationMessagingUtil;
|
||||
|
||||
import java.util.Comparator;
|
||||
@@ -33,18 +34,23 @@ import java.util.Objects;
|
||||
/**
|
||||
* Sorts notifications individually into attention-relevant order.
|
||||
*/
|
||||
public class NotificationComparator
|
||||
implements Comparator<NotificationRecord> {
|
||||
class NotificationComparator implements Comparator<NotificationRecord> {
|
||||
|
||||
private final Context mContext;
|
||||
private final NotificationMessagingUtil mMessagingUtil;
|
||||
private String mDefaultPhoneApp;
|
||||
|
||||
/**
|
||||
* Lock that must be held during a sort() call that uses this {@link Comparator}, AND to make
|
||||
* any changes to the state of this object that could affect the results of {@link #compare}.
|
||||
*/
|
||||
public final Object mStateLock = new Object();
|
||||
|
||||
public NotificationComparator(Context context) {
|
||||
mContext = context;
|
||||
mContext.registerReceiver(mPhoneAppBroadcastReceiver,
|
||||
new IntentFilter(TelecomManager.ACTION_DEFAULT_DIALER_CHANGED));
|
||||
mMessagingUtil = new NotificationMessagingUtil(mContext);
|
||||
mMessagingUtil = new NotificationMessagingUtil(mContext, mStateLock);
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -212,8 +218,13 @@ public class NotificationComparator
|
||||
private final BroadcastReceiver mPhoneAppBroadcastReceiver = new BroadcastReceiver() {
|
||||
@Override
|
||||
public void onReceive(Context context, Intent intent) {
|
||||
mDefaultPhoneApp =
|
||||
intent.getStringExtra(TelecomManager.EXTRA_CHANGE_DEFAULT_DIALER_PACKAGE_NAME);
|
||||
BackgroundThread.getExecutor().execute(() -> {
|
||||
synchronized (mStateLock) {
|
||||
mDefaultPhoneApp =
|
||||
intent.getStringExtra(
|
||||
TelecomManager.EXTRA_CHANGE_DEFAULT_DIALER_PACKAGE_NAME);
|
||||
}
|
||||
});
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
@@ -103,8 +103,11 @@ public class RankingHelper {
|
||||
notificationList.get(i).setGlobalSortKey(null);
|
||||
}
|
||||
|
||||
// rank each record individually
|
||||
Collections.sort(notificationList, mPreliminaryComparator);
|
||||
// Rank each record individually.
|
||||
// Lock comparator state for consistent compare() results.
|
||||
synchronized (mPreliminaryComparator.mStateLock) {
|
||||
notificationList.sort(mPreliminaryComparator);
|
||||
}
|
||||
|
||||
synchronized (mProxyByGroupTmp) {
|
||||
// record individual ranking result and nominate proxies for each group
|
||||
|
||||
@@ -55,7 +55,7 @@ public class ZenModeFiltering {
|
||||
|
||||
public ZenModeFiltering(Context context) {
|
||||
mContext = context;
|
||||
mMessagingUtil = new NotificationMessagingUtil(mContext);
|
||||
mMessagingUtil = new NotificationMessagingUtil(mContext, null);
|
||||
}
|
||||
|
||||
public ZenModeFiltering(Context context, NotificationMessagingUtil messagingUtil) {
|
||||
|
||||
@@ -15,15 +15,20 @@
|
||||
*/
|
||||
package com.android.server.notification;
|
||||
|
||||
import static android.telecom.TelecomManager.EXTRA_CHANGE_DEFAULT_DIALER_PACKAGE_NAME;
|
||||
|
||||
import static com.google.common.truth.Truth.assertThat;
|
||||
import static com.google.common.truth.Truth.assertWithMessage;
|
||||
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.Matchers.anyInt;
|
||||
import static org.mockito.Matchers.anyString;
|
||||
import static org.mockito.Matchers.eq;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.spy;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
import android.app.Notification;
|
||||
@@ -31,8 +36,10 @@ import android.app.NotificationChannel;
|
||||
import android.app.NotificationManager;
|
||||
import android.app.PendingIntent;
|
||||
import android.app.Person;
|
||||
import android.content.BroadcastReceiver;
|
||||
import android.content.ContentResolver;
|
||||
import android.content.Context;
|
||||
import android.content.Intent;
|
||||
import android.content.pm.ApplicationInfo;
|
||||
import android.content.pm.PackageManager;
|
||||
import android.content.res.Resources;
|
||||
@@ -54,12 +61,14 @@ import org.junit.After;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
import org.mockito.ArgumentCaptor;
|
||||
import org.mockito.Mock;
|
||||
import org.mockito.MockitoAnnotations;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.concurrent.CountDownLatch;
|
||||
|
||||
@SmallTest
|
||||
@RunWith(AndroidJUnit4.class)
|
||||
@@ -335,6 +344,73 @@ public class NotificationComparatorTest extends UiServiceTestCase {
|
||||
assertTrue(comp.isImportantPeople(mRecordContact));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testChangeDialerPackageWhileSorting() throws InterruptedException {
|
||||
final int halfList = 100;
|
||||
int userId = UserHandle.myUserId();
|
||||
when(mTm.getDefaultDialerPackage()).thenReturn("B");
|
||||
|
||||
ArgumentCaptor<BroadcastReceiver> broadcastReceiverCaptor = ArgumentCaptor.forClass(
|
||||
BroadcastReceiver.class);
|
||||
NotificationComparator comparator = new NotificationComparator(mMockContext);
|
||||
verify(mMockContext).registerReceiver(broadcastReceiverCaptor.capture(), any());
|
||||
BroadcastReceiver dialerChangedBroadcastReceiver = broadcastReceiverCaptor.getValue();
|
||||
|
||||
ArrayList<NotificationRecord> records = new ArrayList<>();
|
||||
for (int i = 0; i < halfList; i++) {
|
||||
Notification notifCallFromPkgA = new Notification.Builder(mMockContext, TEST_CHANNEL_ID)
|
||||
.setCategory(Notification.CATEGORY_CALL)
|
||||
.setFlag(Notification.FLAG_FOREGROUND_SERVICE, true)
|
||||
.build();
|
||||
records.add(new NotificationRecord(mMockContext,
|
||||
new StatusBarNotification("A", "A", 2 * i, "callA", callUid, callUid,
|
||||
notifCallFromPkgA, new UserHandle(userId), "", 0),
|
||||
getDefaultChannel()));
|
||||
|
||||
Notification notifCallFromPkgB = new Notification.Builder(mMockContext, TEST_CHANNEL_ID)
|
||||
.setCategory(Notification.CATEGORY_CALL)
|
||||
.setFlag(Notification.FLAG_FOREGROUND_SERVICE, true)
|
||||
.build();
|
||||
records.add(new NotificationRecord(mMockContext,
|
||||
new StatusBarNotification("B", "B", 2 * i + 1, "callB", callUid, callUid,
|
||||
notifCallFromPkgB, new UserHandle(userId), "", 0),
|
||||
getDefaultChannel()));
|
||||
}
|
||||
|
||||
CountDownLatch allDone = new CountDownLatch(2);
|
||||
new Thread(() -> {
|
||||
// The lock prevents the other thread from changing the dialer package mid-sort, so:
|
||||
// 1) Results should be "all B before all A" (asserted below).
|
||||
// 2) No "IllegalArgumentException: Comparison method violates its general contract!"
|
||||
synchronized (comparator.mStateLock) {
|
||||
records.sort(comparator);
|
||||
allDone.countDown();
|
||||
}
|
||||
}).start();
|
||||
|
||||
new Thread(() -> {
|
||||
String nextDialer = "A";
|
||||
while (allDone.getCount() == 2) {
|
||||
Intent dialerChangedIntent = new Intent();
|
||||
dialerChangedIntent.putExtra(EXTRA_CHANGE_DEFAULT_DIALER_PACKAGE_NAME, nextDialer);
|
||||
dialerChangedBroadcastReceiver.onReceive(mMockContext, dialerChangedIntent);
|
||||
nextDialer = nextDialer.equals("A") ? "B" : "A";
|
||||
}
|
||||
allDone.countDown();
|
||||
}).start();
|
||||
|
||||
allDone.await();
|
||||
|
||||
for (int i = 0; i < halfList; i++) {
|
||||
assertWithMessage("Wrong element in position #" + i)
|
||||
.that(records.get(i).getSbn().getPackageName()).isEqualTo("B");
|
||||
}
|
||||
for (int i = halfList; i < 2 * halfList; i++) {
|
||||
assertWithMessage("Wrong element in position #" + i)
|
||||
.that(records.get(i).getSbn().getPackageName()).isEqualTo("A");
|
||||
}
|
||||
}
|
||||
|
||||
private NotificationChannel getDefaultChannel() {
|
||||
return new NotificationChannel(NotificationChannel.DEFAULT_CHANNEL_ID, "name",
|
||||
NotificationManager.IMPORTANCE_LOW);
|
||||
|
||||
Reference in New Issue
Block a user