Add UMAs for checking header values against RFC 7230 in //net

Add UMAs for checking header values against RFC 7230 in //net

This CL inserts UMAs to URLRequestHttpJob to count the number of
requests/responses that contains invalid header values in RFC 7230.
This is the first step to investigate the feasibility of updating header value
check to RFC 7230 in //net stack.

BUG=455099
Committed: https://crrev.com/cb76caa4c5d065991a03c54f80e3e6ae239d7a59
Cr-Commit-Position: refs/heads/master@{#366563}

[hiroshige, 2015-09-29 08:54:43]

The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-29 08:55:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1374883002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1374883002/1

[commit-bot: I haz the power, 2015-09-29 10:12:09]

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

[commit-bot: I haz the power, 2015-09-29 10:12:10]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2015-09-29 10:38:48]

hiroshige@chromium.org changed reviewers:
+ davidben@chromium.org

[hiroshige, 2015-09-29 10:38:48]

davidben@, could you take a look?

In your comment in "Intent to Deprecate and Remove: obsolete header values in
Fetch API"
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/K5jd8Y5l6pI,
you suggested to insert UMAs to URLRequest, but I inserted UMAs to
HttpRequest/ResponseHeaders in order to catch header values set by component
other than URLRequest.

[davidben, 2015-09-29 16:32:59]

https://codereview.chromium.org/1374883002/diff/1/net/http/http_request_heade...
File net/http/http_request_headers.cc (right):

https://codereview.chromium.org/1374883002/diff/1/net/http/http_request_heade...
net/http/http_request_headers.cc:91:
HttpUtil::IsValidHeaderValueRFC7230(value.as_string()));
This makes an unnecessary copy. The lines above do too, but they're only
DCHECKs, so it doesn't matter.

https://codereview.chromium.org/1374883002/diff/1/net/http/http_util.h
File net/http/http_util.h (right):

https://codereview.chromium.org/1374883002/diff/1/net/http/http_util.h#newcode88
net/http/http_util.h:88: std::string::const_iterator value_end);
This should use a base::StringPiece. Then you don't need to convert or have both
versions.

[hiroshige, 2015-09-30 08:19:23]

https://codereview.chromium.org/1374883002/diff/1/net/http/http_request_heade...
File net/http/http_request_headers.cc (right):

https://codereview.chromium.org/1374883002/diff/1/net/http/http_request_heade...
net/http/http_request_headers.cc:91:
HttpUtil::IsValidHeaderValueRFC7230(value.as_string()));
On 2015/09/29 16:32:59, David Benjamin wrote:
> This makes an unnecessary copy. The lines above do too, but they're only
> DCHECKs, so it doesn't matter.

Done by using StringPiece.

https://codereview.chromium.org/1374883002/diff/1/net/http/http_util.h
File net/http/http_util.h (right):

https://codereview.chromium.org/1374883002/diff/1/net/http/http_util.h#newcode88
net/http/http_util.h:88: std::string::const_iterator value_end);
On 2015/09/29 16:32:59, David Benjamin wrote:
> This should use a base::StringPiece. Then you don't need to convert or have
both
> versions.

Done.

[hiroshige, 2015-09-30 15:27:16]

hiroshige@chromium.org changed reviewers:
+ asvitkine@chromium.org

[hiroshige, 2015-09-30 15:27:17]

+asvitkine for tools/metrics/ OWNER. Could you take a look?

[davidben, 2015-09-30 16:38:18]

https://codereview.chromium.org/1374883002/diff/20001/net/http/http_util.cc
File net/http/http_util.cc (right):

https://codereview.chromium.org/1374883002/diff/20001/net/http/http_util.cc#n...
net/http/http_util.cc:349: bool
HttpUtil::IsValidHeaderValueRFC7230(base::StringPiece value) {
Optional: Now that you have a base::StringPiece, I think this would be more
readable if you wrote this using those APIs.

if (value.empty())
  return true;

if (IsLWS(value[0]) || IsLWS(value[value.size() - 1]))
  return false;

// Can you just write "for (unsigned char c : value) {"? I'm not
// sure. Did I mention I really hate how C messed up char
// signedness? :-)
for (char c_signed : value) {
  unsigned char c = static_cast<unsigned char>(c_signed);
  if (c == .......)
    return false;
}
return true;

https://codereview.chromium.org/1374883002/diff/20001/net/http/http_util.cc#n...
net/http/http_util.cc:353: // Empty string is a valid header-value.
Nit: "This empty string is a valid header-value.

https://codereview.chromium.org/1374883002/diff/20001/net/http/http_util.cc#n...
net/http/http_util.cc:364: if (c == 0x7F || c > 0xFF || (c < 0x20 && c != '\t'))
c > 0xFF isn't possible. char's are one byte.

https://codereview.chromium.org/1374883002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1374883002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:22742: +    The count of request header
values valid/invalid in RFC 7230 (somehow
For each request header sent out, whether it is valid per RFC 7230.

https://codereview.chromium.org/1374883002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:22744: +    https://crbug.com/455099.
This description is very confusing. What do you mean by "somehow inaccurate
because a header value can be counted multiple times"? Are you saying this
because we might set a header repeatedly? If so, we should fix that. (Indeed
HttpRequestHeaders::SetHeader is a somewhat odd place to measure.)

https://codereview.chromium.org/1374883002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:22766: +    https://crbug.com/455099.
Ditto.

[hiroshige, 2015-10-29 12:49:21]

https://codereview.chromium.org/1374883002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1374883002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:22744: +    https://crbug.com/455099.
On 2015/09/30 16:38:18, davidben wrote:
> Are you saying this because we might set a header repeatedly?
> If so, we should fix that. (Indeed
> HttpRequestHeaders::SetHeader is a somewhat odd place to measure.)
Yes, a header value is counted multiple times while headers are passed across
layers.

An alternative is to check values in URLRequest, as you suggested, which will
remove the duplicated counts.
(Perhaps in StartJob() and OnHeadersComplete()?)

However, checks in URLRequest will miss headers set in layers under URLRequest
(e.g. HttpNetworkTransaction).
Is this OK?
(I think basically OK but I was not 100% sure, so I'd like to confirm)

[davidben, 2015-10-29 21:04:56]

https://codereview.chromium.org/1374883002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1374883002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:22744: +    https://crbug.com/455099.
On 2015/10/29 12:49:20, hiroshige wrote:
> On 2015/09/30 16:38:18, davidben wrote:
> > Are you saying this because we might set a header repeatedly?
> > If so, we should fix that. (Indeed
> > HttpRequestHeaders::SetHeader is a somewhat odd place to measure.)
> Yes, a header value is counted multiple times while headers are passed across
> layers.
> 
> An alternative is to check values in URLRequest, as you suggested, which will
> remove the duplicated counts.
> (Perhaps in StartJob() and OnHeadersComplete()?)
> 
> However, checks in URLRequest will miss headers set in layers under URLRequest
> (e.g. HttpNetworkTransaction).
> Is this OK?
> (I think basically OK but I was not 100% sure, so I'd like to confirm)

I think that's fine. Headers set internally within //net should only be fixed
headers that are part of the HTTP protocol. One would hope all of those are
valid per RFC 7230! :-D

The one thing to think about is extensions and the NetworkDelegate. Probably you
want to sample in URLRequestHTTPJob (there's one per redirect leg that goes
through http/https), sometime after NotifyBeforeSendHeaders is resolved.

https://code.google.com/p/chromium/codesearch#chromium/src/net/url_request/ur...

(Or, if you want, sampling both before and after would tell us if it's
extensions or the URLRequest consumer, likely web content, that's doing it.)

[hiroshige, 2015-12-15 07:42:40]

The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-15 07:43:17]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1374883002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1374883002/60001

[hiroshige, 2015-12-15 07:45:07]

Description was changed from

==========
Add UMAs for checking header values against RFC 7230 in //net

This CL inserts UMAs to HttpRequestHeaders and HttpResponseHeaders classes
to see the ratio of header values invalid against RFC 7230.
This is the first step to investigate the feasibility of updating header value
check to RFC 7230 in //net stack.

This CL is intended to capture all header values passing //net stack, and a
header value can be counted multiple times during request/response processing.
Therefore the stats will be somehow inaccurate, but I expect the accuracy will
be sufficient for the first step (e.g. to see the order of magnitude).

BUG=455099
==========

to

==========
Add UMAs for checking header values against RFC 7230 in //net

This CL inserts UMAs to URLRequestHttpJob to see the number of
requests/responses that contains invalid header values in RFC 7230.
This is the first step to investigate the feasibility of updating header value
check to RFC 7230 in //net stack.

BUG=455099
==========

[commit-bot: I haz the power, 2015-12-15 08:18:29]

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

[commit-bot: I haz the power, 2015-12-15 08:18:29]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[hiroshige, 2015-12-15 09:12:42]

The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-15 09:13:09]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1374883002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1374883002/80001

[commit-bot: I haz the power, 2015-12-15 10:45:50]

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

[commit-bot: I haz the power, 2015-12-15 10:45:50]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2015-12-15 11:59:38]

Description was changed from

==========
Add UMAs for checking header values against RFC 7230 in //net

This CL inserts UMAs to URLRequestHttpJob to see the number of
requests/responses that contains invalid header values in RFC 7230.
This is the first step to investigate the feasibility of updating header value
check to RFC 7230 in //net stack.

BUG=455099
==========

to

==========
Add UMAs for checking header values against RFC 7230 in //net

This CL inserts UMAs to URLRequestHttpJob to count the number of
requests/responses that contains invalid header values in RFC 7230.
This is the first step to investigate the feasibility of updating header value
check to RFC 7230 in //net stack.

BUG=455099
==========

[hiroshige, 2015-12-15 12:00:26]

davidben@,

Thanks for suggestion!
I moved UMAs to URLRequestHTTPJob in Patch Set 5. Could you take a look?

https://codereview.chromium.org/1374883002/diff/20001/net/http/http_util.cc
File net/http/http_util.cc (right):

https://codereview.chromium.org/1374883002/diff/20001/net/http/http_util.cc#n...
net/http/http_util.cc:349: bool
HttpUtil::IsValidHeaderValueRFC7230(base::StringPiece value) {
On 2015/09/30 16:38:18, davidben wrote:
> Optional: Now that you have a base::StringPiece, I think this would be more
> readable if you wrote this using those APIs.
> 
> if (value.empty())
>   return true;
> 
> if (IsLWS(value[0]) || IsLWS(value[value.size() - 1]))
>   return false;
> 
> // Can you just write "for (unsigned char c : value) {"? I'm not
> // sure. Did I mention I really hate how C messed up char
> // signedness? :-)
> for (char c_signed : value) {
>   unsigned char c = static_cast<unsigned char>(c_signed);
>   if (c == .......)
>     return false;
> }
> return true;

Done.

https://codereview.chromium.org/1374883002/diff/20001/net/http/http_util.cc#n...
net/http/http_util.cc:353: // Empty string is a valid header-value.
On 2015/09/30 16:38:18, davidben wrote:
> Nit: "This empty string is a valid header-value.

Done.

https://codereview.chromium.org/1374883002/diff/20001/net/http/http_util.cc#n...
net/http/http_util.cc:364: if (c == 0x7F || c > 0xFF || (c < 0x20 && c != '\t'))
On 2015/09/30 16:38:18, davidben wrote:
> c > 0xFF isn't possible. char's are one byte.

Done.

https://codereview.chromium.org/1374883002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1374883002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:22744: +    https://crbug.com/455099.
On 2015/10/29 21:04:56, davidben wrote:
> On 2015/10/29 12:49:20, hiroshige wrote:
> > On 2015/09/30 16:38:18, davidben wrote:
> > > Are you saying this because we might set a header repeatedly?
> > > If so, we should fix that. (Indeed
> > > HttpRequestHeaders::SetHeader is a somewhat odd place to measure.)
> > Yes, a header value is counted multiple times while headers are passed
across
> > layers.
> > 
> > An alternative is to check values in URLRequest, as you suggested, which
will
> > remove the duplicated counts.
> > (Perhaps in StartJob() and OnHeadersComplete()?)
> > 
> > However, checks in URLRequest will miss headers set in layers under
URLRequest
> > (e.g. HttpNetworkTransaction).
> > Is this OK?
> > (I think basically OK but I was not 100% sure, so I'd like to confirm)
> 
> I think that's fine. Headers set internally within //net should only be fixed
> headers that are part of the HTTP protocol. One would hope all of those are
> valid per RFC 7230! :-D
> 
> The one thing to think about is extensions and the NetworkDelegate. Probably
you
> want to sample in URLRequestHTTPJob (there's one per redirect leg that goes
> through http/https), sometime after NotifyBeforeSendHeaders is resolved.
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/net/url_request/ur...
> 
> (Or, if you want, sampling both before and after would tell us if it's
> extensions or the URLRequest consumer, likely web content, that's doing it.)

Done.

[davidben, 2015-12-15 21:16:36]

https://codereview.chromium.org/1374883002/diff/80001/net/url_request/url_req...
File net/url_request/url_request_http_job.cc (right):

https://codereview.chromium.org/1374883002/diff/80001/net/url_request/url_req...
net/url_request/url_request_http_job.cc:486: int
invalid_header_values_in_RFC7230 = 0;
This should probably be a bool (and then you can break inside the loop when you
set it to true).

https://codereview.chromium.org/1374883002/diff/80001/net/url_request/url_req...
net/url_request/url_request_http_job.cc:487: for
(net::HttpRequestHeaders::Iterator it(request_info_.extra_headers);
Nit: No net:: prefix.

https://codereview.chromium.org/1374883002/diff/80001/net/url_request/url_req...
net/url_request/url_request_http_job.cc:937: if
(scoped_refptr<HttpResponseHeaders> headers = GetResponseHeaders()) {
I don't think we usually declare variables inside conditionals like that.

https://codereview.chromium.org/1374883002/diff/80001/net/url_request/url_req...
net/url_request/url_request_http_job.cc:941: int
invalid_header_values_in_RFC7230 = 0;
Ditto about making this a bool.

https://codereview.chromium.org/1374883002/diff/80001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1374883002/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:24173: +    https://crbug.com/455099.
Probably should add "this is counted once for every redirect leg" or something
of the sort.

https://codereview.chromium.org/1374883002/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:24195: +    https://crbug.com/455099.
Ditto.

[hiroshige, 2015-12-17 01:30:48]

The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-17 02:35:28]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1374883002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1374883002/100001

[commit-bot: I haz the power, 2015-12-17 05:21:38]

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

[commit-bot: I haz the power, 2015-12-17 05:21:38]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2015-12-17 06:36:02]

https://codereview.chromium.org/1374883002/diff/80001/net/url_request/url_req...
File net/url_request/url_request_http_job.cc (right):

https://codereview.chromium.org/1374883002/diff/80001/net/url_request/url_req...
net/url_request/url_request_http_job.cc:486: int
invalid_header_values_in_RFC7230 = 0;
On 2015/12/15 21:16:36, davidben wrote:
> This should probably be a bool (and then you can break inside the loop when
you
> set it to true).

Done.

https://codereview.chromium.org/1374883002/diff/80001/net/url_request/url_req...
net/url_request/url_request_http_job.cc:487: for
(net::HttpRequestHeaders::Iterator it(request_info_.extra_headers);
On 2015/12/15 21:16:36, davidben wrote:
> Nit: No net:: prefix.

Done.

https://codereview.chromium.org/1374883002/diff/80001/net/url_request/url_req...
net/url_request/url_request_http_job.cc:937: if
(scoped_refptr<HttpResponseHeaders> headers = GetResponseHeaders()) {
On 2015/12/15 21:16:36, davidben wrote:
> I don't think we usually declare variables inside conditionals like that.

Done. Also I moved this to exclude error cases.

https://codereview.chromium.org/1374883002/diff/80001/net/url_request/url_req...
net/url_request/url_request_http_job.cc:941: int
invalid_header_values_in_RFC7230 = 0;
On 2015/12/15 21:16:36, davidben wrote:
> Ditto about making this a bool.

Done.

https://codereview.chromium.org/1374883002/diff/80001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1374883002/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:24173: +    https://crbug.com/455099.
On 2015/12/15 21:16:36, davidben wrote:
> Probably should add "this is counted once for every redirect leg" or something
> of the sort.

Done.

https://codereview.chromium.org/1374883002/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:24195: +    https://crbug.com/455099.
On 2015/12/15 21:16:36, davidben wrote:
> Ditto.

Done.

[Alexei Svitkine (slow), 2015-12-17 19:53:23]

lgtm

https://codereview.chromium.org/1374883002/diff/100001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1374883002/diff/100001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:24270: +<histogram
name="Net.HttpResponseContainsInvalidHeaderValuesInRFC7230"
Nit: These are a bit hard to read. Maybe you can reformat as:

Net.HttpResponse.ContainsInvalidHeaderValuesInRFC7230
Net.HttpRequest.ContainsInvalidHeaderValuesInRFC7230

[davidben, 2015-12-17 20:00:41]

lgtm

https://codereview.chromium.org/1374883002/diff/100001/net/url_request/url_re...
File net/url_request/url_request_http_job.cc (right):

https://codereview.chromium.org/1374883002/diff/100001/net/url_request/url_re...
net/url_request/url_request_http_job.cc:486: bool
invalid_header_values_in_RFC7230 = false;
Nit: I would lowercase RFC.

[hiroshige, 2015-12-22 06:33:51]

https://codereview.chromium.org/1374883002/diff/100001/net/url_request/url_re...
File net/url_request/url_request_http_job.cc (right):

https://codereview.chromium.org/1374883002/diff/100001/net/url_request/url_re...
net/url_request/url_request_http_job.cc:486: bool
invalid_header_values_in_RFC7230 = false;
On 2015/12/17 20:00:41, davidben wrote:
> Nit: I would lowercase RFC.

Done.

https://codereview.chromium.org/1374883002/diff/100001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1374883002/diff/100001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:24270: +<histogram
name="Net.HttpResponseContainsInvalidHeaderValuesInRFC7230"
On 2015/12/17 19:53:23, Alexei Svitkine (slow) wrote:
> Nit: These are a bit hard to read. Maybe you can reformat as:
> 
> Net.HttpResponse.ContainsInvalidHeaderValuesInRFC7230
> Net.HttpRequest.ContainsInvalidHeaderValuesInRFC7230

Done.

[hiroshige, 2015-12-22 06:34:04]

The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-22 06:34:13]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1374883002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1374883002/120001

[hiroshige, 2015-12-22 06:37:33]

The CQ bit was unchecked by hiroshige@chromium.org

[hiroshige, 2015-12-22 06:37:38]

The CQ bit was checked by hiroshige@chromium.org

[hiroshige, 2015-12-22 06:37:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org, davidben@chromium.org
Link to the patchset: https://codereview.chromium.org/1374883002/#ps120001
(title: "Reflect comments")

[commit-bot: I haz the power, 2015-12-22 06:37:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1374883002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1374883002/120001

[commit-bot: I haz the power, 2015-12-22 07:50:32]

Description was changed from

==========
Add UMAs for checking header values against RFC 7230 in //net

This CL inserts UMAs to URLRequestHttpJob to count the number of
requests/responses that contains invalid header values in RFC 7230.
This is the first step to investigate the feasibility of updating header value
check to RFC 7230 in //net stack.

BUG=455099
==========

to

==========
Add UMAs for checking header values against RFC 7230 in //net

This CL inserts UMAs to URLRequestHttpJob to count the number of
requests/responses that contains invalid header values in RFC 7230.
This is the first step to investigate the feasibility of updating header value
check to RFC 7230 in //net stack.

BUG=455099
==========

[commit-bot: I haz the power, 2015-12-22 07:50:32]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-12-22 07:51:26]

Description was changed from

==========
Add UMAs for checking header values against RFC 7230 in //net

This CL inserts UMAs to URLRequestHttpJob to count the number of
requests/responses that contains invalid header values in RFC 7230.
This is the first step to investigate the feasibility of updating header value
check to RFC 7230 in //net stack.

BUG=455099
==========

to

==========
Add UMAs for checking header values against RFC 7230 in //net

This CL inserts UMAs to URLRequestHttpJob to count the number of
requests/responses that contains invalid header values in RFC 7230.
This is the first step to investigate the feasibility of updating header value
check to RFC 7230 in //net stack.

BUG=455099

Committed: https://crrev.com/cb76caa4c5d065991a03c54f80e3e6ae239d7a59
Cr-Commit-Position: refs/heads/master@{#366563}
==========

[commit-bot: I haz the power, 2015-12-22 07:51:27]

Patchset 7 (id:??) landed as
https://crrev.com/cb76caa4c5d065991a03c54f80e3e6ae239d7a59
Cr-Commit-Position: refs/heads/master@{#366563}