Add injected bundle SPI for getting favicon and touch icon URLs
Created attachment 278289 [details] Patch
Comment on attachment 278289 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=278289&action=review > Source/WebCore/html/LinkIconCollector.cpp:51 > + { Incorrect indentation. > Source/WebCore/html/LinkIconCollector.cpp:102 > + unsigned size = linkElement.sizes().item(0).string().stripWhiteSpace().toUInt(&ok); It’s almost never good to use stripWhiteSpace, since it’s the wrong set of whitespace characters. We have stripLeadingAndTrailingHTMLSpaces, and we have it on both String and StringView. Here, I think we’d want to use the StringView version—why allocate a string just to convert it to an integer? > Source/WebKit2/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInFrame.h:49 > +// FIXME: These should be tagged nonnull. Why does this have to be a FIXME? We don’t have the correct macro for that?
(In reply to comment #2) > > Source/WebKit2/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInFrame.h:49 > > +// FIXME: These should be tagged nonnull. > > Why does this have to be a FIXME? We don’t have the correct macro for that? We do have the correct macro, but adding an annotation to one method will cause the compiler to warn unless all methods have annotations (which they don't).
(In reply to comment #2) > Comment on attachment 278289 [details] > Patch > > > Source/WebCore/html/LinkIconCollector.cpp:102 > > + unsigned size = linkElement.sizes().item(0).string().stripWhiteSpace().toUInt(&ok); > > It’s almost never good to use stripWhiteSpace, since it’s the wrong set of > whitespace characters. We have stripLeadingAndTrailingHTMLSpaces, and we > have it on both String and StringView. Here, I think we’d want to use the > StringView version—why allocate a string just to convert it to an integer? > It doesn't look like there's a version of stripLeadingAndTrailingHTMLSpaces that works on a StringView - I'll make sure to fix one and adopt StringView in an upcoming patch.
Committed r200591: <http://trac.webkit.org/changeset/200591>