Don't show a placeholder if the response contains the full image.

Don't show a placeholder if the response contains the full image.

When attempting to load a placeholder image, it's possible that the
response contains the full image, e.g. if the image is smaller than the
requested range (e.g. requested the first 2KB of a 1KB image), if the
server ignored the range request header and returned a full 200
response, etc.

In these cases, this CL causes the full image to be shown and not get
treated as a placeholder image, instead of just showing a nondescript
placeholder image.

In order to detect when range responses contain the full image, this CL
refactors Content-Range response header parsing into http_util and
exposes it to Blink.

BUG=657995
Review-Url: https://codereview.chromium.org/2543073002
Cr-Commit-Position: refs/heads/master@{#450830}
Committed: https://chromium.googlesource.com/chromium/src/+/a2ef21fb88052a7c445d8646430170eb8e4447fd

[sclittle, 2016-12-02 00:23:02]

Patchset #1 (id:1) has been deleted

[sclittle, 2016-12-02 00:26:40]

sclittle@chromium.org changed reviewers:
+ bengr@chromium.org, kouhei@chromium.org, mmenke@chromium.org

[sclittle, 2016-12-02 00:26:40]

mmenke: net/*
kouhei: Blink
bengr: all

Thanks in advance.

[kouhei (in TOK), 2016-12-02 05:13:15]

kouhei@chromium.org changed reviewers:
+ hiroshige@chromium.org, yhirano@chromium.org

[kouhei (in TOK), 2016-12-02 05:13:16]

+hiroshige,yhirano

[kouhei (in TOK), 2016-12-02 06:13:45]

third_party/WebKit lgtm.
Please ask platform/ OWNER for rs.

[kinuko, 2016-12-02 07:56:47]

Source/platform/ lgtm (if net people are happy)

[mmenke, 2016-12-02 19:05:08]

https://codereview.chromium.org/2543073002/diff/20001/net/http/http_response_...
File net/http/http_response_headers.cc (right):

https://codereview.chromium.org/2543073002/diff/20001/net/http/http_response_...
net/http/http_response_headers.cc:1311: *first_byte_position =
*last_byte_position = *instance_length = -1;
Can move this down to below the body of the if statement below. Doesn't really
matter, admittedly.

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

https://codereview.chromium.org/2543073002/diff/20001/net/http/http_util.cc#n...
net/http/http_util.cc:311: *first_byte_position = *last_byte_position = -1;
This API seems really bad.  It sets these values to something other than -1 on
some errors, but not on others.  Can we change the API to only set values to
something other than -1 when it returns true?

It looks like nothing depends on the current weirdness, with the possible
exception of chrome/browser/extensions/blob_reader.cc, so should get an owners
review from that for the API change, just in case.

Would also need to update docs.

[sclittle, 2016-12-02 22:19:23]

https://codereview.chromium.org/2543073002/diff/20001/net/http/http_response_...
File net/http/http_response_headers.cc (right):

https://codereview.chromium.org/2543073002/diff/20001/net/http/http_response_...
net/http/http_response_headers.cc:1311: *first_byte_position =
*last_byte_position = *instance_length = -1;
On 2016/12/02 19:05:08, mmenke wrote:
> Can move this down to below the body of the if statement below. Doesn't really
> matter, admittedly.

IIUC this needs to be up here so that the outputs are still -1 even if they
weren't set to -1 before the caller passed them in. I can move it down if you'd
prefer though.

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

https://codereview.chromium.org/2543073002/diff/20001/net/http/http_util.cc#n...
net/http/http_util.cc:311: *first_byte_position = *last_byte_position = -1;
On 2016/12/02 19:05:08, mmenke wrote:
> This API seems really bad.  It sets these values to something other than -1 on
> some errors, but not on others.  Can we change the API to only set values to
> something other than -1 when it returns true?
> 
> It looks like nothing depends on the current weirdness, with the possible
> exception of chrome/browser/extensions/blob_reader.cc, so should get an owners
> review from that for the API change, just in case.
> 
> Would also need to update docs.

In what way are you suggesting changing the API? For example, consider
"Content-Range: bytes 0-99/*", which is a valid Content-Range according to spec
(unless I'm misreading it), such as for a resource where the full length isn't
known, but the server returned the first 100 bytes anyways.

Currently, this method would set the outputs to 0, 99, and -1 respectively, and
return false.

Are you suggesting that it should instead return true in that case, or that it
should set all the outputs to -1 and return false?

If we're changing it, I'd lean towards returning true in that case, since it
*is* a valid Content-Range. blob_reader.cc wouldn't need to be changed in that
case, and the ImageResource stuff in this file would be fine with that too.

[mmenke, 2016-12-02 22:31:56]

https://codereview.chromium.org/2543073002/diff/20001/net/http/http_response_...
File net/http/http_response_headers.cc (right):

https://codereview.chromium.org/2543073002/diff/20001/net/http/http_response_...
net/http/http_response_headers.cc:1311: *first_byte_position =
*last_byte_position = *instance_length = -1;
On 2016/12/02 22:19:22, sclittle wrote:
> On 2016/12/02 19:05:08, mmenke wrote:
> > Can move this down to below the body of the if statement below. Doesn't
really
> > matter, admittedly.
> 
> IIUC this needs to be up here so that the outputs are still -1 even if they
> weren't set to -1 before the caller passed them in. I can move it down if
you'd
> prefer though.

They don't - HttpUtil::ParseContentRangeHeader has the same API contract, so you
only have to set them to -1 if you don't call ParseContentRangeHeader.

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

https://codereview.chromium.org/2543073002/diff/20001/net/http/http_util.cc#n...
net/http/http_util.cc:311: *first_byte_position = *last_byte_position = -1;
On 2016/12/02 22:19:22, sclittle wrote:
> On 2016/12/02 19:05:08, mmenke wrote:
> > This API seems really bad.  It sets these values to something other than -1
on
> > some errors, but not on others.  Can we change the API to only set values to
> > something other than -1 when it returns true?
> > 
> > It looks like nothing depends on the current weirdness, with the possible
> > exception of chrome/browser/extensions/blob_reader.cc, so should get an
owners
> > review from that for the API change, just in case.
> > 
> > Would also need to update docs.
> 
> In what way are you suggesting changing the API? For example, consider
> "Content-Range: bytes 0-99/*", which is a valid Content-Range according to
spec
> (unless I'm misreading it), such as for a resource where the full length isn't
> known, but the server returned the first 100 bytes anyways.
> 
> Currently, this method would set the outputs to 0, 99, and -1 respectively,
and
> return false.
> 
> Are you suggesting that it should instead return true in that case, or that it
> should set all the outputs to -1 and return false?
> 
> If we're changing it, I'd lean towards returning true in that case, since it
> *is* a valid Content-Range. blob_reader.cc wouldn't need to be changed in that
> case, and the ImageResource stuff in this file would be fine with that too.

We return false if it's not a valid response to a 206 request, and nowhere do we
seem to handle the case where it's not a valid 206 response (And the API
description here is unclear).

"bytes 5-0" is an invalid byte range, and when we see it, we return
false/5/0/-1.  "bytes 1-2/*" is valid, but not for a 206 response, but when we
see it, we return false/1/2/-1....So if someone is using this for something
other than a 206, they basically have to do their own validation of the results
of this method, which is weird and bizarre, and no one is doing.

I'd like to get this API to a reasonable state before making more code depend on
it / making it easier for code to use.

[mmenke, 2016-12-02 22:38:47]

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

https://codereview.chromium.org/2543073002/diff/20001/net/http/http_util.cc#n...
net/http/http_util.cc:328: return false;
Actually, we don't provide enough data to validate it.  We return the same thing
for:

"1-2/*" as we do for "1-2/foo", which seems bad.

If we needed to handle the * case, I'd be fine with two methods, one that wraps
the other (One allows * and returns true if present, the other just calls into
it, and returns false and all -1's if the instance length is -1), but I'm not
sure we need both.  Do we have anything that cares about the asterisk?

[sclittle, 2016-12-03 00:43:13]

https://codereview.chromium.org/2543073002/diff/20001/net/http/http_response_...
File net/http/http_response_headers.cc (right):

https://codereview.chromium.org/2543073002/diff/20001/net/http/http_response_...
net/http/http_response_headers.cc:1311: *first_byte_position =
*last_byte_position = *instance_length = -1;
On 2016/12/02 22:31:56, mmenke wrote:
> On 2016/12/02 22:19:22, sclittle wrote:
> > On 2016/12/02 19:05:08, mmenke wrote:
> > > Can move this down to below the body of the if statement below. Doesn't
> really
> > > matter, admittedly.
> > 
> > IIUC this needs to be up here so that the outputs are still -1 even if they
> > weren't set to -1 before the caller passed them in. I can move it down if
> you'd
> > prefer though.
> 
> They don't - HttpUtil::ParseContentRangeHeader has the same API contract, so
you
> only have to set them to -1 if you don't call ParseContentRangeHeader.

Ah, your right, sorry, I misunderstood your earlier comment. Done.

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

https://codereview.chromium.org/2543073002/diff/20001/net/http/http_util.cc#n...
net/http/http_util.cc:311: *first_byte_position = *last_byte_position = -1;
On 2016/12/02 22:31:56, mmenke wrote:
> On 2016/12/02 22:19:22, sclittle wrote:
> > On 2016/12/02 19:05:08, mmenke wrote:
> > > This API seems really bad.  It sets these values to something other than
-1
> on
> > > some errors, but not on others.  Can we change the API to only set values
to
> > > something other than -1 when it returns true?
> > > 
> > > It looks like nothing depends on the current weirdness, with the possible
> > > exception of chrome/browser/extensions/blob_reader.cc, so should get an
> owners
> > > review from that for the API change, just in case.
> > > 
> > > Would also need to update docs.
> > 
> > In what way are you suggesting changing the API? For example, consider
> > "Content-Range: bytes 0-99/*", which is a valid Content-Range according to
> spec
> > (unless I'm misreading it), such as for a resource where the full length
isn't
> > known, but the server returned the first 100 bytes anyways.
> > 
> > Currently, this method would set the outputs to 0, 99, and -1 respectively,
> and
> > return false.
> > 
> > Are you suggesting that it should instead return true in that case, or that
it
> > should set all the outputs to -1 and return false?
> > 
> > If we're changing it, I'd lean towards returning true in that case, since it
> > *is* a valid Content-Range. blob_reader.cc wouldn't need to be changed in
that
> > case, and the ImageResource stuff in this file would be fine with that too.
> 
> We return false if it's not a valid response to a 206 request, and nowhere do
we
> seem to handle the case where it's not a valid 206 response (And the API
> description here is unclear).
> 
> "bytes 5-0" is an invalid byte range, and when we see it, we return
> false/5/0/-1.  "bytes 1-2/*" is valid, but not for a 206 response, but when we
> see it, we return false/1/2/-1....So if someone is using this for something
> other than a 206, they basically have to do their own validation of the
results
> of this method, which is weird and bizarre, and no one is doing.
> 
> I'd like to get this API to a reasonable state before making more code depend
on
> it / making it easier for code to use.

From what I can see, it looks like all the callers of this function only seem to
care about the Content-Range header in the context of 206 responses.

Do you mind if I land this CL keeping the existing behavior, with a TODO on
myself to clean it up? I'll send a follow up CL that changes the behavior and
API of ParseContentRanges.

https://codereview.chromium.org/2543073002/diff/20001/net/http/http_util.cc#n...
net/http/http_util.cc:328: return false;
On 2016/12/02 22:38:47, mmenke wrote:
> Actually, we don't provide enough data to validate it.  We return the same
thing
> for:
> 
> "1-2/*" as we do for "1-2/foo", which seems bad.
> 
> If we needed to handle the * case, I'd be fine with two methods, one that
wraps
> the other (One allows * and returns true if present, the other just calls into
> it, and returns false and all -1's if the instance length is -1), but I'm not
> sure we need both.  Do we have anything that cares about the asterisk?

Looking at the call sites, I don't see any reader that cares about non-206
responses.

[mmenke, 2016-12-03 00:46:21]

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

https://codereview.chromium.org/2543073002/diff/20001/net/http/http_util.cc#n...
net/http/http_util.cc:311: *first_byte_position = *last_byte_position = -1;
On 2016/12/03 00:43:13, sclittle wrote:
> On 2016/12/02 22:31:56, mmenke wrote:
> > On 2016/12/02 22:19:22, sclittle wrote:
> > > On 2016/12/02 19:05:08, mmenke wrote:
> > > > This API seems really bad.  It sets these values to something other than
> -1
> > on
> > > > some errors, but not on others.  Can we change the API to only set
values
> to
> > > > something other than -1 when it returns true?
> > > > 
> > > > It looks like nothing depends on the current weirdness, with the
possible
> > > > exception of chrome/browser/extensions/blob_reader.cc, so should get an
> > owners
> > > > review from that for the API change, just in case.
> > > > 
> > > > Would also need to update docs.
> > > 
> > > In what way are you suggesting changing the API? For example, consider
> > > "Content-Range: bytes 0-99/*", which is a valid Content-Range according to
> > spec
> > > (unless I'm misreading it), such as for a resource where the full length
> isn't
> > > known, but the server returned the first 100 bytes anyways.
> > > 
> > > Currently, this method would set the outputs to 0, 99, and -1
respectively,
> > and
> > > return false.
> > > 
> > > Are you suggesting that it should instead return true in that case, or
that
> it
> > > should set all the outputs to -1 and return false?
> > > 
> > > If we're changing it, I'd lean towards returning true in that case, since
it
> > > *is* a valid Content-Range. blob_reader.cc wouldn't need to be changed in
> that
> > > case, and the ImageResource stuff in this file would be fine with that
too.
> > 
> > We return false if it's not a valid response to a 206 request, and nowhere
do
> we
> > seem to handle the case where it's not a valid 206 response (And the API
> > description here is unclear).
> > 
> > "bytes 5-0" is an invalid byte range, and when we see it, we return
> > false/5/0/-1.  "bytes 1-2/*" is valid, but not for a 206 response, but when
we
> > see it, we return false/1/2/-1....So if someone is using this for something
> > other than a 206, they basically have to do their own validation of the
> results
> > of this method, which is weird and bizarre, and no one is doing.
> > 
> > I'd like to get this API to a reasonable state before making more code
depend
> on
> > it / making it easier for code to use.
> 
> From what I can see, it looks like all the callers of this function only seem
to
> care about the Content-Range header in the context of 206 responses.
> 
> Do you mind if I land this CL keeping the existing behavior, with a TODO on
> myself to clean it up? I'll send a follow up CL that changes the behavior and
> API of ParseContentRanges.

I'm fine with splitting it into multiple CLs, but I'd rather have webkit start
depending on this code only after we've cleaned up the API (i.e., just land the
net/ part of this CL, then clean up the API, then land the WebKit change.  Yes,
I'm paranoid).

[mmenke, 2016-12-03 00:48:37]

On 2016/12/03 00:46:21, mmenke wrote:
> https://codereview.chromium.org/2543073002/diff/20001/net/http/http_util.cc
> File net/http/http_util.cc (right):
> 
>
https://codereview.chromium.org/2543073002/diff/20001/net/http/http_util.cc#n...
> net/http/http_util.cc:311: *first_byte_position = *last_byte_position = -1;
> On 2016/12/03 00:43:13, sclittle wrote:
> > On 2016/12/02 22:31:56, mmenke wrote:
> > > On 2016/12/02 22:19:22, sclittle wrote:
> > > > On 2016/12/02 19:05:08, mmenke wrote:
> > > > > This API seems really bad.  It sets these values to something other
than
> > -1
> > > on
> > > > > some errors, but not on others.  Can we change the API to only set
> values
> > to
> > > > > something other than -1 when it returns true?
> > > > > 
> > > > > It looks like nothing depends on the current weirdness, with the
> possible
> > > > > exception of chrome/browser/extensions/blob_reader.cc, so should get
an
> > > owners
> > > > > review from that for the API change, just in case.
> > > > > 
> > > > > Would also need to update docs.
> > > > 
> > > > In what way are you suggesting changing the API? For example, consider
> > > > "Content-Range: bytes 0-99/*", which is a valid Content-Range according
to
> > > spec
> > > > (unless I'm misreading it), such as for a resource where the full length
> > isn't
> > > > known, but the server returned the first 100 bytes anyways.
> > > > 
> > > > Currently, this method would set the outputs to 0, 99, and -1
> respectively,
> > > and
> > > > return false.
> > > > 
> > > > Are you suggesting that it should instead return true in that case, or
> that
> > it
> > > > should set all the outputs to -1 and return false?
> > > > 
> > > > If we're changing it, I'd lean towards returning true in that case,
since
> it
> > > > *is* a valid Content-Range. blob_reader.cc wouldn't need to be changed
in
> > that
> > > > case, and the ImageResource stuff in this file would be fine with that
> too.
> > > 
> > > We return false if it's not a valid response to a 206 request, and nowhere
> do
> > we
> > > seem to handle the case where it's not a valid 206 response (And the API
> > > description here is unclear).
> > > 
> > > "bytes 5-0" is an invalid byte range, and when we see it, we return
> > > false/5/0/-1.  "bytes 1-2/*" is valid, but not for a 206 response, but
when
> we
> > > see it, we return false/1/2/-1....So if someone is using this for
something
> > > other than a 206, they basically have to do their own validation of the
> > results
> > > of this method, which is weird and bizarre, and no one is doing.
> > > 
> > > I'd like to get this API to a reasonable state before making more code
> depend
> > on
> > > it / making it easier for code to use.
> > 
> > From what I can see, it looks like all the callers of this function only
seem
> to
> > care about the Content-Range header in the context of 206 responses.
> > 
> > Do you mind if I land this CL keeping the existing behavior, with a TODO on
> > myself to clean it up? I'll send a follow up CL that changes the behavior
and
> > API of ParseContentRanges.
> 
> I'm fine with splitting it into multiple CLs, but I'd rather have webkit start
> depending on this code only after we've cleaned up the API (i.e., just land
the
> net/ part of this CL, then clean up the API, then land the WebKit change. 
Yes,
> I'm paranoid).

I'm also happy to do the fixes myself next week, actually.  There are other
things with the old code that could be improved.

[sclittle, 2016-12-03 00:56:34]

Re discussion with mmenke, I'm going to split the net/ part of this CL out into
another two CLs, with one moving the ParseContentRange impl into HttpUtil and
another cleaning up the API of that method.

I'll then update this CL to contain only the Blink changes.

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

https://codereview.chromium.org/2543073002/diff/20001/net/http/http_util.cc#n...
net/http/http_util.cc:311: *first_byte_position = *last_byte_position = -1;
On 2016/12/03 00:46:21, mmenke wrote:
> On 2016/12/03 00:43:13, sclittle wrote:
> > On 2016/12/02 22:31:56, mmenke wrote:
> > > On 2016/12/02 22:19:22, sclittle wrote:
> > > > On 2016/12/02 19:05:08, mmenke wrote:
> > > > > This API seems really bad.  It sets these values to something other
than
> > -1
> > > on
> > > > > some errors, but not on others.  Can we change the API to only set
> values
> > to
> > > > > something other than -1 when it returns true?
> > > > > 
> > > > > It looks like nothing depends on the current weirdness, with the
> possible
> > > > > exception of chrome/browser/extensions/blob_reader.cc, so should get
an
> > > owners
> > > > > review from that for the API change, just in case.
> > > > > 
> > > > > Would also need to update docs.
> > > > 
> > > > In what way are you suggesting changing the API? For example, consider
> > > > "Content-Range: bytes 0-99/*", which is a valid Content-Range according
to
> > > spec
> > > > (unless I'm misreading it), such as for a resource where the full length
> > isn't
> > > > known, but the server returned the first 100 bytes anyways.
> > > > 
> > > > Currently, this method would set the outputs to 0, 99, and -1
> respectively,
> > > and
> > > > return false.
> > > > 
> > > > Are you suggesting that it should instead return true in that case, or
> that
> > it
> > > > should set all the outputs to -1 and return false?
> > > > 
> > > > If we're changing it, I'd lean towards returning true in that case,
since
> it
> > > > *is* a valid Content-Range. blob_reader.cc wouldn't need to be changed
in
> > that
> > > > case, and the ImageResource stuff in this file would be fine with that
> too.
> > > 
> > > We return false if it's not a valid response to a 206 request, and nowhere
> do
> > we
> > > seem to handle the case where it's not a valid 206 response (And the API
> > > description here is unclear).
> > > 
> > > "bytes 5-0" is an invalid byte range, and when we see it, we return
> > > false/5/0/-1.  "bytes 1-2/*" is valid, but not for a 206 response, but
when
> we
> > > see it, we return false/1/2/-1....So if someone is using this for
something
> > > other than a 206, they basically have to do their own validation of the
> > results
> > > of this method, which is weird and bizarre, and no one is doing.
> > > 
> > > I'd like to get this API to a reasonable state before making more code
> depend
> > on
> > > it / making it easier for code to use.
> > 
> > From what I can see, it looks like all the callers of this function only
seem
> to
> > care about the Content-Range header in the context of 206 responses.
> > 
> > Do you mind if I land this CL keeping the existing behavior, with a TODO on
> > myself to clean it up? I'll send a follow up CL that changes the behavior
and
> > API of ParseContentRanges.
> 
> I'm fine with splitting it into multiple CLs, but I'd rather have webkit start
> depending on this code only after we've cleaned up the API (i.e., just land
the
> net/ part of this CL, then clean up the API, then land the WebKit change. 
Yes,
> I'm paranoid).

Woah, that is paranoid. Ok, I guess I'll do that then.

[sclittle, 2016-12-08 21:37:36]

Patchset #3 (id:60001) has been deleted

[sclittle, 2016-12-08 21:47:30]

sclittle@chromium.org changed reviewers:
+ kinuko@chromium.org
- bengr@chromium.org, hiroshige@chromium.org, yhirano@chromium.org

[sclittle, 2016-12-08 21:47:30]

I've separated the net/ changes out into other CLs, and pruned the list of
reviewers.

I'll still wait for an LGTM from mmenke@ before landing this, since this CL does
still add a new dependency on the net::HttpUtil::ParseContentRangeHeaderFor206
method.

[mmenke, 2016-12-08 22:48:08]

On 2016/12/08 21:47:30, sclittle wrote:
> I've separated the net/ changes out into other CLs, and pruned the list of
> reviewers.
> 
> I'll still wait for an LGTM from mmenke@ before landing this, since this CL
does
> still add a new dependency on the net::HttpUtil::ParseContentRangeHeaderFor206
> method.

The additional dependency LGTM, thanks for being so accommodating!

[sclittle, 2017-02-01 00:58:38]

I've rebased this change on the refactored ImageResource/ImageResourceContent
and on https://codereview.chromium.org/2638383008 that makes it possible for the
ImageResourceContent to mark itself as not a placeholder through
ImageResourceInfo.

I'll submit this once https://codereview.chromium.org/2638383008 lands.

[hiroshige, 2017-02-06 22:57:10]

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

[commit-bot: I haz the power, 2017-02-06 22:58:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-07 00:34:05]

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

[commit-bot: I haz the power, 2017-02-07 00:34:06]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2017-02-11 00:28:46]

On 2017/02/01 00:58:38, sclittle wrote:
> I've rebased this change on the refactored ImageResource/ImageResourceContent
> and on https://codereview.chromium.org/2638383008 that makes it possible for
the
> ImageResourceContent to mark itself as not a placeholder through
> ImageResourceInfo.
> 
> I'll submit this once https://codereview.chromium.org/2638383008 lands.

FYI all depending CLs are landed.
(Still we need rebase though; my local rebase is at
https://codereview.chromium.org/2643923006#ps120001)

[sclittle, 2017-02-14 22:24:26]

@hiroshige: I've rebased this on master, I'll wait for your LGTM before
committing.

[hiroshige, 2017-02-15 18:39:49]

lgtm

[sclittle, 2017-02-15 18:40:45]

The CQ bit was checked by sclittle@chromium.org

[sclittle, 2017-02-15 18:40:46]

The patchset sent to the CQ was uploaded after l-g-t-m from kouhei@chromium.org,
kinuko@chromium.org, mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2543073002/#ps120001
(title: "Rebased on master")

[commit-bot: I haz the power, 2017-02-15 18:41:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-15 23:09:37]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1487184045767660,
"parent_rev": "dfe9f69720945551289fd7dba30f1b825e72c830", "commit_rev":
"a2ef21fb88052a7c445d8646430170eb8e4447fd"}

[commit-bot: I haz the power, 2017-02-15 23:10:22]

Description was changed from

==========
Don't show a placeholder if the response contains the full image.

When attempting to load a placeholder image, it's possible that the
response contains the full image, e.g. if the image is smaller than the
requested range (e.g. requested the first 2KB of a 1KB image), if the
server ignored the range request header and returned a full 200
response, etc.

In these cases, this CL causes the full image to be shown and not get
treated as a placeholder image, instead of just showing a nondescript
placeholder image.

In order to detect when range responses contain the full image, this CL
refactors Content-Range response header parsing into http_util and
exposes it to Blink.

BUG=657995
==========

to

==========
Don't show a placeholder if the response contains the full image.

When attempting to load a placeholder image, it's possible that the
response contains the full image, e.g. if the image is smaller than the
requested range (e.g. requested the first 2KB of a 1KB image), if the
server ignored the range request header and returned a full 200
response, etc.

In these cases, this CL causes the full image to be shown and not get
treated as a placeholder image, instead of just showing a nondescript
placeholder image.

In order to detect when range responses contain the full image, this CL
refactors Content-Range response header parsing into http_util and
exposes it to Blink.

BUG=657995

Review-Url: https://codereview.chromium.org/2543073002
Cr-Commit-Position: refs/heads/master@{#450830}
Committed:
https://chromium.googlesource.com/chromium/src/+/a2ef21fb88052a7c445d86464301...
==========

[commit-bot: I haz the power, 2017-02-15 23:10:24]

Committed patchset #5 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/a2ef21fb88052a7c445d86464301...