Photo Picker dialog: Show the dialog when image types are being requested.

ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java

Index: ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java
diff --git a/ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java b/ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java
index 753a559340d994f64956f794b4dcb295ea7ed12a..71502436a688eb8ebc2c3ac2516cb5d36d71d768 100644
--- a/ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java
+++ b/ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java
@@ -18,6 +18,7 @@ import android.os.AsyncTask;
 import android.os.Build;
 import android.provider.MediaStore;
 import android.text.TextUtils;
+import android.webkit.MimeTypeMap;
 
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.ContentUriUtils;
@@ -251,7 +252,8 @@ public class SelectFileDialog implements WindowAndroid.IntentCallback,
 
         // Use new photo picker, if available.
         Activity activity = mWindowAndroid.getActivity().get();
-        if (activity != null && UiUtils.showPhotoPicker(activity, this, mAllowMultiple)) {
+        if (activity != null && usePhotoPicker()

[Ted C, 2017/05/26 20:21:14]

Could usePhotoPicker() be:

TextUtils.equals(getContentIntent.getType(), ALL_IMAGE_TYPES) &&
!captureCamera()

I wonder if the block with noSpecificType() is doing what we want above.  Or
maybe we would want to set a boolean in the shouldShowImageTypes() branch above.
 That "seems" what we want.

[Finnur, 2017/05/29 15:18:36]

I don't see how that can work. 

There's two ways to specify images in HTML: with MIME types (image/jpg) or with
Extensions (.jpg).

Lets take an example, where I specify both, as in: "image/jpg,*.gif".

noSpecificType() would in that case return true, which makes the branch point at
line 225 above skip lines 226-242, causing getContentIntent to have type */*,
and the photo picker wouldn't be shown.

I do however, think that this code belongs above all the code that constructs a
getContentIntent, so I have moved it higher up (and the UMA to the top, above
the early returns).

Also added a test.

[Ted C, 2017/05/30 04:20:52]

If that is the case, then I'd argue the previous/existing behavior was wrong. 
With
the example you showed, shouldn't shouldShowImageTypes() return true (not
in the current form, but conceptually)?

It seems odd to me to have two methods that check the same thing.  Especially
if the existing Android photo picker would handle your example correctly (
filtering to just those file formats).

I don't know if it needs to block this anymore, but it seems that we should
reconcile this and explain the difference.

[Finnur, 2017/05/30 11:16:10]

I tend to agree with you. I suspect the previous behavior is wrong, but am not
100% sure. But yeah, we should reconcile this.

+                && UiUtils.showPhotoPicker(activity, this, mAllowMultiple)) {
             return;
         }
 
@@ -267,6 +269,26 @@ public class SelectFileDialog implements WindowAndroid.IntentCallback,
         }
     }
 
+    /**
+     * Determines if a photo picker can be used instead of the stock Android picker.
+     * @return True if only images types are being requested.
+     *

[Ted C, 2017/05/26 20:21:14]

remove extra blank line

[Finnur, 2017/05/29 15:18:36]

Done.

+     */
+    private boolean usePhotoPicker() {
+        boolean usePhotoPicker = false;
+        for (String type : mFileTypes) {
+            String mimeType;
+            if (type.startsWith(".")) {
+                String extension = type.substring(1, type.length());
+                mimeType = MimeTypeMap.getSingleton().getMimeTypeFromExtension(extension);
+            } else {
+                mimeType = type;
+            }
+            usePhotoPicker = mimeType != null && mimeType.startsWith("image/");
+        }
+        return usePhotoPicker;
+    }
+
     @Override
     public void onPickerUserAction(Action action, String[] photos) {
         UiUtils.dismissPhotoPicker();