[webkit-reviews] review granted: [Bug 100598] [EFL][AC] Fix bugs preventing us from running layout tests with AC turned on : [Attachment 171148] Patch

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has granted Yael
<yael.aharon.m at gmail.com>'s request for review:
Bug 100598: [EFL][AC] Fix bugs preventing us from running layout tests with AC
turned on
https://bugs.webkit.org/show_bug.cgi?id=100598

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

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=171148&action=review


> Tools/EWebLauncher/main.c:813
>      app->ee = ecore_evas_new(userArgs->engine, 0, 0, userArgs->geometry.w,
userArgs->geometry.h, NULL);

shouldn't that be indented now?

> Tools/WebKitTestRunner/efl/PlatformWebViewEfl.cpp:39
>  static Ecore_Evas* initEcoreEvas()
>  {
> +#ifdef WTF_USE_ACCELERATED_COMPOSITING && defined HAVE_ECORE_X
> +    const char* engine = "opengl_x11";
> +    Ecore_Evas* ecoreEvas = ecore_evas_new(engine, 0, 0, 800, 600, 0);
> +#else
>      Ecore_Evas* ecoreEvas = ecore_evas_new(0, 0, 0, 800, 600, 0);
> +#endif
>      if (!ecoreEvas)

I would prefer

const char* engine = 0;

#ifdef WTF_USE_ACCELERATED_COMPOSITING && defined HAVE_ECORE_X
engine = "opengl_x11";
#endif

Ecore_Evas* ecoreEvas = ecore_evas_new(engine, 0, 0, 800, 600, 0);

> Tools/DumpRenderTree/efl/DumpRenderTree.cpp:427
> +#ifdef WTF_USE_ACCELERATED_COMPOSITING && defined HAVE_ECORE_X
> +    const char* engine = "opelgl_x11";
> +    Ecore_Evas* ecoreEvas = ecore_evas_new(engine, 0, 0, 800, 600, 0);
> +#else
>      Ecore_Evas* ecoreEvas = ecore_evas_new(0, 0, 0, 800, 600, 0);
> +#endif

so much copied code, can't we concentrate it?