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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jul 18 07:09:45 PDT 2012


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


Gustavo Noronha (kov) <gns at gnome.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #152986|review?                     |review-
               Flag|                            |




--- Comment #7 from Gustavo Noronha (kov) <gns at gnome.org>  2012-07-18 07:09:43 PST ---
(From update of attachment 152986)
View in context: https://bugs.webkit.org/attachment.cgi?id=152986&action=review

> Source/WebCore/platform/network/soup/SocketStreamHandleSoup.cpp:109
> +    if (!m_readBuffer)
> +        return;

This check seems unnecessary.

> Source/WebCore/platform/network/soup/SocketStreamHandleSoup.cpp:121
> +    if (m_client) {
> +        // The client can close the handle, potentially removing the last reference.
> +        RefPtr<SocketStreamHandle> protect(this); 
> +        m_client->didOpenSocketStream(this);
> +    }

Protecting this should not be necessary here, since we are not doing anything else after didOpenSocketStream's called.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:375
> +#if PLATFORM(GTK)
> +    // will read host address from environment variables
> +    if (m_pageGroup->preferences()->developerExtrasEnabled())
> +        WebInspectorServer::shared().listen("", 2999);
> +    else
> +        WebInspectorServer::shared().close();
> +#endif

This doesn't look right... WebSocketServer::platform{Listen,Close} should already be called when appropriate, shouldn't they?

> Source/WebKit2/UIProcess/WebPageProxy.cpp:1747
> +#if PLATFORM(GTK)
> +    // will read host address from environment variables
> +    if (m_pageGroup->preferences()->developerExtrasEnabled())
> +        WebInspectorServer::shared().listen("", 2999);
> +    else
> +        WebInspectorServer::shared().close();
> +#endif

Ditto.

> Source/WebKit2/UIProcess/InspectorServer/gtk/WebInspectorServerGtk.cpp:38
> +    if (path == "/") {

So, as I understand it, if the remote side asks for / we want to give it the default page, which is the inspector.html file, but we want any other resource requested to be delivered as is. I don't understand what the meta refresh is about; also, is there a reason why we are not delivering pagelist.json in a special-case like qt?

> Source/WebKit2/UIProcess/InspectorServer/gtk/WebInspectorServerGtk.cpp:50
> +        if (!url)
> +            url = "http://trac.webkit.org/export/head/trunk/Source/WebCore/inspector/front-end/inspector.html";

Hmm, no, we should use the locally installed file, not the one hosted in webkit.org.

> Source/WebKit2/UIProcess/InspectorServer/gtk/WebSocketServerGtk.cpp:87
> +        portValue = 2999;

Why 2999? Is it the same one used by the other ports?

> Source/WebKit2/UIProcess/InspectorServer/gtk/WebSocketServerGtk.cpp:88
> +        const char* envCStr = getenv("GTKWEBKIT_INSPECTOR_SERVER");

We use WEBKIT_ as our environment variables prefix.

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



More information about the webkit-unassigned mailing list