From 12aeda802ed91a49977a22166319ce74a3352e30 Mon Sep 17 00:00:00 2001 From: Christoph Studer Date: Tue, 23 Sep 2014 19:08:56 +0200 Subject: [PATCH] NoMan: Add 3s timeout in matchesCallFilter() In Priority mode, NoMan can block the telecom Ringer indefinitely when looking up the caller contact details via ContactsProvider. Fix this by introducing a 3s timeout and assume the call matches the call filter when the timeout is hit. Bug: 17598245 Change-Id: Ia0893cf203e6ffd943c749719ebb9e0f0958eb01 --- .../NotificationManagerService.java | 18 ++++++- .../ValidateNotificationPeople.java | 47 +++++++++++++++++-- .../server/notification/ZenModeHelper.java | 11 ++++- 3 files changed, 68 insertions(+), 8 deletions(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 2829e2b7aa2c5..d8e5a982c44a3 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -154,6 +154,16 @@ public class NotificationManagerService extends SystemService { static final boolean ENABLE_BLOCKED_NOTIFICATIONS = true; static final boolean ENABLE_BLOCKED_TOASTS = true; + // When #matchesCallFilter is called from the ringer, wait at most + // 3s to resolve the contacts. This timeout is required since + // ContactsProvider might take a long time to start up. + // + // Return STARRED_CONTACT when the timeout is hit in order to avoid + // missed calls in ZEN mode "Important". + static final int MATCHES_CALL_FILTER_CONTACTS_TIMEOUT_MS = 3000; + static final float MATCHES_CALL_FILTER_TIMEOUT_AFFINITY = + ValidateNotificationPeople.STARRED_CONTACT; + private IActivityManager mAm; AudioManager mAudioManager; StatusBarManagerInternal mStatusBar; @@ -1474,8 +1484,12 @@ public class NotificationManagerService extends SystemService { @Override public boolean matchesCallFilter(Bundle extras) { enforceSystemOrSystemUI("INotificationManager.matchesCallFilter"); - return mZenModeHelper.matchesCallFilter(UserHandle.getCallingUserHandle(), extras, - mRankingHelper.findExtractor(ValidateNotificationPeople.class)); + return mZenModeHelper.matchesCallFilter( + UserHandle.getCallingUserHandle(), + extras, + mRankingHelper.findExtractor(ValidateNotificationPeople.class), + MATCHES_CALL_FILTER_CONTACTS_TIMEOUT_MS, + MATCHES_CALL_FILTER_TIMEOUT_AFFINITY); } }; diff --git a/services/core/java/com/android/server/notification/ValidateNotificationPeople.java b/services/core/java/com/android/server/notification/ValidateNotificationPeople.java index 1c1a034492b34..6e2a8ada32f00 100644 --- a/services/core/java/com/android/server/notification/ValidateNotificationPeople.java +++ b/services/core/java/com/android/server/notification/ValidateNotificationPeople.java @@ -22,6 +22,7 @@ import android.content.pm.PackageManager; import android.database.ContentObserver; import android.database.Cursor; import android.net.Uri; +import android.os.AsyncTask; import android.os.Bundle; import android.os.Handler; import android.os.UserHandle; @@ -37,6 +38,8 @@ import android.util.Slog; import java.util.ArrayList; import java.util.LinkedList; import java.util.Map; +import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; /** * This {@link NotificationSignalExtractor} attempts to validate @@ -45,9 +48,10 @@ import java.util.Map; * {@hide} */ public class ValidateNotificationPeople implements NotificationSignalExtractor { - private static final String TAG = "ValidateNotificationPeople"; + // Using a shorter log tag since setprop has a limit of 32chars on variable name. + private static final String TAG = "ValidateNoPeople"; private static final boolean INFO = true; - private static final boolean DEBUG = false; + private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); private static final boolean ENABLE_PEOPLE_VALIDATOR = true; private static final String SETTING_ENABLE_PEOPLE_VALIDATOR = @@ -132,7 +136,14 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { // ignore: config has no relevant information yet. } - public float getContactAffinity(UserHandle userHandle, Bundle extras) { + /** + * @param extras extras of the notification with EXTRA_PEOPLE populated + * @param timeoutMs timeout in milliseconds to wait for contacts response + * @param timeoutAffinity affinity to return when the timeout specified via + * timeoutMs is hit + */ + public float getContactAffinity(UserHandle userHandle, Bundle extras, int timeoutMs, + float timeoutAffinity) { if (DEBUG) Slog.d(TAG, "checking affinity for " + userHandle); if (extras == null) return NONE; final String key = Long.toString(System.nanoTime()); @@ -143,8 +154,31 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { } final PeopleRankingReconsideration prr = validatePeople(context, key, extras, affinityOut); float affinity = affinityOut[0]; + if (prr != null) { - prr.work(); + // Perform the heavy work on a background thread so we can abort when we hit the + // timeout. + final Semaphore s = new Semaphore(0); + AsyncTask.THREAD_POOL_EXECUTOR.execute(new Runnable() { + @Override + public void run() { + prr.work(); + s.release(); + } + }); + + try { + if (!s.tryAcquire(timeoutMs, TimeUnit.MILLISECONDS)) { + Slog.w(TAG, "Timeout while waiting for affinity: " + key + ". " + + "Returning timeoutAffinity=" + timeoutAffinity); + return timeoutAffinity; + } + } catch (InterruptedException e) { + Slog.w(TAG, "InterruptedException while waiting for affinity: " + key + ". " + + "Returning affinity=" + affinity, e); + return affinity; + } + affinity = Math.max(prr.getContactAffinity(), affinity); } return affinity; @@ -391,6 +425,7 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { @Override public void work() { if (INFO) Slog.i(TAG, "Executing: validation for: " + mKey); + long timeStartMs = System.currentTimeMillis(); for (final String handle: mPendingLookups) { LookupResult lookupResult = null; final Uri uri = Uri.parse(handle); @@ -415,6 +450,10 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { mContactAffinity = Math.max(mContactAffinity, lookupResult.getAffinity()); } } + if (DEBUG) { + Slog.d(TAG, "Validation finished in " + (System.currentTimeMillis() - timeStartMs) + + "ms"); + } } @Override diff --git a/services/core/java/com/android/server/notification/ZenModeHelper.java b/services/core/java/com/android/server/notification/ZenModeHelper.java index 5038d03a48af4..557a44e0cb7ba 100644 --- a/services/core/java/com/android/server/notification/ZenModeHelper.java +++ b/services/core/java/com/android/server/notification/ZenModeHelper.java @@ -376,14 +376,21 @@ public class ZenModeHelper { return record.isCategory(Notification.CATEGORY_MESSAGE) || isDefaultMessagingApp(record); } + /** + * @param extras extras of the notification with EXTRA_PEOPLE populated + * @param contactsTimeoutMs timeout in milliseconds to wait for contacts response + * @param timeoutAffinity affinity to return when the timeout specified via + * contactsTimeoutMs is hit + */ public boolean matchesCallFilter(UserHandle userHandle, Bundle extras, - ValidateNotificationPeople validator) { + ValidateNotificationPeople validator, int contactsTimeoutMs, float timeoutAffinity) { final int zen = mZenMode; if (zen == Global.ZEN_MODE_NO_INTERRUPTIONS) return false; // nothing gets through if (zen == Global.ZEN_MODE_IMPORTANT_INTERRUPTIONS) { if (!mConfig.allowCalls) return false; // no calls get through if (validator != null) { - final float contactAffinity = validator.getContactAffinity(userHandle, extras); + final float contactAffinity = validator.getContactAffinity(userHandle, extras, + contactsTimeoutMs, timeoutAffinity); return audienceMatches(contactAffinity); } }