Figure out the display name for a content uri.

Figure out the display name for a content uri.

Plumb it through the File listener so blink can display the right
content on <input type=file> elements.

BUG=338929
NOTRY=true

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

[Miguel Garcia, 2014-01-30 14:25:32]

After https://codereview.chromium.org/75533002/ we no longer respect the display
name of the file, this was calculated on the fly based on the file path but
since we now pass content uris you get a random string as the file name. This
change figures out the display name in java and uses the ExtraInfo that Blink
accepts to display the right file name.

[Miguel Garcia, 2014-01-30 14:26:31]

On 2014/01/30 14:25:32, Miguel Garcia wrote:
> After https://codereview.chromium.org/75533002/ we no longer respect the
display
> name of the file, this was calculated on the fly based on the file path but
> since we now pass content uris you get a random string as the file name. This
> change figures out the display name in java and uses the ExtraInfo that Blink
> accepts to display the right file name.

sky: for ui/OWNERS (since there are no OWNERS in src/ui/shell_dialogs/)
Min and Yaron as the main reviewers.

[Miguel Garcia, 2014-01-31 11:03:45]

Friendly ping on this review. Thanks!

[qinmin, 2014-01-31 16:17:29]

lgtm % nit

https://codereview.chromium.org/150283002/diff/1/ui/android/java/src/org/chro...
File ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java (right):

https://codereview.chromium.org/150283002/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:193:
results.getData().toString(),
nit: align with mNativeSelectFileDialog here and below

[Miguel Garcia, 2014-01-31 16:35:48]

https://codereview.chromium.org/150283002/diff/1/ui/android/java/src/org/chro...
File ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java (right):

https://codereview.chromium.org/150283002/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:193:
results.getData().toString(),
On 2014/01/31 16:17:29, qinmin wrote:
> nit: align with mNativeSelectFileDialog here and below

Done.

[sky, 2014-02-03 04:50:32]

I'm not a good reviewer for any of these files. If there are no OWNERs for the
android files, they should be added.

https://codereview.chromium.org/150283002/diff/40001/ui/shell_dialogs/select_...
File ui/shell_dialogs/select_file_dialog_android.h (right):

https://codereview.chromium.org/150283002/diff/40001/ui/shell_dialogs/select_...
ui/shell_dialogs/select_file_dialog_android.h:21: void OnFileSelected(JNIEnv*
env, jobject java_object, jstring filepath,
nit: one param per line.

[Miguel Garcia, 2014-02-03 09:41:58]

Ok, I added OWNERS for the Android files. 

Min and me have been contributing recently to this area and know the code.

Aurimas was the original author.

https://codereview.chromium.org/150283002/diff/40001/ui/shell_dialogs/select_...
File ui/shell_dialogs/select_file_dialog_android.h (right):

https://codereview.chromium.org/150283002/diff/40001/ui/shell_dialogs/select_...
ui/shell_dialogs/select_file_dialog_android.h:21: void OnFileSelected(JNIEnv*
env, jobject java_object, jstring filepath,
On 2014/02/03 04:50:33, sky wrote:
> nit: one param per line.

Done.

[sky, 2014-02-03 21:20:11]

Thanks, LGTM
+yfriedman as I don't know any of these people.

[aurimas (slooooooooow), 2014-02-03 21:27:02]

LGTM with nits

https://codereview.chromium.org/150283002/diff/150001/ui/android/java/src/org...
File ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java (right):

https://codereview.chromium.org/150283002/diff/150001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:145: return
cursor.getString(index);
You should just make it into one line:
if (index > -1) return cursor.getString(index);

https://codereview.chromium.org/150283002/diff/150001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:187:
results.getData().getSchemeSpecificPart(), "");
In Java we use 8 space offset instead of assigning with the left parenthesis.

[Yaron, 2014-02-04 02:30:34]

lgtm for owners file. one question I'd like clarified

https://codereview.chromium.org/150283002/diff/150001/ui/android/java/src/org...
File ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java (right):

https://codereview.chromium.org/150283002/diff/150001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:133: * @return
the display name of the @code uri if present in the data base
s/data base/database

https://codereview.chromium.org/150283002/diff/150001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:139: cursor =
contentResolver.query(uri, null, null, null, null);
I suspect this should be pretty fast, right? We just queried the content
resolver for the file info and now just getting an additional field is cheap?
Just want to make sure we aren't inducing jank since we're pinging another
process seemingly form the UI thread.

[Miguel Garcia, 2014-02-04 10:12:50]

https://codereview.chromium.org/150283002/diff/150001/ui/android/java/src/org...
File ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java (right):

https://codereview.chromium.org/150283002/diff/150001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:133: * @return
the display name of the @code uri if present in the data base
On 2014/02/04 02:30:35, Yaron wrote:
> s/data base/database

Done.

https://codereview.chromium.org/150283002/diff/150001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:139: cursor =
contentResolver.query(uri, null, null, null, null);
Indeed, this should not be introducing any jank.


On 2014/02/04 02:30:35, Yaron wrote:
> I suspect this should be pretty fast, right? We just queried the content
> resolver for the file info and now just getting an additional field is cheap?
> Just want to make sure we aren't inducing jank since we're pinging another
> process seemingly form the UI thread.

https://codereview.chromium.org/150283002/diff/150001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:145: return
cursor.getString(index);
On 2014/02/03 21:27:03, aurimas wrote:
> You should just make it into one line:
> if (index > -1) return cursor.getString(index);

I didn't know this was allowed! nice.

https://codereview.chromium.org/150283002/diff/150001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:187:
results.getData().getSchemeSpecificPart(), "");
On 2014/02/03 21:27:03, aurimas wrote:
> In Java we use 8 space offset instead of assigning with the left parenthesis.

Done.

[Miguel Garcia, 2014-02-04 10:12:57]

The CQ bit was checked by miguelg@chromium.org

[commit-bot: I haz the power, 2014-02-04 10:13:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/miguelg@chromium.org/150283002/210001

[commit-bot: I haz the power, 2014-02-04 11:04:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-04 11:04:55]

Retried try job too often on ios_dbg_simulator for step(s) url_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[Miguel Garcia, 2014-02-04 11:13:58]

The CQ bit was checked by miguelg@chromium.org

[commit-bot: I haz the power, 2014-02-04 11:14:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/miguelg@chromium.org/150283002/210001

[commit-bot: I haz the power, 2014-02-04 12:13:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-04 12:13:31]

Retried try job too often on ios_dbg_simulator for step(s) url_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[Miguel Garcia, 2014-02-04 12:32:42]

The CQ bit was checked by miguelg@chromium.org

[commit-bot: I haz the power, 2014-02-04 12:32:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/miguelg@chromium.org/150283002/210001

[Miguel Garcia, 2014-02-04 12:33:25]

The CQ bit was unchecked by miguelg@chromium.org

[Miguel Garcia, 2014-02-04 12:37:12]

The CQ bit was checked by miguelg@chromium.org

[commit-bot: I haz the power, 2014-02-04 12:37:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/miguelg@chromium.org/150283002/210001

[commit-bot: I haz the power, 2014-02-04 13:09:45]

Change committed as 248707