Commit 2c33a879 authored by Xing Liu's avatar Xing Liu Committed by Commit Bot

Download location: Don't delete DB record for missing files on SD card.

If the user pulls out the SD card, currently the download database
record will be removed and user can no longer see the download.

Instead, we shouldn't erase the database records for files on SD card
when checking if they are externally removed. Later we can add a timestamp
to persist the time when the file is found missing for the first time, and
erase the db record 30 days after this timestamp.

TBR=mpearson@chromium.org

Bug: 845369
Change-Id: I5b129df435119d501867cedde85ef49ec844aa38
Reviewed-on: https://chromium-review.googlesource.com/1068273
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561369}
parent 6def5d92
...@@ -10,6 +10,7 @@ import android.content.Context; ...@@ -10,6 +10,7 @@ import android.content.Context;
import android.content.Intent; import android.content.Intent;
import android.content.res.ColorStateList; import android.content.res.ColorStateList;
import android.net.Uri; import android.net.Uri;
import android.os.Environment;
import android.os.StrictMode; import android.os.StrictMode;
import android.support.annotation.IntDef; import android.support.annotation.IntDef;
import android.support.annotation.Nullable; import android.support.annotation.Nullable;
...@@ -22,6 +23,7 @@ import org.chromium.base.ContextUtils; ...@@ -22,6 +23,7 @@ import org.chromium.base.ContextUtils;
import org.chromium.base.FileUtils; import org.chromium.base.FileUtils;
import org.chromium.base.Log; import org.chromium.base.Log;
import org.chromium.base.PathUtils; import org.chromium.base.PathUtils;
import org.chromium.base.StrictModeContext;
import org.chromium.base.VisibleForTesting; import org.chromium.base.VisibleForTesting;
import org.chromium.base.library_loader.LibraryProcessType; import org.chromium.base.library_loader.LibraryProcessType;
import org.chromium.base.metrics.RecordHistogram; import org.chromium.base.metrics.RecordHistogram;
...@@ -1066,4 +1068,18 @@ public class DownloadUtils { ...@@ -1066,4 +1068,18 @@ public class DownloadUtils {
public static String[] getAllDownloadDirectories() { public static String[] getAllDownloadDirectories() {
return PathUtils.getAllPrivateDownloadsDirectories(); return PathUtils.getAllPrivateDownloadsDirectories();
} }
/**
* Returns if the path is in the download directory on primary storage.
* @param path The directory to check.
* @return If the path is in the download directory on primary storage.
*/
public static boolean isInPrimaryStorageDownloadDirectory(String path) {
StrictModeContext c = StrictModeContext.allowDiskReads();
File primaryDir = Environment.getExternalStorageDirectory();
if (primaryDir == null || path == null) return false;
String primaryPath = primaryDir.getAbsolutePath();
if (primaryPath == null) return false;
return path.contains(primaryPath);
}
} }
...@@ -309,15 +309,19 @@ public class DownloadHistoryAdapter extends DateDividedAdapter ...@@ -309,15 +309,19 @@ public class DownloadHistoryAdapter extends DateDividedAdapter
// downloads rather than passing them to Java. // downloads rather than passing them to Java.
if (sDeletedFileTracker.contains(wrapper)) return true; if (sDeletedFileTracker.contains(wrapper)) return true;
if (wrapper.hasBeenExternallyRemoved()) { if (!wrapper.hasBeenExternallyRemoved()) return false;
if (DownloadUtils.isInPrimaryStorageDownloadDirectory(wrapper.getFilePath())) {
sDeletedFileTracker.add(wrapper); sDeletedFileTracker.add(wrapper);
wrapper.removePermanently(); wrapper.removePermanently();
mFilePathsToItemsMap.removeItem(wrapper); mFilePathsToItemsMap.removeItem(wrapper);
RecordUserAction.record("Android.DownloadManager.Item.ExternallyDeleted"); RecordUserAction.record("Android.DownloadManager.Item.ExternallyDeleted");
return true; return true;
} else {
// Keeps the download record when the file is on external SD card.
RecordUserAction.record("Android.DownloadManager.Item.ExternallyDeletedKeepRecord");
return false;
} }
return false;
} }
private boolean addDownloadHistoryItemWrapper(DownloadHistoryItemWrapper wrapper) { private boolean addDownloadHistoryItemWrapper(DownloadHistoryItemWrapper wrapper) {
......
...@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.download.ui; ...@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.download.ui;
import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertNull; import static junit.framework.Assert.assertNull;
import android.os.Environment;
import android.os.Handler; import android.os.Handler;
import android.os.Looper; import android.os.Looper;
...@@ -296,13 +297,15 @@ public class StubbedProvider implements BackendProvider { ...@@ -296,13 +297,15 @@ public class StubbedProvider implements BackendProvider {
.setDownloadGuid("sixth_guid") .setDownloadGuid("sixth_guid")
.setMimeType("audio/mp3"); .setMimeType("audio/mp3");
} else if (which == 6) { } else if (which == 6) {
builder = new DownloadInfo.Builder() builder =
.setUrl("https://sigh.com") new DownloadInfo.Builder()
.setBytesReceived(ONE_GIGABYTE) .setUrl("https://sigh.com")
.setFileName("huge_image.png") .setBytesReceived(ONE_GIGABYTE)
.setFilePath("/storage/fake_path/Downloads/huge_image.png") .setFileName("huge_image.png")
.setDownloadGuid("seventh_guid") .setFilePath(Environment.getExternalStorageDirectory().getAbsolutePath()
.setMimeType("image/png"); + "/fake_path/Downloads/huge_image.png")
.setDownloadGuid("seventh_guid")
.setMimeType("image/png");
} else if (which == 7) { } else if (which == 7) {
builder = new DownloadInfo.Builder() builder = new DownloadInfo.Builder()
.setUrl("https://sleepy.com") .setUrl("https://sleepy.com")
......
...@@ -1568,8 +1568,18 @@ should be able to be added at any place in this file. ...@@ -1568,8 +1568,18 @@ should be able to be added at any place in this file.
<owner>dtrainor@chromium.org</owner> <owner>dtrainor@chromium.org</owner>
<owner>twellington@chromium.org</owner> <owner>twellington@chromium.org</owner>
<description> <description>
A download tracked by the Chrome download service was externally deleted A download file on primary storage was externally deleted by other
e.g. through a third-party download manager. application. e.g. through a third-party download manager. The associated
database record will be also deleted.
</description>
</action>
<action name="Android.DownloadManager.Item.ExternallyDeletedKeepRecord">
<owner>xingliu@chromium.org</owner>
<description>
A download file was externally deleted by other application, e.g. through a
third-party download manager. However the associated database record will
not be immediately deleted.
</description> </description>
</action> </action>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment