Bug 99455 – [EFL][WK2] Inherit Ewk_Download_Job, Ewk_Back_Forward_List_Item, Ewk_Url_Response, Ewk_Navigation_Policy_Decision from RefCounted

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Mikhail Pozdnyakov]

Sub bug for bug#99321

[Mikhail Pozdnyakov]

Created attachment 168923 [details]
patch

[Kenneth Rohde Christiansen]

Comment on attachment 168923 [details]
patch

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

> Source/WebKit2/UIProcess/API/efl/ewk_back_forward_list.cpp:NaN
>  static inline Ewk_Back_Forward_List_Item* addItemToWrapperCache(const Ewk_Back_F

it's internal, maybe we should return the PassRefPtr?

> Source/WebKit2/UIProcess/API/efl/ewk_navigation_policy_decision_private.h:51
> +    _Ewk_Navigation_Policy_Decision(WKFramePolicyListenerRef _listener, Ewk_Navigation_Type _navigationType, Event_Mouse_Button _mouseButton, Event_Modifier_Keys _modifiers, PassRefPtr<Ewk_Url_Request> _request, const char* _frameName)

C++ doesnt require you to add _ here

: listener(listener) should work

[Mikhail Pozdnyakov]

(In reply to comment #2)
> (From update of attachment 168923 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=168923&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_back_forward_list.cpp:NaN
> >  static inline Ewk_Back_Forward_List_Item* addItemToWrapperCache(const Ewk_Back_F
> 
> it's internal, maybe we should return the PassRefPtr?

To my mind it would not be convenient as we have several functions like following:
Ewk_Back_Forward_List_Item* ewk_back_forward_list_current_item_get(const Ewk_Back_Forward_List* list)
{
    EWK_BACK_FORWARD_LIST_WK_GET_OR_RETURN(list, wkList, 0);

    return addItemToWrapperCache(list, WKBackForwardListGetCurrentItem(wkList));
}

if PassRefPtr was returned it would be 

return RefPtr<Ewk_Back_Forward_List_Item>(addItemToWrapperCache(list, WKBackForwardListGetCurrentItem(wkList))).get();

that looks worse IMHO

> 
> > Source/WebKit2/UIProcess/API/efl/ewk_navigation_policy_decision_private.h:51
> > +    _Ewk_Navigation_Policy_Decision(WKFramePolicyListenerRef _listener, Ewk_Navigation_Type _navigationType, Event_Mouse_Button _mouseButton, Event_Modifier_Keys _modifiers, PassRefPtr<Ewk_Url_Request> _request, const char* _frameName)
> 
> C++ doesnt require you to add _ here
> 
> : listener(listener) should work

sure

[Mikhail Pozdnyakov]

Created attachment 168936 [details]
patch v2

Took Kenneth comment into consideration.

[WebKit Review Bot]

Comment on attachment 168936 [details]
patch v2

Clearing flags on attachment: 168936

Committed r131460: <http://trac.webkit.org/changeset/131460>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.