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.
Created attachment 192226 [details] patch
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?
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)
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
Created attachment 192505 [details] patch v2 Fixed comments above.
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
(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.
Created attachment 192775 [details] patch v2 Re-uploading to verify EWS
Comment on attachment 192775 [details] patch v2 Clearing flags on attachment: 192775 Committed r145710: <http://trac.webkit.org/changeset/145710>
All reviewed patches have been landed. Closing bug.