Bug 107652

Summary: [WK2] Replace some internal API usage in EwkView with C API
Product: WebKit Reporter: Kenneth Rohde Christiansen <kenneth>
Component: WebKit2Assignee: Kenneth Rohde Christiansen <kenneth>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, d-r, gyuyoung.kim, mikhail.pozdnyakov, rakuco, tmpsantos, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 107657    
Attachments:
Description Flags
Patch none

Description Kenneth Rohde Christiansen 2013-01-23 02:10:38 PST 
SSIA
Comment 1 Kenneth Rohde Christiansen 2013-01-23 02:15:57 PST 
Created attachment 184183 [details]
Patch
Comment 2 Chris Dumez 2013-01-23 02:28:53 PST 
Comment on attachment 184183 [details]
Patch

LGTM.
Comment 3 Gyuyoung Kim 2013-01-23 02:30:47 PST 
Comment on attachment 184183 [details]
Patch

Looks fine.
Comment 4 Benjamin Poulain 2013-01-23 12:43:46 PST 
Comment on attachment 184183 [details]
Patch

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

Really nice.

Just one API thingy you might want to look into:

> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:758
> -    if (unreachableUrl && *unreachableUrl)
> -        impl->page()->loadAlternateHTMLString(String::fromUTF8(html), baseUrl ? String::fromUTF8(baseUrl) : "", String::fromUTF8(unreachableUrl));
> -    else
> -        impl->page()->loadHTMLString(String::fromUTF8(html), baseUrl ? String::fromUTF8(baseUrl) : "");
> +    WKRetainPtr<WKStringRef> wkHTMLString = adoptWK(WKStringCreateWithUTF8CString(html));
> +    WKRetainPtr<WKURLRef> wkBaseURL = adoptWK(WKURLCreateWithUTF8CString(baseUrl));
> +
> +    if (unreachableUrl && *unreachableUrl) {
> +        WKRetainPtr<WKURLRef> wkUnreachableURL = adoptWK(WKURLCreateWithUTF8CString(unreachableUrl));
> +        WKPageLoadAlternateHTMLString(impl->wkPage(), wkHTMLString.get(), wkBaseURL.get(), wkUnreachableURL.get());
> +    } else
> +        WKPageLoadHTMLString(impl->wkPage(), wkHTMLString.get(), wkBaseURL.get());

The new code is nicer than the old one.
Comment 5 Benjamin Poulain 2013-01-23 14:18:46 PST 
Comment on attachment 184183 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:568
> -    impl->page()->loadURL(url);
> +    WKRetainPtr<WKURLRef> wkUrl = adoptWK(WKURLCreateWithUTF8CString(url));
> +    WKPageLoadURL(impl->wkPage(), wkUrl.get());

This change the behavior. You know supports UTF8 instead of Latin1.
You may want to update the documentation and tests.
Comment 6 Kenneth Rohde Christiansen 2013-01-23 14:39:14 PST 
Created #107739 to track the unit test and documentation update.
Comment 7 WebKit Review Bot 2013-01-23 14:54:39 PST 
Comment on attachment 184183 [details]
Patch

Clearing flags on attachment: 184183

Committed r140600: <http://trac.webkit.org/changeset/140600>
Comment 8 WebKit Review Bot 2013-01-23 14:54:44 PST 
All reviewed patches have been landed.  Closing bug.