spqchan@ - would you please take a look at this? We'll want to cherry-pick this back to M52 once you land a fix.

Just checked and yes, it's a cross platform issue.
Note: this favicon is not a vector.

The solution I'm currently working on is to replace that icon with the OMNIBOX_HTTP vector when using Material design. Thoughts?

That sounds like the right thing to do.

Great. I'll do that on Mac then, and I can look into this for Windows too

No, don't use the OMNIBOX_HTTP icon.  That icon is smaller.  We temporarily had that as the favicon, and it looked broken.

I thought the default favicon was also a vector icon at this point, at least on views.  I'd need to see the code.  In any case, it should be a vector icon, and it should be given the same color as the tab text, but it shouldn't be the omnibox icon.

OMNIBOX_HTTP is a vector icon so it should just be a matter of requesting a different size. The Omnibox size is 16x22, which is the same proportion as the favicon which is 20x28. It looks like passing 16 as the size returns the 16x22 icon so please try passing in 20 as the icon size.

Here's the change with "20" as the icon size. Let me know what you think

https://codereview.chromium.org/2052153002/

Any chance you can screenshot old vs. new on Win?

I didn't make any changes to Windows. Here it is on Mac though.
The height the same but the width is different.

Deleted: Screen Shot 2016-06-10 at 4.55.16 PM.png 17.5 KB

	Screen Shot 2016-06-10 at 4.55.16 PM.png 17.5 KB View Download

Forgot to mention that the top is the new one

I don't think that's going to work.

* The dimensions look smaller -- not as small as the thing we had to revert, but still less good than the current icon.
* I think the outlines must not be strokes, but rather filled regions, so when scaled up slightly, you get non-pixel-grid-snapped areas, leading to the blurry AA around the edges.

I'm also a bit worried about the implementation in your CL regardless.  Tracing (by hand, not in a debugger) through the views code leads me to ChromeContentBrowserClient::GetDefaultFavicon().  I would think that would control the default favicon in a cross-platform way.  Worse, there are more than a dozen other references around the codebase to IDR_DEFAULT_FAVICON.  Presumably, any change we make to the actual icon should apply to all of them, and at least some of them are likely affected by the incognito color scheme as well.

So probably fixing this right involves finding the right place to put a "default favicon getter" and changing all these places to refer to that.

Sure thing, I can look into refactoring the codebase so that they will grab the default favicon from one source.

For the incognito color scheme, what should we do about the icon? AFAIK, a default favicon vector does not exists, so we'll need to get one if we're using vectors. 

Yeah, I would ask sgabriel to get one to you.

Re: #11 baed on a reading of the vector icon source, the vector is composed of stroked lines, not filled regions. The icon "size" is baked into the vector source file so I suspect the vector code is rendering at "16" and then scaling the bitmap to "20" (ugh). If true, that makes vector images kind of braindead.

spqchan@ - we don't have a lot of time to fix this bug because it's a blocker for M52, and I also don't want to cherry-pick a heap of code back to M52. I think the best course of action is to draw the icon in code, and to replace that with pkasting@'s "default favicon getter" suggestion in a follow-up cl. I will see about writing that vector code.

FWIW, I would not block a release on this on the Windows side, although it would be a P2 to try to get in.

Sure thing, that sounds good to me

I removed Windows and created a separate bug for it:  Issue 619627  

I don't think Windows should be tracked separately here.  This isn't something that we're going to end up fixing in a different way on each platform.  My comment 15 was intended to imply "I suggest you not make this RB-Stable on any platform, but up to you".

I also don't think it makes sense to change just the drawing of the favicon in this particular case without at least auditing the other affected sites and understanding what the impact of not addressing them is.

 Issue 619627  has been merged into this issue.

pkasting@ - we are going to fix this particular issue directly on the Mac so that we can easily cherry-pick the change back to M52. A more global fix is needed, but it will be made in a separate cl. The point of  Issue 619627   was to track that future cl (which should have been made OS-All. I don't want to just undo your merge of that issue from this bug, but this bug is only tracking the specific change we plan to make to temporarily fix this problem on the Mac.

If you really want those to be on separate bugs, feel free to reopen, but it seems like spqchan would still own the new bug, and I'd do the auditing etc. before even doing a temporary fix on this bug, in case it turns out the "right" solution is actually mergeable to M-52.

I duped because all the description and investigation is already on this bug, and assumed that if you checked in a temporary fix here the bug would just remain open for the complete fix.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b2557875f1783f455212f45fdd85fcd3af9b35f6

commit b2557875f1783f455212f45fdd85fcd3af9b35f6
Author: spqchan <spqchan@chromium.org>
Date: Wed Jun 15 17:06:19 2016

[Material][Mac] Replace the default favicon with a vector

BUG= 618742 

Review-Url: https://codereview.chromium.org/2052153002
Cr-Commit-Position: refs/heads/master@{#399930}

[modify] https://crrev.com/b2557875f1783f455212f45fdd85fcd3af9b35f6/chrome/browser/ui/cocoa/tab_contents/favicon_util_mac.h
[modify] https://crrev.com/b2557875f1783f455212f45fdd85fcd3af9b35f6/chrome/browser/ui/cocoa/tab_contents/favicon_util_mac.mm
[modify] https://crrev.com/b2557875f1783f455212f45fdd85fcd3af9b35f6/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm

Your change meets the bar and is auto-approved for M52 (branch: 2743)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a482e9417e45afd49ac0e7a00a2109e83e232d4c

commit a482e9417e45afd49ac0e7a00a2109e83e232d4c
Author: spqchan <spqchan@chromium.org>
Date: Thu Jun 16 19:09:17 2016

[Material][Mac] Replace the default favicon with a vector

BUG= 618742 

Review-Url: https://codereview.chromium.org/2052153002
Cr-Commit-Position: refs/heads/master@{#399930}
(cherry picked from commit b2557875f1783f455212f45fdd85fcd3af9b35f6)

Review URL: https://codereview.chromium.org/2072833002 .

Cr-Commit-Position: refs/branch-heads/2743@{#368}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/a482e9417e45afd49ac0e7a00a2109e83e232d4c/chrome/browser/ui/cocoa/tab_contents/favicon_util_mac.h
[modify] https://crrev.com/a482e9417e45afd49ac0e7a00a2109e83e232d4c/chrome/browser/ui/cocoa/tab_contents/favicon_util_mac.mm
[modify] https://crrev.com/a482e9417e45afd49ac0e7a00a2109e83e232d4c/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm

Re: my comment in #14, I will restate and say the current vector system is unfortunate. Ideally a vector icon can be scaled for any size use but one issue is the stroke width. When a designer creates a bit of vector art the stroke width is part of the artwork, so if I scale up to 4x, say, the stroke should scale up as well. That's part of what tripped us up in trying to reuse the omnibox document icon in tabs - we instead want to scale the art but not the stroke width, but there's no way to say that in the API (and at first glance that ability would make the API unwieldy).

Even if the line width were not an issue, the omnibox document vector icon has different proportions than the tab document icon, which is the other thing that derailed us :-/.

Verified the issue on Latest Chrome# 52.0.2743.49 on Mac OS X 10.11.5 and is working as intended.
Favicon of error page is visible in Incognito Window. Hence adding TE-Verified labels.
Attaching a screenshot for reference.
Thank You.