Fix data URL bug reported on Wikipedia

Fix data URL bug reported on Wikipedia

The previous parser got confused if charset wasn't the last parameter.

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

[abarth-chromium, 2011-05-18 20:18:06]

I'm not really sure who to send this change to.  Will, if you're not he right
person, please route appropriately.  Thx!

[willchan no longer on Chromium, 2011-05-18 21:00:14]

Need a sec to double check logic but stuck in meetings for a bit. Will review
later this afternoon.

http://codereview.chromium.org/7039048/diff/1/net/base/data_url.cc
File net/base/data_url.cc (right):

http://codereview.chromium.org/7039048/diff/1/net/base/data_url.cc#newcode13
net/base/data_url.cc:13: #include "base/string_split.h"
nit: sort alphabetically

http://codereview.chromium.org/7039048/diff/1/net/base/data_url.cc#newcode47
net/base/data_url.cc:47: const char kBase64Tag[] = "base64";
static, otherwise this gets initialized again on each function call

http://codereview.chromium.org/7039048/diff/1/net/base/data_url.cc#newcode48
net/base/data_url.cc:48: const char kCharsetTag[] = "charset=";
ditto

http://codereview.chromium.org/7039048/diff/1/net/base/data_url.cc#newcode49
net/base/data_url.cc:49: const size_t kCharsetTagLength = sizeof(kCharsetTag) -
1;
arraysize or strlen (compilers understand strlen and will turn it into a
compile-time constant)

[Peter Kasting, 2011-05-18 21:05:04]

http://codereview.chromium.org/7039048/diff/1/net/base/data_url.cc
File net/base/data_url.cc (right):

http://codereview.chromium.org/7039048/diff/1/net/base/data_url.cc#newcode49
net/base/data_url.cc:49: const size_t kCharsetTagLength = sizeof(kCharsetTag) -
1;
On 2011/05/18 21:00:14, willchan wrote:
> arraysize or strlen (compilers understand strlen and will turn it into a
> compile-time constant)

I think we've typically used arraysize() for this in other code.

[abarth-chromium, 2011-05-18 21:07:06]

Uploaded fixed version.

[willchan no longer on Chromium, 2011-05-18 23:52:02]

LGTM, thanks for the fix.

http://codereview.chromium.org/7039048/diff/6/net/base/data_url.cc
File net/base/data_url.cc (right):

http://codereview.chromium.org/7039048/diff/6/net/base/data_url.cc#newcode49
net/base/data_url.cc:49: const size_t kCharsetTagLength = arraysize(kCharsetTag)
- 1;
You should probably include basictypes.h to get the arraysize() definition.
Chromium is going to use a include-what-you-use checker to enforce this in the
not so distant future.

http://codereview.chromium.org/7039048/diff/6/net/base/data_url.cc#newcode58
net/base/data_url.cc:58: StringToLowerASCII(charset);
Does this have to be lowercased? I see below that we assign US-ASCII here. If it
must be lowercased, please add a test for it.

[abarth-chromium, 2011-05-18 23:56:50]

Thanks.  Lowercasing not needed, apparently.

[commit-bot: I haz the power, 2011-05-19 00:22:59]

Try job failure for 7039048-1004 on linux:
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux&numb...