Bug 75586 – WebFrameLoaderClient::userAgent does unnecessary NSString/NSURL conversions

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Pratik Solanki]

WebFrameLoaderClient::userAgent calls -(NSString *)userAgentForURL:(NSURL *)url on WebView to get the user agent string. This results in

- converting KURL to NSURL* before passing to userAgentForURL (note that the function does't use the argument at all!)
- Since userAgentForURL returns an NSString* but stores the user agent internally as WTF::String, it converts the String to NSString* before returning
- WebFrameLoaderClient::userAgent converts that NSString back into a WTF::String when returning

We can optimize this by having a separate code path that takes a KURL and returns String.

[Pratik Solanki]

<rdar://problem/10637328&10637371>

[Pratik Solanki]

Created attachment 121222 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 121222 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=121222&action=review

It seems that the knowledge of obsolete url argument can be isolated in API method - no need to add a new new that's still going to ignore its argument. Dead code for "No current site-specific spoofs" can go away, too.

> Source/WebKit/mac/WebView/WebView.mm:3892
> +    return [self _userAgentStringForURL:(KURL*)url];

Even though the argument is unused in practice, casting to an unrelated type is a bug.

[Pratik Solanki]

(In reply to comment #3)
> (From update of attachment 121222 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=121222&action=review
> 
> It seems that the knowledge of obsolete url argument can be isolated in API method - no need to add a new new that's still going to ignore its argument. Dead code for "No current site-specific spoofs" can go away, too.
> 

Ok.

> > Source/WebKit/mac/WebView/WebView.mm:3892
> > +    return [self _userAgentStringForURL:(KURL*)url];
> 
> Even though the argument is unused in practice, casting to an unrelated type is a bug.

Yeah, I was debating on this cast as well. I'll just remove it given that the new function shouldn't take a URL argument.

[Pratik Solanki]

Created attachment 121298 [details]
Patch

[Pratik Solanki]

Committed r104181: <http://trac.webkit.org/changeset/104181>