Update the HTML Media Capture implementation.

ui/shell_dialogs/select_file_dialog_android.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "select_file_dialog_android.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"
@@ skipping 45 matching lines @@
     const base::string16& title,
     const base::FilePath& default_path,
     const SelectFileDialog::FileTypeInfo* file_types,
     int file_type_index,
     const std::string& default_extension,
     gfx::NativeWindow owning_window,
     void* params) {
   JNIEnv* env = base::android::AttachCurrentThread();
 
   ScopedJavaLocalRef<jstring> capture_value;
-  std::vector<base::string16> accept_types;
+
+  // The first element in the pair is a list of accepted types, the second
+  // indicates whether the device's capture capabilities should be used.
+  typedef std::pair<std::vector<base::string16>, bool> AcceptTypes;
+  AcceptTypes accept_types;
+
   if (params) {
-    accept_types = *(reinterpret_cast<std::vector<base::string16>*>(params));
+    accept_types = *(reinterpret_cast<AcceptTypes*>(params));
 
-    // The last string in params is expected to be the string
-    // with capture value.
-    capture_value = base::android::ConvertUTF16ToJavaString(env,
-          StringToLowerASCII(accept_types.back()));
+    // The latest version of the HTML Media Capture spec says the "capture"

[joth, 2013/07/03 17:41:42]

nit: 'latest' is ambiguous when read in future. Include a date, or say "As of
spec version x.y..."

+    // attribute should be a boolean, and implementations are supposed to infer
+    // which application to launch based on the value of the "accept"
+    // attribute. However, since we have already shipped an implementation of

[joth, 2013/07/03 17:41:42]

likewise, could say versions of chrome up to M29 shipped with....

+    // an earlier version of the spec in which the "capture" attribute was an
+    // enum, we need to keep compatibility for a while. We do that by creating
+    // a valid string value accepted by SelectFileDialog.java based on the
+    // contents of the "accept" attribute. This approximation is not 100%
+    // compatible with the previous behavior, as we cannot infer a value for
+    // "capture" if "accept" is set to something like "*/*" or
+    // "audio/*,video/*".
+    // TODO: crbug.com/240252 We need to phase out support for the old version
+    // of the spec at some point.

[joth, 2013/07/03 17:41:42]

I'm still feeling mislead by this comment. An old website that says
"capture=camera" will have the 'camera' value ignored, regardless of the block
of code here.
I think that's acceptable. But I just think we need to update the comment to not
suggest this if/else ladder is trying to retain site backwards compat. I think
it's really just retaining Java SelectFileDialog backwards compat. (And we can
remove that... see next comment)

+    bool has_capture_attribute = accept_types.second;
+    string16 normalized_capture_value;
+
+    if (has_capture_attribute && accept_types.first.size() == 1) {
+      if (StartsWithASCII(accept_types.front(), "image/*", false)) {
+        normalized_capture_value = "camera";
+      } else if (StartsWithASCII(accept_types.front(), "video/*", false)) {
+        normalized_capture_value = "camcorder";
+      } else if (StartsWithASCII(accept_types.front(), "audio/*", false)) {
+        normalized_capture_value = "microphone";
+      }
+    }
+
+    capture_value = base::android::ConvertUTF16ToJavaString(
+        env, normalized_capture_value);
     base::android::CheckException(env);
-    accept_types.pop_back();
   } else {
     capture_value = base::android::ConvertUTF8ToJavaString(env, "filesystem");
   }
 
-  // The rest params elements are expected to be accept_types.
   ScopedJavaLocalRef<jobjectArray> accept_types_java =
-      base::android::ToJavaArrayOfStrings(env, accept_types);
+      base::android::ToJavaArrayOfStrings(env, accept_types.first);
 
   Java_SelectFileDialog_selectFile(env, java_object_.obj(),
                                    accept_types_java.obj(),
                                    capture_value.obj(),

[joth, 2013/07/03 17:41:42]

Can't we go the full way and pass 'has_capture_attribute' here as the 4th param,
and move all the policy about which source to use when into the Java side? It's
hard to read having it split part here, and part in Java.

e.g. modify all the Java side helper methods like so:-

    private boolean specificTypeIs(String type) {
        return mFileTypes.size() == 1 && mFileTypes.at(0).equals(type);
    }

    private boolean captureCamcorder() {
        return mCapture && specificTypeIs(ALL_VIDEO_TYPES);
    }

etc...

                                    owning_window->GetJavaObject().obj());
   is_running_ = true;
 }
 
 bool SelectFileDialogImpl::RegisterSelectFileDialog(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 SelectFileDialogImpl::~SelectFileDialogImpl() {
 }
@@ skipping 11 matching lines @@
   return false;
 }
 
 SelectFileDialog* CreateAndroidSelectFileDialog(
     SelectFileDialog::Listener* listener,
     SelectFilePolicy* policy) {
   return SelectFileDialogImpl::Create(listener, policy);
 }
 
 }  // namespace ui