Download location: Fix issue that can't access file on SD card.

Chrome uses file URL to access file on disk. The path needs to be in
a directory white list.

This CL adds the external SD card directory to the white list, so file
can be accessed through file URL. Also moves the core function
getAllDownloadDirectories to PathUtils for easier access in native code.


Bug: 838419,792775
Change-Id: If52202d6a62f89106b1e8533f8c1884b17cc9e0b
Reviewed-on: https://chromium-review.googlesource.com/1050394
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557759}

base/android/java/src/org/chromium/base/PathUtils.java

@@ -210,6 +210,27 @@ public abstract class PathUtils {
         }
     }
 
+    /**
+     * @return Download directories including the default storage directory on SD card, and a
+     * private directory on external SD card.
+     */
+    @SuppressWarnings("unused")
+    @CalledByNative
+    public static String[] getAllPrivateDownloadsDirectories() {
+        File[] files;
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) {
+            files = ContextUtils.getApplicationContext().getExternalFilesDirs(
+                    Environment.DIRECTORY_DOWNLOADS);
+        } else {
+            files = new File[] {Environment.getExternalStorageDirectory()};
+        }
+        String[] result = new String[files.length];
+        for (int i = 0; i < files.length; ++i) {
+            result[i] = files[i].getAbsolutePath();
+        }
+        return result;
+    }
+
     /**
      * @return the path to native libraries.
      */

base/android/path_utils.cc

@@ -5,6 +5,7 @@
 #include "base/android/path_utils.h"
 
 #include "base/android/jni_android.h"
+#include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/files/file_path.h"
@@ -47,6 +48,18 @@ bool GetDownloadsDirectory(FilePath* result) {
   return true;
 }
 
+std::vector<FilePath> GetAllPrivateDownloadsDirectories() {
+  std::vector<std::string> dirs;
+  JNIEnv* env = AttachCurrentThread();
+  auto jarray = Java_PathUtils_getAllPrivateDownloadsDirectories(env);
+  base::android::AppendJavaStringArrayToStringVector(env, jarray.obj(), &dirs);
+
+  std::vector<base::FilePath> file_paths;
+  for (const auto& dir : dirs)
+    file_paths.emplace_back(dir);
+  return file_paths;
+}
+
 bool GetNativeLibraryDirectory(FilePath* result) {
   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jstring> path =

base/android/path_utils.h

@@ -6,6 +6,7 @@
 #define BASE_ANDROID_PATH_UTILS_H_
 
 #include <jni.h>
+#include <vector>
 
 #include "base/base_export.h"
 
@@ -35,6 +36,10 @@ BASE_EXPORT bool GetThumbnailCacheDirectory(FilePath* result);
 // in the FilePath pointed to by 'result'.
 BASE_EXPORT bool GetDownloadsDirectory(FilePath* result);
 
+// Retrieves the paths to all download directories, including default storage
+// directory, and a private directory on external SD card.
+BASE_EXPORT std::vector<FilePath> GetAllPrivateDownloadsDirectories();
+
 // Retrieves the path to the native JNI libraries via
 // ApplicationInfo.nativeLibraryDir on the Java side. The result is placed in
 // the FilePath pointed to by 'result'.

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java

@@ -1543,10 +1543,9 @@ public class DownloadManagerService
             return false;
         }
 
-        File[] externalDirs = DownloadUtils.getAllDownloadDirectories(mContext);
-        for (File dir : externalDirs) {
-            if (dir == null) continue;
-            if (filePath.contains(dir.getAbsolutePath())) return false;
+        for (String dir : DownloadUtils.getAllDownloadDirectories()) {
+            if (TextUtils.isEmpty(dir)) continue;
+            if (filePath.contains(dir)) return false;
         }
 
         return true;

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java

@@ -10,8 +10,6 @@ import android.content.Context;
 import android.content.Intent;
 import android.content.res.ColorStateList;
 import android.net.Uri;
-import android.os.Build;
-import android.os.Environment;
 import android.os.StrictMode;
 import android.support.annotation.IntDef;
 import android.support.annotation.Nullable;
@@ -23,6 +21,7 @@ import org.chromium.base.Callback;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.FileUtils;
 import org.chromium.base.Log;
+import org.chromium.base.PathUtils;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.library_loader.LibraryProcessType;
 import org.chromium.base.metrics.RecordHistogram;
@@ -1038,13 +1037,9 @@ public class DownloadUtils {
      * If the external directories are not available for querying (on older versions of Android),
      * return an array with just the internal directory.
      *
-     * @param context   Context from which to look for the directories.
-     * @return          The list of directories or empty array if no directories.
+     * @return          The absolute paths of download directories.
      */
-    public static File[] getAllDownloadDirectories(Context context) {
-        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) {
-            return context.getExternalFilesDirs(Environment.DIRECTORY_DOWNLOADS);
-        }
-        return new File[] {Environment.getExternalStorageDirectory()};
+    public static String[] getAllDownloadDirectories() {
+        return PathUtils.getAllPrivateDownloadsDirectories();
     }
 }

chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadManagerUi.java

@@ -567,7 +567,7 @@ public class DownloadManagerUi
 
     private void maybeShowDownloadSettingsTextBubble(final Tracker tracker) {
         // If the user doesn't have an SD card don't show the IPH.
-        File[] externalDirs = DownloadUtils.getAllDownloadDirectories(mActivity);
+        String[] externalDirs = DownloadUtils.getAllDownloadDirectories();
         if (externalDirs.length < 2) return;
 
         // Check to see if the help UI should be triggered.

chrome/android/java/src/org/chromium/chrome/browser/preferences/download/DownloadDirectoryAdapter.java

@@ -9,6 +9,7 @@ import android.os.Environment;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.support.v4.widget.TextViewCompat;
+import android.text.TextUtils;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
@@ -239,18 +240,15 @@ public class DownloadDirectoryAdapter extends ArrayAdapter<Object> {
         mAdditionalOptions.clear();
 
         // If there are no more additional directories, it is only the primary storage available.
-        File[] externalDirs = DownloadUtils.getAllDownloadDirectories(mContext);
+        String[] externalDirs = DownloadUtils.getAllDownloadDirectories();
         if (externalDirs.length <= 1) return;
 
         int numOtherAdditionalDirectories = 0;
-        for (File dir : externalDirs) {
-            if (dir == null) continue;
+        for (String dir : externalDirs) {
+            if (TextUtils.isEmpty(dir)) continue;
 
             // Skip the directory that is in primary storage.
-            if (dir.getAbsolutePath().contains(
-                        Environment.getExternalStorageDirectory().getAbsolutePath())) {
-                continue;
-            }
+            if (dir.contains(Environment.getExternalStorageDirectory().getAbsolutePath())) continue;
 
             // Add index (ie. SD Card 2) if there is more than one secondary storage option.
             String directoryName = (numOtherAdditionalDirectories > 0)
@@ -259,7 +257,8 @@ public class DownloadDirectoryAdapter extends ArrayAdapter<Object> {
                               numOtherAdditionalDirectories + 1)
                     : mContext.getString(org.chromium.chrome.R.string.downloads_location_sd_card);
 
-            mAdditionalOptions.add(new DirectoryOption(directoryName, dir, dir.getUsableSpace()));
+            File file = new File(dir);
+            mAdditionalOptions.add(new DirectoryOption(directoryName, file, file.getUsableSpace()));
             numOtherAdditionalDirectories++;
         }
     }

chrome/browser/net/chrome_network_delegate.cc

@@ -60,6 +60,7 @@
 #include "net/url_request/url_request.h"
 
 #if defined(OS_ANDROID)
+#include "base/android/path_utils.h"
 #include "chrome/browser/io_thread.h"
 #endif
 
@@ -170,6 +171,10 @@ bool IsAccessAllowedInternal(const base::FilePath& path,
   if (external_storage_path.IsParent(path))
     return true;
 
+  auto all_download_dirs = base::android::GetAllPrivateDownloadsDirectories();
+  for (const auto& dir : all_download_dirs)
+    whitelist.push_back(dir);
+
   // Whitelist of other allowed directories.
   static const base::FilePath::CharType* const kLocalAccessWhiteList[] = {
       "/sdcard", "/mnt/sdcard",