Send an image-like 'Accept' header for favicons.

Send an image-like 'Accept' header for favicons.

Requests for images send an `Accept` header of `image/webp,image/*,*/*;q=0.8`.
Favicon requests currently send `*/*`, which seems like an oversight. This
patch aligns the latter with the former.

BUG=645901
R=jochen@chromium.org, japhet@chromium.org

Committed: https://crrev.com/8b4368150456fb577442e83b8a32b8cf3763c33f
Cr-Commit-Position: refs/heads/master@{#417911}

[Mike West, 2016-09-01 10:57:52]

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

[commit-bot: I haz the power, 2016-09-01 10:58:08]

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, 2016-09-01 11:19:10]

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

[commit-bot: I haz the power, 2016-09-01 11:19:10]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Mike West, 2016-09-01 12:00:54]

mkwst@chromium.org changed reviewers:
+ japhet@chromium.org, yoav@yoav.ws

[Mike West, 2016-09-01 12:00:54]

Hey Nate! Simon (CC'd) noted that we're sending a default `Accept` header for
favicon requests rather than the same `Accept` header we're sending for images.
Is that intentional, or just an oversight in the refactoring that pulled all the
`Accept` bits and pieces into this central place?

Poking Yoav as well, who probably has opinions...

[Yoav Weiss, 2016-09-01 12:32:52]

On 2016/09/01 12:00:54, Mike West (OOO until 29th) wrote:
> Hey Nate! Simon (CC'd) noted that we're sending a default `Accept` header for
> favicon requests rather than the same `Accept` header we're sending for
images.
> Is that intentional, or just an oversight in the refactoring that pulled all
the
> `Accept` bits and pieces into this central place?
> 
> Poking Yoav as well, who probably has opinions...

Do we support all the image formats for favicons as well? (and in particular, do
we support webp there?)

Otherwise, the code change looks good, but I'm not an owner there. Also, failing
tests seems related.

[Nate Chapin, 2016-09-06 18:17:52]

On 2016/09/01 12:32:52, Yoav Weiss wrote:
> On 2016/09/01 12:00:54, Mike West (OOO until 29th) wrote:
> > Hey Nate! Simon (CC'd) noted that we're sending a default `Accept` header
for
> > favicon requests rather than the same `Accept` header we're sending for
> images.
> > Is that intentional, or just an oversight in the refactoring that pulled all
> the
> > `Accept` bits and pieces into this central place?
> > 
> > Poking Yoav as well, who probably has opinions...
> 
> Do we support all the image formats for favicons as well? (and in particular,
do
> we support webp there?)
> 
> Otherwise, the code change looks good, but I'm not an owner there. Also,
failing
> tests seems related.

It was an oversight. Agreed on all of yoav's questions, too. :)

[Yoav Weiss, 2016-09-07 13:16:47]

On 2016/09/01 12:32:52, Yoav Weiss wrote:
> On 2016/09/01 12:00:54, Mike West (OOO until 29th) wrote:
> > Hey Nate! Simon (CC'd) noted that we're sending a default `Accept` header
for
> > favicon requests rather than the same `Accept` header we're sending for
> images.
> > Is that intentional, or just an oversight in the refactoring that pulled all
> the
> > `Accept` bits and pieces into this central place?
> > 
> > Poking Yoav as well, who probably has opinions...
> 
> Do we support all the image formats for favicons as well? (and in particular,
do
> we support webp there?)
> 

Answering my own question, webps are supported as favicon, so we should be good.

[Mike West, 2016-09-07 13:20:37]

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

[Mike West, 2016-09-07 13:20:56]

Ok. Updated the relevant test. WDYT? Shall we make Simon happy?

[commit-bot: I haz the power, 2016-09-07 13:20:57]

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

[Yoav Weiss, 2016-09-07 13:22:58]

On 2016/09/07 13:20:56, Mike West (OOO) wrote:
> Ok. Updated the relevant test. WDYT? Shall we make Simon happy?

ship it! :)

[Yoav Weiss, 2016-09-07 13:23:52]

On 2016/09/07 13:22:58, Yoav Weiss wrote:
> On 2016/09/07 13:20:56, Mike West (OOO) wrote:
> > Ok. Updated the relevant test. WDYT? Shall we make Simon happy?
> 
> ship it! :)

Oh, it still requires Nate to LGT & M

[Mike West, 2016-09-07 13:39:20]

mkwst@chromium.org changed reviewers:
+ rdsmith@chromium.org

[Mike West, 2016-09-07 13:39:21]

Hey Randy! Yoav is happy, and hopefully Nate is happy too. Assuming that
background, would you mind taking a look at this small change to
//content/browser/loader?

[commit-bot: I haz the power, 2016-09-07 14:12:27]

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

[commit-bot: I haz the power, 2016-09-07 14:12:28]

Dry run: This issue passed the CQ dry run.

[Nate Chapin, 2016-09-07 19:15:13]

On 2016/09/07 13:39:21, Mike West (OOO) wrote:
> Hey Randy! Yoav is happy, and hopefully Nate is happy too. Assuming that
> background, would you mind taking a look at this small change to
> //content/browser/loader?

I am happy! LGTM

[Mike West, 2016-09-08 08:01:20]

mkwst@chromium.org changed reviewers:
+ jochen@chromium.org

[Mike West, 2016-09-08 08:01:20]

Great!

Maybe jochen@ can stamp the //content/browser/loader bit?

[jochen (gone - plz use gerrit), 2016-09-08 10:06:29]

lgtm if you add a tracking bug

[zcorpan, 2016-09-09 10:47:14]

\o/

FYI, on the spec side:

https://html.spec.whatwg.org/multipage/semantics.html#rel-icon sets type to
"image" for /favicon.ico (but doesn't say anything about <link rel=icon>;
https://github.com/whatwg/html/issues/1769 )

https://fetch.spec.whatwg.org/#concept-fetch sets `Accept` depending on type

[Mike West, 2016-09-12 09:32:58]

Description was changed from

==========
Send an image-like 'Accept' header for favicons.

BUG=
==========

to

==========
Send an image-like 'Accept' header for favicons.

Requests for images send an `Accept` header of `image/webp,image/*,*/*;q=0.8`.
Favicon requests currently send `*/*`, which seems like an oversight. This
patch aligns the latter with the former.

BUG=645901
R=jochen@chromium.org, japhet@chromium.org
==========

[Mike West, 2016-09-12 09:33:02]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2016-09-12 09:33:15]

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-09-12 10:32:57]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-12 10:34:52]

Description was changed from

==========
Send an image-like 'Accept' header for favicons.

Requests for images send an `Accept` header of `image/webp,image/*,*/*;q=0.8`.
Favicon requests currently send `*/*`, which seems like an oversight. This
patch aligns the latter with the former.

BUG=645901
R=jochen@chromium.org, japhet@chromium.org
==========

to

==========
Send an image-like 'Accept' header for favicons.

Requests for images send an `Accept` header of `image/webp,image/*,*/*;q=0.8`.
Favicon requests currently send `*/*`, which seems like an oversight. This
patch aligns the latter with the former.

BUG=645901
R=jochen@chromium.org, japhet@chromium.org

Committed: https://crrev.com/8b4368150456fb577442e83b8a32b8cf3763c33f
Cr-Commit-Position: refs/heads/master@{#417911}
==========

[commit-bot: I haz the power, 2016-09-12 10:34:53]

Patchset 2 (id:??) landed as
https://crrev.com/8b4368150456fb577442e83b8a32b8cf3763c33f
Cr-Commit-Position: refs/heads/master@{#417911}