diff options
12 files changed, 84 insertions, 127 deletions
diff --git a/src/com/android/providers/downloads/DownloadHandler.java b/src/com/android/providers/downloads/DownloadHandler.java index 2f02864e..c376ff1e 100644 --- a/src/com/android/providers/downloads/DownloadHandler.java +++ b/src/com/android/providers/downloads/DownloadHandler.java @@ -96,28 +96,4 @@ public class DownloadHandler { public synchronized long getCurrentSpeed(long id) { return mCurrentSpeed.get(id, -1L); } - - // right now this is only used by tests. but there is no reason why it can't be used - // by any module using DownloadManager (TODO add API to DownloadManager.java) - public synchronized void waitUntilDownloadsTerminate() throws InterruptedException { - if (mDownloadsInProgress.size() == 0 && mDownloadsQueue.size() == 0) { - if (Constants.LOGVV) { - Log.i(TAG, "nothing to wait on"); - } - return; - } - if (Constants.LOGVV) { - for (DownloadInfo info : mDownloadsInProgress.values()) { - Log.i(TAG, "** progress: " + info.mId + ", " + info.mUri); - } - for (DownloadInfo info : mDownloadsQueue.values()) { - Log.i(TAG, "** in Q: " + info.mId + ", " + info.mUri); - } - } - if (Constants.LOGVV) { - Log.i(TAG, "waiting for 5 sec"); - } - // wait upto 5 sec - wait(5 * 1000); - } } diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java index 2ea7d84d..e64db289 100644 --- a/src/com/android/providers/downloads/DownloadInfo.java +++ b/src/com/android/providers/downloads/DownloadInfo.java @@ -575,9 +575,8 @@ public class DownloadInfo { } void startDownloadThread() { - DownloadThread downloader = new DownloadThread(mContext, mSystemFacade, this, - mStorageManager); - mSystemFacade.startThread(downloader); + // TODO: keep this thread strongly referenced + new DownloadThread(mContext, mSystemFacade, this, mStorageManager).start(); } /** diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java index e0fe4c55..4a1b40d5 100644 --- a/src/com/android/providers/downloads/DownloadService.java +++ b/src/com/android/providers/downloads/DownloadService.java @@ -259,7 +259,7 @@ public class DownloadService extends Service { mPendingUpdate = true; if (mUpdateThread == null) { mUpdateThread = new UpdateThread(); - mSystemFacade.startThread(mUpdateThread); + mUpdateThread.start(); } } } diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java index eb59d3f7..ae279260 100644 --- a/src/com/android/providers/downloads/DownloadThread.java +++ b/src/com/android/providers/downloads/DownloadThread.java @@ -261,10 +261,9 @@ public class DownloadThread extends Thread { try { // Asking for response code will execute the request final int statusCode = conn.getResponseCode(); - in = conn.getInputStream(); - handleExceptionalStatus(state, conn, statusCode); processResponseHeaders(state, conn); + in = conn.getInputStream(); } catch (IOException e) { throw new StopRequestException( getFinalStatusForHttpError(state), "Request failed: " + e, e); diff --git a/src/com/android/providers/downloads/RealSystemFacade.java b/src/com/android/providers/downloads/RealSystemFacade.java index 228c7165..fa4f3488 100644 --- a/src/com/android/providers/downloads/RealSystemFacade.java +++ b/src/com/android/providers/downloads/RealSystemFacade.java @@ -32,10 +32,12 @@ class RealSystemFacade implements SystemFacade { mContext = context; } + @Override public long currentTimeMillis() { return System.currentTimeMillis(); } + @Override public NetworkInfo getActiveNetworkInfo(int uid) { ConnectivityManager connectivity = (ConnectivityManager) mContext.getSystemService(Context.CONNECTIVITY_SERVICE); @@ -57,6 +59,7 @@ class RealSystemFacade implements SystemFacade { return conn.isActiveNetworkMetered(); } + @Override public boolean isNetworkRoaming() { ConnectivityManager connectivity = (ConnectivityManager) mContext.getSystemService(Context.CONNECTIVITY_SERVICE); @@ -74,6 +77,7 @@ class RealSystemFacade implements SystemFacade { return isRoaming; } + @Override public Long getMaxBytesOverMobile() { return DownloadManager.getMaxBytesOverMobile(mContext); } @@ -92,9 +96,4 @@ class RealSystemFacade implements SystemFacade { public boolean userOwnsPackage(int uid, String packageName) throws NameNotFoundException { return mContext.getPackageManager().getApplicationInfo(packageName, 0).uid == uid; } - - @Override - public void startThread(Thread thread) { - thread.start(); - } } diff --git a/src/com/android/providers/downloads/SystemFacade.java b/src/com/android/providers/downloads/SystemFacade.java index fda97e08..15fc31f9 100644 --- a/src/com/android/providers/downloads/SystemFacade.java +++ b/src/com/android/providers/downloads/SystemFacade.java @@ -61,9 +61,4 @@ interface SystemFacade { * Returns true if the specified UID owns the specified package name. */ public boolean userOwnsPackage(int uid, String pckg) throws NameNotFoundException; - - /** - * Start a thread. - */ - public void startThread(Thread thread); } diff --git a/tests/src/com/android/providers/downloads/AbstractDownloadProviderFunctionalTest.java b/tests/src/com/android/providers/downloads/AbstractDownloadProviderFunctionalTest.java index a65693fa..48eb0b47 100644 --- a/tests/src/com/android/providers/downloads/AbstractDownloadProviderFunctionalTest.java +++ b/tests/src/com/android/providers/downloads/AbstractDownloadProviderFunctionalTest.java @@ -161,6 +161,7 @@ public abstract class AbstractDownloadProviderFunctionalTest extends setContext(mTestContext); setupService(); getService().mSystemFacade = mSystemFacade; + mSystemFacade.setUp(); assertTrue(isDatabaseEmpty()); // ensure we're not messing with real data mServer = new MockWebServer(); mServer.play(); @@ -246,11 +247,6 @@ public abstract class AbstractDownloadProviderFunctionalTest extends return mServer.getUrl(path).toString(); } - public void runService() throws Exception { - startService(null); - mSystemFacade.runAllThreads(); - } - protected String readStream(InputStream inputStream) throws IOException { BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream)); try { diff --git a/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java b/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java index cda607aa..12c04f6a 100644 --- a/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java +++ b/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java @@ -16,17 +16,22 @@ package com.android.providers.downloads; +import static android.app.DownloadManager.STATUS_FAILED; +import static android.app.DownloadManager.STATUS_SUCCESSFUL; +import static android.text.format.DateUtils.SECOND_IN_MILLIS; + import android.app.DownloadManager; import android.database.Cursor; import android.net.Uri; import android.os.ParcelFileDescriptor; -import android.provider.Downloads; +import android.os.SystemClock; import android.util.Log; import java.io.FileInputStream; import java.io.InputStream; import java.net.MalformedURLException; import java.net.UnknownHostException; +import java.util.concurrent.TimeoutException; /** * Code common to tests that use the download manager public API. @@ -94,9 +99,28 @@ public abstract class AbstractPublicApiTest extends AbstractDownloadProviderFunc } } - void runUntilStatus(int status) throws Exception { - runService(); - assertEquals(status, getStatus()); + void runUntilStatus(int status) throws TimeoutException { + startService(null); + waitForStatus(status); + } + + void waitForStatus(int expected) throws TimeoutException { + int actual = -1; + + final long timeout = SystemClock.elapsedRealtime() + (15 * SECOND_IN_MILLIS); + while (SystemClock.elapsedRealtime() < timeout) { + actual = getStatus(); + if (actual == STATUS_SUCCESSFUL || actual == STATUS_FAILED) { + assertEquals(expected, actual); + return; + } else if (actual == expected) { + return; + } + + SystemClock.sleep(100); + } + + throw new TimeoutException("Expected status " + expected + "; only reached " + actual); } // max time to wait before giving up on the current download operation. @@ -105,22 +129,10 @@ public abstract class AbstractPublicApiTest extends AbstractDownloadProviderFunc // download thread private static final int TIME_TO_SLEEP = 1000; - int runUntilDone() throws InterruptedException { - int sleepCounter = MAX_TIME_TO_WAIT_FOR_OPERATION * 1000 / TIME_TO_SLEEP; - for (int i = 0; i < sleepCounter; i++) { - int status = getStatusIfExists(); - if (status == -1 || Downloads.Impl.isStatusCompleted(getStatus())) { - // row doesn't exist or the download is done - return status; - } - // download not done yet. sleep a while and try again - Thread.sleep(TIME_TO_SLEEP); - } - return 0; // failed - } - // waits until progress_so_far is >= (progress)% boolean runUntilProgress(int progress) throws InterruptedException { + startService(null); + int sleepCounter = MAX_TIME_TO_WAIT_FOR_OPERATION * 1000 / TIME_TO_SLEEP; int numBytesReceivedSoFar = 0; int totalBytes = 0; diff --git a/tests/src/com/android/providers/downloads/DownloadProviderFunctionalTest.java b/tests/src/com/android/providers/downloads/DownloadProviderFunctionalTest.java index 23d300f8..d5249378 100644 --- a/tests/src/com/android/providers/downloads/DownloadProviderFunctionalTest.java +++ b/tests/src/com/android/providers/downloads/DownloadProviderFunctionalTest.java @@ -16,14 +16,16 @@ package com.android.providers.downloads; +import static android.text.format.DateUtils.SECOND_IN_MILLIS; + import android.content.ContentValues; import android.database.Cursor; import android.net.ConnectivityManager; import android.net.Uri; import android.os.Environment; +import android.os.SystemClock; import android.provider.Downloads; import android.test.suitebuilder.annotation.LargeTest; -import android.util.Log; import com.google.mockwebserver.MockWebServer; import com.google.mockwebserver.RecordedRequest; @@ -31,6 +33,7 @@ import com.google.mockwebserver.RecordedRequest; import java.io.InputStream; import java.net.MalformedURLException; import java.net.UnknownHostException; +import java.util.concurrent.TimeoutException; /** * This test exercises the entire download manager working together -- it requests downloads through @@ -109,20 +112,22 @@ public class DownloadProviderFunctionalTest extends AbstractDownloadProviderFunc } } - private void runUntilStatus(Uri downloadUri, int status) throws Exception { - runService(); - boolean done = false; - while (!done) { - int rslt = getDownloadStatus(downloadUri); - if (rslt == Downloads.Impl.STATUS_RUNNING || rslt == Downloads.Impl.STATUS_PENDING) { - Log.i(TAG, "status is: " + rslt + ", for: " + downloadUri); - DownloadHandler.getInstance().waitUntilDownloadsTerminate(); - Thread.sleep(100); - } else { - done = true; + private void runUntilStatus(Uri downloadUri, int expected) throws Exception { + startService(null); + + int actual = -1; + + final long timeout = SystemClock.elapsedRealtime() + (15 * SECOND_IN_MILLIS); + while (SystemClock.elapsedRealtime() < timeout) { + actual = getDownloadStatus(downloadUri); + if (expected == actual) { + return; } + + SystemClock.sleep(100); } - assertEquals(status, getDownloadStatus(downloadUri)); + + throw new TimeoutException("Expected status " + expected + "; only reached " + actual); } protected int getDownloadStatus(Uri downloadUri) { diff --git a/tests/src/com/android/providers/downloads/FakeSystemFacade.java b/tests/src/com/android/providers/downloads/FakeSystemFacade.java index 481b5cba..d54c1224 100644 --- a/tests/src/com/android/providers/downloads/FakeSystemFacade.java +++ b/tests/src/com/android/providers/downloads/FakeSystemFacade.java @@ -7,10 +7,7 @@ import android.net.NetworkInfo; import android.net.NetworkInfo.DetailedState; import java.util.ArrayList; -import java.util.LinkedList; import java.util.List; -import java.util.Queue; - public class FakeSystemFacade implements SystemFacade { long mTimeMillis = 0; Integer mActiveNetworkType = ConnectivityManager.TYPE_WIFI; @@ -19,20 +16,32 @@ public class FakeSystemFacade implements SystemFacade { Long mMaxBytesOverMobile = null; Long mRecommendedMaxBytesOverMobile = null; List<Intent> mBroadcastsSent = new ArrayList<Intent>(); - Queue<Thread> mStartedThreads = new LinkedList<Thread>(); - private boolean returnActualTime = false; + private boolean mReturnActualTime = false; + + public void setUp() { + mTimeMillis = 0; + mActiveNetworkType = ConnectivityManager.TYPE_WIFI; + mIsRoaming = false; + mIsMetered = false; + mMaxBytesOverMobile = null; + mRecommendedMaxBytesOverMobile = null; + mBroadcastsSent.clear(); + mReturnActualTime = false; + } void incrementTimeMillis(long delta) { mTimeMillis += delta; } + @Override public long currentTimeMillis() { - if (returnActualTime) { + if (mReturnActualTime) { return System.currentTimeMillis(); } return mTimeMillis; } + @Override public NetworkInfo getActiveNetworkInfo(int uid) { if (mActiveNetworkType == null) { return null; @@ -48,14 +57,17 @@ public class FakeSystemFacade implements SystemFacade { return mIsMetered; } + @Override public boolean isNetworkRoaming() { return mIsRoaming; } + @Override public Long getMaxBytesOverMobile() { return mMaxBytesOverMobile ; } + @Override public Long getRecommendedMaxBytesOverMobile() { return mRecommendedMaxBytesOverMobile ; } @@ -70,27 +82,7 @@ public class FakeSystemFacade implements SystemFacade { return true; } - public boolean startThreadsWithoutWaiting = false; - public void setStartThreadsWithoutWaiting(boolean flag) { - this.startThreadsWithoutWaiting = flag; - } - - @Override - public void startThread(Thread thread) { - if (startThreadsWithoutWaiting) { - thread.start(); - } else { - mStartedThreads.add(thread); - } - } - - public void runAllThreads() { - while (!mStartedThreads.isEmpty()) { - mStartedThreads.poll().run(); - } - } - public void setReturnActualTime(boolean flag) { - returnActualTime = flag; + mReturnActualTime = flag; } } diff --git a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java index 1e6b7053..4159beb6 100644 --- a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java +++ b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java @@ -47,7 +47,6 @@ import java.io.InputStream; import java.net.MalformedURLException; import java.util.List; - @LargeTest public class PublicApiFunctionalTest extends AbstractPublicApiTest { private static final String REDIRECTED_PATH = "/other_path"; @@ -76,7 +75,6 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { } else { mTestDirectory.mkdir(); } - mSystemFacade.setStartThreadsWithoutWaiting(false); } @Override @@ -412,22 +410,18 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { } public void testCancel() throws Exception { - mSystemFacade.setStartThreadsWithoutWaiting(true); // return 'real time' from FakeSystemFacade so that DownloadThread will report progress mSystemFacade.setReturnActualTime(true); enqueueResponse(buildContinuingResponse()); Download download = enqueueRequest(getRequest()); - startService(null); // give the download time to get started and progress to 1% completion // before cancelling it. boolean rslt = download.runUntilProgress(1); assertTrue(rslt); mManager.remove(download.mId); - startService(null); - int status = download.runUntilDone(); + // make sure the row is gone from the database - assertEquals(-1, status); - mSystemFacade.setReturnActualTime(false); + download.waitForStatus(-1); } public void testDownloadCompleteBroadcast() throws Exception { @@ -512,9 +506,9 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { public void testContentObserver() throws Exception { enqueueResponse(buildEmptyResponse(HTTP_OK)); - enqueueRequest(getRequest()); mResolver.resetNotified(); - runService(); + final Download download = enqueueRequest(getRequest()); + download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); assertTrue(mResolver.mNotifyWasCalled); } @@ -524,10 +518,9 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { final Download download = enqueueRequest( getRequest().setNotificationVisibility(DownloadManager.Request.VISIBILITY_HIDDEN)); download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); - runService(); + verify(mNotifManager, times(1)).cancelAll(); verify(mNotifManager, never()).notify(anyString(), anyInt(), isA(Notification.class)); - // TODO: verify that it never cancels } public void testNotificationVisible() throws Exception { @@ -536,11 +529,10 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { // only shows in-progress notifications final Download download = enqueueRequest(getRequest()); download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); - runService(); // TODO: verify different notif types with tags + verify(mNotifManager, times(1)).cancelAll(); verify(mNotifManager, atLeastOnce()).notify(anyString(), anyInt(), isA(Notification.class)); - verify(mNotifManager, times(1)).cancel(anyString(), anyInt()); } public void testNotificationVisibleComplete() throws Exception { @@ -549,11 +541,10 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { final Download download = enqueueRequest(getRequest().setNotificationVisibility( DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED)); download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); - runService(); // TODO: verify different notif types with tags + verify(mNotifManager, times(1)).cancelAll(); verify(mNotifManager, atLeastOnce()).notify(anyString(), anyInt(), isA(Notification.class)); - verify(mNotifManager, times(1)).cancel(anyString(), anyInt()); } public void testRetryAfter() throws Exception { diff --git a/tests/src/com/android/providers/downloads/ThreadingTest.java b/tests/src/com/android/providers/downloads/ThreadingTest.java index 8605c76b..e73bae2a 100644 --- a/tests/src/com/android/providers/downloads/ThreadingTest.java +++ b/tests/src/com/android/providers/downloads/ThreadingTest.java @@ -24,15 +24,8 @@ import android.test.suitebuilder.annotation.LargeTest; */ @LargeTest public class ThreadingTest extends AbstractPublicApiTest { - private static class FakeSystemFacadeWithThreading extends FakeSystemFacade { - @Override - public void startThread(Thread thread) { - thread.start(); - } - } - public ThreadingTest() { - super(new FakeSystemFacadeWithThreading()); + super(new FakeSystemFacade()); } @Override |