Enables HiDPI favicon to be displayed in tab strip on CrOS

Enables HiDPI favicon to be displayed in tab strip on CrOS

BUG=138550
Test=Manual
Run ChromeOS with --force-device-scale-factor=2
Go to a website for the first time. Favicon appears in HiDPI in tab strip on CrOS.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=151942

[pkotwicz, 2012-08-14 20:16:06]

Enables HiDPI favicon to be displayed in tab strip on CrOS

Most of the patch is converting calls using SkBitmap to use ImageSkia instead

[Nico, 2012-08-14 20:19:36]

One nit below, other than that looks fine.

Looks like this was more than one line, sorry about that :-/

http://codereview.chromium.org/10824296/diff/6001/ui/gfx/image/image_skia.h
File ui/gfx/image/image_skia.h (right):

http://codereview.chromium.org/10824296/diff/6001/ui/gfx/image/image_skia.h#n...
ui/gfx/image/image_skia.h:72: bool Equals(const gfx::ImageSkia& other) const;
I'd call this RefersToSamePixelsAs() or something. Equals() is usually supposed
to do comparisons based on contents.

[pkotwicz, 2012-08-14 20:22:38]

Adding Scott to get his comments as well

http://codereview.chromium.org/10824296/diff/6001/ui/gfx/image/image_skia.h
File ui/gfx/image/image_skia.h (right):

http://codereview.chromium.org/10824296/diff/6001/ui/gfx/image/image_skia.h#n...
ui/gfx/image/image_skia.h:72: bool Equals(const gfx::ImageSkia& other) const;
I agree.

[sky, 2012-08-14 22:26:44]

LGTM - can you make sure the crashed tab animation doesn't look wonky with this
change.

[commit-bot: I haz the power, 2012-08-16 14:28:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/10824296/11001

[commit-bot: I haz the power, 2012-08-16 14:28:06]

Failed to apply patch for chrome/browser/favicon/favicon_tab_helper.cc:
While running patch -p1 --forward --force;
patching file chrome/browser/favicon/favicon_tab_helper.cc
Hunk #1 FAILED at 198.
1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/favicon/favicon_tab_helper.cc.rej

[commit-bot: I haz the power, 2012-08-16 15:11:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/10824296/3003

[commit-bot: I haz the power, 2012-08-16 15:26:20]

Try job failure for 10824296-3003 (retry) on mac_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-08-16 15:34:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/10824296/1036

[commit-bot: I haz the power, 2012-08-16 16:48:52]

Try job failure for 10824296-1036 (retry) on mac_rel for step
"content_browsertests".
It's a second try, previously, steps "browser_tests, content_browsertests"
failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-08-16 17:07:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/10824296/15

[commit-bot: I haz the power, 2012-08-16 19:23:44]

Change committed as 151942