[Webkit-unassigned] [Bug 59143] Need populate touch-icon url to FrameLoaderClient

[bugzilla-daemon at webkit.org]

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


David Kilzer (ddkilzer) <ddkilzer at webkit.org> changed:

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




--- Comment #19 from David Kilzer (ddkilzer) <ddkilzer at webkit.org>  2011-05-03 13:25:02 PST ---
(From update of attachment 92110)
View in context: https://bugs.webkit.org/attachment.cgi?id=92110&action=review

r- to update/add ChangeLog entries, fix FeatureDefines.xcconfig sort order (just ENABLE_TOUCH_ICON_LOADING), move toIconIndex out of struct IconURL and into the WebCore namespace, and use Vector<IconURL, ICON_COUNT> to limit the capacity of the vector when it's first created.

> Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:120
> +ENABLE_TOUCH_ICON_LOADING = ;

Need an entry in JavaScriptCore/ChangeLog for the change to JavaScriptCore/Configurations/FeatureDefines.xcconfig.

> Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:136
> +FEATURE_DEFINES = $(ENABLE_LINK_PREFETCH) $(ENABLE_ACCELERATED_2D_CANVAS) $(ENABLE_WEBGL) $(ENABLE_3D_RENDERING) $(ENABLE_ANIMATION_API) $(ENABLE_BLOB) $(ENABLE_CHANNEL_MESSAGING) $(ENABLE_CLIENT_BASED_GEOLOCATION) $(ENABLE_DATABASE) $(ENABLE_DATALIST) $(ENABLE_DATA_TRANSFER_ITEMS) $(ENABLE_DETAILS) $(ENABLE_DEVICE_ORIENTATION) $(ENABLE_DIRECTORY_UPLOAD) $(ENABLE_DOM_STORAGE) $(ENABLE_EVENTSOURCE) $(ENABLE_FILTERS) $(ENABLE_FILE_SYSTEM) $(ENABLE_FULLSCREEN_API) $(ENABLE_GEOLOCATION) $(ENABLE_ICONDATABASE) $(ENABLE_INDEXED_DATABASE) $(ENABLE_INPUT_SPEECH) $(ENABLE_JAVASCRIPT_DEBUGGER) $(ENABLE_MATHML) $(ENABLE_METER_TAG) $(ENABLE_NOTIFICATIONS) $(ENABLE_OFFLINE_WEB_APPLICATIONS) $(ENABLE_PAGE_VISIBILITY_API) $(ENABLE_PROGRESS_TAG) $(ENABLE_REGISTER_PROTOCOL_HANDLER) $(ENABLE_QUOTA) $(ENABLE_SHARED_WORKERS) $(ENABLE_SVG) $(ENABLE_SVG_ANIMATION) $(ENABLE_SVG_AS_IMAGE) $(ENABLE_SVG_DOM_OBJC_BINDINGS) $(ENABLE_SVG_FONTS) $(ENABLE_SVG_FOREIGN_OBJECT) $(ENABLE_SVG_USE) $(ENABLE_V
 IDEO) $(ENABLE_VIDEO_TRACK) $(ENABLE_MEDIA_STATISTICS) $(ENABLE_WEB_AUDIO) $(ENABLE_WEB_SOCKETS) $(ENABLE_WEB_TIMING) $(ENABLE_WORKERS) $(ENABLE_XHTMLMP) $(ENABLE_XPATH) $(ENABLE_XSLT) $(ENABLE_TOUCH_ICON_LOADING);

Please add $(ENABLE_TOUCH_ICON_LOADING) alphabetically after $(ENABLE_SVG_USE) like (most) of the rest of the items.

> Source/WebCore/dom/IconURL.h:68
> +    static size_t toIconIndex(IconType type)

Nit:  You didn't have to add the method to struct IconURL--you could have added it to the WebCore namespace as a stand-alone method.  That's what we do for similar conversion methods.  I see now why you didn't think it made sense to add this method "to IconURL".

> Source/WebCore/loader/FrameLoader.cpp:477
> +    Vector<IconURL> iconURLs;

I believe you could use "Vector<IconURL, ICON_COUNT>" here to hint that you only need a capacity of ICON_COUNT allocated.

> Source/WebCore/loader/FrameLoader.h:241
> +    Vector<IconURL> iconURLs(int iconTypes);

I believe you could use "Vector<IconURL, ICON_COUNT>" here to hint that you only need a capacity of ICON_COUNT allocated.

> Source/WebKit2/ChangeLog:14
> +        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
> +        (WebKit::WebFrameLoaderClient::dispatchDidChangeIcons):
> +        * WebProcess/WebCoreSupport/WebFrameLoaderClient.h:

Need to update the WebKit2/ChangeLog to list the changes to Configurations/FeatureDefines.xcconfig.

> Source/WebKit/mac/ChangeLog:13
> +        * WebCoreSupport/WebFrameLoaderClient.h:
> +        * WebCoreSupport/WebFrameLoaderClient.mm:
> +        (WebFrameLoaderClient::dispatchDidChangeIcons):

Need to update WebKit/mac/ChangeLog to list the changes to Configurations/FeatureDefines.xcconfig.

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