Implement WebSocketExtensionParser

Implement WebSocketExtensionParser

Implement WebSocketExtensionParser, which parses Sec-WebSocket-Extension
header value as specified in RFC6455.

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

[Adam Rice, 2013-09-17 04:30:32]

https://codereview.chromium.org/23872029/diff/1/net/websockets/websocket_exte...
File net/websockets/websocket_extension_parser.cc (right):

https://codereview.chromium.org/23872029/diff/1/net/websockets/websocket_exte...
net/websockets/websocket_extension_parser.cc:13: const char kHeaderFieldName[] =
"Sec-WebSocket-Extensions";
Please use the declaration in websocket_handshake_constants.h instead.

https://codereview.chromium.org/23872029/diff/3001/net/websockets/websocket_e...
File net/websockets/websocket_extension.h (right):

https://codereview.chromium.org/23872029/diff/3001/net/websockets/websocket_e...
net/websockets/websocket_extension.h:31: bool operator == (const Parameter&
other) const {
There should be no spaces around the == signs.

Also, the style guide is pretty negative about operator overloading:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Operat...
and so you should avoid it unless you explicitly require it.

https://codereview.chromium.org/23872029/diff/3001/net/websockets/websocket_e...
net/websockets/websocket_extension.h:34: 
Please add a comment that you are using the compiler-generated copy constructor
and assignment operator.

https://codereview.chromium.org/23872029/diff/3001/net/websockets/websocket_e...
File net/websockets/websocket_extension_parser.cc (right):

https://codereview.chromium.org/23872029/diff/3001/net/websockets/websocket_e...
net/websockets/websocket_extension_parser.cc:11: // Backup the head pointer if
the parser has error.
I think this grammar can be parsed with single-character lookahead. Backtracking
should not be necessary.

[yhirano, 2013-09-17 06:03:30]

https://codereview.chromium.org/23872029/diff/1/net/websockets/websocket_exte...
File net/websockets/websocket_extension_parser.cc (right):

https://codereview.chromium.org/23872029/diff/1/net/websockets/websocket_exte...
net/websockets/websocket_extension_parser.cc:13: const char kHeaderFieldName[] =
"Sec-WebSocket-Extensions";
On 2013/09/17 04:30:32, Adam Rice wrote:
> Please use the declaration in websocket_handshake_constants.h instead.

Sorry, I don't need it anymore. I deleted it.

https://codereview.chromium.org/23872029/diff/3001/net/websockets/websocket_e...
File net/websockets/websocket_extension.h (right):

https://codereview.chromium.org/23872029/diff/3001/net/websockets/websocket_e...
net/websockets/websocket_extension.h:31: bool operator == (const Parameter&
other) const {
On 2013/09/17 04:30:33, Adam Rice wrote:
> There should be no spaces around the == signs.
> 
> Also, the style guide is pretty negative about operator overloading:
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Operat...
> and so you should avoid it unless you explicitly require it.

Done.

https://codereview.chromium.org/23872029/diff/3001/net/websockets/websocket_e...
net/websockets/websocket_extension.h:34: 
On 2013/09/17 04:30:33, Adam Rice wrote:
> Please add a comment that you are using the compiler-generated copy
constructor
> and assignment operator.

Done.

https://codereview.chromium.org/23872029/diff/3001/net/websockets/websocket_e...
File net/websockets/websocket_extension_parser.cc (right):

https://codereview.chromium.org/23872029/diff/3001/net/websockets/websocket_e...
net/websockets/websocket_extension_parser.cc:11: // Backup the head pointer if
the parser has error.
On 2013/09/17 04:30:33, Adam Rice wrote:
> I think this grammar can be parsed with single-character lookahead.
Backtracking
> should not be necessary.

I deleted some |has_error_ = false| statements.
The rule "extension-list = 1#extension" needs potential backtracking, though
nobody uses it. At any rate, it doesn't hurt performance.

[Adam Rice, 2013-09-17 07:45:57]

https://codereview.chromium.org/23872029/diff/26001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser.cc (right):

https://codereview.chromium.org/23872029/diff/26001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:12: class
WebSocketExtensionParser::CurrentPointerBackup {
Sorry my comment was not clear enough. The parser does not need backtracking, so
this class is not needed. Since this class is not needed, there is not need for
the methods for each rule to set or check the has_error_ member; they can just
return true or false. So has_error_ can go away and the code can be much
simpler.

https://codereview.chromium.org/23872029/diff/26001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:64: void
WebSocketExtensionParser::ConsumeExtensionList() {
We only need to parse one extension at a time, so this method is not needed.
Although the documentation for HttpResoonseHeader::EnumerateHeader is confusing,
the test at
https://code.google.com/p/chromium/codesearch#chromium/src/net/http/http_resp...
makes it clear enough that it will separate the extensions for us.

https://codereview.chromium.org/23872029/diff/26001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:166: current_[0] == '\r' &&
current_[1] == '\n' &&
Line coalescing is implemented by HttpResponseHeaders; no need to re-implement.

https://codereview.chromium.org/23872029/diff/26001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:180: bool
WebSocketExtensionParser::Lookahead(const char* data, size_t size) {
Having "size" makes this method much more complicated, but none of the callers
use it. char Lookahead() would be simpler and easier to use.

https://codereview.chromium.org/23872029/diff/26001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser_test.cc (right):

https://codereview.chromium.org/23872029/diff/26001/net/websockets/websocket_...
net/websockets/websocket_extension_parser_test.cc:94: parser.Parse("foo ; bar
baz\thoge\r\n ;fuga=piyo");
This is not correct; the header would be formatted as

X-Sec-WebSocket-Extensions: foo ; bar, baz     ,hoge
 ;fuga=piyo

Also, the separation at "," is done by HttpResponseHeaders::EnumerateHeader so
does not need implementing here.

[yhirano, 2013-09-17 09:17:38]

https://codereview.chromium.org/23872029/diff/26001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser.cc (right):

https://codereview.chromium.org/23872029/diff/26001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:12: class
WebSocketExtensionParser::CurrentPointerBackup {
On 2013/09/17 07:45:58, Adam Rice wrote:
> Sorry my comment was not clear enough. The parser does not need backtracking,
so
> this class is not needed. Since this class is not needed, there is not need
for
> the methods for each rule to set or check the has_error_ member; they can just
> return true or false. So has_error_ can go away and the code can be much
> simpler.

Done.

https://codereview.chromium.org/23872029/diff/26001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:64: void
WebSocketExtensionParser::ConsumeExtensionList() {
On 2013/09/17 07:45:58, Adam Rice wrote:
> We only need to parse one extension at a time, so this method is not needed.
> Although the documentation for HttpResoonseHeader::EnumerateHeader is
confusing,
> the test at
>
https://code.google.com/p/chromium/codesearch#chromium/src/net/http/http_resp...
> makes it clear enough that it will separate the extensions for us.

Done.

https://codereview.chromium.org/23872029/diff/26001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:166: current_[0] == '\r' &&
current_[1] == '\n' &&
On 2013/09/17 07:45:58, Adam Rice wrote:
> Line coalescing is implemented by HttpResponseHeaders; no need to
re-implement.

Done.

https://codereview.chromium.org/23872029/diff/26001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:180: bool
WebSocketExtensionParser::Lookahead(const char* data, size_t size) {
On 2013/09/17 07:45:58, Adam Rice wrote:
> Having "size" makes this method much more complicated, but none of the callers
> use it. char Lookahead() would be simpler and easier to use.

I like this prototype because we can check an error easily.
I replaced parameters of Consume, Lookahead and ConsumeIfMatch from (const
char*, size_t) to (char).

https://codereview.chromium.org/23872029/diff/26001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser_test.cc (right):

https://codereview.chromium.org/23872029/diff/26001/net/websockets/websocket_...
net/websockets/websocket_extension_parser_test.cc:94: parser.Parse("foo ; bar
baz\thoge\r\n ;fuga=piyo");
On 2013/09/17 07:45:58, Adam Rice wrote:
> This is not correct; the header would be formatted as
> 
> X-Sec-WebSocket-Extensions: foo ; bar, baz     ,hoge
>  ;fuga=piyo
> 
> Also, the separation at "," is done by HttpResponseHeaders::EnumerateHeader so
> does not need implementing here.

Deleted

[Adam Rice, 2013-09-17 12:41:36]

https://codereview.chromium.org/23872029/diff/33001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser.cc (right):

https://codereview.chromium.org/23872029/diff/33001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:30: if (current_ == end_) {
FYI: I would combine these two if statements, ie.

if (current_ == end_ || c != *current)

but I think that's a matter of taste.

https://codereview.chromium.org/23872029/diff/33001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:128: if (!has_error_)
ConsumeSpaces() can no longer set has_error_, so this check is unneeded. In
additional, Consume() includes a call to ConsumeSpaces(), so the call in this
method is unnecessary.

https://codereview.chromium.org/23872029/diff/33001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:137: bool
WebSocketExtensionParser::ConsumeIfMatch(char c) {
Lookahead() and ConsumeIfMatch() are now very similar. You might be able to
benefit by sharing some code between them, but I am not sure.

https://codereview.chromium.org/23872029/diff/33001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser_test.cc (right):

https://codereview.chromium.org/23872029/diff/33001/net/websockets/websocket_...
net/websockets/websocket_extension_parser_test.cc:93: "foo, bar"  // multiple
extensions
Some other strings I think are invalid:

"foo;",  // extension parameter name missing
"foo; bar=\"baz",  // unterminated quoted string  
"foo; bar baz",  // missing "="
"foo; bar -"baz",  // "-" instead of "="
"foo bar"  // missing ";"

[yhirano, 2013-09-18 01:15:43]

https://codereview.chromium.org/23872029/diff/33001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser.cc (right):

https://codereview.chromium.org/23872029/diff/33001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:30: if (current_ == end_) {
On 2013/09/17 12:41:36, Adam Rice wrote:
> FYI: I would combine these two if statements, ie.
> 
> if (current_ == end_ || c != *current)
> 
> but I think that's a matter of taste.

Done.

https://codereview.chromium.org/23872029/diff/33001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:128: if (!has_error_)
On 2013/09/17 12:41:36, Adam Rice wrote:
> ConsumeSpaces() can no longer set has_error_, so this check is unneeded. In
> additional, Consume() includes a call to ConsumeSpaces(), so the call in this
> method is unnecessary.

Done.

https://codereview.chromium.org/23872029/diff/33001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:137: bool
WebSocketExtensionParser::ConsumeIfMatch(char c) {
On 2013/09/17 12:41:36, Adam Rice wrote:
> Lookahead() and ConsumeIfMatch() are now very similar. You might be able to
> benefit by sharing some code between them, but I am not sure.
I would like to keep them because they are both simple and straightforward
enough.

https://codereview.chromium.org/23872029/diff/33001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser_test.cc (right):

https://codereview.chromium.org/23872029/diff/33001/net/websockets/websocket_...
net/websockets/websocket_extension_parser_test.cc:93: "foo, bar"  // multiple
extensions
On 2013/09/17 12:41:36, Adam Rice wrote:
> Some other strings I think are invalid:
> 
> "foo;",  // extension parameter name missing
> "foo; bar=\"baz",  // unterminated quoted string  
> "foo; bar baz",  // missing "="
> "foo; bar -"baz",  // "-" instead of "="
> "foo bar"  // missing ";"
Done.

[Adam Rice, 2013-09-18 06:26:38]

https://codereview.chromium.org/23872029/diff/46001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser.cc (right):

https://codereview.chromium.org/23872029/diff/46001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:17: end_ = &data[size];
FYI: I would prefer end_ = data + size;

https://codereview.chromium.org/23872029/diff/46001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:45: WebSocketExtension::Parameter
parameter("");
Nitpick: parameter(std::string()) is likely to produce smaller code.

https://codereview.chromium.org/23872029/diff/46001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:108: Consume('"');
It looks like this will accept the invalid string: foo; bar="baz "

Please add a test for it and fix it if it does.

https://codereview.chromium.org/23872029/diff/46001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser.h (right):

https://codereview.chromium.org/23872029/diff/46001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.h:27: //    have already joined.
s/parsers/parses/
s/joined/been done/

[yhirano, 2013-09-18 06:54:40]

https://codereview.chromium.org/23872029/diff/46001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser.cc (right):

https://codereview.chromium.org/23872029/diff/46001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:17: end_ = &data[size];
On 2013/09/18 06:26:38, Adam Rice wrote:
> FYI: I would prefer end_ = data + size;

Done.

https://codereview.chromium.org/23872029/diff/46001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:45: WebSocketExtension::Parameter
parameter("");
On 2013/09/18 06:26:38, Adam Rice wrote:
> Nitpick: parameter(std::string()) is likely to produce smaller code.

Done.

https://codereview.chromium.org/23872029/diff/46001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:108: Consume('"');
On 2013/09/18 06:26:38, Adam Rice wrote:
> It looks like this will accept the invalid string: foo; bar="baz "
> 
> Please add a test for it and fix it if it does.
Nice catch. done.

https://codereview.chromium.org/23872029/diff/46001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser.h (right):

https://codereview.chromium.org/23872029/diff/46001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.h:27: //    have already joined.
On 2013/09/18 06:26:38, Adam Rice wrote:
> s/parsers/parses/
> s/joined/been done/

Thanks, done.

[Adam Rice, 2013-09-18 07:15:49]

lgtm

[yhirano, 2013-09-18 12:55:38]

+tyoshino for net/websockets OWNER review.
+szym for net/net.gyp OWNER review.

[szym, 2013-09-18 14:41:09]

lgtm

https://codereview.chromium.org/23872029/diff/51001/net/websockets/README
File net/websockets/README (right):

https://codereview.chromium.org/23872029/diff/51001/net/websockets/README#new...
net/websockets/README:44: websocket_extension.cc
nit: keep the ordering cc <-> h consistent. Lexicographically .cc should precede
.h

https://codereview.chromium.org/23872029/diff/51001/net/websockets/websocket_...
File net/websockets/websocket_extension.h (right):

https://codereview.chromium.org/23872029/diff/51001/net/websockets/websocket_...
net/websockets/websocket_extension.h:53: };
missing DISALLOW_COPY_AND_ASSIGN?

https://codereview.chromium.org/23872029/diff/51001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser.h (right):

https://codereview.chromium.org/23872029/diff/51001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.h:54: };
DISALLOW_COPY_AND_ASSIGN

[yhirano, 2013-09-19 01:29:36]

https://codereview.chromium.org/23872029/diff/51001/net/websockets/README
File net/websockets/README (right):

https://codereview.chromium.org/23872029/diff/51001/net/websockets/README#new...
net/websockets/README:44: websocket_extension.cc
On 2013/09/18 14:41:09, szym wrote:
> nit: keep the ordering cc <-> h consistent. Lexicographically .cc should
precede
> .h

Done.

https://codereview.chromium.org/23872029/diff/51001/net/websockets/websocket_...
File net/websockets/websocket_extension.h (right):

https://codereview.chromium.org/23872029/diff/51001/net/websockets/websocket_...
net/websockets/websocket_extension.h:53: };
On 2013/09/18 14:41:09, szym wrote:
> missing DISALLOW_COPY_AND_ASSIGN?

I want the class to be copyable, because it is a value object. We may use
std::vector<WebSocketExtension>.

https://codereview.chromium.org/23872029/diff/51001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser.h (right):

https://codereview.chromium.org/23872029/diff/51001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.h:54: };
On 2013/09/18 14:41:09, szym wrote:
> DISALLOW_COPY_AND_ASSIGN

Done.

[tyoshino (SeeGerritForStatus), 2013-09-19 03:53:42]

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
File net/websockets/websocket_extension.h (right):

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
net/websockets/websocket_extension.h:48: // The default copy constructor and the
assignment operator is defined:
is -> are

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser.cc (right):

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:81: !IsControl(current_[0]) &&
!IsSeparator(current_[0]))
both foo[0] and *foo are used in this file. pick either of them and use it?

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser.h (right):

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.h:9: #include <vector>
not used?

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser_test.cc (right):

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
net/websockets/websocket_extension_parser_test.cc:49: parser.Parse("foo ; bar=
baz\t");
test
foo; bar= "baz"
please

[yhirano, 2013-09-19 04:16:19]

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
File net/websockets/websocket_extension.h (right):

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
net/websockets/websocket_extension.h:48: // The default copy constructor and the
assignment operator is defined:
On 2013/09/19 03:53:43, tyoshino wrote:
> is -> are

Done.

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser.cc (right):

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.cc:81: !IsControl(current_[0]) &&
!IsSeparator(current_[0]))
On 2013/09/19 03:53:43, tyoshino wrote:
> both foo[0] and *foo are used in this file. pick either of them and use it?

Thanks, I would use current_[i] for arrays. Done.

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser.h (right):

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
net/websockets/websocket_extension_parser.h:9: #include <vector>
On 2013/09/19 03:53:43, tyoshino wrote:
> not used?

Done.

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser_test.cc (right):

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
net/websockets/websocket_extension_parser_test.cc:49: parser.Parse("foo ; bar=
baz\t");
On 2013/09/19 03:53:43, tyoshino wrote:
> test
> foo; bar= "baz"
> please

We have a test case
 foo; bar="ba\z"
as QuotedParameterValue. Is that sufficient?

[tyoshino (SeeGerritForStatus), 2013-09-19 04:39:32]

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser_test.cc (right):

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
net/websockets/websocket_extension_parser_test.cc:49: parser.Parse("foo ; bar=
baz\t");
On 2013/09/19 04:16:19, yhirano wrote:
> On 2013/09/19 03:53:43, tyoshino wrote:
> > test
> > foo; bar= "baz"
> > please
> 
> We have a test case
>  foo; bar="ba\z"
> as QuotedParameterValue. Is that sufficient?

To check that LookAhead skips space though the code looks correct.

[yhirano, 2013-09-19 04:43:00]

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser_test.cc (right):

https://codereview.chromium.org/23872029/diff/62001/net/websockets/websocket_...
net/websockets/websocket_extension_parser_test.cc:49: parser.Parse("foo ; bar=
baz\t");
On 2013/09/19 04:39:32, tyoshino wrote:
> On 2013/09/19 04:16:19, yhirano wrote:
> > On 2013/09/19 03:53:43, tyoshino wrote:
> > > test
> > > foo; bar= "baz"
> > > please
> > 
> > We have a test case
> >  foo; bar="ba\z"
> > as QuotedParameterValue. Is that sufficient?
> 
> To check that LookAhead skips space though the code looks correct.
Done in PS12

[tyoshino (SeeGerritForStatus), 2013-09-19 04:49:21]

LGTM

https://codereview.chromium.org/23872029/diff/70001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser_test.cc (right):

https://codereview.chromium.org/23872029/diff/70001/net/websockets/websocket_...
net/websockets/websocket_extension_parser_test.cc:92: "foo; bar=\"\"",  //
quoted emptyparameter value
empty parameter

[yhirano, 2013-09-19 04:52:59]

https://codereview.chromium.org/23872029/diff/70001/net/websockets/websocket_...
File net/websockets/websocket_extension_parser_test.cc (right):

https://codereview.chromium.org/23872029/diff/70001/net/websockets/websocket_...
net/websockets/websocket_extension_parser_test.cc:92: "foo; bar=\"\"",  //
quoted emptyparameter value
On 2013/09/19 04:49:22, tyoshino wrote:
> empty parameter

Done.

[commit-bot: I haz the power, 2013-09-19 04:54:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yhirano@chromium.org/23872029/81001

[commit-bot: I haz the power, 2013-09-19 05:45:04]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win7_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-09-19 05:57:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yhirano@chromium.org/23872029/91001

[commit-bot: I haz the power, 2013-09-19 06:57:31]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-09-19 07:44:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yhirano@chromium.org/23872029/106001

[commit-bot: I haz the power, 2013-09-19 10:31:45]

Change committed as 224102