[Webkit-unassigned] [Bug 60247] Handle the touch icon in WebKit/Chromium

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=60247


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #92804|review?                     |review-
               Flag|                            |




--- Comment #14 from David Levin <levin at chromium.org>  2011-05-09 10:59:40 PST ---
(From update of attachment 92804)
View in context: https://bugs.webkit.org/attachment.cgi?id=92804&action=review

Mostly following up Darin's comments.

> Source/WebKit/chromium/public/WebIconURL.h:43
> +        TouchPrecomposedIcon = 1 << 2

Even though this is inside of the struct, I look at a number of other classes in WebKit/chromium/public and they prefix the name with the enum, so it would be
WebIconTypeInvalid, etc.

> Source/WebKit/chromium/public/WebIconURL.h:57
> +    WebIconType iconType;

I suspect Darin meant RAII and I believe he was suggesting making these member variables private (and making this a class essentially with m_ variables and accesssors for the values where needed), so the only place that the object become immutable (except for the copy operator, which is used in WebFrameImpl::favIconURL).

> Source/WebKit/chromium/src/WebFrameImpl.cpp:529
> +        WTF::Vector<WebCore::IconURL> iconURLs = frameLoader->iconURLs(webIconTypes);

I doubt that you need the WTF:: prefix.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.