[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 #48 from Carlos Garcia Campos <cgarcia at igalia.com>  2012-11-08 07:02:44 PST ---
(From update of attachment 169656)
View in context: https://bugs.webkit.org/attachment.cgi?id=169656&action=review

Looks good to me, I have only a couple of minor comments. Gustavo, are your concerns about the changes to WebSocketServerConnection fixed now?

> Source/WebKit2/GNUmakefile.am:560
> +dist_remoteinspector_DATA = \

do we really need the dist prefix? I guess you can use remoteinspector_DATA and add the inspectorPageIndex.html to EXTRA_DIST.

> Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp:32
> +static const char* s_testServerAppName = "InspectorTestServer";

We don't use that prefix for global variables, we use k or g without the underscore.

> Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp:35
> +static const int s_maxWaitForChild = 5;

Ditto.

> Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp:38
> +static GPid s_childProcessPid = 0;

Ditto.

> Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp:41
> +static bool s_childIsReady = false;

Ditto.

> Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp:193
> +    String validInspectorURL = String("/webinspector/inspector.html?page=") + String::number(pageId);
> +    g_assert_cmpstr(valueString.get(), ==, validInspectorURL.latin1().data());

utf8() instead of latin1()

> Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp:241
> +    String resolvedURL = String("http://127.0.0.1:2999/") + WebViewTest::javascriptResultToCString(javascriptResult);

I think this should be 

String("http://127.0.0.1:2999/") + String::fromUTF8(WebViewTest::javascriptResultToCString(javascriptResult).data())

because js result is a CString utf8 encoded.

> Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp:242
> +    test->loadURI(resolvedURL.latin1().data());

utf8() instead of latin1()

> Source/WebKit2/UIProcess/InspectorServer/gtk/WebInspectorServerGtk.cpp:28
> +#include "FileSystem.h"

Use <WebCore/FileSystem.h>

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