diff options
8 files changed, 103 insertions, 23 deletions
diff --git a/src/com/android/providers/downloads/DownloadNotification.java b/src/com/android/providers/downloads/DownloadNotification.java index 3c0b7d9b..76bffbc6 100644 --- a/src/com/android/providers/downloads/DownloadNotification.java +++ b/src/com/android/providers/downloads/DownloadNotification.java @@ -17,7 +17,6 @@ package com.android.providers.downloads; import android.app.Notification; -import android.app.NotificationManager; import android.app.PendingIntent; import android.content.Context; import android.content.Intent; @@ -38,8 +37,8 @@ import java.util.HashMap; class DownloadNotification { Context mContext; - public NotificationManager mNotificationMgr; HashMap <String, NotificationItem> mNotifications; + private SystemFacade mSystemFacade; static final String LOGTAG = "DownloadNotification"; static final String WHERE_RUNNING = @@ -93,10 +92,9 @@ class DownloadNotification { * @param ctx The context to use to obtain access to the * Notification Service */ - DownloadNotification(Context ctx) { + DownloadNotification(Context ctx, SystemFacade systemFacade) { mContext = ctx; - mNotificationMgr = (NotificationManager) mContext - .getSystemService(Context.NOTIFICATION_SERVICE); + mSystemFacade = systemFacade; mNotifications = new HashMap<String, NotificationItem>(); } @@ -208,7 +206,7 @@ class DownloadNotification { n.contentIntent = PendingIntent.getBroadcast(mContext, 0, intent, 0); - mNotificationMgr.notify(item.mId, n); + mSystemFacade.postNotification(item.mId, n); } } @@ -287,7 +285,7 @@ class DownloadNotification { intent.setData(contentUri); n.deleteIntent = PendingIntent.getBroadcast(mContext, 0, intent, 0); - mNotificationMgr.notify(c.getInt(idColumn), n); + mSystemFacade.postNotification(c.getInt(idColumn), n); } c.close(); } diff --git a/src/com/android/providers/downloads/DownloadReceiver.java b/src/com/android/providers/downloads/DownloadReceiver.java index 98c37103..852c3712 100644 --- a/src/com/android/providers/downloads/DownloadReceiver.java +++ b/src/com/android/providers/downloads/DownloadReceiver.java @@ -16,9 +16,6 @@ package com.android.providers.downloads; -import com.google.common.annotations.VisibleForTesting; - -import android.app.NotificationManager; import android.content.ActivityNotFoundException; import android.content.BroadcastReceiver; import android.content.ContentUris; @@ -34,6 +31,8 @@ import android.provider.Downloads; import android.util.Config; import android.util.Log; +import com.google.common.annotations.VisibleForTesting; + import java.io.File; /** @@ -164,11 +163,7 @@ public class DownloadReceiver extends BroadcastReceiver { } cursor.close(); } - NotificationManager notMgr = (NotificationManager) context - .getSystemService(Context.NOTIFICATION_SERVICE); - if (notMgr != null) { - notMgr.cancel((int) ContentUris.parseId(intent.getData())); - } + mSystemFacade.cancelNotification((int) ContentUris.parseId(intent.getData())); } else if (intent.getAction().equals(Constants.ACTION_HIDE)) { if (Constants.LOGVV) { Log.v(Constants.TAG, "Receiver hide for " + intent.getData()); diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java index 472b4c5f..a6175953 100644 --- a/src/com/android/providers/downloads/DownloadService.java +++ b/src/com/android/providers/downloads/DownloadService.java @@ -218,8 +218,8 @@ public class DownloadService extends Service { mMediaScannerConnecting = false; mMediaScannerConnection = new MediaScannerConnection(); - mNotifier = new DownloadNotification(this); - mNotifier.mNotificationMgr.cancelAll(); + mNotifier = new DownloadNotification(this, mSystemFacade); + mSystemFacade.cancelAllNotifications(); mNotifier.updateNotification(); trimDatabase(); @@ -641,7 +641,7 @@ public class DownloadService extends Service { if (info.mVisibility == Downloads.Impl.VISIBILITY_VISIBLE_NOTIFY_COMPLETED && newVisibility != Downloads.Impl.VISIBILITY_VISIBLE_NOTIFY_COMPLETED && Downloads.Impl.isStatusCompleted(info.mStatus)) { - mNotifier.mNotificationMgr.cancel(info.mId); + mSystemFacade.cancelNotification(info.mId); } info.mVisibility = newVisibility; synchronized (info) { @@ -651,7 +651,7 @@ public class DownloadService extends Service { int newStatus = cursor.getInt(statusColumn); if (!Downloads.Impl.isStatusCompleted(info.mStatus) && Downloads.Impl.isStatusCompleted(newStatus)) { - mNotifier.mNotificationMgr.cancel(info.mId); + mSystemFacade.cancelNotification(info.mId); } info.mStatus = newStatus; info.mNumFailed = cursor.getInt(failedColumn); @@ -724,7 +724,7 @@ public class DownloadService extends Service { && info.mFileName != null) { new File(info.mFileName).delete(); } - mNotifier.mNotificationMgr.cancel(info.mId); + mSystemFacade.cancelNotification(info.mId); mDownloads.remove(arrayPos); } diff --git a/src/com/android/providers/downloads/RealSystemFacade.java b/src/com/android/providers/downloads/RealSystemFacade.java index f89f1659..1d9e64a9 100644 --- a/src/com/android/providers/downloads/RealSystemFacade.java +++ b/src/com/android/providers/downloads/RealSystemFacade.java @@ -1,5 +1,7 @@ package com.android.providers.downloads; +import android.app.Notification; +import android.app.NotificationManager; import android.content.Context; import android.content.Intent; import android.content.pm.PackageManager.NameNotFoundException; @@ -10,9 +12,12 @@ import android.util.Log; class RealSystemFacade implements SystemFacade { private Context mContext; + private NotificationManager mNotificationManager; public RealSystemFacade(Context context) { mContext = context; + mNotificationManager = (NotificationManager) + mContext.getSystemService(Context.NOTIFICATION_SERVICE); } public long currentTimeMillis() { @@ -67,4 +72,19 @@ class RealSystemFacade implements SystemFacade { public boolean userOwnsPackage(int uid, String packageName) throws NameNotFoundException { return mContext.getPackageManager().getApplicationInfo(packageName, 0).uid == uid; } + + @Override + public void postNotification(int id, Notification notification) { + mNotificationManager.notify(id, notification); + } + + @Override + public void cancelNotification(int id) { + mNotificationManager.cancel(id); + } + + @Override + public void cancelAllNotifications() { + mNotificationManager.cancelAll(); + } } diff --git a/src/com/android/providers/downloads/SystemFacade.java b/src/com/android/providers/downloads/SystemFacade.java index d48e6b80..e41644ab 100644 --- a/src/com/android/providers/downloads/SystemFacade.java +++ b/src/com/android/providers/downloads/SystemFacade.java @@ -1,6 +1,7 @@ package com.android.providers.downloads; +import android.app.Notification; import android.content.Intent; import android.content.pm.PackageManager.NameNotFoundException; @@ -37,4 +38,19 @@ interface SystemFacade { * Returns true if the specified UID owns the specified package name. */ public boolean userOwnsPackage(int uid, String pckg) throws NameNotFoundException; + + /** + * Post a system notification to the NotificationManager. + */ + public void postNotification(int id, Notification notification); + + /** + * Cancel a system notification. + */ + public void cancelNotification(int id); + + /** + * Cancel all system notifications. + */ + public void cancelAllNotifications(); } diff --git a/tests/src/com/android/providers/downloads/AbstractDownloadManagerFunctionalTest.java b/tests/src/com/android/providers/downloads/AbstractDownloadManagerFunctionalTest.java index cb4ad8c9..a401a5b8 100644 --- a/tests/src/com/android/providers/downloads/AbstractDownloadManagerFunctionalTest.java +++ b/tests/src/com/android/providers/downloads/AbstractDownloadManagerFunctionalTest.java @@ -165,12 +165,12 @@ public abstract class AbstractDownloadManagerFunctionalTest extends @Override protected void tearDown() throws Exception { - waitForUpdateThread(); + waitForThreads(); cleanUpDownloads(); super.tearDown(); } - private void waitForUpdateThread() throws InterruptedException { + private void waitForThreads() throws InterruptedException { DownloadService service = getService(); if (service == null) { return; @@ -181,6 +181,10 @@ public abstract class AbstractDownloadManagerFunctionalTest extends && System.currentTimeMillis() < startTimeMillis + 1000) { Thread.sleep(50); } + + // We can't explicitly wait for DownloadThreads, so just throw this last sleep in. Ugly, + // but necessary to avoid unbearable flakiness until I can find a better solution. + Thread.sleep(50); } private boolean isDatabaseEmpty() { @@ -289,7 +293,7 @@ public abstract class AbstractDownloadManagerFunctionalTest extends status = reader.getStatus(); } - long delta = startTimeMillis - startTimeMillis; + long delta = System.currentTimeMillis() - startTimeMillis; Log.d(LOG_TAG, "Status " + status + " reached after " + delta + "ms"); } diff --git a/tests/src/com/android/providers/downloads/FakeSystemFacade.java b/tests/src/com/android/providers/downloads/FakeSystemFacade.java index 0f8a9801..d35b558b 100644 --- a/tests/src/com/android/providers/downloads/FakeSystemFacade.java +++ b/tests/src/com/android/providers/downloads/FakeSystemFacade.java @@ -1,11 +1,15 @@ package com.android.providers.downloads; +import android.app.Notification; import android.content.Intent; import android.content.pm.PackageManager.NameNotFoundException; import android.net.ConnectivityManager; +import android.test.AssertionFailedError; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; public class FakeSystemFacade implements SystemFacade { long mTimeMillis = 0; @@ -13,6 +17,8 @@ public class FakeSystemFacade implements SystemFacade { boolean mIsRoaming = false; Integer mMaxBytesOverMobile = null; List<Intent> mBroadcastsSent = new ArrayList<Intent>(); + Map<Integer,Notification> mActiveNotifications = new HashMap<Integer,Notification>(); + List<Notification> mCanceledNotifications = new ArrayList<Notification>(); void incrementTimeMillis(long delta) { mTimeMillis += delta; @@ -43,4 +49,27 @@ public class FakeSystemFacade implements SystemFacade { public boolean userOwnsPackage(int uid, String pckg) throws NameNotFoundException { return true; } + + @Override + public void postNotification(int id, Notification notification) { + if (notification == null) { + throw new AssertionFailedError("Posting null notification"); + } + mActiveNotifications.put(id, notification); + } + + @Override + public void cancelNotification(int id) { + Notification notification = mActiveNotifications.remove(id); + if (notification != null) { + mCanceledNotifications.add(notification); + } + } + + @Override + public void cancelAllNotifications() { + for (int id : mActiveNotifications.keySet()) { + cancelNotification(id); + } + } } diff --git a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java index 3d32ae3c..00419a3d 100644 --- a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java +++ b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java @@ -485,6 +485,24 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe assertTrue(mResolver.mNotifyWasCalled); } + public void testNotifications() throws Exception { + enqueueEmptyResponse(HTTP_OK); + Download download = enqueueRequest(getRequest()); // no visibility requested + download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); + assertEquals(0, mSystemFacade.mActiveNotifications.size()); + assertEquals(0, mSystemFacade.mCanceledNotifications.size()); + + enqueueEmptyResponse(HTTP_OK); + download = enqueueRequest( + getRequest() + .setShowNotification(DownloadManager.Request.NOTIFICATION_WHEN_RUNNING)); + download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); + assertEquals(1, mSystemFacade.mActiveNotifications.size()); + // The notification doesn't actually get canceled until the UpdateThread runs again, which + // gets triggered by the DownloadThread updating the status in the provider. This is + // tough to test right now, so I'll leave it until the overall structure is changed. + } + private void runSimpleFailureTest(int expectedErrorCode) throws Exception { Download download = enqueueRequest(getRequest()); download.runUntilStatus(DownloadManager.STATUS_FAILED); |