[Android] Expose showFileChooser in AwContentsClient interface

[Android] Expose showFileChooser in AwContentsClient interface

Adapts the WebContentsDelegate::RunFileChooser into a java embedding
callback.

BUG=Internal b/6930981
TBR=ben@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=216811

[mkosiba (inactive), 2013-07-29 10:59:03]

https://codereview.chromium.org/20666003/diff/15002/android_webview/java/src/...
File
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java
(right):

https://codereview.chromium.org/20666003/diff/15002/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java:159:
Log.w(TAG, "Ignroing spurious showFileChooser result");
same thing here - IMHO this should throw because it's likely a programmer error.

https://codereview.chromium.org/20666003/diff/15002/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java:165:
if (!ret && !completed.getAndSet(true)) {
ISTM we should throw some sort of exception the 3rd party returns false but
invokes the callback. IMHO doing that is probably some sort of programmer error
and should be pointed out to the user of the API.

https://codereview.chromium.org/20666003/diff/15002/android_webview/native/aw...
File android_webview/native/aw_web_contents_delegate.cc (right):

https://codereview.chromium.org/20666003/diff/15002/android_webview/native/aw...
android_webview/native/aw_web_contents_delegate.cc:216: LOG(INFO) << "File
Chooser result: mode = " << mode
VLOG?

[joth, 2013-08-03 19:01:05]

thx. Much cleaner with no bool return paths.

https://codereview.chromium.org/20666003/diff/15002/android_webview/java/src/...
File
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java
(right):

https://codereview.chromium.org/20666003/diff/15002/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java:159:
Log.w(TAG, "Ignroing spurious showFileChooser result");
On 2013/07/29 10:59:03, mkosiba wrote:
> same thing here - IMHO this should throw because it's likely a programmer
error.

Done.

https://codereview.chromium.org/20666003/diff/15002/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java:165:
if (!ret && !completed.getAndSet(true)) {
On 2013/07/29 10:59:03, mkosiba wrote:
> ISTM we should throw some sort of exception the 3rd party returns false but
> invokes the callback. IMHO doing that is probably some sort of programmer
error
> and should be pointed out to the user of the API.

Done.

https://codereview.chromium.org/20666003/diff/15002/android_webview/native/aw...
File android_webview/native/aw_web_contents_delegate.cc (right):

https://codereview.chromium.org/20666003/diff/15002/android_webview/native/aw...
android_webview/native/aw_web_contents_delegate.cc:216: LOG(INFO) << "File
Chooser result: mode = " << mode
On 2013/07/29 10:59:03, mkosiba wrote:
> VLOG?

This is rare enough and interesting enough that I think it's worth having in the
full log for bugreports etc.

[joth, 2013-08-07 00:10:17]

ping! would be handy to get this through this week (the hook-up steps can wait)

[benm (inactive), 2013-08-07 09:50:58]

https://codereview.chromium.org/20666003/diff/29001/android_webview/native/aw...
File android_webview/native/aw_web_contents_delegate.cc (right):

https://codereview.chromium.org/20666003/diff/29001/android_webview/native/aw...
android_webview/native/aw_web_contents_delegate.cc:99: mode_flags |=
kFileChooserModeSave;
We're exposing more things than the old file picker API I think here - what does
it mean to get a Save file request in WebView, how should the mebedder respond
to that?

Should we abort this early rather than propagating to Java? Same for Open
folder/open multiple, and just support the simple open case for now?

[mkosiba (inactive), 2013-08-07 11:18:58]

LGTM % any feedback from Ben

[joth, 2013-08-07 18:22:44]

On 2013/08/07 09:50:58, benm wrote:
>
https://codereview.chromium.org/20666003/diff/29001/android_webview/native/aw...
> File android_webview/native/aw_web_contents_delegate.cc (right):
> 
>
https://codereview.chromium.org/20666003/diff/29001/android_webview/native/aw...
> android_webview/native/aw_web_contents_delegate.cc:99: mode_flags |=
> kFileChooserModeSave;
> We're exposing more things than the old file picker API I think here - what
does
> it mean to get a Save file request in WebView, how should the mebedder respond
> to that?
> 

I agree Save is spurious will drop this from here

> Should we abort this early rather than propagating to Java? Same for Open
> folder/open multiple, and just support the simple open case for now?

Folder and allow multiple can fallback to selecting a single file very nicely,
(this is what happens already if you go to a website with <input type=file
multiple> on an older browsr that does not recognize the multiple tag anyway,
likewise for [webkit]directory )

Adding multiple later is a pain due to the ValueCallback needing to take an
array vs single item.

[commit-bot: I haz the power, 2013-08-08 01:11:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joth@chromium.org/20666003/42001

[joth, 2013-08-08 01:13:36]

riding mkosiba lgtm. Let me know if you have more thoughts and I can make a
follow-up

https://codereview.chromium.org/20666003/diff/29001/android_webview/native/aw...
File android_webview/native/aw_web_contents_delegate.cc (right):

https://codereview.chromium.org/20666003/diff/29001/android_webview/native/aw...
android_webview/native/aw_web_contents_delegate.cc:99: mode_flags |=
kFileChooserModeSave;
On 2013/08/07 09:50:59, benm wrote:
> We're exposing more things than the old file picker API I think here - what
does
> it mean to get a Save file request in WebView, how should the mebedder respond
> to that?
> 
> Should we abort this early rather than propagating to Java? Same for Open
> folder/open multiple, and just support the simple open case for now?

Done (removed Save)

[joth, 2013-08-08 01:41:23]

+ben@ for the DEPS change (adding ui/shell_dialogs)

- I don't really use the shell_dialogs, but it's mandatory to depend on it if
you want to call the RenderViewHost::FilesSelectedInChooser as it takes a  
ui::SelectedFileInfo

[joth, 2013-08-08 21:26:02]

+sky for DEPS approval as ben is not replied. (I'm OOO monday but would like to
get this landed before the m30 branch point)

[benm (inactive), 2013-08-09 14:34:42]

On 2013/08/08 21:26:02, joth wrote:
> +sky for DEPS approval as ben is not replied. (I'm OOO monday but would like
to
> get this landed before the m30 branch point)

LGTM. TBR for deps?

[joth, 2013-08-09 20:55:41]

On 2013/08/09 14:34:42, benm wrote:
> On 2013/08/08 21:26:02, joth wrote:
> > +sky for DEPS approval as ben is not replied. (I'm OOO monday but would like
> to
> > get this landed before the m30 branch point)
> 
> LGTM. 

Thanks

> TBR for deps?

Yep will do. Ben G please do take a look at
https://codereview.chromium.org/20666003/diff/49001/android_webview/native/DEPS
when you have chance. I can't imagine this is a problem given
ui::SelectedFileInfo is in the content public API, which I know you're fine with
android_webview/ depending on :)

[commit-bot: I haz the power, 2013-08-09 20:59:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joth@chromium.org/20666003/49001

[commit-bot: I haz the power, 2013-08-10 03:31:02]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-08-10 04:51:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joth@chromium.org/20666003/49001

[commit-bot: I haz the power, 2013-08-10 07:05:51]

Change committed as 216811