Bug 99455 - [EFL][WK2] Inherit Ewk_Download_Job, Ewk_Back_Forward_List_Item, Ewk_Url_Response, Ewk_Navigation_Policy_Decision from RefCounted
Summary: [EFL][WK2] Inherit Ewk_Download_Job, Ewk_Back_Forward_List_Item, Ewk_Url_Resp...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mikhail Pozdnyakov
URL:
Keywords:
Depends on:
Blocks: 99321
  Show dependency treegraph
 
Reported: 2012-10-16 05:04 PDT by Mikhail Pozdnyakov
Modified: 2012-10-16 09:08 PDT (History)
4 users (show)

See Also:


Attachments
patch (28.50 KB, patch)
2012-10-16 05:09 PDT, Mikhail Pozdnyakov
kenneth: review+
Details | Formatted Diff | Diff
patch v2 (28.50 KB, patch)
2012-10-16 06:32 PDT, Mikhail Pozdnyakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail Pozdnyakov 2012-10-16 05:04:03 PDT
Sub bug for bug#99321
Comment 1 Mikhail Pozdnyakov 2012-10-16 05:09:58 PDT
Created attachment 168923 [details]
patch
Comment 2 Kenneth Rohde Christiansen 2012-10-16 06:05:17 PDT
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
Comment 3 Mikhail Pozdnyakov 2012-10-16 06:29:28 PDT
(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
Comment 4 Mikhail Pozdnyakov 2012-10-16 06:32:45 PDT
Created attachment 168936 [details]
patch v2

Took Kenneth comment into consideration.
Comment 5 WebKit Review Bot 2012-10-16 09:08:03 PDT
Comment on attachment 168936 [details]
patch v2

Clearing flags on attachment: 168936

Committed r131460: <http://trac.webkit.org/changeset/131460>
Comment 6 WebKit Review Bot 2012-10-16 09:08:07 PDT
All reviewed patches have been landed.  Closing bug.