Bug 1506649 - Part 1: Avoid exeception-based control-flow for resolving content://-URIs. r=snorp
authorJan Henning <jh+bugzilla@buttercookie.de>
Wed, 26 Dec 2018 20:19:15 +0000
changeset 509287 242e66d8473405df9973d38ebed676b84edc9241
parent 509275 ad2f233430e80a4c3764e5aacfbb164d169a9cac
child 509288 fd078630d6661dfa3cc43dc793c45fda91e4b78c
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp
bugs1506649
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1506649 - Part 1: Avoid exeception-based control-flow for resolving content://-URIs. r=snorp Instead, getOriginalFilePathFromUri() will simply *always* return null if it cannot divine the original file path, and consequently resolveContentUri() will then always fall back to the temp file method if getOriginalFilePathFromUri() returns null. Differential Revision: https://phabricator.services.mozilla.com/D15257
mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/ContentUriUtils.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/FileUtils.java
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/ContentUriUtils.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/ContentUriUtils.java
@@ -42,18 +42,17 @@ public class ContentUriUtils {
      * Callers should check whether the path is local before assuming it
      * represents a local file.
      *
      * @param context The context.
      * @param uri The Uri to query.
      * @author paulburke
      */
     @SuppressLint("NewAPI")
-    public static @Nullable String getOriginalFilePathFromUri(final Context context, final Uri uri) throws IllegalArgumentException {
-
+    public static @Nullable String getOriginalFilePathFromUri(final Context context, final Uri uri) {
         final boolean isKitKat = Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT;
 
         // DocumentProvider
         if (isKitKat && DocumentsContract.isDocumentUri(context, uri)) {
             // ExternalStorageProvider
             if (isExternalStorageDocument(uri)) {
                 final String docId = DocumentsContract.getDocumentId(uri);
                 final String[] split = docId.split(":");
@@ -154,28 +153,27 @@ public class ContentUriUtils {
      * @param context The context.
      * @param uri The Uri to query.
      * @param selection (Optional) Filter used in the query.
      * @param selectionArgs (Optional) Selection arguments used in the query.
      * @return The value of the _data column, which is typically a file path.
      * @author paulburke
      */
     private static String getDataColumn(Context context, Uri uri, String selection,
-                                        String[] selectionArgs) throws IllegalArgumentException {
-
+                                        String[] selectionArgs) {
         final String column = "_data";
         final String[] projection = {
                 column
         };
 
         try (Cursor cursor = context.getContentResolver().query(uri, projection, selection, selectionArgs,
                 null)) {
             if (cursor != null && cursor.moveToFirst()) {
-                final int column_index = cursor.getColumnIndexOrThrow(column);
-                return cursor.getString(column_index);
+                final int column_index = cursor.getColumnIndex(column);
+                return column_index >= 0 ? cursor.getString(column_index) : null;
             }
         }
         return null;
     }
 
     /**
      * @param uri The Uri to check.
      * @return Whether the Uri authority is ExternalStorageProvider.
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/FileUtils.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/FileUtils.java
@@ -289,20 +289,18 @@ public class FileUtils {
         Random random = new Random();
         do {
             result = new File(directory, prefix + random.nextInt());
         } while (!result.mkdirs());
         return result;
     }
 
     public static String resolveContentUri(final Context context, final Uri uri) {
-        String path;
-        try {
-            path = getOriginalFilePathFromUri(context, uri);
-        } catch (IllegalArgumentException ex) {
+        String path = getOriginalFilePathFromUri(context, uri);
+        if (TextUtils.isEmpty(path)) {
             // We cannot always successfully guess the original path of the file behind the
             // content:// URI, so we need a fallback. This will break local subresources and
             // relative links, but unfortunately there's nothing else we can do
             // (see https://issuetracker.google.com/issues/77406791).
             path = getTempFilePathFromContentUri(context, uri);
         }
         return !TextUtils.isEmpty(path) ? String.format(FILE_ABSOLUTE_URI, path) : path;
     }