[webkit-reviews] review denied: [Bug 107820] [EFL][WK2] Use C API inside ewk_settings : [Attachment 187245] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Christophe Dumez
<dchris at gmail.com>'s request for review:
Bug 107820: [EFL][WK2] Use C API inside ewk_settings
https://bugs.webkit.org/show_bug.cgi?id=107820

Attachment 187245: Patch
https://bugs.webkit.org/attachment.cgi?id=187245&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=187245&action=review


> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:241
> -    , m_settings(EwkSettings::create(this))
> +    ,
m_settings(EwkSettings::create(WKPageGroupGetPreferences(WKPageGetPageGroup(m_w
ebView->pageRef()))))

I don't get this. You get the PageGroup as an argument of the constructor, yet
you extract (an other?) PageGroup from m_webView?
Why not construct the attribute from your parameters?

> Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:99
> +
> +    return
WKPreferencesGetFullScreenEnabled(const_cast<Ewk_Settings*>(settings)->preferen
ces());

Why not make preferences() const instead of const-cast every access?

> Source/WebKit2/UIProcess/API/efl/ewk_settings_private.h:49
> +    inline WKPreferencesRef preferences() {	return m_preferences.get(); }

You can remove the inline keyword, it is redundant. Make this const?