Convert mostly favicon related code from SkBitmap to ImageSkia and Image

Convert mostly favicon related code from SkBitmap to ImageSkia and Image.

BUG=None
Test=Compiles

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

[pkotwicz, 2012-08-16 20:13:50]

This CL Converts mostly favicon related code from SkBitmap to ImageSkia and
Image

[Nico, 2012-08-16 20:26:27]

Awesome, thanks for doing this!

http://codereview.chromium.org/10834368/diff/1/chrome/browser/ui/cocoa/locati...
File chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm (right):

http://codereview.chromium.org/10834368/diff/1/chrome/browser/ui/cocoa/locati...
chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm:66: #include
"third_party/skia/include/core/SkBitmap.h"
can this go?

http://codereview.chromium.org/10834368/diff/1/chrome/browser/ui/cocoa/locati...
File chrome/browser/ui/cocoa/location_bar/location_icon_decoration.mm (right):

http://codereview.chromium.org/10834368/diff/1/chrome/browser/ui/cocoa/locati...
chrome/browser/ui/cocoa/location_bar/location_icon_decoration.mm:67: NSImage*
favicon = owner_->GetFavicon().ToNSImage();
This will crash if GetFavicon() IsEmpty(). (You can use AsNSImage(), I'll clean
that up later.)

http://codereview.chromium.org/10834368/diff/1/chrome/browser/ui/gtk/browser_...
File chrome/browser/ui/gtk/browser_titlebar.cc (right):

http://codereview.chromium.org/10834368/diff/1/chrome/browser/ui/gtk/browser_...
chrome/browser/ui/gtk/browser_titlebar.cc:610: g_object_unref(icon_pixbuf);
ToGdkPixbuf() returns an unref'd icon, so you need to remove this unref

http://codereview.chromium.org/10834368/diff/1/chrome/browser/ui/gtk/browser_...
chrome/browser/ui/gtk/browser_titlebar.cc:647: g_object_unref(icon_pixbuf);
ditto

http://codereview.chromium.org/10834368/diff/1/chrome/browser/ui/gtk/location...
File chrome/browser/ui/gtk/location_bar_view_gtk.cc (right):

http://codereview.chromium.org/10834368/diff/1/chrome/browser/ui/gtk/location...
chrome/browser/ui/gtk/location_bar_view_gtk.cc:1353: GdkPixbuf* pixbuf =
favicon.ToGdkPixbuf();
This will fail if favicon.IsEmpty(). Move the if(!pixbuf) check above this line
(and change it to `if (favicon.IsEmpty())`, then it's safe.

http://codereview.chromium.org/10834368/diff/1/chrome/browser/ui/gtk/location...
chrome/browser/ui/gtk/location_bar_view_gtk.cc:1358: g_object_unref(pixbuf);
This is now wrong.

[pkotwicz, 2012-08-16 21:32:31]

Changes as requested

http://codereview.chromium.org/10834368/diff/1/chrome/browser/ui/cocoa/locati...
File chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm (right):

http://codereview.chromium.org/10834368/diff/1/chrome/browser/ui/cocoa/locati...
chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm:66: #include
"third_party/skia/include/core/SkBitmap.h"
SkBitmap is still used by LocationBarViewMac::GetKeywordImage

[Nico, 2012-08-16 21:35:06]

lgtm!

http://codereview.chromium.org/10834368/diff/1/chrome/browser/ui/cocoa/locati...
File chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm (right):

http://codereview.chromium.org/10834368/diff/1/chrome/browser/ui/cocoa/locati...
chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm:66: #include
"third_party/skia/include/core/SkBitmap.h"
On 2012/08/16 21:32:31, pkotwicz wrote:
> SkBitmap is still used by LocationBarViewMac::GetKeywordImage

But only in reference form, so the definition shouldn't be necessary (?).
Doesn't matter much though.

http://codereview.chromium.org/10834368/diff/4003/chrome/browser/ui/gtk/brows...
File chrome/browser/ui/gtk/browser_titlebar.cc (right):

http://codereview.chromium.org/10834368/diff/4003/chrome/browser/ui/gtk/brows...
chrome/browser/ui/gtk/browser_titlebar.cc:607: GdkPixbuf* icon_pixbuf =
icon.ToGdkPixbuf();
nit: no need for the local variable

http://codereview.chromium.org/10834368/diff/4003/chrome/browser/ui/gtk/brows...
chrome/browser/ui/gtk/browser_titlebar.cc:644:
gtk_image_set_from_pixbuf(GTK_IMAGE(app_mode_favicon_), icon_pixbuf);
likewise

http://codereview.chromium.org/10834368/diff/4003/chrome/browser/ui/gtk/locat...
File chrome/browser/ui/gtk/location_bar_view_gtk.cc (right):

http://codereview.chromium.org/10834368/diff/4003/chrome/browser/ui/gtk/locat...
chrome/browser/ui/gtk/location_bar_view_gtk.cc:1355: GdkPixbuf* pixbuf =
favicon.ToGdkPixbuf();
nit: no need for the local variable

[commit-bot: I haz the power, 2012-08-16 22:57:05]

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

[commit-bot: I haz the power, 2012-08-16 23:40:51]

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

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

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

[commit-bot: I haz the power, 2012-08-17 01:17:54]

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

[commit-bot: I haz the power, 2012-08-17 01:29:58]

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

[commit-bot: I haz the power, 2012-08-17 04:43:57]

Change committed as 152050