Bug 173495 – Cleanup: WebContentMenuClient::searchWithGoogle() should parse URL using URL { URL { }, ... } instead of URL { ParsedURLString, ... }

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

[Daniel Bates]

When parsing an arbitrary string s that looks like a URL we should use URL { URL { }, s }. The constructor URL { ParsedURLString, s' } should only be used when s' is the result from URL::string(). WebContentMenuClient::searchWithGoogle() should use URL { URL { }, ... } to construct the URL object for the string it built.

[Daniel Bates]

Created attachment 313129 [details]
Patch

[Daniel Bates]

Created attachment 313130 [details]
Patch

[Daniel Bates]

Created attachment 313131 [details]
Patch

[Alex Christensen]

Comment on attachment 313131 [details]
Patch

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

> Source/WebKit/win/WebCoreSupport/WebContextMenuClient.cpp:66
> +    String url = "http://www.google.com/search?q=" + encoded + "&ie=UTF-8&oe=UTF-8";

I think makeString is even more efficient than calling operator+ twice.

[Daniel Bates]

(In reply to Alex Christensen from comment #4)
> Comment on attachment 313131 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=313131&action=review
> 
> > Source/WebKit/win/WebCoreSupport/WebContextMenuClient.cpp:66
> > +    String url = "http://www.google.com/search?q=" + encoded + "&ie=UTF-8&oe=UTF-8";
> 
> I think makeString is even more efficient than calling operator+ twice.

The StringOperators usage in this line is almost as efficient as makeString(), which is efficient enough for our use here. StringOperators ensures that we allocate for the finalized size once and then copy the parts just like makeString(). The difference between StringOperators and makeString() is the StringOperators can create temporaries and ultimately we need to convert from StringAppend to String. These differences likely go away with a good optimizing compiler.

[Radar WebKit Bug Importer]

<rdar://problem/32827445>

[Daniel Bates]

Committed r218435: <http://trac.webkit.org/changeset/218435>