[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 #13 from Adam Roben (aroben) <aroben at apple.com>  2011-02-25 08:50:26 PST ---
(From update of attachment 82975)
View in context: https://bugs.webkit.org/attachment.cgi?id=82975&action=review

> Source/WebCore/WebCore.exp.in:-632
> -__ZN7WebCore4KURLC1ENS_18ParsedURLStringTagEPKc

You should also revert r77816.

> Source/WebKit2/Shared/API/c/WKString.h:50
> -WK_EXPORT WKStringRef WKStringCreateWithUTF8CString(const char* string);
> +WK_EXPORT WKStringRef WKStringCreateWithUTF8CString(const char*);
>  
> -WK_EXPORT bool WKStringIsEmpty(WKStringRef string);
> +WK_EXPORT bool WKStringIsEmpty(WKStringRef);
>  
> -WK_EXPORT size_t WKStringGetMaximumUTF8CStringSize(WKStringRef string);
> +WK_EXPORT size_t WKStringGetMaximumUTF8CStringSize(WKStringRef);
>  WK_EXPORT size_t WKStringGetUTF8CString(WKStringRef string, char* buffer, size_t bufferSize);
>  
> -WK_EXPORT bool WKStringIsEqual(WKStringRef a, WKStringRef b);
> -WK_EXPORT bool WKStringIsEqualToUTF8CString(WKStringRef a, const char* b);
> +WK_EXPORT bool WKStringIsEqual(WKStringRef, WKStringRef);
> +WK_EXPORT bool WKStringIsEqualToUTF8CString(WKStringRef, const char*);
> +WK_EXPORT bool WKStringIsEqualToUTF8CStringIgnoringCase(WKStringRef, const char*);

So far all of our C API headers do not omit argument names. I'd suggest undoing those changes. We can always discuss that separately from this patch.

> Source/WebKit2/Shared/API/c/WKURL.h:41
> -WK_EXPORT WKURLRef WKURLCreateWithUTF8CString(const char* string);
> +WK_EXPORT WKURLRef WKURLCreateWithUTF8CString(const char*);
>  
> -WK_EXPORT WKStringRef WKURLCopyString(WKURLRef URL);
> +WK_EXPORT WKStringRef WKURLCopyString(WKURLRef);
>  
> -WK_EXPORT bool WKURLIsEqual(WKURLRef a, WKURLRef b);
> +WK_EXPORT bool WKURLIsEqual(WKURLRef, WKURLRef);

Same comment here about argument names.

> Source/WebKit2/Shared/API/c/WKURL.h:45
> +WK_EXPORT WKStringRef WKURLCopyProtocol(WKURLRef);

The correct term is "scheme", not "protocol".

> 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();
> +    }

It seems unfortunate to create and throw away a KURL each time these functions are called.

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:627
> +    WKRetainPtr<WKStringRef> host = WKURLCopyHostName(url.get());
> +    WKRetainPtr<WKStringRef> protocol = WKURLCopyProtocol(url.get());

These two lines will leak. You need to adopt the result of Copy-style functions.

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:628
> +    if (host.get()

No need for .get() here.

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:632
> +        && (!WKStringIsEqualToUTF8CString(host.get(), "127.0.0.1"))
> +        && (!WKStringIsEqualToUTF8CString(host.get(), "255.255.255.255")) // Used in some tests that expect to get back an error.
> +        && (!WKStringIsEqualToUTF8CStringIgnoringCase(host.get(), "localhost"))) {

I don't think the extra parentheses are helping here.

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:633
> +        InjectedBundle::shared().os() << "Blocked access to external URL " << toSTD(adoptWK(WKURLCopyString(url.get()))) << "\n";

We should add an operator<< that takes a WKURLRef.

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