Commit 281e07f7 authored by Xing Liu's avatar Xing Liu Committed by Commit Bot

Android download: Fix storage space UI on Android Q.

On Android Q, Environment.DIRECTORY_DOWNLOADS is an invalid directory,
and all disk space queries like getUsableSpace() or getTotalSpace()
will return 0, and other related calls may throw exception. This is
because we didn't create the directory in external storage. Q basically
exposes a bug in our code.

This CL adds a utility function to retrieve the directory and tries to
create it. Also updates DownloadUtils.isInPrimaryStorageDownloadDirectory
to work with content URI file path on Q.


Bug: 945028
Change-Id: I22cce2e5ac81bd574eee96719648689c5a8cdbf7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1538876Reviewed-by: default avatarMin Qin <qinmin@chromium.org>
Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644621}
parent eea73325
...@@ -48,8 +48,7 @@ public class DownloadDirectoryProvider { ...@@ -48,8 +48,7 @@ public class DownloadDirectoryProvider {
ArrayList<DirectoryOption> dirs = new ArrayList<>(); ArrayList<DirectoryOption> dirs = new ArrayList<>();
// Retrieve default directory. // Retrieve default directory.
File defaultDirectory = File defaultDirectory = DownloadUtils.getPrimaryDownloadDirectory();
Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS);
// If no default directory, return an error option. // If no default directory, return an error option.
if (defaultDirectory == null) { if (defaultDirectory == null) {
......
...@@ -1211,6 +1211,10 @@ public class DownloadUtils { ...@@ -1211,6 +1211,10 @@ public class DownloadUtils {
* @return If the path is in the download directory on primary storage. * @return If the path is in the download directory on primary storage.
*/ */
public static boolean isInPrimaryStorageDownloadDirectory(String path) { public static boolean isInPrimaryStorageDownloadDirectory(String path) {
// Only primary storage can have content URI as file path.
if (ContentUriUtils.isContentUri(path)) return true;
// Check if the file path contains the external public directory.
File primaryDir = null; File primaryDir = null;
try (StrictModeContext unused = StrictModeContext.allowDiskReads()) { try (StrictModeContext unused = StrictModeContext.allowDiskReads()) {
primaryDir = Environment.getExternalStorageDirectory(); primaryDir = Environment.getExternalStorageDirectory();
...@@ -1220,6 +1224,27 @@ public class DownloadUtils { ...@@ -1220,6 +1224,27 @@ public class DownloadUtils {
return primaryPath == null ? false : path.contains(primaryPath); return primaryPath == null ? false : path.contains(primaryPath);
} }
/**
* Get the primary download directory in public external storage. The directory will be created
* if it doesn't exist.
* @return The download directory. Can be an invalid directory if failed to create the
* directory.
*/
public static File getPrimaryDownloadDirectory() {
File downloadDir =
Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS);
// Create the directory if needed.
if (!downloadDir.exists()) {
try {
downloadDir.mkdirs();
} catch (SecurityException e) {
Log.e(TAG, "Exception when creating download directory.", e);
}
}
return downloadDir;
}
/** /**
* Parses an originating URL string and returns a valid Uri that can be inserted into * Parses an originating URL string and returns a valid Uri that can be inserted into
* DownloadProvider. The returned Uri has to be null or non-empty http(s) scheme. * DownloadProvider. The returned Uri has to be null or non-empty http(s) scheme.
......
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
package org.chromium.chrome.browser.download.home.storage; package org.chromium.chrome.browser.download.home.storage;
import android.content.Context; import android.content.Context;
import android.os.Environment;
import android.support.annotation.Nullable; import android.support.annotation.Nullable;
import org.chromium.base.task.AsyncTask; import org.chromium.base.task.AsyncTask;
...@@ -89,8 +88,7 @@ public class StorageSummaryProvider implements OfflineItemFilterObserver { ...@@ -89,8 +88,7 @@ public class StorageSummaryProvider implements OfflineItemFilterObserver {
new AsyncTask<DirectoryOption>() { new AsyncTask<DirectoryOption>() {
@Override @Override
protected DirectoryOption doInBackground() { protected DirectoryOption doInBackground() {
File defaultDownloadDir = Environment.getExternalStoragePublicDirectory( File defaultDownloadDir = DownloadUtils.getPrimaryDownloadDirectory();
Environment.DIRECTORY_DOWNLOADS);
DirectoryOption directoryOption = new DirectoryOption("", DirectoryOption directoryOption = new DirectoryOption("",
defaultDownloadDir.getAbsolutePath(), defaultDownloadDir.getUsableSpace(), defaultDownloadDir.getAbsolutePath(), defaultDownloadDir.getUsableSpace(),
defaultDownloadDir.getTotalSpace(), defaultDownloadDir.getTotalSpace(),
......
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
package org.chromium.chrome.browser.download.ui; package org.chromium.chrome.browser.download.ui;
import android.content.Context; import android.content.Context;
import android.os.Environment;
import android.os.StatFs; import android.os.StatFs;
import android.support.v7.widget.RecyclerView; import android.support.v7.widget.RecyclerView;
import android.view.LayoutInflater; import android.view.LayoutInflater;
...@@ -63,19 +62,7 @@ public class SpaceDisplay extends RecyclerView.AdapterDataObserver { ...@@ -63,19 +62,7 @@ public class SpaceDisplay extends RecyclerView.AdapterDataObserver {
@Override @Override
protected Long doInBackground() { protected Long doInBackground() {
File downloadDirectory = Environment.getExternalStoragePublicDirectory( File downloadDirectory = DownloadUtils.getPrimaryDownloadDirectory();
Environment.DIRECTORY_DOWNLOADS);
// Create the downloads directory, if necessary.
if (!downloadDirectory.exists()) {
try {
// mkdirs() can fail, so we still need to check if the directory exists
// later.
downloadDirectory.mkdirs();
} catch (SecurityException e) {
Log.e(TAG, "SecurityException when creating download directory.", e);
}
}
// Determine how much space is available on the storage device where downloads // Determine how much space is available on the storage device where downloads
// reside. If the downloads directory doesn't exist, it is likely that the user // reside. If the downloads directory doesn't exist, it is likely that the user
......
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