Protect against image conversion failure in mac::FaviconForTabContents().

Protect against image conversion failure in mac::FaviconForTabContents().

BUG=80579
TEST=Monitor crash server.


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

[Scott Hess - ex-Googler, 2011-04-26 17:55:34]

Actual code and placement LGTM.

Note that up in [HungRendererController showForTabContents:], there is an
explicit test for empty title.  So I had been putting the nil check there.

http://codereview.chromium.org/6906001/diff/1/chrome/browser/ui/cocoa/tab_con...
File chrome/browser/ui/cocoa/tab_contents/favicon_util.mm (right):

http://codereview.chromium.org/6906001/diff/1/chrome/browser/ui/cocoa/tab_con...
chrome/browser/ui/cocoa/tab_contents/favicon_util.mm:26: // to the default
image.
AFAICT, the only way it returns nil is if skiaBitmap.isNull().  So I'm not sure
about this comment.

http://codereview.chromium.org/6906001/diff/1/chrome/browser/ui/cocoa/tab_con...
chrome/browser/ui/cocoa/tab_contents/favicon_util.mm:29: }
Is it just me, or do those {} look extraneous?

[Scott Hess - ex-Googler, 2011-04-26 17:56:42]

BTW,

BUG=80579
TEST=Monitor crash server.

[Robert Sesek, 2011-04-26 18:08:51]

On 2011/04/26 17:55:34, shess wrote:
> Note that up in [HungRendererController showForTabContents:], there is an
> explicit test for empty title.  So I had been putting the nil check there.

A nil check there would of course work, but I think making this function never
return nil is best.

http://codereview.chromium.org/6906001/diff/1/chrome/browser/ui/cocoa/tab_con...
File chrome/browser/ui/cocoa/tab_contents/favicon_util.mm (right):

http://codereview.chromium.org/6906001/diff/1/chrome/browser/ui/cocoa/tab_con...
chrome/browser/ui/cocoa/tab_contents/favicon_util.mm:26: // to the default
image.
On 2011/04/26 17:55:34, shess wrote:
> AFAICT, the only way it returns nil is if skiaBitmap.isNull().  So I'm not
sure
> about this comment.

I wrote the comment from memory, but you're right. Updated the comment.

http://codereview.chromium.org/6906001/diff/1/chrome/browser/ui/cocoa/tab_con...
chrome/browser/ui/cocoa/tab_contents/favicon_util.mm:29: }
On 2011/04/26 17:55:34, shess wrote:
> Is it just me, or do those {} look extraneous?

I like extraneous braces in this instance because of the weird wrapping
whitespace above and to make the scope indentation clear with the closing brace
below.

[Scott Hess - ex-Googler, 2011-04-26 18:26:04]

lgtm.

[commit-bot: I haz the power, 2011-04-26 19:30:57]

Change committed as 83056