[Webkit-unassigned] [Bug 54109] [Qt][WK2]Unbreak InjectedBundle on Qt
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Mar 11 08:09:18 PST 2011
https://bugs.webkit.org/show_bug.cgi?id=54109
Adam Roben (:aroben) <aroben at apple.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #83899|review?, commit-queue? |review-
Flag| |
--- Comment #18 from Adam Roben (:aroben) <aroben at apple.com> 2011-03-11 08:09:17 PST ---
(From update of attachment 83899)
View in context: https://bugs.webkit.org/attachment.cgi?id=83899&action=review
I think this is really close! Just a few comments.
>> Source/WebKit2/Shared/API/c/WKURL.h:43
>> +WK_EXPORT WKStringRef WKURLCopyHostName(WKURLRef url);
>
> The parameter name "url" adds no information, so it should be removed. [readability/parameter_name] [5]
I think you should put these new functions just beneath WKURLCopyString, all in the same paragraph.
> Source/WebKit2/Shared/WebURL.h:62
> + String host() const
> + {
> + WebCore::KURL url(WebCore::KURL(), m_string);
> + return url.isValid() ? url.host() : String();
> + }
> +
> + String protocol() const
> + {
> + WebCore::KURL url(WebCore::KURL(), m_string);
> + return url.isValid() ? url.protocol() : String();
> + }
Maybe it would be better to have an OwnPtr<KURL> data member. It would be lazily initialized using m_string. That way only the first call to one of these functions would have to pay the cost of parsing the string.
> Tools/WebKitTestRunner/StringFunctions.h:101
> + WKRetainPtr<WKStringRef> urlString(AdoptWK, WKURLCopyString(urlRef));
> + return out << toSTD(urlString.get());
Can you use the adoptWK function here?
No need for the .get().
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list