summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorVasu Nori <vnori@google.com>2010-10-12 23:27:49 -0700
committerVasu Nori <vnori@google.com>2010-10-13 16:23:03 -0700
commite00c31208405bd2e4c88e069df7a2b15237f70bf (patch)
treeb5e8c3da030e821681e36660360ad5cf34c0d9b7
parentcd990514feb2b17848809d9262e0d73a828b2142 (diff)
downloadandroid_packages_providers_DownloadProvider-e00c31208405bd2e4c88e069df7a2b15237f70bf.zip
android_packages_providers_DownloadProvider-e00c31208405bd2e4c88e069df7a2b15237f70bf.tar.gz
android_packages_providers_DownloadProvider-e00c31208405bd2e4c88e069df7a2b15237f70bf.tar.bz2
bug:3069735 in Download UI app, handle deletes correctly
gingerbread. High-level details 1. When a file is downloaded by DownloadManager, metadata about the file is stored in 2 databases: DownloadProvider and MediaProvider. 2. So, when it is to be deleted, its metadata needs to be cleaned up from both the databases. 3. But the 2 databases use differnt content-uri's as "primary keys" and DownloadProvider loses the "primary-key" of the row in MediaProvider database. 4. Easiest thing would have been to have DownloadProvider give filepath to MediaProvider and let MediaProvider linearly scan its database to locate the row and delete it. 5. The other - faster but more coding for now - option is to have DownloadProvider store the "primary-key" of the MediaProvider's row. implemented in this CL. Low-level details 1. add 2 new columns to downloads table in downloads.db: mediaprovider_uri = downloaded file's content_uri in mediaprovider db this column is null for downloads that finished before this column is added to the database. deleted = flag is set to true if a file is to be deleted 2. download UI app shows only those files whose 'deleted' flag is not set. 3. when the user deletes downloads from download UI app, 3.1. if mediaprovider_uri is NOT null, then the row is deleted from downloads table AND from the mediaprovider database. 3.2 if mediaprovider_uri is NULL, then its row in downloads database is marked 'tp be deleted' by setting 'deleted' column to '1'. 4. When DownloadService (in DownloadProvider) processes all rows from downloads table, if it sees any rows wth 'deleted' = 1, then it uses MediaScanner Service to re-scan the file, get the mediaprovider_uri from MediaProvider and update the row in downloads table with this mediaprovider_uri value and then delete the row by doing the following 1. delete it from MediaProvider database using mediaprovider_uri 2. delete it from DownloadProvider database Problem with this solution: There is a small window where it is deleted by the user on the Download app (and the row disappears from the display) but it is still present in Gallery app. Thats due to the following asynchronous operations 1. DownladService which processes rows-to-be-deleted is not always up 2. DownloadService uses asynchronous call to have the file re-scanned by MediaScanner to get mediaprovider_uri Change-Id: Ib90eb9e647f543312c865d3bbf9a06fb867a648b
-rw-r--r--src/com/android/providers/downloads/DownloadInfo.java8
-rw-r--r--src/com/android/providers/downloads/DownloadProvider.java25
-rw-r--r--src/com/android/providers/downloads/DownloadService.java124
-rw-r--r--ui/src/com/android/providers/downloads/ui/DownloadList.java37
4 files changed, 157 insertions, 37 deletions
diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index 3327e80..36816b5 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -29,6 +29,7 @@ import android.net.ConnectivityManager;
import android.net.Uri;
import android.provider.Downloads;
import android.provider.Downloads.Impl;
+import android.text.TextUtils;
import android.util.Log;
import android.util.Pair;
@@ -84,6 +85,9 @@ public class DownloadInfo {
info.mCurrentBytes = getLong(Downloads.Impl.COLUMN_CURRENT_BYTES);
info.mETag = getString(info.mETag, Constants.ETAG);
info.mMediaScanned = getInt(Constants.MEDIA_SCANNED) == 1;
+ info.mDeleted = getInt(Downloads.Impl.COLUMN_DELETED) == 1;
+ info.mMediaProviderUri = getString(info.mMediaProviderUri,
+ Downloads.Impl.COLUMN_MEDIAPROVIDER_URI);
info.mIsPublicApi = getInt(Downloads.Impl.COLUMN_IS_PUBLIC_API) != 0;
info.mAllowedNetworkTypes = getInt(Downloads.Impl.COLUMN_ALLOWED_NETWORK_TYPES);
info.mAllowRoaming = getInt(Downloads.Impl.COLUMN_ALLOW_ROAMING) != 0;
@@ -231,6 +235,8 @@ public class DownloadInfo {
public long mCurrentBytes;
public String mETag;
public boolean mMediaScanned;
+ public boolean mDeleted;
+ public String mMediaProviderUri;
public boolean mIsPublicApi;
public int mAllowedNetworkTypes;
public boolean mAllowRoaming;
@@ -511,6 +517,8 @@ public class DownloadInfo {
Log.v(Constants.TAG, "CURRENT : " + mCurrentBytes);
Log.v(Constants.TAG, "ETAG : " + mETag);
Log.v(Constants.TAG, "SCANNED : " + mMediaScanned);
+ Log.v(Constants.TAG, "DELETED : " + mDeleted);
+ Log.v(Constants.TAG, "MEDIAPROVIDER_URI : " + mMediaProviderUri);
}
/**
diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java
index 3201463..d3fe4ab 100644
--- a/src/com/android/providers/downloads/DownloadProvider.java
+++ b/src/com/android/providers/downloads/DownloadProvider.java
@@ -49,6 +49,7 @@ import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Set;
/**
@@ -58,7 +59,7 @@ public final class DownloadProvider extends ContentProvider {
/** Database filename */
private static final String DB_NAME = "downloads.db";
/** Current database version */
- private static final int DB_VERSION = 105;
+ private static final int DB_VERSION = 106;
/** Name of table in the database */
private static final String DB_TABLE = "downloads";
@@ -123,6 +124,8 @@ public final class DownloadProvider extends ContentProvider {
Downloads.Impl.COLUMN_URI,
Downloads.Impl.COLUMN_IS_VISIBLE_IN_DOWNLOADS_UI,
Downloads.Impl.COLUMN_FILE_NAME_HINT,
+ Downloads.Impl.COLUMN_MEDIAPROVIDER_URI,
+ Downloads.Impl.COLUMN_DELETED,
};
private static HashSet<String> sAppReadableColumnsSet;
@@ -270,6 +273,12 @@ public final class DownloadProvider extends ContentProvider {
fillNullValues(db);
break;
+ case 106:
+ addColumn(db, DB_TABLE, Downloads.Impl.COLUMN_MEDIAPROVIDER_URI, "TEXT");
+ addColumn(db, DB_TABLE, Downloads.Impl.COLUMN_DELETED,
+ "BOOLEAN NOT NULL DEFAULT 0");
+ break;
+
default:
throw new IllegalStateException("Don't know how to upgrade to " + version);
}
@@ -356,7 +365,9 @@ public final class DownloadProvider extends ContentProvider {
Downloads.Impl.COLUMN_OTHER_UID + " INTEGER, " +
Downloads.Impl.COLUMN_TITLE + " TEXT, " +
Downloads.Impl.COLUMN_DESCRIPTION + " TEXT, " +
- Constants.MEDIA_SCANNED + " BOOLEAN);");
+ Constants.MEDIA_SCANNED + " BOOLEAN, " +
+ Downloads.Impl.COLUMN_MEDIAPROVIDER_URI + " TEXT, " +
+ Downloads.Impl.COLUMN_DELETED + " BOOLEAN NOT NULL DEFAULT 1);");
} catch (SQLException ex) {
Log.e(Constants.TAG, "couldn't create table in downloads database");
throw ex;
@@ -869,6 +880,13 @@ public final class DownloadProvider extends ContentProvider {
int count;
boolean startService = false;
+ if (values.containsKey(Downloads.Impl.COLUMN_DELETED)) {
+ if (values.getAsInteger(Downloads.Impl.COLUMN_DELETED) == 1) {
+ // some rows are to be 'deleted'. need to start DownloadService.
+ startService = true;
+ }
+ }
+
ContentValues filteredValues;
if (Binder.getCallingPid() != Process.myPid()) {
filteredValues = new ContentValues();
@@ -879,9 +897,12 @@ public final class DownloadProvider extends ContentProvider {
filteredValues.put(Downloads.Impl.COLUMN_CONTROL, i);
startService = true;
}
+
copyInteger(Downloads.Impl.COLUMN_CONTROL, values, filteredValues);
copyString(Downloads.Impl.COLUMN_TITLE, values, filteredValues);
+ copyString(Downloads.Impl.COLUMN_MEDIAPROVIDER_URI, values, filteredValues);
copyString(Downloads.Impl.COLUMN_DESCRIPTION, values, filteredValues);
+ copyInteger(Downloads.Impl.COLUMN_DELETED, values, filteredValues);
} else {
filteredValues = values;
String filename = values.getAsString(Downloads.Impl._DATA);
diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java
index 8fbcf4b..0850af7 100644
--- a/src/com/android/providers/downloads/DownloadService.java
+++ b/src/com/android/providers/downloads/DownloadService.java
@@ -27,6 +27,7 @@ import android.content.Intent;
import android.content.ServiceConnection;
import android.database.ContentObserver;
import android.database.Cursor;
+import android.media.IMediaScannerListener;
import android.media.IMediaScannerService;
import android.net.Uri;
import android.os.Environment;
@@ -35,12 +36,14 @@ import android.os.IBinder;
import android.os.Process;
import android.os.RemoteException;
import android.provider.Downloads;
+import android.text.TextUtils;
import android.util.Log;
import com.google.android.collect.Maps;
import com.google.common.annotations.VisibleForTesting;
import java.io.File;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
@@ -94,6 +97,18 @@ public class DownloadService extends Service {
@VisibleForTesting
SystemFacade mSystemFacade;
+ /** Before calling
+ * {@link IMediaScannerService#requestScanFile(String, String, IMediaScannerListener)},
+ * a (key,value) pair of (filepath, uri-to-update-the-row-in-dowloads-db)
+ * is stored in the following struct.
+ * When the callback from
+ * {@link IMediaScannerService#requestScanFile(String, String, IMediaScannerListener)} is
+ * received with params (filepath, uri-to-update-the-row-in-mediaprovider-db),
+ * this struct comes in handy to locate the row in downloads table whose mediaprovier-uri
+ * column is to be updated with the value returned by this callback method.
+ */
+ private final HashMap<String, Uri> downloadsToBeUpdated = new HashMap<String, Uri>();
+
/**
* Receives notifications when the data in the content provider changes
*/
@@ -125,17 +140,20 @@ public class DownloadService extends Service {
if (Constants.LOGVV) {
Log.v(Constants.TAG, "Connected to Media Scanner");
}
- mMediaScannerConnecting = false;
synchronized (DownloadService.this) {
+ mMediaScannerConnecting = false;
mMediaScannerService = IMediaScannerService.Stub.asInterface(service);
if (mMediaScannerService != null) {
updateFromProvider();
}
+ // 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) {
@@ -144,11 +162,11 @@ public class DownloadService extends Service {
try {
unbindService(this);
} catch (IllegalArgumentException ex) {
- if (Constants.LOGV) {
- Log.v(Constants.TAG, "unbindService threw up: " + ex);
- }
+ Log.w(Constants.TAG, "unbindService failed: " + ex);
}
}
+ // notify anyone waiting on unsuccessful connection to MediaService
+ DownloadService.this.notifyAll();
}
}
@@ -158,6 +176,9 @@ public class DownloadService extends Service {
}
synchronized (DownloadService.this) {
mMediaScannerService = null;
+ mMediaScannerConnecting = false;
+ // notify anyone waiting on disconnect from MediaService
+ DownloadService.this.notifyAll();
}
}
}
@@ -315,13 +336,48 @@ public class DownloadService extends Service {
deleteDownload(id);
}
+ // 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.updateNotification(mDownloads.values());
-
if (mustScan) {
bindMediaScanner();
} else {
mMediaScannerConnection.disconnectMediaScanner();
}
+
+ // 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)) {
+ // initiate rescan of the file to - which will populate mediaProviderUri
+ // column in this row
+ if (!scanFile(info, true)) {
+ throw new IllegalStateException("scanFile failed!");
+ }
+ } else {
+ // yes it has mediaProviderUri column already filled in.
+ // delete it from MediaProvider database and then from downloads table
+ // in DownProvider database (the order of deletion is important).
+ getContentResolver().delete(Uri.parse(info.mMediaProviderUri), null,
+ null);
+ getContentResolver().delete(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
+ Downloads.Impl._ID + " = ? ",
+ new String[]{String.valueOf(info.mId)});
+ }
+ }
+ }
}
}
@@ -491,24 +547,60 @@ public class DownloadService extends Service {
private boolean scanFile(DownloadInfo info, boolean updateDatabase) {
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();
+ } 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;
}
- try {
- if (Constants.LOGV) {
- Log.v(Constants.TAG, "Scanning file " + info.mFileName);
- }
- mMediaScannerService.scanFile(info.mFileName, info.mMimeType);
- if (updateDatabase) {
- ContentValues values = new ContentValues();
- values.put(Constants.MEDIA_SCANNED, 1);
- getContentResolver().update(info.getAllDownloadsUri(), values, null, null);
+ if (Constants.LOGV) {
+ Log.v(Constants.TAG, "Scanning file " + info.mFileName);
+ }
+ if (updateDatabase) {
+ synchronized(downloadsToBeUpdated) {
+ Uri value = info.getAllDownloadsUri();
+ if (value != null) {
+ downloadsToBeUpdated.put(info.mFileName, value);
+ }
}
+ }
+ try {
+ mMediaScannerService.requestScanFile(info.mFileName, info.mMimeType,
+ new IMediaScannerListener.Stub() {
+ public void scanCompleted(String path, Uri uri) {
+ Uri key;
+ boolean updateMediaproviderUriColumn;
+ synchronized(downloadsToBeUpdated) {
+ key = downloadsToBeUpdated.get(path);
+ updateMediaproviderUriColumn = (key != null);
+ }
+ if (updateMediaproviderUriColumn) {
+ ContentValues values = new ContentValues();
+ values.put(Constants.MEDIA_SCANNED, 1);
+ values.put(Downloads.Impl.COLUMN_MEDIAPROVIDER_URI,
+ uri.toString());
+ getContentResolver().update(key, values, null, null);
+ synchronized(downloadsToBeUpdated) {
+ downloadsToBeUpdated.remove(path);
+ }
+ }
+ }
+ });
return true;
} catch (RemoteException e) {
- Log.d(Constants.TAG, "Failed to scan file " + info.mFileName);
+ Log.w(Constants.TAG, "Failed to scan file " + info.mFileName);
return false;
}
}
}
-
}
diff --git a/ui/src/com/android/providers/downloads/ui/DownloadList.java b/ui/src/com/android/providers/downloads/ui/DownloadList.java
index f1cb91f..33f821f 100644
--- a/ui/src/com/android/providers/downloads/ui/DownloadList.java
+++ b/ui/src/com/android/providers/downloads/ui/DownloadList.java
@@ -33,6 +33,7 @@ import android.os.Bundle;
import android.os.Environment;
import android.os.Handler;
import android.provider.Downloads;
+import android.text.TextUtils;
import android.util.Log;
import android.view.Menu;
import android.view.MenuInflater;
@@ -51,7 +52,6 @@ import android.widget.Toast;
import com.android.providers.downloads.ui.DownloadItem.DownloadSelectListener;
-import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.HashSet;
@@ -86,6 +86,7 @@ public class DownloadList extends Activity
private int mLocalUriColumnId;
private int mMediaTypeColumnId;
private int mReasonColumndId;
+ private int mMediaProviderUriId;
private boolean mIsSortedBySize = false;
private Set<Long> mSelectedIds = new HashSet<Long>();
@@ -148,6 +149,9 @@ public class DownloadList extends Activity
mDateSortedCursor.getColumnIndexOrThrow(DownloadManager.COLUMN_MEDIA_TYPE);
mReasonColumndId =
mDateSortedCursor.getColumnIndexOrThrow(DownloadManager.COLUMN_REASON);
+ mMediaProviderUriId =
+ mDateSortedCursor.getColumnIndexOrThrow(
+ DownloadManager.COLUMN_MEDIAPROVIDER_URI);
mDateSortedAdapter = new DateSortedDownloadAdapter(this, mDateSortedCursor, this);
mDateOrderedListView.setAdapter(mDateSortedAdapter);
@@ -588,30 +592,25 @@ public class DownloadList extends Activity
if (isComplete && localUri != null) {
String path = Uri.parse(localUri).getPath();
if (path.startsWith(Environment.getExternalStorageDirectory().getPath())) {
- String mediaType = mDateSortedCursor.getString(mMediaTypeColumnId);
- deleteDownloadedFile(path, mediaType);
+ String mediaProviderUri = mDateSortedCursor.getString(mMediaProviderUriId);
+ if (TextUtils.isEmpty(mediaProviderUri)) {
+ // downloads database doesn't have the mediaprovider_uri. It means
+ // this download occurred before mediaprovider_uri column existed
+ // in downloads table. Since MediaProvider needs the mediaprovider_uri to
+ // delete this download, just set the 'deleted' flag to 1 on this row
+ // in the database. DownloadService, upon seeing this flag set to 1, will
+ // re-scan the file and get the MediaProviderUri and then delete the file
+ mDownloadManager.markRowDeleted(downloadId);
+ return;
+ } else {
+ getContentResolver().delete(Uri.parse(mediaProviderUri), null, null);
+ }
}
}
}
mDownloadManager.remove(downloadId);
}
- /**
- * Delete the file at the given path. Try sending an intent to delete it, but if that goes
- * unhandled, delete it ourselves.
- * @param path path to the file to delete
- */
- private void deleteDownloadedFile(String path, String mediaType) {
- Intent intent = new Intent(Intent.ACTION_DELETE);
- File file = new File(path);
- intent.setDataAndType(Uri.fromFile(file), mediaType);
- try {
- startActivity(intent);
- } catch (ActivityNotFoundException exc) {
- file.delete();
- }
- }
-
@Override
public boolean isDownloadSelected(long id) {
return mSelectedIds.contains(id);