[webkit-reviews] review denied: [Bug 35932] [EFL] Add ewk_view.{cpp, h} to WK/efl/ewk. : [Attachment 50331] Add ewk_view to WK/efl/ewk.

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has denied Leandro Pereira
<leandro at profusion.mobi>'s request for review:
Bug 35932: [EFL] Add ewk_view.{cpp,h} to WK/efl/ewk.
https://bugs.webkit.org/show_bug.cgi?id=35932

Attachment 50331: Add ewk_view to WK/efl/ewk.
https://bugs.webkit.org/attachment.cgi?id=50331&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
 169 static inline void _ewk_view_smart_changed(Ewk_View_Smart_Data* sd)

There are several inline markers in this code. In WebKit land we're averse to
premature, unmeasured optimization. If you have measured that this helps, this
should be OK, but if you haven't I'd recommend leaving this up to the compiler.


 478 static inline WTF::PassRefPtr<WebCore::Frame>
_ewk_view_core_frame_new(Ewk_View_Smart_Data* sd, Ewk_View_Private_Data* priv,
WebCore::HTMLFrameOwnerElement* owner)
[...]
 488	 return WebCore::Frame::create(priv->page, owner, flc).get();

This is wrong. Frame::create will adopt the ref a RefPtr is holding, and return
a PassRefPtr already. What you're doing here is you are getting the raw
pointer, and returning it. When the returned PassRefPtr goes out of scope the
frame will likely be deleted.