Bug 99549 – [EFL][WK2] Ewk_Url_Scheme_Request has to be refactored

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Mikhail Pozdnyakov]

Ewk_Url_Scheme_Request has to be refactored according to the pattern from bug#99321 + it should use WKEinaSharedString

[Mikhail Pozdnyakov]

Created attachment 169119 [details]
patch

[Chris Dumez]

Comment on attachment 169119 [details]
patch

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

> Source/WebKit2/ChangeLog:8
> +        Ewk_Url_Scheme_Requestis inherited from RefCounted, WKEinaSharedString are used,

"Ewk_Url_Scheme_Request is"

> Source/WebKit2/UIProcess/API/efl/ewk_url_scheme_request_private.h:47
> +    static PassRefPtr<Ewk_Url_Scheme_Request> create(WKSoupRequestManagerRef manager, WKStringRef url, uint64_t requestID)

I think we should pass a WKURLRef here instead of a WKStringRef.

> Source/WebKit2/UIProcess/API/efl/ewk_url_scheme_request_private.h:56
> +    Ewk_Url_Scheme_Request(WKSoupRequestManagerRef manager, WKStringRef urlString, uint64_t requestID)

Ditto.

[Kenneth Rohde Christiansen]

Comment on attachment 169119 [details]
patch

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

> Source/WebKit2/UIProcess/API/efl/ewk_context_request_manager_client.cpp:43
> +    RefPtr<Ewk_Url_Scheme_Request> schemeRequest = Ewk_Url_Scheme_Request::create(soupRequestManagerRef, adoptWK(WKURLCopyString(urlRef)).get(), requestID);

So you copy the string then you adopt it (turning it into a RetainPtr<WKStringRef>, which you then passes the WKStringRef of (a pointer; const struct OpaqueWKString*), then it gets out of scope, meaning that it will be freed after this function call ends. That doesn't seem right, unless you copy it inbetween

[Mikhail Pozdnyakov]

(In reply to comment #3)
> (From update of attachment 169119 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=169119&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_context_request_manager_client.cpp:43
> > +    RefPtr<Ewk_Url_Scheme_Request> schemeRequest = Ewk_Url_Scheme_Request::create(soupRequestManagerRef, adoptWK(WKURLCopyString(urlRef)).get(), requestID);
> 
> So you copy the string then you adopt it (turning it into a RetainPtr<WKStringRef>, which you then passes the WKStringRef of (a pointer; const struct OpaqueWKString*), then it gets out of scope, meaning that it will be freed after this function call ends. That doesn't seem right, unless you copy it inbetween

It'll be freed after Ewk_Url_Scheme_Request::create() exits because RefPtr will be kept on stack before. However, guess I should pass WKURLRef as Chris recommends

[Mikhail Pozdnyakov]

Created attachment 169138 [details]
patch v2

Took comments from Chris and Kenneth into consideration.

[WebKit Review Bot]

Comment on attachment 169138 [details]
patch v2

Clearing flags on attachment: 169138

Committed r131575: <http://trac.webkit.org/changeset/131575>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.