Summary: | [EFL][WK2] Ewk_Url_Scheme_Request has to be refactored | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mikhail Pozdnyakov <mikhail.pozdnyakov> | ||||||
Component: | WebKit EFL | Assignee: | Mikhail Pozdnyakov <mikhail.pozdnyakov> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cdumez, gyuyoung.kim, kenneth, lucas.de.marchi, rakuco, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 99321 | ||||||||
Attachments: |
|
Description
Mikhail Pozdnyakov
2012-10-16 23:52:45 PDT
Created attachment 169119 [details]
patch
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. 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 (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 Created attachment 169138 [details]
patch v2
Took comments from Chris and Kenneth into consideration.
Comment on attachment 169138 [details] patch v2 Clearing flags on attachment: 169138 Committed r131575: <http://trac.webkit.org/changeset/131575> All reviewed patches have been landed. Closing bug. |