[webkit-reviews] review denied: [Bug 70592] [EFL] Windowless plugins implementation : [Attachment 111952] EFL windowless plugins implementation

[bugzilla-daemon at webkit.org]

Leandro Pereira <leandro at profusion.mobi> has denied Mariusz Grzegorczyk
<mariusz.g at samsung.com>'s request for review:
Bug 70592: [EFL] Windowless plugins implementation
https://bugs.webkit.org/show_bug.cgi?id=70592

Attachment 111952: EFL windowless plugins implementation
https://bugs.webkit.org/attachment.cgi?id=111952&action=review

------- Additional Comments from Leandro Pereira <leandro at profusion.mobi>
View in context: https://bugs.webkit.org/attachment.cgi?id=111952&action=review


> Source/WebCore/plugins/efl/PluginViewEfl.cpp:51
> +#include <PlatformContextCairo.h>
> +#include <X11/Xlib.h>
> +#include <cairo/cairo-xlib.h>

Be aware that even though Ecore_X is present during compilation, it might not
be during runtime. For instance, DumpRenderTree won't be able to run plugin
tests because it uses Ecore_Evas_Buffer (which performs all operations in
memory, without a connection to a X server). It would be nice if either the
plugin subsystem were disabled if it's not running under X, or provide an
implementation independent of the graphical backend.

> Source/WebCore/plugins/efl/PluginViewEfl.cpp:60
> +    // sanity check

Unneeded comment.

> Source/WebCore/plugins/efl/PluginViewEfl.cpp:216
> +    const bool syncX = m_pluginDisplay && m_pluginDisplay != display;

Please precede ``syncX'' by ``should'': shouldSyncWithX.

> Source/WebCore/plugins/efl/PluginViewEfl.cpp:233
> +	   RefPtr<cairo_t> cr = adoptRef(cairo_create(drawableSurface.get()));

``cr'' doesn't look like a good variable name.

> Source/WebCore/plugins/efl/PluginViewEfl.cpp:261
> +    exposeEvent.width = exposedRect.x() + exposedRect.width(); // flash bug?
it thinks width is the right in transparent mode
> +    exposeEvent.height = exposedRect.y() + exposedRect.height(); // flash
bug? it thinks height is the bottom in transparent mode

Please use proper sentences in comments.