Treat responses with legacy LoFi headers as LoFi images.

Treat responses with legacy LoFi headers as LoFi images.

This CL causes responses with legacy LoFi headers to trigger the LoFi
infobar and get reloaded if the user opts out of LoFi on the page.

This way, legacy LoFi responses shown from cache will trigger the LoFi
infobar and gives users a way to see the original images.

BUG=707384
Review-Url: https://codereview.chromium.org/2800473002
Cr-Commit-Position: refs/heads/master@{#463516}
Committed: https://chromium.googlesource.com/chromium/src/+/ac85591ea501f4181bfa7b22e0e4b32a4a03886e

[sclittle, 2017-04-04 21:04:49]

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

[Nate Chapin, 2017-04-06 22:01:56]

lgtm

[megjablon, 2017-04-07 21:34:22]

lgtm % comment

https://codereview.chromium.org/2800473002/diff/20001/components/data_reducti...
File components/data_reduction_proxy/core/common/data_reduction_proxy_headers.cc
(right):

https://codereview.chromium.org/2800473002/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_headers.cc:95:
return base::LowerCaseEqualsASCII(token, transform_type);
Is there are reason we only support it being the first value? Maybe add a
comment that we're only checking first directive?

[sclittle, 2017-04-08 00:54:56]

sclittle@chromium.org changed reviewers:
+ thestig@chromium.org

[sclittle, 2017-04-08 00:54:57]

thestig: chrome_content_renderer_client.cc

Thanks!

https://codereview.chromium.org/2800473002/diff/20001/components/data_reducti...
File components/data_reduction_proxy/core/common/data_reduction_proxy_headers.cc
(right):

https://codereview.chromium.org/2800473002/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_headers.cc:95:
return base::LowerCaseEqualsASCII(token, transform_type);
On 2017/04/07 21:34:22, megjablon wrote:
> Is there are reason we only support it being the first value? Maybe add a
> comment that we're only checking first directive?

Done.

[Lei Zhang, 2017-04-08 01:01:07]

https://codereview.chromium.org/2800473002/diff/40001/chrome/renderer/chrome_...
File chrome/renderer/chrome_content_renderer_client.cc (right):

https://codereview.chromium.org/2800473002/diff/40001/chrome/renderer/chrome_...
chrome/renderer/chrome_content_renderer_client.cc:1388: WebString cpct_value =
response.httpHeaderField(WebString::fromASCII(
Why do we have to go from const char* to WebString to std::string?

[sclittle, 2017-04-10 20:56:50]

https://codereview.chromium.org/2800473002/diff/40001/chrome/renderer/chrome_...
File chrome/renderer/chrome_content_renderer_client.cc (right):

https://codereview.chromium.org/2800473002/diff/40001/chrome/renderer/chrome_...
chrome/renderer/chrome_content_renderer_client.cc:1388: WebString cpct_value =
response.httpHeaderField(WebString::fromASCII(
On 2017/04/08 01:01:07, Lei Zhang wrote:
> Why do we have to go from const char* to WebString to std::string?

We basically want to call:

drp::IsEmptyImagePreview(
    response.HttpHeaderField(drp::chrome_proxy_header()),
    response.HttpHeaderField(
        drp::chrome_proxy_content_transform_header()));

But since response.HttpHeaderField() both takes and returns WebStrings, we need
to convert the header names to WebStrings and the returned header values to
std::strings. Do you know of a more direct way to do this?

[Lei Zhang, 2017-04-10 22:41:17]

On 2017/04/10 20:56:50, sclittle wrote:
> But since response.HttpHeaderField() both takes and returns WebStrings, we
need
> to convert the header names to WebStrings and the returned header values to
> std::strings. Do you know of a more direct way to do this?

Oh, I didn't realize WebURLResponse was blink::WebURLResponse from 10 lines of
context.. Guess there's no other way, so lgtm.

[sclittle, 2017-04-11 00:34:41]

The CQ bit was checked by sclittle@chromium.org

[sclittle, 2017-04-11 00:34:41]

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

[commit-bot: I haz the power, 2017-04-11 00:35:27]

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-04-11 03:23:20]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1491870881165450,
"parent_rev": "be7bac4630c397e4cb8773fc93e1c559c3bfebdb", "commit_rev":
"ac85591ea501f4181bfa7b22e0e4b32a4a03886e"}

[commit-bot: I haz the power, 2017-04-11 03:24:45]

Description was changed from

==========
Treat responses with legacy LoFi headers as LoFi images.

This CL causes responses with legacy LoFi headers to trigger the LoFi
infobar and get reloaded if the user opts out of LoFi on the page.

This way, legacy LoFi responses shown from cache will trigger the LoFi
infobar and gives users a way to see the original images.

BUG=707384
==========

to

==========
Treat responses with legacy LoFi headers as LoFi images.

This CL causes responses with legacy LoFi headers to trigger the LoFi
infobar and get reloaded if the user opts out of LoFi on the page.

This way, legacy LoFi responses shown from cache will trigger the LoFi
infobar and gives users a way to see the original images.

BUG=707384

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

[commit-bot: I haz the power, 2017-04-11 03:24:46]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/ac85591ea501f4181bfa7b22e0e4...