[Webkit-unassigned] [Bug 88094] Web Inspector: Add a WebInspectorServer on Linux using the GSocket API for the GTK port

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=88094





--- Comment #34 from Yuta Kitamura <yutak at chromium.org>  2012-10-08 23:28:15 PST ---
(From update of attachment 167539)
View in context: https://bugs.webkit.org/attachment.cgi?id=167539&action=review

Just added style comments. SocketStream part looks good to me. I'd like to let GTK folks to do the final review.

> Source/WebKit2/ChangeLog:9
> +        The server listens on port 2999 by default. Ip address of the server can be set

Ip -> IP

> Source/WebKit2/UIProcess/InspectorServer/WebInspectorServer.h:64
> +#if (PLATFORM(QT) || PLATFORM(GTK))

Outermost parentheses are not necessary.

> Source/WebKit2/UIProcess/InspectorServer/gtk/WebSocketServerGtk.cpp:44
> +#ifndef LOG_DISABLED

I think this line should be "#if !LOG_DISABLED".

> Source/WebKit2/UIProcess/InspectorServer/gtk/WebSocketServerGtk.cpp:68
> +#ifndef LOG_DISABLED
> +    LOG(Network, "Listen to address=%s, port=%d.", bindAddress.utf8().data(), port);
> +#endif

This #ifndef block is not necessary because LOG() becomes a no-op when LOG_DISABLED is true. See wtf/Assertions.h.

> Source/WebKit2/UIProcess/gtk/WebContextGtk.cpp:46
> +    String envStr(g_getenv("WEBKIT_INSPECTOR_SERVER"));

We tend to avoid abbreviations like "env" or "Str" and spell the words fully.

Or, as an alternative, "serverAddress" or "serverSpecification" may sound better.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.