Support accept attribute for an <input type=file> element....

Support accept attribute for an <input type=file> element.

BUG=54009
TEST=none


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

[darin (slow to review), 2010-09-07 23:09:14]

http://codereview.chromium.org/3311016/diff/9001/10002
File chrome/browser/tab_contents/tab_contents_file_select_helper.cc (right):

http://codereview.chromium.org/3311016/diff/9001/10002#newcode140
chrome/browser/tab_contents/tab_contents_file_select_helper.cc:140:
file_type->extensions.push_back(extensions);
nit: use .swap here to reduce allocation?

http://codereview.chromium.org/3311016/diff/9001/10003
File chrome/browser/tab_contents/tab_contents_file_select_helper.h (right):

http://codereview.chromium.org/3311016/diff/9001/10003#newcode51
chrome/browser/tab_contents/tab_contents_file_select_helper.h:51: //
http://www.whatwg.org/specs/web-apps/current-work/multipage/number-state.html...
nit: you can shorten this URL to:

  http://whatwg.org/html/number-state.html#attr-input-accept

http://codereview.chromium.org/3311016/diff/9001/10003#newcode53
chrome/browser/tab_contents/tab_contents_file_select_helper.h:53: string16
accept_types);
nit: "string16 accept_types" -> "const string16& accept_types"

http://codereview.chromium.org/3311016/diff/9001/10007
File net/base/mime_util.cc (right):

http://codereview.chromium.org/3311016/diff/9001/10007#newcode537
net/base/mime_util.cc:537: static const char* standard_image_types[] = {
nit: i think these should be written in kFooBar style since they are constants. 
i see that some of the const data at the top of this file does not follow that
style rule.

http://codereview.chromium.org/3311016/diff/9001/10007#newcode596
net/base/mime_util.cc:596: void GetExtensionsFromHardCodedMappings(
these helper functions and the static data should really all be in an anonymous
namespace to avoid link time conflicts with other things in the net:: namespace.

http://codereview.chromium.org/3311016/diff/9001/10007#newcode614
net/base/mime_util.cc:614: extensions->push_back(extension);
do you care about de-duping here?  it looks like .png will appear more than once
for example.

http://codereview.chromium.org/3311016/diff/9001/10008
File net/base/mime_util.h (right):

http://codereview.chromium.org/3311016/diff/9001/10008#newcode74
net/base/mime_util.h:74: // Get the extensions for images files.
you should probably mention that these functions append to the given vector
without erasing what might already be there.

[jianli, 2010-09-08 00:43:42]

All fixed. Thanks.

http://codereview.chromium.org/3311016/diff/9001/10002
File chrome/browser/tab_contents/tab_contents_file_select_helper.cc (right):

http://codereview.chromium.org/3311016/diff/9001/10002#newcode140
chrome/browser/tab_contents/tab_contents_file_select_helper.cc:140:
file_type->extensions.push_back(extensions);
On 2010/09/07 23:09:14, darin wrote:
> nit: use .swap here to reduce allocation?

Changed to preallocate before the loop.

http://codereview.chromium.org/3311016/diff/9001/10003
File chrome/browser/tab_contents/tab_contents_file_select_helper.h (right):

http://codereview.chromium.org/3311016/diff/9001/10003#newcode51
chrome/browser/tab_contents/tab_contents_file_select_helper.h:51: //
http://www.whatwg.org/specs/web-apps/current-work/multipage/number-state.html...
On 2010/09/07 23:09:14, darin wrote:
> nit: you can shorten this URL to:
> 
>   http://whatwg.org/html/number-state.html#attr-input-accept

Done.

http://codereview.chromium.org/3311016/diff/9001/10003#newcode53
chrome/browser/tab_contents/tab_contents_file_select_helper.h:53: string16
accept_types);
On 2010/09/07 23:09:14, darin wrote:
> nit: "string16 accept_types" -> "const string16& accept_types"

Done.

http://codereview.chromium.org/3311016/diff/9001/10007
File net/base/mime_util.cc (right):

http://codereview.chromium.org/3311016/diff/9001/10007#newcode537
net/base/mime_util.cc:537: static const char* standard_image_types[] = {
On 2010/09/07 23:09:14, darin wrote:
> nit: i think these should be written in kFooBar style since they are
constants. 
> i see that some of the const data at the top of this file does not follow that
> style rule.

Done.

http://codereview.chromium.org/3311016/diff/9001/10007#newcode596
net/base/mime_util.cc:596: void GetExtensionsFromHardCodedMappings(
On 2010/09/07 23:09:14, darin wrote:
> these helper functions and the static data should really all be in an
anonymous
> namespace to avoid link time conflicts with other things in the net::
namespace.

Done.

http://codereview.chromium.org/3311016/diff/9001/10007#newcode614
net/base/mime_util.cc:614: extensions->push_back(extension);
On 2010/09/07 23:09:14, darin wrote:
> do you care about de-duping here?  it looks like .png will appear more than
once
> for example.

Done.

http://codereview.chromium.org/3311016/diff/9001/10008
File net/base/mime_util.h (right):

http://codereview.chromium.org/3311016/diff/9001/10008#newcode74
net/base/mime_util.h:74: // Get the extensions for images files.
On 2010/09/07 23:09:14, darin wrote:
> you should probably mention that these functions append to the given vector
> without erasing what might already be there.

Done.

[darin (slow to review), 2010-09-08 05:18:32]

LGTM... mime_util.cc was a mess to begin with... probably not worth trying to
clean it all up in this CL.  what you've done is a good improvement though.

http://codereview.chromium.org/3311016/diff/19001/20007
File net/base/mime_util.cc (right):

http://codereview.chromium.org/3311016/diff/19001/20007#newcode20
net/base/mime_util.cc:20: namespace {
it would have been OK to put the anonymous namespace within the net namespace
too.