[webkit-reviews] review denied: [Bug 107198] [TexMap] Enable debug borders and repaint counter via Settings. : [Attachment 183308] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Huang Dongsung
<luxtella at company100.net>'s request for review:
Bug 107198: [TexMap] Enable debug borders and repaint counter via Settings.
https://bugs.webkit.org/show_bug.cgi?id=107198

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

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


> Source/WebKit/efl/ewk/ewk_view.cpp:842
> +    bool showDebugVisuals =
String(getenv("WEBKIT_SHOW_COMPOSITING_DEBUG_VISUALS")) == "1";

This is quite overkill. If you care about start up time, you want to avoid
creating a new String here.

Just use StringImpl's equal() functions here?

> Source/WebKit/gtk/webkit/webkitwebview.cpp:3438
> +    bool showDebugVisuals =
String(getenv("WEBKIT_SHOW_COMPOSITING_DEBUG_VISUALS")) == "1";

Ditto.

> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:154
> +    bool showDebugVisuals =
String(getenv("WEBKIT_SHOW_COMPOSITING_DEBUG_VISUALS")) == "1";

ditto.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:261
> +	   if (g_value_get_boolean(value))
> +	       webkit_settings_set_draw_compositing_indicators(settings,
g_value_get_boolean(value));

It would looks nicer to keep the value of g_value_get_boolean(value) instead of
calling it twice.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:263
> +	       bool showDebugVisuals =
String(getenv("WEBKIT_SHOW_COMPOSITING_DEBUG_VISUALS")) == "1";

Ditto regarding mem allocations.