Add Blink support for showing image placeholders using range requests.

Add Blink support for showing image placeholders using range requests.

Design doc:
https://docs.google.com/document/d/1691W7yFDI1FJv69N2MEtaSzpnqO2EqkgGD3T0O-pQ08/edit?usp=sharing

This CL introduces support for issuing a range request for just the
first few bytes of an image, and showing a translucent gray box of the
same size as the image in the image's place if the original image's
dimensions can be decoded from the returned range.

Currently, this behavior is only activated via a blink-settings flag. In
the future, features such as Data Saver will activate this functionality
to save data for users.

BUG=605350, 605351
Committed: https://crrev.com/72b7454b7d1ea60317365a076187da54e646e88e
Cr-Commit-Position: refs/heads/master@{#427141}

[sclittle, 2016-10-15 02:23:32]

sclittle@chromium.org changed reviewers:
+ dglazkov@chromium.org, japhet@chromium.org

[Stephen Chennney, 2016-10-17 18:09:16]

schenney@chromium.org changed reviewers:
+ fmalita@chromium.org, schenney@chromium.org

[Stephen Chennney, 2016-10-17 18:09:17]

fmalita@ should also take a look. I'm not sure who is good to review resource
fetching changes.

https://codereview.chromium.org/2423683002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/fetch/ImageResource.cpp (right):

https://codereview.chromium.org/2423683002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/ImageResource.cpp:604: setLoFiStateOff();
It concerns me a little that if you reload the placehlder you're also changing
the LoFiState. It's not clear to me that the two should be linked. Do we ever
expect to load lo-fi once the placeholder is done?

https://codereview.chromium.org/2423683002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/fetch/ImageResource.h (right):

https://codereview.chromium.org/2423683002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/ImageResource.h:114: void
reloadIfLoFi(ResourceFetcher*, bool bypassCache = true);
Rename this to reloadIfLoFiOrPlaceholder.

https://codereview.chromium.org/2423683002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/PlaceholderImage.cpp (right):

https://codereview.chromium.org/2423683002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/PlaceholderImage.cpp:31:
sk_sp<SkImage> PlaceholderImage::imageForCurrentFrame() {
You're always creating a new image here, but my understanding is that this
method may be called over and over again so you should retain a handle to the
image you make, and only create it if the handle has not been created yet.

[sclittle, 2016-10-18 00:54:11]

https://codereview.chromium.org/2423683002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/fetch/ImageResource.cpp (right):

https://codereview.chromium.org/2423683002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/ImageResource.cpp:604: setLoFiStateOff();
On 2016/10/17 18:09:17, Stephen Chennney wrote:
> It concerns me a little that if you reload the placehlder you're also changing
> the LoFiState. It's not clear to me that the two should be linked. Do we ever
> expect to load lo-fi once the placeholder is done?

We don't want to load a Lo-Fi image if a placeholder is reloaded. A placeholder
image is shown to the user as a substitute for the original image with the hope
that the user will never need to load the original image, in order to save data.

The main use case of this placeholder functionality that we're focusing on right
now is to essentially extend the Data Saver Lo-Fi feature so that we can save
data on images that aren't proxied through the Data Saver proxy (e.g. https://
images), via this range request + placeholder mechanism.

There isn't really any advantage to generating a placeholder for a Lo-Fi image,
since Lo-Fi images are already tiny. Ideally, Blink would favor the Lo-Fi image
response over generating a placeholder, and this will be handled in practice by
showing the original image if the subrange response received is the entire
resource, see the TODO in ImageResource::updateImage().

See the design doc linked in the CL description and the Intent to Implement
thread on blink-dev
(https://groups.google.com/a/chromium.org/d/msg/blink-dev/pOTcSsMsAuo/YFiYRy7f...)
for more details.

https://codereview.chromium.org/2423683002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/fetch/ImageResource.h (right):

https://codereview.chromium.org/2423683002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/ImageResource.h:114: void
reloadIfLoFi(ResourceFetcher*, bool bypassCache = true);
On 2016/10/17 18:09:17, Stephen Chennney wrote:
> Rename this to reloadIfLoFiOrPlaceholder.

Done.

https://codereview.chromium.org/2423683002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/PlaceholderImage.cpp (right):

https://codereview.chromium.org/2423683002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/PlaceholderImage.cpp:31:
sk_sp<SkImage> PlaceholderImage::imageForCurrentFrame() {
On 2016/10/17 18:09:17, Stephen Chennney wrote:
> You're always creating a new image here, but my understanding is that this
> method may be called over and over again so you should retain a handle to the
> image you make, and only create it if the handle has not been created yet.

Done.

[Stephen Chennney, 2016-10-18 17:36:57]

It looks good to me but I would still like a second pair of eyes. Could someone
else please also take a look?

[Nate Chapin, 2016-10-18 18:23:17]

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
File third_party/WebKit/Source/core/fetch/ImageResource.cpp (right):

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
third_party/WebKit/Source/core/fetch/ImageResource.cpp:67: const FetchRequest*
m_fetchRequest;
Instead of stashing the FetchRequest here and adding a parameter to the
constructor, could we just detect the range header on the ResourceRequest in the
constructor?

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
third_party/WebKit/Source/core/fetch/ImageResource.cpp:589: bool bypassCache) {
bypassCache should probably be an enum.

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
third_party/WebKit/Source/core/fetch/ImageResource.cpp:590: if (!m_isPlaceholder
&&
This if-statement is sufficiently complex that it might be be time to pull it
out into a helper, even though there's only one callsite.

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
File third_party/WebKit/Source/core/fetch/ImageResource.h (right):

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
third_party/WebKit/Source/core/fetch/ImageResource.h:149: bool isPlaceholder()
const { return m_isPlaceholder; }
Looks like this is test-only? Maybe note it? Or test the range header instead of
exposing this bit?

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
File third_party/WebKit/Source/core/fetch/Resource.cpp (right):

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
third_party/WebKit/Source/core/fetch/Resource.cpp:935: void
Resource::clearRangeRequestHeader() {
Potentially out of scope nit: I don't like that we need to add a new method on
Resource every time we want to modify a header. We should consider adding a
protected Resource::mutableResourceRequest() for the Resource subclasses that
need to modify m_resourceRequest.

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right):

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:492:
request.setPlaceholderImageRequestType(FetchRequest::DisallowPlaceholder);
Everything in this function should probably all be done as part of
setPlaceholderImageRequestType()?

Also, this seems like the only place where setPlaceholderImageRequestType
changes an AllowPlaceholder to a DisallowPlaceholder. Can we change it to
setAllowImagePlaceholder() with no arguments, since the default will always be
DisallowPlaceholder?

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:1195:
toImageResource(resource)->reloadIfLoFiOrPlaceholder(this);
I may have asked this in the design doc, but if I did, I can't find the answer
:(

Is it important that this happens after didLoadResource()? Could we do these
calls in ImageResource::error and ImageResource::finish instead?

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
File third_party/WebKit/Source/core/page/DragController.cpp (right):

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
third_party/WebKit/Source/core/page/DragController.cpp:1067: if (!image ||
image->isNull() || !image->data() || !image->data()->size())
Why is this needed?

[sclittle, 2016-10-18 23:53:04]

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
File third_party/WebKit/Source/core/fetch/ImageResource.cpp (right):

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
third_party/WebKit/Source/core/fetch/ImageResource.cpp:67: const FetchRequest*
m_fetchRequest;
On 2016/10/18 18:23:17, Nate Chapin wrote:
> Instead of stashing the FetchRequest here and adding a parameter to the
> constructor, could we just detect the range header on the ResourceRequest in
the
> constructor?

I'm hesitant to just detect the range header, since there's nothing stopping
some other caller of ImageResource::fetch() from adding their own range header
for an image fetch for some reason. Currently, the code only adds Range header
and starts the placeholder logic if there isn't already a Range header on the
request.

I don't know of any other feature that uses range headers for images, or if it's
possible for JS to add a range header somehow. I can change this to just check
the range header if you think that's better though.

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
third_party/WebKit/Source/core/fetch/ImageResource.cpp:589: bool bypassCache) {
On 2016/10/18 18:23:17, Nate Chapin wrote:
> bypassCache should probably be an enum.

Done.

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
third_party/WebKit/Source/core/fetch/ImageResource.cpp:590: if (!m_isPlaceholder
&&
On 2016/10/18 18:23:17, Nate Chapin wrote:
> This if-statement is sufficiently complex that it might be be time to pull it
> out into a helper, even though there's only one callsite.

Done.

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
File third_party/WebKit/Source/core/fetch/ImageResource.h (right):

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
third_party/WebKit/Source/core/fetch/ImageResource.h:149: bool isPlaceholder()
const { return m_isPlaceholder; }
On 2016/10/18 18:23:17, Nate Chapin wrote:
> Looks like this is test-only? Maybe note it? Or test the range header instead
of
> exposing this bit?

Added comment saying that it's test-only.

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
File third_party/WebKit/Source/core/fetch/Resource.cpp (right):

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
third_party/WebKit/Source/core/fetch/Resource.cpp:935: void
Resource::clearRangeRequestHeader() {
On 2016/10/18 18:23:17, Nate Chapin wrote:
> Potentially out of scope nit: I don't like that we need to add a new method on
> Resource every time we want to modify a header. We should consider adding a
> protected Resource::mutableResourceRequest() for the Resource subclasses that
> need to modify m_resourceRequest.

I agree completely, plus I'll be modifying it even more in a later CL I add the
request bit to make the embedder return the full cached resource if it's all
there and fresh. Do you mind if I make that Resource::mutableResourceRequest()
change in a follow up CL?

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right):

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:492:
request.setPlaceholderImageRequestType(FetchRequest::DisallowPlaceholder);
On 2016/10/18 18:23:17, Nate Chapin wrote:
> Everything in this function should probably all be done as part of
> setPlaceholderImageRequestType()?
> 
> Also, this seems like the only place where setPlaceholderImageRequestType
> changes an AllowPlaceholder to a DisallowPlaceholder. Can we change it to
> setAllowImagePlaceholder() with no arguments, since the default will always be
> DisallowPlaceholder?

Done.

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:1195:
toImageResource(resource)->reloadIfLoFiOrPlaceholder(this);
On 2016/10/18 18:23:17, Nate Chapin wrote:
> I may have asked this in the design doc, but if I did, I can't find the answer
> :(
> 
> Is it important that this happens after didLoadResource()? Could we do these
> calls in ImageResource::error and ImageResource::finish instead?

I don't think the ordering here is important with respect to didLoadResource.

You did ask this in the design doc, but I resolved the comment:
https://docs.google.com/document/d/1691W7yFDI1FJv69N2MEtaSzpnqO2EqkgGD3T0O-pQ...

The issue with doing the reload in ImageResource::finish/error is that we need a
ResourceFetcher in order to start the reload. To get that ResourceFetcher, we'd
either need to pass it into finish()/error(), or ImageResource would need to
hold on to a ResourceFetcher.

Doing the reload here seems a bit nicer.

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
File third_party/WebKit/Source/core/page/DragController.cpp (right):

https://chromiumcodereview.appspot.com/2423683002/diff/20001/third_party/WebK...
third_party/WebKit/Source/core/page/DragController.cpp:1067: if (!image ||
image->isNull() || !image->data() || !image->data()->size())
On 2016/10/18 18:23:17, Nate Chapin wrote:
> Why is this needed?

Without this condition, the DCHECK immediately below this will fail for
placeholder images if the user clicks and drags on a placeholder, since
PlaceholderImage doesn't have a canonical filename extension (nor should it).

At drop time, DataTransfer checks and cancels the data transfer if the image has
no data
(https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/clipboard...),
so I'm just duplicating that condition here before the drag starts.

This also has the added bonus of disabling drag-and-drop for PlaceholderImages,
which seems good since there's not really anything there to drag or drop.

[Nate Chapin, 2016-10-21 18:23:55]

https://codereview.chromium.org/2423683002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/FetchRequest.cpp (right):

https://codereview.chromium.org/2423683002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/FetchRequest.cpp:132:
m_placeholderImageRequestType = DisallowPlaceholder;
Is this needed? Can we DCHECK(m_placeholderImageRequestType ==
DisallowPlaceholder) instead?

https://codereview.chromium.org/2423683002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/ImageResource.cpp (right):

https://codereview.chromium.org/2423683002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/ImageResource.cpp:57: Resource*
create(const ResourceRequest& request,
Every caller of ResourceFactory::create() just generates the parameters from a
FetchRequest. Maybe we should just take a FetchRequest as the sole parameter
here.

https://codereview.chromium.org/2423683002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/ProgressTracker.cpp (left):

https://codereview.chromium.org/2423683002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/ProgressTracker.cpp:144:
DCHECK(!m_progressItems.get(identifier));
I missed this...when does this DCHECK get hit? This implies that we're reloading
the ImageResource without fulling cancelling it first.

[sclittle, 2016-10-21 18:57:25]

https://codereview.chromium.org/2423683002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/FetchRequest.cpp (right):

https://codereview.chromium.org/2423683002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/FetchRequest.cpp:132:
m_placeholderImageRequestType = DisallowPlaceholder;
On 2016/10/21 18:23:55, Nate Chapin wrote:
> Is this needed? Can we DCHECK(m_placeholderImageRequestType ==
> DisallowPlaceholder) instead?

Done.

https://codereview.chromium.org/2423683002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/ImageResource.cpp (right):

https://codereview.chromium.org/2423683002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/ImageResource.cpp:57: Resource*
create(const ResourceRequest& request,
On 2016/10/21 18:23:55, Nate Chapin wrote:
> Every caller of ResourceFactory::create() just generates the parameters from a
> FetchRequest. Maybe we should just take a FetchRequest as the sole parameter
> here.

Ok; the String charset parameter is probably still needed, but |request| and
|options| can certainly just be replaced by the FetchRequest.

Do you mind if I change that in a follow up CL, or would you strongly prefer it
to be part of this CL?

https://codereview.chromium.org/2423683002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/ProgressTracker.cpp (left):

https://codereview.chromium.org/2423683002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/ProgressTracker.cpp:144:
DCHECK(!m_progressItems.get(identifier));
On 2016/10/21 18:23:55, Nate Chapin wrote:
> I missed this...when does this DCHECK get hit? This implies that we're
reloading
> the ImageResource without fulling cancelling it first.

The ProgressTracker doesn't remove resources from its |m_progressItems| map as
the resources complete or are cancelled, so this DCHECK would fire for any image
that is being tracked by the ProgressTracker and is reloaded for some reason
while the page is still loading, e.g. if the image subrange didn't contain the
dimensions.

I'm pretty sure this DCHECK could also trigger even before this CL if the user's
in Lo-Fi mode and causes the image to be reloaded while the page is still
loading, although I haven't actually reproduced that myself.

[Nate Chapin, 2016-10-21 19:13:10]

lgtm

https://codereview.chromium.org/2423683002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/ImageResource.cpp (right):

https://codereview.chromium.org/2423683002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/ImageResource.cpp:57: Resource*
create(const ResourceRequest& request,
On 2016/10/21 18:57:25, sclittle wrote:
> On 2016/10/21 18:23:55, Nate Chapin wrote:
> > Every caller of ResourceFactory::create() just generates the parameters from
a
> > FetchRequest. Maybe we should just take a FetchRequest as the sole parameter
> > here.
> 
> Ok; the String charset parameter is probably still needed, but |request| and
> |options| can certainly just be replaced by the FetchRequest.
> 
> Do you mind if I change that in a follow up CL, or would you strongly prefer
it
> to be part of this CL?

I thought it looked like the charset param always originated (sometimes
indirectly) from FetchRequest::charset(), but maybe I was wrong.

Separate CL is fine.

https://codereview.chromium.org/2423683002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/ProgressTracker.cpp (left):

https://codereview.chromium.org/2423683002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/ProgressTracker.cpp:144:
DCHECK(!m_progressItems.get(identifier));
On 2016/10/21 18:57:25, sclittle wrote:
> On 2016/10/21 18:23:55, Nate Chapin wrote:
> > I missed this...when does this DCHECK get hit? This implies that we're
> reloading
> > the ImageResource without fulling cancelling it first.
> 
> The ProgressTracker doesn't remove resources from its |m_progressItems| map as
> the resources complete or are cancelled, so this DCHECK would fire for any
image
> that is being tracked by the ProgressTracker and is reloaded for some reason
> while the page is still loading, e.g. if the image subrange didn't contain the
> dimensions.
> 
> I'm pretty sure this DCHECK could also trigger even before this CL if the
user's
> in Lo-Fi mode and causes the image to be reloaded while the page is still
> loading, although I haven't actually reproduced that myself.

Fair enough. I'm debating whether it's better to drop the DCHECK or to instead
check that if there is a ProgressItem, it's "finished" (it's estimated and
received byte counts are the same).

[f(malita), 2016-10-21 19:27:06]

platform/graphics LGTM

[sclittle, 2016-10-21 19:36:35]

The CQ bit was checked by sclittle@chromium.org

[commit-bot: I haz the power, 2016-10-21 19:37:32]

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, 2016-10-21 19:40:56]

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

[commit-bot: I haz the power, 2016-10-21 19:40:57]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[sclittle, 2016-10-21 19:44:23]

The CQ bit was checked by sclittle@chromium.org

[sclittle, 2016-10-21 19:44:24]

The patchset sent to the CQ was uploaded after l-g-t-m from japhet@chromium.org,
fmalita@chromium.org
Link to the patchset: https://codereview.chromium.org/2423683002/#ps80001
(title: "Rebased on master")

[commit-bot: I haz the power, 2016-10-21 19:45:11]

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, 2016-10-21 20:58:30]

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

[commit-bot: I haz the power, 2016-10-21 20:58:31]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[sclittle, 2016-10-24 18:46:34]

The CQ bit was checked by sclittle@chromium.org

[sclittle, 2016-10-24 18:46:35]

The patchset sent to the CQ was uploaded after l-g-t-m from japhet@chromium.org,
fmalita@chromium.org
Link to the patchset: https://codereview.chromium.org/2423683002/#ps100001
(title: "Change test data to "const unsigned char" instead of "const char" to
fix warnings in windows build")

[commit-bot: I haz the power, 2016-10-24 18:47:20]

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, 2016-10-24 20:49:33]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-10-24 20:51:44]

Description was changed from

==========
Add Blink support for showing image placeholders using range requests.

Design doc:
https://docs.google.com/document/d/1691W7yFDI1FJv69N2MEtaSzpnqO2EqkgGD3T0O-pQ...

This CL introduces support for issuing a range request for just the
first few bytes of an image, and showing a translucent gray box of the
same size as the image in the image's place if the original image's
dimensions can be decoded from the returned range.

Currently, this behavior is only activated via a blink-settings flag. In
the future, features such as Data Saver will activate this functionality
to save data for users.

BUG=605350,605351
==========

to

==========
Add Blink support for showing image placeholders using range requests.

Design doc:
https://docs.google.com/document/d/1691W7yFDI1FJv69N2MEtaSzpnqO2EqkgGD3T0O-pQ...

This CL introduces support for issuing a range request for just the
first few bytes of an image, and showing a translucent gray box of the
same size as the image in the image's place if the original image's
dimensions can be decoded from the returned range.

Currently, this behavior is only activated via a blink-settings flag. In
the future, features such as Data Saver will activate this functionality
to save data for users.

BUG=605350,605351

Committed: https://crrev.com/72b7454b7d1ea60317365a076187da54e646e88e
Cr-Commit-Position: refs/heads/master@{#427141}
==========

[commit-bot: I haz the power, 2016-10-24 20:51:45]

Patchset 6 (id:??) landed as
https://crrev.com/72b7454b7d1ea60317365a076187da54e646e88e
Cr-Commit-Position: refs/heads/master@{#427141}