Adds the optional suggestedName to the chooseFile fileSystem API call.

Adds the optional suggestedName to the chooseFile fileSystem API call.

BUG=
TEST=


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=146771

[benwells, 2012-07-12 08:23:32]

https://chromiumcodereview.appspot.com/10696188/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/api/file_system/file_system_api.cc (right):

https://chromiumcodereview.appspot.com/10696188/diff/1/chrome/browser/extensi...
chrome/browser/extensions/api/file_system/file_system_api.cc:247: :
suggested_path_(suggested_path),
Nit: change suggested_path here to suggested_name.

https://chromiumcodereview.appspot.com/10696188/diff/1/chrome/browser/extensi...
chrome/browser/extensions/api/file_system/file_system_api.cc:375: FilePath
suggested_path;
Nit: suggested_name

https://chromiumcodereview.appspot.com/10696188/diff/1/chrome/browser/extensi...
chrome/browser/extensions/api/file_system/file_system_api.cc:394: suggested_path
= FilePath::FromUTF8Unsafe(*options->suggested_name.get())
We should just use BaseName, so if an extension tries to put in a complete path
the directory information would be stripped.

[thorogood, 2012-07-12 08:40:19]

https://chromiumcodereview.appspot.com/10696188/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/api/file_system/file_system_api.cc (right):

https://chromiumcodereview.appspot.com/10696188/diff/1/chrome/browser/extensi...
chrome/browser/extensions/api/file_system/file_system_api.cc:247: :
suggested_path_(suggested_path),
On 2012/07/12 08:23:33, benwells wrote:
> Nit: change suggested_path here to suggested_name.

Done, and I've changed it in a few other places in this file.

https://chromiumcodereview.appspot.com/10696188/diff/1/chrome/browser/extensi...
chrome/browser/extensions/api/file_system/file_system_api.cc:375: FilePath
suggested_path;
On 2012/07/12 08:23:33, benwells wrote:
> Nit: suggested_name

Done.

https://chromiumcodereview.appspot.com/10696188/diff/1/chrome/browser/extensi...
chrome/browser/extensions/api/file_system/file_system_api.cc:394: suggested_path
= FilePath::FromUTF8Unsafe(*options->suggested_name.get())
On 2012/07/12 08:23:33, benwells wrote:
> We should just use BaseName, so if an extension tries to put in a complete
path
> the directory information would be stripped.

Done. BaseName may still return an absolute path if the path is a root path
name. So if an app says suggestedName: '/', then we'd end up always opening the
user in the root folder (and similar on Windows for C:\, D:\ etc). I've added a
check for this too which just clears the name.

[benwells, 2012-07-12 08:48:01]

lgtm

https://chromiumcodereview.appspot.com/10696188/diff/4/chrome/browser/extensi...
File chrome/browser/extensions/api/file_system/file_system_api.cc (right):

https://chromiumcodereview.appspot.com/10696188/diff/4/chrome/browser/extensi...
chrome/browser/extensions/api/file_system/file_system_api.cc:398: // result in a
relative path, but in some cases may not be. Clear the
nit: ...cases may not.

[Mihai Parparita -not on Chrome, 2012-07-13 00:43:55]

LGTM

[thorogood, 2012-07-13 01:15:22]

https://chromiumcodereview.appspot.com/10696188/diff/4/chrome/browser/extensi...
File chrome/browser/extensions/api/file_system/file_system_api.cc (right):

https://chromiumcodereview.appspot.com/10696188/diff/4/chrome/browser/extensi...
chrome/browser/extensions/api/file_system/file_system_api.cc:398: // result in a
relative path, but in some cases may not be. Clear the
On 2012/07/12 08:48:01, benwells wrote:
> nit: ...cases may not.

Done.

[commit-bot: I haz the power, 2012-07-13 01:15:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thorogood@chromium.org/10696188/7001

[thorogood, 2012-07-13 01:15:30]

On 2012/07/13 01:15:22, thorogood wrote:
>
https://chromiumcodereview.appspot.com/10696188/diff/4/chrome/browser/extensi...
> File chrome/browser/extensions/api/file_system/file_system_api.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10696188/diff/4/chrome/browser/extensi...
> chrome/browser/extensions/api/file_system/file_system_api.cc:398: // result in
a
> relative path, but in some cases may not be. Clear the
> On 2012/07/12 08:48:01, benwells wrote:
> > nit: ...cases may not.
> 
> Done.

[commit-bot: I haz the power, 2012-07-13 03:04:20]

Try job failure for 10696188-7001 (retry) on win_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[thorogood, 2012-07-13 03:05:45]

On 2012/07/13 03:04:20, I haz the power (commit-bot) wrote:
> Try job failure for 10696188-7001 (retry) on win_rel for step "browser_tests".
> It's a second try, previously, step "browser_tests" failed.
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-07-13 03:06:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thorogood@chromium.org/10696188/7001

[commit-bot: I haz the power, 2012-07-13 03:55:03]

Try job failure for 10696188-7001 (retry) on linux_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[thorogood, 2012-07-13 04:42:22]

On 2012/07/13 03:55:03, I haz the power (commit-bot) wrote:
> Try job failure for 10696188-7001 (retry) on linux_rel for step
"browser_tests".
> It's a second try, previously, step "browser_tests" failed.
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-07-13 04:42:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thorogood@chromium.org/10696188/6003

[commit-bot: I haz the power, 2012-07-13 05:52:41]

Try job failure for 10696188-6003 (retry) on linux_rel for step "browser_tests".
It's a second try, previously, steps "browser_tests, check_deps" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[thorogood, 2012-07-13 13:32:53]

On 2012/07/13 05:52:41, I haz the power (commit-bot) wrote:
> Try job failure for 10696188-6003 (retry) on linux_rel for step
"browser_tests".
> It's a second try, previously, steps "browser_tests, check_deps" failed.
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-07-13 13:32:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thorogood@chromium.org/10696188/6003

[commit-bot: I haz the power, 2012-07-13 15:46:00]

Try job failure for 10696188-6003 (retry) (retry) on linux_rel for step
"browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[thorogood, 2012-07-15 23:36:20]

On 2012/07/13 15:46:00, I haz the power (commit-bot) wrote:
> Try job failure for 10696188-6003 (retry) (retry) on linux_rel for step
> "browser_tests".
> It's a second try, previously, step "browser_tests" failed.
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-07-15 23:36:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thorogood@chromium.org/10696188/6003

[commit-bot: I haz the power, 2012-07-16 01:49:19]

Change committed as 146771