[Webkit-unassigned] [Bug 54109] [Qt][WK2]Unbreak InjectedBundle on Qt

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=54109





--- Comment #21 from Balazs Kelemen <kbalazs at webkit.org>  2011-03-12 11:29:23 PST ---
(In reply to comment #18)
> (From update of attachment 83899 [details])
> 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.

Done.

> 
> > 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.

Done.

> 
> > Tools/WebKitTestRunner/StringFunctions.h:101
> > +    WKRetainPtr<WKStringRef> urlString(AdoptWK, WKURLCopyString(urlRef));
> > +    return out << toSTD(urlString.get());
> 
> Can you use the adoptWK function here?

Done. adoptWK was defined in InjectedBundlePage.cpp so I moved it to here
(this file is included almost everywhere).

I will upload the bug for the check-webkit-style as discussed on IRC in a day
or two.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.