[Webkit-unassigned] [Bug 237601] [GTK][WPE] Web Inspector: make it possible to use the remote inspector from other browsers

[bugzilla-daemon at webkit.org]

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

Michael Catanzaro <mcatanzaro at gnome.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mcatanzaro at gnome.org
 Attachment #454114|review?                     |review+
              Flags|                            |

--- Comment #2 from Michael Catanzaro <mcatanzaro at gnome.org> ---
Comment on attachment 454114
  --> https://bugs.webkit.org/attachment.cgi?id=454114
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=454114&action=review

Cool.

> Source/WebKit/UIProcess/API/glib/WebKitInitialize.cpp:78
> +        g_setenv("WEBKIT_INSPECTOR_SERVER", inspectorAddress.get(), TRUE);

Careful: this function is called inside std::call_once, which uses threads, so it's too late to modify the environment.

IMO you are insufficiently fearful of setenv. It's not just a theoretical problem: remember we have documented cases where this broke gnome-session (https://bugzilla.gnome.org/show_bug.cgi?id=754951) and also gnome-initial-setup (downstream fork).

> Source/WebKit/UIProcess/Inspector/glib/RemoteInspectorHTTPServer.cpp:50
> +        g_warning("Failed to start remote inspector HTTP server on %s:%u: invalid address\n", address, port);

Oops: should be no trailing \n here

Also: if address is an IPv6 address, then %s:%u will be an invalid URI: it should be [%s]:%u in that case. I suppose that's an edge case that's probably not worth handling manually, but there is a g_inet_address_to_string() you might be able to use to avoid it.

> Source/WebKit/UIProcess/Inspector/glib/RemoteInspectorHTTPServer.cpp:56
> +        g_warning("Failed to start remote inspector HTTP server on %s:%u: %s\n", address, port, error->message);

Ditto

> Source/WebKit/UIProcess/Inspector/glib/RemoteInspectorHTTPServer.cpp:157
> +    g_signal_handlers_disconnect_by_data(webSocketConnection.get(), this);

Good.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220308/38a8af35/attachment-0001.htm>