Summary: | Add WebCore::protocolIsJavaScript(StringView) | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||
Component: | WebCore Misc. | Assignee: | Daniel Bates <dbates> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | achristensen, darin, sam | ||||
Priority: | P2 | ||||||
Version: | WebKit Local Build | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 170925 | ||||||
Attachments: |
|
Description
Daniel Bates
2017-04-27 14:50:13 PDT
Created attachment 308455 [details]
Patch
Comment on attachment 308455 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308455&action=review r=me I think we should get rid of all instances of protocolIs that take Strings instead of URLs. They're probably a marginal performance benefit, but we should really parse URLs rather than maintain a separate code path that looks enough like the URLParser. > Source/WebCore/platform/URL.h:315 > +bool protocolIsJavaScript(StringView url); Did you want to use this somewhere? This patch only changes URL.h and URL.cpp. (In reply to Alex Christensen from comment #2) > [...] > I think we should get rid of all instances of protocolIs that take Strings > instead of URLs. They're probably a marginal performance benefit, but we > should really parse URLs rather than maintain a separate code path that > looks enough like the URLParser. > We should investigate this. We should do this in the separate bug. > > Source/WebCore/platform/URL.h:315 > > +bool protocolIsJavaScript(StringView url); > > Did you want to use this somewhere? This patch only changes URL.h and > URL.cpp. As I mentioned in the ChangeLog entry, I plan to use this function in the patch for bug #170925. Comment on attachment 308455 [details] Patch Clearing flags on attachment: 308455 Committed r215949: <http://trac.webkit.org/changeset/215949> All reviewed patches have been landed. Closing bug. (In reply to Alex Christensen from comment #2) > I think we should get rid of all instances of protocolIs that take Strings > instead of URLs. They're probably a marginal performance benefit, but we > should really parse URLs rather than maintain a separate code path that > looks enough like the URLParser. I am not sure I agree. If the performance benefit is tiny, then that’s OK. Parsing an entire URL to do a properly-case-insensitive prefix check seems like massive overkill to me, so we should be sure a performance benefit is not there before dropping. I think there is at *least* an order of magnitude difference in the costs of the two operations. |