Add idle service to clean orphan downloads.
Jeff Sharkey [Thu, 6 Feb 2014 23:19:01 +0000 (15:19 -0800)]
Periodically reconcile database against disk contents.  This handles
the case where a user/app deletes files directly from disk without
updating the database, and the rare case where a database delete
didn't make it to deleting the underlying file.

Also cleans up any downloads belonging to a UID when removed.

Bug: 12924143
Change-Id: I4899d09df7ef71f2625491ac01ceeafa8a2013ce

AndroidManifest.xml
src/com/android/providers/downloads/DownloadIdleService.java [new file with mode: 0644]
src/com/android/providers/downloads/DownloadProvider.java
src/com/android/providers/downloads/DownloadReceiver.java
src/com/android/providers/downloads/DownloadThread.java
src/com/android/providers/downloads/StorageUtils.java

index 423538a..9ea1dc3 100644 (file)
             </intent-filter>
         </provider>
 
-        <service android:name=".DownloadService"
-                android:permission="android.permission.ACCESS_DOWNLOAD_MANAGER" />
+        <service
+            android:name=".DownloadService"
+            android:permission="android.permission.ACCESS_DOWNLOAD_MANAGER" />
+
+        <service
+            android:name="com.android.providers.downloads.DownloadIdleService"
+            android:permission="android.permission.BIND_IDLE_SERVICE">
+            <intent-filter>
+                <action android:name="android.service.idle.IdleService" />
+            </intent-filter>
+        </service>
+
         <receiver android:name=".DownloadReceiver" android:exported="false">
             <intent-filter>
                 <action android:name="android.intent.action.BOOT_COMPLETED" />
                 <action android:name="android.net.conn.CONNECTIVITY_CHANGE" />
+                <action android:name="android.intent.action.UID_REMOVED" />
             </intent-filter>
             <intent-filter>
                 <action android:name="android.intent.action.MEDIA_MOUNTED" />
diff --git a/src/com/android/providers/downloads/DownloadIdleService.java b/src/com/android/providers/downloads/DownloadIdleService.java
new file mode 100644 (file)
index 0000000..c562ae4
--- /dev/null
@@ -0,0 +1,135 @@
+/*
+ * Copyright (C) 2014 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 com.android.providers.downloads.Constants.TAG;
+import static com.android.providers.downloads.StorageUtils.listFilesRecursive;
+
+import android.app.DownloadManager;
+import android.app.maintenance.IdleService;
+import android.content.ContentResolver;
+import android.content.ContentUris;
+import android.database.Cursor;
+import android.os.Environment;
+import android.provider.Downloads;
+import android.text.TextUtils;
+import android.util.Slog;
+
+import com.android.providers.downloads.StorageUtils.ConcreteFile;
+import com.google.android.collect.Lists;
+import com.google.android.collect.Sets;
+
+import libcore.io.ErrnoException;
+import libcore.io.IoUtils;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.HashSet;
+
+/**
+ * Idle maintenance service for {@link DownloadManager}. Reconciles database
+ * metadata and files on disk, which can become inconsistent when files are
+ * deleted directly on disk.
+ */
+public class DownloadIdleService extends IdleService {
+
+    private final Runnable mIdleRunnable = new Runnable() {
+        @Override
+        public void run() {
+            cleanOrphans();
+            finishIdle();
+        }
+    };
+
+    @Override
+    public boolean onIdleStart() {
+        new Thread(mIdleRunnable).start();
+        return true;
+    }
+
+    @Override
+    public void onIdleStop() {
+        // We're okay being killed at any point, so we don't worry about
+        // checkpointing before tearing down.
+    }
+
+    private interface DownloadQuery {
+        final String[] PROJECTION = new String[] {
+                Downloads.Impl._ID,
+                Downloads.Impl._DATA };
+
+        final int _ID = 0;
+        final int _DATA = 1;
+    }
+
+    /**
+     * Clean up orphan downloads, both in database and on disk.
+     */
+    public void cleanOrphans() {
+        final ContentResolver resolver = getContentResolver();
+
+        // Collect known files from database
+        final HashSet<ConcreteFile> fromDb = Sets.newHashSet();
+        final Cursor cursor = resolver.query(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
+                DownloadQuery.PROJECTION, null, null, null);
+        try {
+            while (cursor.moveToNext()) {
+                final String path = cursor.getString(DownloadQuery._DATA);
+                if (TextUtils.isEmpty(path)) continue;
+
+                final File file = new File(path);
+                try {
+                    fromDb.add(new ConcreteFile(file));
+                } catch (ErrnoException e) {
+                    // File probably no longer exists
+                    final String state = Environment.getExternalStorageState(file);
+                    if (Environment.MEDIA_UNKNOWN.equals(state)
+                            || Environment.MEDIA_MOUNTED.equals(state)) {
+                        // File appears to live on internal storage, or a
+                        // currently mounted device, so remove it from database.
+                        // This logic preserves files on external storage while
+                        // media is removed.
+                        final long id = cursor.getLong(DownloadQuery._ID);
+                        Slog.d(TAG, "Missing " + file + ", deleting " + id);
+                        resolver.delete(ContentUris.withAppendedId(
+                                Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, id), null, null);
+                    }
+                }
+            }
+        } finally {
+            IoUtils.closeQuietly(cursor);
+        }
+
+        // Collect known files from disk
+        final int uid = android.os.Process.myUid();
+        final ArrayList<ConcreteFile> fromDisk = Lists.newArrayList();
+        fromDisk.addAll(listFilesRecursive(getCacheDir(), null, uid));
+        fromDisk.addAll(listFilesRecursive(getFilesDir(), null, uid));
+        fromDisk.addAll(listFilesRecursive(Environment.getDownloadCacheDirectory(), null, uid));
+
+        Slog.d(TAG, "Found " + fromDb.size() + " files in database");
+        Slog.d(TAG, "Found " + fromDisk.size() + " files on disk");
+
+        // Delete files no longer referenced by database
+        for (ConcreteFile file : fromDisk) {
+            if (!fromDb.contains(file)) {
+                Slog.d(TAG, "Missing db entry, deleting " + file.file);
+                file.file.delete();
+            }
+        }
+    }
+}
index dc3c480..cd55eee 100644 (file)
@@ -1137,7 +1137,9 @@ public final class DownloadProvider extends ContentProvider {
     public int delete(final Uri uri, final String where,
             final String[] whereArgs) {
 
-        Helpers.validateSelection(where, sAppReadableColumnsSet);
+        if (shouldRestrictVisibility()) {
+            Helpers.validateSelection(where, sAppReadableColumnsSet);
+        }
 
         SQLiteDatabase db = mOpenHelper.getWritableDatabase();
         int count;
index f3d2376..28e2a67 100644 (file)
@@ -18,9 +18,11 @@ package com.android.providers.downloads;
 
 import static android.app.DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED;
 import static android.app.DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_ONLY_COMPLETION;
+import static com.android.providers.downloads.Constants.TAG;
 
 import android.app.DownloadManager;
 import android.content.BroadcastReceiver;
+import android.content.ContentResolver;
 import android.content.ContentUris;
 import android.content.ContentValues;
 import android.content.Context;
@@ -34,6 +36,7 @@ import android.os.HandlerThread;
 import android.provider.Downloads;
 import android.text.TextUtils;
 import android.util.Log;
+import android.util.Slog;
 import android.widget.Toast;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -42,12 +45,10 @@ import com.google.common.annotations.VisibleForTesting;
  * Receives system broadcasts (boot, network connectivity)
  */
 public class DownloadReceiver extends BroadcastReceiver {
-    private static final String TAG = "DownloadReceiver";
-
     private static Handler sAsyncHandler;
 
     static {
-        final HandlerThread thread = new HandlerThread(TAG);
+        final HandlerThread thread = new HandlerThread("DownloadReceiver");
         thread.start();
         sAsyncHandler = new Handler(thread.getLooper());
     }
@@ -61,31 +62,37 @@ public class DownloadReceiver extends BroadcastReceiver {
             mSystemFacade = new RealSystemFacade(context);
         }
 
-        String action = intent.getAction();
-        if (action.equals(Intent.ACTION_BOOT_COMPLETED)) {
-            if (Constants.LOGVV) {
-                Log.v(Constants.TAG, "Received broadcast intent for " +
-                        Intent.ACTION_BOOT_COMPLETED);
-            }
+        final String action = intent.getAction();
+        if (Intent.ACTION_BOOT_COMPLETED.equals(action)) {
             startService(context);
-        } else if (action.equals(Intent.ACTION_MEDIA_MOUNTED)) {
-            if (Constants.LOGVV) {
-                Log.v(Constants.TAG, "Received broadcast intent for " +
-                        Intent.ACTION_MEDIA_MOUNTED);
-            }
+
+        } else if (Intent.ACTION_MEDIA_MOUNTED.equals(action)) {
             startService(context);
-        } else if (action.equals(ConnectivityManager.CONNECTIVITY_ACTION)) {
+
+        } else if (ConnectivityManager.CONNECTIVITY_ACTION.equals(action)) {
             final ConnectivityManager connManager = (ConnectivityManager) context
                     .getSystemService(Context.CONNECTIVITY_SERVICE);
             final NetworkInfo info = connManager.getActiveNetworkInfo();
             if (info != null && info.isConnected()) {
                 startService(context);
             }
-        } else if (action.equals(Constants.ACTION_RETRY)) {
+
+        } else if (Intent.ACTION_UID_REMOVED.equals(action)) {
+            final PendingResult result = goAsync();
+            sAsyncHandler.post(new Runnable() {
+                @Override
+                public void run() {
+                    handleUidRemoved(context, intent);
+                    result.finish();
+                }
+            });
+
+        } else if (Constants.ACTION_RETRY.equals(action)) {
             startService(context);
-        } else if (action.equals(Constants.ACTION_OPEN)
-                || action.equals(Constants.ACTION_LIST)
-                || action.equals(Constants.ACTION_HIDE)) {
+
+        } else if (Constants.ACTION_OPEN.equals(action)
+                || Constants.ACTION_LIST.equals(action)
+                || Constants.ACTION_HIDE.equals(action)) {
 
             final PendingResult result = goAsync();
             if (result == null) {
@@ -103,6 +110,18 @@ public class DownloadReceiver extends BroadcastReceiver {
         }
     }
 
+    private void handleUidRemoved(Context context, Intent intent) {
+        final ContentResolver resolver = context.getContentResolver();
+
+        final int uid = intent.getIntExtra(Intent.EXTRA_UID, -1);
+        final int count = resolver.delete(
+                Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, Constants.UID + "=" + uid, null);
+
+        if (count > 0) {
+            Slog.d(TAG, "Deleted " + count + " downloads owned by UID " + uid);
+        }
+    }
+
     /**
      * Handle any broadcast related to a system notification.
      */
index fd4e89a..6c7cdc6 100644 (file)
@@ -588,6 +588,7 @@ public class DownloadThread implements Runnable {
             // Delete if local file
             if (mInfoDelta.mFileName != null) {
                 new File(mInfoDelta.mFileName).delete();
+                mInfoDelta.mFileName = null;
             }
 
         } else if (Downloads.Impl.isStatusSuccess(mInfoDelta.mStatus)) {
index 53da8e1..ad08c5d 100644 (file)
@@ -62,8 +62,6 @@ import java.util.concurrent.TimeUnit;
  */
 public class StorageUtils {
 
-    // TODO: run idle maint service to clean up untracked downloads
-
     /**
      * Minimum age for a file to be considered for deletion.
      */
@@ -174,69 +172,6 @@ public class StorageUtils {
         }
     }
 
-    private interface DownloadQuery {
-        final String[] PROJECTION = new String[] {
-                Downloads.Impl._ID,
-                Downloads.Impl._DATA };
-
-        final int _ID = 0;
-        final int _DATA = 1;
-    }
-
-    /**
-     * Clean up orphan downloads, both in database and on disk.
-     */
-    public static void cleanOrphans(Context context) {
-        final ContentResolver resolver = context.getContentResolver();
-
-        // Collect known files from database
-        final HashSet<ConcreteFile> fromDb = Sets.newHashSet();
-        final Cursor cursor = resolver.query(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
-                DownloadQuery.PROJECTION, null, null, null);
-        try {
-            while (cursor.moveToNext()) {
-                final String path = cursor.getString(DownloadQuery._DATA);
-                if (TextUtils.isEmpty(path)) continue;
-
-                final File file = new File(path);
-                try {
-                    fromDb.add(new ConcreteFile(file));
-                } catch (ErrnoException e) {
-                    // File probably no longer exists
-                    final String state = Environment.getExternalStorageState(file);
-                    if (Environment.MEDIA_UNKNOWN.equals(state)
-                            || Environment.MEDIA_MOUNTED.equals(state)) {
-                        // File appears to live on internal storage, or a
-                        // currently mounted device, so remove it from database.
-                        // This preserves files on external storage while media
-                        // is removed.
-                        final long id = cursor.getLong(DownloadQuery._ID);
-                        Slog.d(TAG, "Missing " + file + ", deleting " + id);
-                        resolver.delete(ContentUris.withAppendedId(
-                                Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, id), null, null);
-                    }
-                }
-            }
-        } finally {
-            IoUtils.closeQuietly(cursor);
-        }
-
-        // Collect known files from disk
-        final int uid = android.os.Process.myUid();
-        final ArrayList<ConcreteFile> fromDisk = Lists.newArrayList();
-        fromDisk.addAll(listFilesRecursive(context.getCacheDir(), null, uid));
-        fromDisk.addAll(listFilesRecursive(context.getFilesDir(), null, uid));
-        fromDisk.addAll(listFilesRecursive(Environment.getDownloadCacheDirectory(), null, uid));
-
-        // Delete files no longer referenced by database
-        for (ConcreteFile file : fromDisk) {
-            if (!fromDb.contains(file)) {
-                Slog.d(TAG, "Missing db entry, deleting " + file.file);
-                file.file.delete();
-            }
-        }
-    }
-
     /**
      * Return number of available bytes on the filesystem backing the given
      * {@link FileDescriptor}, minus any {@link #RESERVED_BYTES} buffer.
@@ -266,7 +201,7 @@ public class StorageUtils {
      * @param exclude ignore dirs with this name, or {@code null} to ignore.
      * @param uid only return files owned by this UID, or {@code -1} to ignore.
      */
-    private static List<ConcreteFile> listFilesRecursive(File startDir, String exclude, int uid) {
+    static List<ConcreteFile> listFilesRecursive(File startDir, String exclude, int uid) {
         final ArrayList<ConcreteFile> files = Lists.newArrayList();
         final LinkedList<File> dirs = new LinkedList<File>();
         dirs.add(startDir);
@@ -298,7 +233,7 @@ public class StorageUtils {
      * Concrete file on disk that has a backing device and inode. Faster than
      * {@code realpath()} when looking for identical files.
      */
-    public static class ConcreteFile {
+    static class ConcreteFile {
         public final File file;
         public final StructStat stat;