Bug 111858 – [EFL] Start using evas object directly in Widget class

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

[Mikhail Pozdnyakov]

EFL Widget class should use evas object pointer directly rather than MAC specific WidgetPrivate* m_data. 
The reason: 1) to be compliant with other ports 2) it will make code much cleaner.

[Mikhail Pozdnyakov]

Created attachment 192226 [details]
patch

[Kenneth Rohde Christiansen]

Comment on attachment 192226 [details]
patch

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

> Source/WebCore/ChangeLog:10
> +        EFL Widget class should use evas object pointer directly rather
> +        than MAC-specific WidgetPrivate* m_data. The reasons are: firstly
> +        to be compliant with other ports, secondly to make code cleaner.

I would say that the platformWidget() is a Mac specific implementation detail that they are moving away from with WebKit2.

You could also point out that the evasObject is only directly used in the ScrollbarEfl subclass of Widget and not in Widget or ScrollView or any other subclass, thus the evas_object code has now been concentrated in ScrollbarEfl

> Source/WebCore/platform/efl/ScrollbarEfl.cpp:98
> +    if (!evasObject())
> +        return;
>  
> -    if (!object) {
> -        if (!view || !view->evas() || !view->evasObject())
> -            return;
> +    evas_object_show(evasObject());
> +}

Do you know if evas_object_show handles 0 pointer?

[Mikhail Pozdnyakov]

Comment on attachment 192226 [details]
patch

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

>> Source/WebCore/platform/efl/ScrollbarEfl.cpp:98
>> +}
> 
> Do you know if evas_object_show handles 0 pointer?

Think recommended practice is to check pointer before call (http://docs.enlightenment.org/auto/efl/Example_Evas_Images.html)

[Mikhail Pozdnyakov]

Comment on attachment 192226 [details]
patch

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

>> Source/WebCore/ChangeLog:10
>> +        to be compliant with other ports, secondly to make code cleaner.
> 
> I would say that the platformWidget() is a Mac specific implementation detail that they are moving away from with WebKit2.
> 
> You could also point out that the evasObject is only directly used in the ScrollbarEfl subclass of Widget and not in Widget or ScrollView or any other subclass, thus the evas_object code has now been concentrated in ScrollbarEfl

Thanks, I will add it

[Mikhail Pozdnyakov]

Created attachment 192505 [details]
patch v2

Fixed comments above.

[Build Bot]

Comment on attachment 192505 [details]
patch v2

Attachment 192505 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17177291

New failing tests:
editing/selection/selection-modify-crash.html

[Mikhail Pozdnyakov]

(In reply to comment #6)
> (From update of attachment 192505 [details])
> Attachment 192505 [details] did not pass mac-ews (mac):
> Output: http://webkit-commit-queue.appspot.com/results/17177291
> 
> New failing tests:
> editing/selection/selection-modify-crash.html

Has to be unrelated as only efl-specific code is touched.
Will re-upload patch however to be sure.

[Mikhail Pozdnyakov]

Created attachment 192775 [details]
patch v2

Re-uploading to verify EWS

[WebKit Review Bot]

Comment on attachment 192775 [details]
patch v2

Clearing flags on attachment: 192775

Committed r145710: <http://trac.webkit.org/changeset/145710>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.