Redesign of DownloadManager update loop.

Previously, the service lifecycle was managed through a large for()
loop which was extremely tricky to reason about.  This resulted in
several race conditions that could leave the service running
indefinitely, or terminate it early before tasks had finished.

This change redesigns the update loop to be event driven based on
database updates, and to collapse mutiple pending update passes.  It
is much easier to reason about service termination conditions, and
it correctly uses startId to handle races during command delivery.

Also moves scanner into isolated class, and switches to using public
API instead of binding to private interface.

Bug: 7638470, 7455406, 7162341
Change-Id: I380e77f5432223b2acb4e819e37f29f98ee4782b

4 files changed, 401 insertions, 371 deletions

diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index 74b52d48..65242227 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -244,10 +244,10 @@ public class DownloadInfo {
 
     /**
      * Result of last {@link DownloadThread} started by
-     * {@link #startIfReady(ExecutorService)}.
+     * {@link #startDownloadIfReady(ExecutorService)}.
      */
     @GuardedBy("this")
-    private Future<?> mActiveTask;
+    private Future<?> mActiveDownload;
 
     private final Context mContext;
     private final SystemFacade mSystemFacade;
@@ -312,7 +312,7 @@ public class DownloadInfo {
     /**
      * Returns whether this download should be enqueued.
      */
-    private boolean isReadyToStart() {
+    private boolean isReadyToDownload() {
         if (mControl == Downloads.Impl.CONTROL_PAUSED) {
             // the download is paused, so it's not going to start
             return false;
@@ -450,11 +450,14 @@ public class DownloadInfo {
      * If download is ready to start, and isn't already pending or executing,
      * create a {@link DownloadThread} and enqueue it into given
      * {@link Executor}.
+     *
+     * @return If actively downloading.
      */
-    public void startIfReady(ExecutorService executor) {
+    public boolean startDownloadIfReady(ExecutorService executor) {
         synchronized (this) {
-            final boolean isActive = mActiveTask != null && !mActiveTask.isDone();
-            if (isReadyToStart() && !isActive) {
+            final boolean isReady = isReadyToDownload();
+            final boolean isActive = mActiveDownload != null && !mActiveDownload.isDone();
+            if (isReady && !isActive) {
                 if (mStatus != Impl.STATUS_RUNNING) {
                     mStatus = Impl.STATUS_RUNNING;
                     ContentValues values = new ContentValues();
@@ -464,8 +467,25 @@ public class DownloadInfo {
 
                 final DownloadThread task = new DownloadThread(
                         mContext, mSystemFacade, this, mStorageManager, mNotifier);
-                mActiveTask = executor.submit(task);
+                mActiveDownload = executor.submit(task);
             }
+            return isReady;
+        }
+    }
+
+    /**
+     * If download is ready to be scanned, enqueue it into the given
+     * {@link DownloadScanner}.
+     *
+     * @return If actively scanning.
+     */
+    public boolean startScanIfReady(DownloadScanner scanner) {
+        synchronized (this) {
+            final boolean isReady = shouldScanFile();
+            if (isReady) {
+                scanner.requestScan(this);
+            }
+            return isReady;
         }
     }
 
@@ -527,15 +547,15 @@ public class DownloadInfo {
     }
 
     /**
-     * Returns the amount of time (as measured from the "now" parameter)
-     * at which a download will be active.
-     * 0 = immediately - service should stick around to handle this download.
-     * -1 = never - service can go away without ever waking up.
-     * positive value - service must wake up in the future, as specified in ms from "now"
+     * Return time when this download will be ready for its next action, in
+     * milliseconds after given time.
+     *
+     * @return If {@code 0}, download is ready to proceed immediately. If
+     *         {@link Long#MAX_VALUE}, then download has no future actions.
      */
-    long nextAction(long now) {
+    public long nextActionMillis(long now) {
         if (Downloads.Impl.isStatusCompleted(mStatus)) {
-            return -1;
+            return Long.MAX_VALUE;
         }
         if (mStatus != Downloads.Impl.STATUS_WAITING_TO_RETRY) {
             return 0;
@@ -550,7 +570,7 @@ public class DownloadInfo {
     /**
      * Returns whether a file should be scanned
      */
-    boolean shouldScanFile() {
+    public boolean shouldScanFile() {
         return (mMediaScanned == 0)
                 && (mDestination == Downloads.Impl.DESTINATION_EXTERNAL ||
                         mDestination == Downloads.Impl.DESTINATION_FILE_URI ||
diff --git a/src/com/android/providers/downloads/DownloadScanner.java b/src/com/android/providers/downloads/DownloadScanner.java
new file mode 100644
index 00000000..ca795062
--- /dev/null
+++ b/src/com/android/providers/downloads/DownloadScanner.java
@@ -0,0 +1,157 @@
+/*
+ * Copyright (C) 2013 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.providers.downloads;
+
+import static android.text.format.DateUtils.MINUTE_IN_MILLIS;
+import static com.android.providers.downloads.Constants.LOGV;
+import static com.android.providers.downloads.Constants.TAG;
+
+import android.content.ContentResolver;
+import android.content.ContentUris;
+import android.content.ContentValues;
+import android.content.Context;
+import android.media.MediaScannerConnection;
+import android.media.MediaScannerConnection.MediaScannerConnectionClient;
+import android.net.Uri;
+import android.os.SystemClock;
+import android.provider.Downloads;
+import android.util.Log;
+
+import com.android.internal.annotations.GuardedBy;
+import com.google.common.collect.Maps;
+
+import java.util.HashMap;
+
+/**
+ * Manages asynchronous scanning of completed downloads.
+ */
+public class DownloadScanner implements MediaScannerConnectionClient {
+    private static final long SCAN_TIMEOUT = MINUTE_IN_MILLIS;
+
+    private final Context mContext;
+    private final MediaScannerConnection mConnection;
+
+    private static class ScanRequest {
+        public final long id;
+        public final String path;
+        public final String mimeType;
+        public final long requestRealtime;
+
+        public ScanRequest(long id, String path, String mimeType) {
+            this.id = id;
+            this.path = path;
+            this.mimeType = mimeType;
+            this.requestRealtime = SystemClock.elapsedRealtime();
+        }
+
+        public void exec(MediaScannerConnection conn) {
+            conn.scanFile(path, mimeType);
+        }
+    }
+
+    @GuardedBy("mConnection")
+    private HashMap<String, ScanRequest> mPending = Maps.newHashMap();
+
+    public DownloadScanner(Context context) {
+        mContext = context;
+        mConnection = new MediaScannerConnection(context, this);
+    }
+
+    /**
+     * Check if requested scans are still pending. Scans may timeout after an
+     * internal duration.
+     */
+    public boolean hasPendingScans() {
+        synchronized (mConnection) {
+            if (mPending.isEmpty()) {
+                return false;
+            } else {
+                // Check if pending scans have timed out
+                final long nowRealtime = SystemClock.elapsedRealtime();
+                for (ScanRequest req : mPending.values()) {
+                    if (nowRealtime < req.requestRealtime + SCAN_TIMEOUT) {
+                        return true;
+                    }
+                }
+                return false;
+            }
+        }
+    }
+
+    /**
+     * Request that given {@link DownloadInfo} be scanned at some point in
+     * future. Enqueues the request to be scanned asynchronously.
+     *
+     * @see #hasPendingScans()
+     */
+    public void requestScan(DownloadInfo info) {
+        if (LOGV) Log.v(TAG, "requestScan() for " + info.mFileName);
+        synchronized (mConnection) {
+            final ScanRequest req = new ScanRequest(info.mId, info.mFileName, info.mMimeType);
+            mPending.put(req.path, req);
+
+            if (mConnection.isConnected()) {
+                req.exec(mConnection);
+            } else {
+                mConnection.connect();
+            }
+        }
+    }
+
+    public void shutdown() {
+        mConnection.disconnect();
+    }
+
+    @Override
+    public void onMediaScannerConnected() {
+        synchronized (mConnection) {
+            for (ScanRequest req : mPending.values()) {
+                req.exec(mConnection);
+            }
+        }
+    }
+
+    @Override
+    public void onScanCompleted(String path, Uri uri) {
+        final ScanRequest req;
+        synchronized (mConnection) {
+            req = mPending.remove(path);
+        }
+        if (req == null) {
+            Log.w(TAG, "Missing request for path " + path);
+            return;
+        }
+
+        // Update scanned column, which will kick off a database update pass,
+        // eventually deciding if overall service is ready for teardown.
+        final ContentValues values = new ContentValues();
+        values.put(Downloads.Impl.COLUMN_MEDIA_SCANNED, 1);
+        if (uri != null) {
+            values.put(Downloads.Impl.COLUMN_MEDIAPROVIDER_URI, uri.toString());
+        }
+
+        final ContentResolver resolver = mContext.getContentResolver();
+        final Uri downloadUri = ContentUris.withAppendedId(
+                Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, req.id);
+        final int rows = resolver.update(downloadUri, values, null, null);
+        if (rows == 0) {
+            // Local row disappeared during scan; download was probably deleted
+            // so clean up now-orphaned media entry.
+            resolver.delete(uri, null, null);
+        }
+    }
+}
diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java
index 039f12cd..34b1b495 100644
--- a/src/com/android/providers/downloads/DownloadService.java
+++ b/src/com/android/providers/downloads/DownloadService.java
@@ -16,26 +16,25 @@
 
 package com.android.providers.downloads;
 
+import static android.text.format.DateUtils.MINUTE_IN_MILLIS;
 import static com.android.providers.downloads.Constants.TAG;
 
 import android.app.AlarmManager;
+import android.app.DownloadManager;
 import android.app.PendingIntent;
 import android.app.Service;
-import android.content.ComponentName;
-import android.content.ContentValues;
+import android.content.ContentResolver;
 import android.content.Context;
 import android.content.Intent;
-import android.content.ServiceConnection;
 import android.content.res.Resources;
 import android.database.ContentObserver;
 import android.database.Cursor;
-import android.media.IMediaScannerListener;
-import android.media.IMediaScannerService;
 import android.net.Uri;
 import android.os.Handler;
+import android.os.HandlerThread;
 import android.os.IBinder;
+import android.os.Message;
 import android.os.Process;
-import android.os.RemoteException;
 import android.provider.Downloads;
 import android.text.TextUtils;
 import android.util.Log;
@@ -45,12 +44,12 @@ import com.android.internal.util.IndentingPrintWriter;
 import com.google.android.collect.Maps;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
 
 import java.io.File;
 import java.io.FileDescriptor;
 import java.io.PrintWriter;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -60,11 +59,25 @@ import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 
 /**
- * Performs the background downloads requested by applications that use the Downloads provider.
+ * Performs background downloads as requested by applications that use
+ * {@link DownloadManager}. Multiple start commands can be issued at this
+ * service, and it will continue running until no downloads are being actively
+ * processed. It may schedule alarms to resume downloads in future.
+ * <p>
+ * Any database updates important enough to initiate tasks should always be
+ * delivered through {@link Context#startService(Intent)}.
  */
 public class DownloadService extends Service {
-    /** amount of time to wait to connect to MediaScannerService before timing out */
-    private static final long WAIT_TIMEOUT = 10 * 1000;
+    // TODO: migrate WakeLock from individual DownloadThreads out into
+    // DownloadReceiver to protect our entire workflow.
+
+    private static final boolean DEBUG_LIFECYCLE = true;
+
+    @VisibleForTesting
+    SystemFacade mSystemFacade;
+
+    private AlarmManager mAlarmManager;
+    private StorageManager mStorageManager;
 
     /** Observer to get notified when the content observer's data changes */
     private DownloadManagerContentObserver mObserver;
@@ -79,7 +92,7 @@ public class DownloadService extends Service {
      * content provider changes or disappears.
      */
     @GuardedBy("mDownloads")
-    private Map<Long, DownloadInfo> mDownloads = Maps.newHashMap();
+    private final Map<Long, DownloadInfo> mDownloads = Maps.newHashMap();
 
     private final ExecutorService mExecutor = buildDownloadExecutor();
 
@@ -96,115 +109,24 @@ public class DownloadService extends Service {
         return executor;
     }
 
-    /**
-     * The thread that updates the internal download list from the content
-     * provider.
-     */
-    private UpdateThread mUpdateThread;
+    private DownloadScanner mScanner;
 
-    /**
-     * Whether the internal download list should be updated from the content
-     * provider.
-     */
-    private boolean mPendingUpdate;
+    private HandlerThread mUpdateThread;
+    private Handler mUpdateHandler;
 
-    /**
-     * The ServiceConnection object that tells us when we're connected to and disconnected from
-     * the Media Scanner
-     */
-    private MediaScannerConnection mMediaScannerConnection;
-
-    private boolean mMediaScannerConnecting;
-
-    /**
-     * The IPC interface to the Media Scanner
-     */
-    private IMediaScannerService mMediaScannerService;
-
-    @VisibleForTesting
-    SystemFacade mSystemFacade;
-
-    private StorageManager mStorageManager;
+    private volatile int mLastStartId;
 
     /**
      * Receives notifications when the data in the content provider changes
      */
     private class DownloadManagerContentObserver extends ContentObserver {
-
         public DownloadManagerContentObserver() {
             super(new Handler());
         }
 
-        /**
-         * Receives notification when the data in the observed content
-         * provider changes.
-         */
         @Override
         public void onChange(final boolean selfChange) {
-            if (Constants.LOGVV) {
-                Log.v(Constants.TAG, "Service ContentObserver received notification");
-            }
-            updateFromProvider();
-        }
-
-    }
-
-    /**
-     * Gets called back when the connection to the media
-     * scanner is established or lost.
-     */
-    public class MediaScannerConnection implements ServiceConnection {
-        public void onServiceConnected(ComponentName className, IBinder service) {
-            if (Constants.LOGVV) {
-                Log.v(Constants.TAG, "Connected to Media Scanner");
-            }
-            synchronized (DownloadService.this) {
-                try {
-                    mMediaScannerConnecting = false;
-                    mMediaScannerService = IMediaScannerService.Stub.asInterface(service);
-                    if (mMediaScannerService != null) {
-                        updateFromProvider();
-                    }
-                } finally {
-                    // notify anyone waiting on successful connection to MediaService
-                    DownloadService.this.notifyAll();
-                }
-            }
-        }
-
-        public void disconnectMediaScanner() {
-            synchronized (DownloadService.this) {
-                mMediaScannerConnecting = false;
-                if (mMediaScannerService != null) {
-                    mMediaScannerService = null;
-                    if (Constants.LOGVV) {
-                        Log.v(Constants.TAG, "Disconnecting from Media Scanner");
-                    }
-                    try {
-                        unbindService(this);
-                    } catch (IllegalArgumentException ex) {
-                        Log.w(Constants.TAG, "unbindService failed: " + ex);
-                    } finally {
-                        // notify anyone waiting on unsuccessful connection to MediaService
-                        DownloadService.this.notifyAll();
-                    }
-                }
-            }
-        }
-
-        public void onServiceDisconnected(ComponentName className) {
-            try {
-                if (Constants.LOGVV) {
-                    Log.v(Constants.TAG, "Disconnected from Media Scanner");
-                }
-            } finally {
-                synchronized (DownloadService.this) {
-                    mMediaScannerService = null;
-                    mMediaScannerConnecting = false;
-                    // notify anyone waiting on disconnect from MediaService
-                    DownloadService.this.notifyAll();
-                }
-            }
+            enqueueUpdate();
         }
     }
 
@@ -233,19 +155,21 @@ public class DownloadService extends Service {
             mSystemFacade = new RealSystemFacade(this);
         }
 
-        mObserver = new DownloadManagerContentObserver();
-        getContentResolver().registerContentObserver(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
-                true, mObserver);
+        mAlarmManager = (AlarmManager) getSystemService(Context.ALARM_SERVICE);
+        mStorageManager = new StorageManager(this);
+
+        mUpdateThread = new HandlerThread(TAG + "-UpdateThread");
+        mUpdateThread.start();
+        mUpdateHandler = new Handler(mUpdateThread.getLooper(), mUpdateCallback);
 
-        mMediaScannerService = null;
-        mMediaScannerConnecting = false;
-        mMediaScannerConnection = new MediaScannerConnection();
+        mScanner = new DownloadScanner(this);
 
         mNotifier = new DownloadNotifier(this);
         mNotifier.cancelAll();
 
-        mStorageManager = new StorageManager(this);
-        updateFromProvider();
+        mObserver = new DownloadManagerContentObserver();
+        getContentResolver().registerContentObserver(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
+                true, mObserver);
     }
 
     @Override
@@ -254,15 +178,14 @@ public class DownloadService extends Service {
         if (Constants.LOGVV) {
             Log.v(Constants.TAG, "Service onStart");
         }
-        updateFromProvider();
+        mLastStartId = startId;
+        enqueueUpdate();
         return returnValue;
     }
 
-    /**
-     * Cleans up when the service is destroyed
-     */
     @Override
     public void onDestroy() {
+        mScanner.shutdown();
         getContentResolver().unregisterContentObserver(mObserver);
         if (Constants.LOGVV) {
             Log.v(Constants.TAG, "Service onDestroy");
@@ -271,182 +194,167 @@ public class DownloadService extends Service {
     }
 
     /**
-     * Parses data from the content provider into private array
+     * Enqueue an {@link #updateLocked()} pass to occur in future.
      */
-    private void updateFromProvider() {
-        synchronized (this) {
-            mPendingUpdate = true;
-            if (mUpdateThread == null) {
-                mUpdateThread = new UpdateThread();
-                mUpdateThread.start();
-            }
-        }
+    private void enqueueUpdate() {
+        mUpdateHandler.removeMessages(MSG_UPDATE);
+        mUpdateHandler.obtainMessage(MSG_UPDATE, mLastStartId, -1).sendToTarget();
     }
 
-    private class UpdateThread extends Thread {
-        public UpdateThread() {
-            super("Download Service");
-        }
+    /**
+     * Enqueue an {@link #updateLocked()} pass to occur after delay, usually to
+     * catch any finished operations that didn't trigger an update pass.
+     */
+    private void enqueueFinalUpdate() {
+        mUpdateHandler.removeMessages(MSG_FINAL_UPDATE);
+        mUpdateHandler.sendMessageDelayed(
+                mUpdateHandler.obtainMessage(MSG_FINAL_UPDATE, mLastStartId, -1),
+                MINUTE_IN_MILLIS);
+    }
+
+    private static final int MSG_UPDATE = 1;
+    private static final int MSG_FINAL_UPDATE = 2;
 
+    private Handler.Callback mUpdateCallback = new Handler.Callback() {
         @Override
-        public void run() {
+        public boolean handleMessage(Message msg) {
             Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND);
-            boolean keepService = false;
-            // for each update from the database, remember which download is
-            // supposed to get restarted soonest in the future
-            long wakeUp = Long.MAX_VALUE;
-            for (;;) {
-                synchronized (DownloadService.this) {
-                    if (mUpdateThread != this) {
-                        throw new IllegalStateException(
-                                "multiple UpdateThreads in DownloadService");
-                    }
-                    if (!mPendingUpdate) {
-                        mUpdateThread = null;
-                        if (!keepService) {
-                            stopSelf();
-                        }
-                        if (wakeUp != Long.MAX_VALUE) {
-                            scheduleAlarm(wakeUp);
-                        }
-                        return;
-                    }
-                    mPendingUpdate = false;
+
+            final int startId = msg.arg1;
+            if (DEBUG_LIFECYCLE) Log.v(TAG, "Updating for startId " + startId);
+
+            // Since database is current source of truth, our "active" status
+            // depends on database state. We always get one final update pass
+            // once the real actions have finished and persisted their state.
+
+            // TODO: switch to asking real tasks to derive active state
+            // TODO: handle media scanner timeouts
+
+            final boolean isActive;
+            synchronized (mDownloads) {
+                isActive = updateLocked();
+            }
+
+            if (msg.what == MSG_FINAL_UPDATE) {
+                Log.wtf(TAG, "Final update pass triggered, isActive=" + isActive
+                        + "; someone didn't update correctly.");
+            }
+
+            if (isActive) {
+                // Still doing useful work, keep service alive. These active
+                // tasks will trigger another update pass when they're finished.
+
+                // Enqueue delayed update pass to catch finished operations that
+                // didn't trigger an update pass; these are bugs.
+                enqueueFinalUpdate();
+
+            } else {
+                // No active tasks, and any pending update messages can be
+                // ignored, since any updates important enough to initiate tasks
+                // will always be delivered with a new startId.
+
+                if (stopSelfResult(startId)) {
+                    if (DEBUG_LIFECYCLE) Log.v(TAG, "Nothing left; stopped");
+                    mUpdateHandler.removeMessages(MSG_UPDATE);
+                    mUpdateHandler.removeMessages(MSG_FINAL_UPDATE);
                 }
+            }
 
-                synchronized (mDownloads) {
-                    long now = mSystemFacade.currentTimeMillis();
-                    boolean mustScan = false;
-                    keepService = false;
-                    wakeUp = Long.MAX_VALUE;
-                    Set<Long> idsNoLongerInDatabase = new HashSet<Long>(mDownloads.keySet());
-
-                    Cursor cursor = getContentResolver().query(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
-                            null, null, null, null);
-                    if (cursor == null) {
-                        continue;
-                    }
-                    try {
-                        DownloadInfo.Reader reader =
-                                new DownloadInfo.Reader(getContentResolver(), cursor);
-                        int idColumn = cursor.getColumnIndexOrThrow(Downloads.Impl._ID);
-                        if (Constants.LOGVV) {
-                            Log.i(Constants.TAG, "number of rows from downloads-db: " +
-                                    cursor.getCount());
-                        }
-                        for (cursor.moveToFirst(); !cursor.isAfterLast(); cursor.moveToNext()) {
-                            long id = cursor.getLong(idColumn);
-                            idsNoLongerInDatabase.remove(id);
-                            DownloadInfo info = mDownloads.get(id);
-                            if (info != null) {
-                                updateDownload(reader, info, now);
-                            } else {
-                                info = insertDownloadLocked(reader, now);
-                            }
-
-                            if (info.shouldScanFile() && !scanFile(info, true, false)) {
-                                mustScan = true;
-                                keepService = true;
-                            }
-                            if (info.hasCompletionNotification()) {
-                                keepService = true;
-                            }
-                            long next = info.nextAction(now);
-                            if (next == 0) {
-                                keepService = true;
-                            } else if (next > 0 && next < wakeUp) {
-                                wakeUp = next;
-                            }
-                        }
-                    } finally {
-                        cursor.close();
-                    }
+            return true;
+        }
+    };
 
-                    for (Long id : idsNoLongerInDatabase) {
-                        deleteDownloadLocked(id);
-                    }
+    /**
+     * Update {@link #mDownloads} to match {@link DownloadProvider} state.
+     * Depending on current download state it may enqueue {@link DownloadThread}
+     * instances, request {@link DownloadScanner} scans, update user-visible
+     * notifications, and/or schedule future actions with {@link AlarmManager}.
+     * <p>
+     * Should only be called from {@link #mUpdateThread} as after being
+     * requested through {@link #enqueueUpdate()}.
+     *
+     * @return If there are active tasks being processed, as of the database
+     *         snapshot taken in this update.
+     */
+    private boolean updateLocked() {
+        final long now = mSystemFacade.currentTimeMillis();
 
-                    // is there a need to start the DownloadService? yes, if there are rows to be
-                    // deleted.
-                    if (!mustScan) {
-                        for (DownloadInfo info : mDownloads.values()) {
-                            if (info.mDeleted && TextUtils.isEmpty(info.mMediaProviderUri)) {
-                                mustScan = true;
-                                keepService = true;
-                                break;
-                            }
-                        }
-                    }
-                    mNotifier.updateWith(mDownloads.values());
-                    if (mustScan) {
-                        bindMediaScanner();
-                    } else {
-                        mMediaScannerConnection.disconnectMediaScanner();
+        boolean isActive = false;
+        long nextActionMillis = Long.MAX_VALUE;
+
+        final Set<Long> staleIds = Sets.newHashSet(mDownloads.keySet());
+
+        final ContentResolver resolver = getContentResolver();
+        final Cursor cursor = resolver.query(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
+                null, null, null, null);
+        try {
+            final DownloadInfo.Reader reader = new DownloadInfo.Reader(resolver, cursor);
+            final int idColumn = cursor.getColumnIndexOrThrow(Downloads.Impl._ID);
+            while (cursor.moveToNext()) {
+                final long id = cursor.getLong(idColumn);
+                staleIds.remove(id);
+
+                DownloadInfo info = mDownloads.get(id);
+                if (info != null) {
+                    updateDownload(reader, info, now);
+                } else {
+                    info = insertDownloadLocked(reader, now);
+                }
+
+                if (info.mDeleted) {
+                    // Delete download if requested, but only after cleaning up
+                    if (!TextUtils.isEmpty(info.mMediaProviderUri)) {
+                        resolver.delete(Uri.parse(info.mMediaProviderUri), null, null);
                     }
 
-                    // look for all rows with deleted flag set and delete the rows from the database
-                    // permanently
-                    for (DownloadInfo info : mDownloads.values()) {
-                        if (info.mDeleted) {
-                            // this row is to be deleted from the database. but does it have
-                            // mediaProviderUri?
-                            if (TextUtils.isEmpty(info.mMediaProviderUri)) {
-                                if (info.shouldScanFile()) {
-                                    // initiate rescan of the file to - which will populate
-                                    // mediaProviderUri column in this row
-                                    if (!scanFile(info, false, true)) {
-                                        throw new IllegalStateException("scanFile failed!");
-                                    }
-                                    continue;
-                                }
-                            } else {
-                                // yes it has mediaProviderUri column already filled in.
-                                // delete it from MediaProvider database.
-                                getContentResolver().delete(Uri.parse(info.mMediaProviderUri), null,
-                                        null);
-                            }
-                            // delete the file
-                            deleteFileIfExists(info.mFileName);
-                            // delete from the downloads db
-                            getContentResolver().delete(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
-                                    Downloads.Impl._ID + " = ? ",
-                                    new String[]{String.valueOf(info.mId)});
-                        }
+                    deleteFileIfExists(info.mFileName);
+                    resolver.delete(info.getAllDownloadsUri(), null, null);
+
+                } else {
+                    // Kick off download task if ready
+                    final boolean activeDownload = info.startDownloadIfReady(mExecutor);
+
+                    // Kick off media scan if completed
+                    final boolean activeScan = info.startScanIfReady(mScanner);
+
+                    if (DEBUG_LIFECYCLE && (activeDownload || activeScan)) {
+                        Log.v(TAG, "Download " + info.mId + ": activeDownload=" + activeDownload
+                                + ", activeScan=" + activeScan);
                     }
+
+                    isActive |= activeDownload;
+                    isActive |= activeScan;
                 }
+
+                // Keep track of nearest next action
+                nextActionMillis = Math.min(info.nextActionMillis(now), nextActionMillis);
             }
+        } finally {
+            cursor.close();
         }
 
-        private void bindMediaScanner() {
-            if (!mMediaScannerConnecting) {
-                Intent intent = new Intent();
-                intent.setClassName("com.android.providers.media",
-                        "com.android.providers.media.MediaScannerService");
-                mMediaScannerConnecting = true;
-                bindService(intent, mMediaScannerConnection, BIND_AUTO_CREATE);
-            }
+        // Clean up stale downloads that disappeared
+        for (Long id : staleIds) {
+            deleteDownloadLocked(id);
         }
 
-        private void scheduleAlarm(long wakeUp) {
-            AlarmManager alarms = (AlarmManager) getSystemService(Context.ALARM_SERVICE);
-            if (alarms == null) {
-                Log.e(Constants.TAG, "couldn't get alarm manager");
-                return;
-            }
+        // Update notifications visible to user
+        mNotifier.updateWith(mDownloads.values());
 
+        // Set alarm when next action is in future. It's okay if the service
+        // continues to run in meantime, since it will kick off an update pass.
+        if (nextActionMillis > 0 && nextActionMillis < Long.MAX_VALUE) {
             if (Constants.LOGV) {
-                Log.v(Constants.TAG, "scheduling retry in " + wakeUp + "ms");
+                Log.v(TAG, "scheduling start in " + nextActionMillis + "ms");
             }
 
-            Intent intent = new Intent(Constants.ACTION_RETRY);
-            intent.setClassName("com.android.providers.downloads",
-                    DownloadReceiver.class.getName());
-            alarms.set(
-                    AlarmManager.RTC_WAKEUP,
-                    mSystemFacade.currentTimeMillis() + wakeUp,
-                    PendingIntent.getBroadcast(DownloadService.this, 0, intent,
-                            PendingIntent.FLAG_ONE_SHOT));
+            final Intent intent = new Intent(Constants.ACTION_RETRY);
+            intent.setClass(this, DownloadReceiver.class);
+            mAlarmManager.set(AlarmManager.RTC_WAKEUP, now + nextActionMillis,
+                    PendingIntent.getBroadcast(this, 0, intent, PendingIntent.FLAG_ONE_SHOT));
         }
+
+        return isActive;
     }
 
     /**
@@ -462,7 +370,6 @@ public class DownloadService extends Service {
             Log.v(Constants.TAG, "processing inserted download " + info.mId);
         }
 
-        info.startIfReady(mExecutor);
         return info;
     }
 
@@ -470,15 +377,11 @@ public class DownloadService extends Service {
      * Updates the local copy of the info about a download.
      */
     private void updateDownload(DownloadInfo.Reader reader, DownloadInfo info, long now) {
-        int oldVisibility = info.mVisibility;
-        int oldStatus = info.mStatus;
-
         reader.updateFromDatabase(info);
         if (Constants.LOGVV) {
             Log.v(Constants.TAG, "processing updated download " + info.mId +
                     ", status: " + info.mStatus);
         }
-        info.startIfReady(mExecutor);
     }
 
     /**
@@ -493,88 +396,20 @@ public class DownloadService extends Service {
             if (Constants.LOGVV) {
                 Log.d(TAG, "deleteDownloadLocked() deleting " + info.mFileName);
             }
-            new File(info.mFileName).delete();
+            deleteFileIfExists(info.mFileName);
         }
         mDownloads.remove(info.mId);
     }
 
-    /**
-     * Attempts to scan the file if necessary.
-     * @return true if the file has been properly scanned.
-     */
-    private boolean scanFile(DownloadInfo info, final boolean updateDatabase,
-            final boolean deleteFile) {
-        synchronized (this) {
-            if (mMediaScannerService == null) {
-                // not bound to mediaservice. but if in the process of connecting to it, wait until
-                // connection is resolved
-                while (mMediaScannerConnecting) {
-                    Log.d(Constants.TAG, "waiting for mMediaScannerService service: ");
-                    try {
-                        this.wait(WAIT_TIMEOUT);
-                    } catch (InterruptedException e1) {
-                        throw new IllegalStateException("wait interrupted");
-                    }
-                }
-            }
-            // do we have mediaservice?
-            if (mMediaScannerService == null) {
-                // no available MediaService And not even in the process of connecting to it
-                return false;
-            }
-            if (Constants.LOGV) {
-                Log.v(Constants.TAG, "Scanning file " + info.mFileName);
-            }
-            try {
-                final Uri key = info.getAllDownloadsUri();
-                final long id = info.mId;
-                mMediaScannerService.requestScanFile(info.mFileName, info.mMimeType,
-                        new IMediaScannerListener.Stub() {
-                            public void scanCompleted(String path, Uri uri) {
-                                if (updateDatabase) {
-                                    // Mark this as 'scanned' in the database
-                                    // so that it is NOT subject to re-scanning by MediaScanner
-                                    // next time this database row row is encountered
-                                    ContentValues values = new ContentValues();
-                                    values.put(Constants.MEDIA_SCANNED, 1);
-                                    if (uri != null) {
-                                        values.put(Downloads.Impl.COLUMN_MEDIAPROVIDER_URI,
-                                                uri.toString());
-                                    }
-                                    getContentResolver().update(key, values, null, null);
-                                } else if (deleteFile) {
-                                    if (uri != null) {
-                                        // use the Uri returned to delete it from the MediaProvider
-                                        getContentResolver().delete(uri, null, null);
-                                    }
-                                    // delete the file and delete its row from the downloads db
-                                    deleteFileIfExists(path);
-                                    getContentResolver().delete(
-                                            Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
-                                            Downloads.Impl._ID + " = ? ",
-                                            new String[]{String.valueOf(id)});
-                                }
-                            }
-                        });
-                return true;
-            } catch (RemoteException e) {
-                Log.w(Constants.TAG, "Failed to scan file " + info.mFileName);
-                return false;
-            }
-        }
-    }
-
     private void deleteFileIfExists(String path) {
-        try {
-            if (!TextUtils.isEmpty(path)) {
-                if (Constants.LOGVV) {
-                    Log.d(TAG, "deleteFileIfExists() deleting " + path);
-                }
-                File file = new File(path);
-                file.delete();
+        if (!TextUtils.isEmpty(path)) {
+            if (Constants.LOGVV) {
+                Log.d(TAG, "deleteFileIfExists() deleting " + path);
+            }
+            final File file = new File(path);
+            if (file.exists() && !file.delete()) {
+                Log.w(TAG, "file: '" + path + "' couldn't be deleted");
             }
-        } catch (Exception e) {
-            Log.w(Constants.TAG, "file: '" + path + "' couldn't be deleted", e);
         }
     }
 
diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index 0d427fdd..c60b02a0 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -20,7 +20,9 @@ import static android.provider.Downloads.Impl.STATUS_BAD_REQUEST;
 import static android.provider.Downloads.Impl.STATUS_CANNOT_RESUME;
 import static android.provider.Downloads.Impl.STATUS_FILE_ERROR;
 import static android.provider.Downloads.Impl.STATUS_HTTP_DATA_ERROR;
+import static android.provider.Downloads.Impl.STATUS_QUEUED_FOR_WIFI;
 import static android.provider.Downloads.Impl.STATUS_TOO_MANY_REDIRECTS;
+import static android.provider.Downloads.Impl.STATUS_WAITING_FOR_NETWORK;
 import static android.provider.Downloads.Impl.STATUS_WAITING_TO_RETRY;
 import static android.text.format.DateUtils.MINUTE_IN_MILLIS;
 import static com.android.providers.downloads.Constants.TAG;
@@ -29,7 +31,6 @@ import static java.net.HttpURLConnection.HTTP_MOVED_PERM;
 import static java.net.HttpURLConnection.HTTP_MOVED_TEMP;
 import static java.net.HttpURLConnection.HTTP_OK;
 import static java.net.HttpURLConnection.HTTP_PARTIAL;
-import static java.net.HttpURLConnection.HTTP_PRECON_FAILED;
 import static java.net.HttpURLConnection.HTTP_SEE_OTHER;
 import static java.net.HttpURLConnection.HTTP_UNAVAILABLE;
 
@@ -218,11 +219,13 @@ public class DownloadThread implements Runnable {
             }
             finalStatus = error.getFinalStatus();
 
+            // Nobody below our level should request retries, since we handle
+            // failure counts at this level.
             if (finalStatus == STATUS_WAITING_TO_RETRY) {
                 throw new IllegalStateException("Execution should always throw final error codes");
             }
 
-            // Some errors should be retryable later, unless we fail too many times.
+            // Some errors should be retryable, unless we fail too many times.
             if (isStatusRetryable(finalStatus)) {
                 if (state.mGotData) {
                     numFailed = 1;
@@ -231,7 +234,7 @@ public class DownloadThread implements Runnable {
                 }
 
                 if (numFailed < Constants.MAX_RETRIES) {
-                    finalStatus = STATUS_WAITING_TO_RETRY;
+                    finalStatus = getFinalRetryStatus();
                 }
             }
 
@@ -428,6 +431,21 @@ public class DownloadThread implements Runnable {
     }
 
     /**
+     * Return retry status appropriate for current network conditions.
+     */
+    private int getFinalRetryStatus() {
+        switch (mInfo.checkCanUseNetwork()) {
+            case OK:
+                return STATUS_WAITING_TO_RETRY;
+            case UNUSABLE_DUE_TO_SIZE:
+            case RECOMMENDED_UNUSABLE_DUE_TO_SIZE:
+                return STATUS_QUEUED_FOR_WIFI;
+            default:
+                return STATUS_WAITING_FOR_NETWORK;
+        }
+    }
+
+    /**
      * Transfer as much data as possible from the HTTP response to the
      * destination file.
      */
@@ -805,10 +823,10 @@ public class DownloadThread implements Runnable {
      */
     private void notifyDownloadCompleted(
             State state, int finalStatus, String errorMsg, int numFailed) {
-        notifyThroughDatabase(state, finalStatus, errorMsg, numFailed);
         if (Downloads.Impl.isStatusCompleted(finalStatus)) {
             mInfo.sendIntentIfRequested();
         }
+        notifyThroughDatabase(state, finalStatus, errorMsg, numFailed);
     }
 
     private void notifyThroughDatabase(