Bug 117137 – [GTK] Remote inspector server should send an error page if resources not found

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Anton Obzhirov]

[GTK] Remote inspector server should send an error page if resources not found

[Anton Obzhirov]

Created attachment 203568 [details]
Patch

[Anton Obzhirov]

Comment on attachment 203568 [details]
Patch

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

> Source/WebKit2/UIProcess/InspectorServer/gtk/WebInspectorServerGtk.cpp:59
> +        if (error->code == G_IO_ERROR_NOT_FOUND) {

I wonder if I should send some basic error page for other errors - unknown error during retrieving inspector resources - or something like that.

[Gustavo Noronha (kov)]

Comment on attachment 203568 [details]
Patch

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

Have you considered doing a g_warning on all of the return false cases? That should make it much easier to debug things, generally.

>> Source/WebKit2/UIProcess/InspectorServer/gtk/WebInspectorServerGtk.cpp:59
>> +        if (error->code == G_IO_ERROR_NOT_FOUND) {
> 
> I wonder if I should send some basic error page for other errors - unknown error during retrieving inspector resources - or something like that.

I think that makes sense, yeah. On second thought, this is likely to hit technical people, so maybe just don't do the error->code check: show it and error->message as well, should make it easier for those people to debug it.

> Source/WebKit2/UIProcess/InspectorServer/gtk/WebInspectorServerGtk.cpp:63
> +            builder.appendLiteral(", make sure that you have set WEBKIT_INSPECTOR_SERVER_PATH in your environment to point to webinspector folder.</body></html>");

How about "make sure you ran make install or have set WEBKIT_…"

[Anton Obzhirov]

Comment on attachment 203568 [details]
Patch

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

>>> Source/WebKit2/UIProcess/InspectorServer/gtk/WebInspectorServerGtk.cpp:59
>>> +        if (error->code == G_IO_ERROR_NOT_FOUND) {
>> 
>> I wonder if I should send some basic error page for other errors - unknown error during retrieving inspector resources - or something like that.
> 
> I think that makes sense, yeah. On second thought, this is likely to hit technical people, so maybe just don't do the error->code check: show it and error->message as well, should make it easier for those people to debug it.

yep, makes sense. Something like "Error (Code: %d, Message: %s) occurred during fetching webinspector resource files. Make sure you ran make install or have set WEBKIT_INSPECTOR_SERVER_PATH in your environment to point to webinspector folder"?

>> Source/WebKit2/UIProcess/InspectorServer/gtk/WebInspectorServerGtk.cpp:63
>> +            builder.appendLiteral(", make sure that you have set WEBKIT_INSPECTOR_SERVER_PATH in your environment to point to webinspector folder.</body></html>");
> 
> How about "make sure you ran make install or have set WEBKIT_…"

ok

[Anton Obzhirov]

I can add also LOG(InspectorServer, ... . It should be consistent with other remote inspector code (see for example in 
./Source/WebKit2/UIProcess/gtk/WebContextGtk.cpp:77:    LOG(InspectorServer, "To start inspector server set WEBKIT_INSPECTOR_SERVER to 127.0.0.1:2999 for example.");

[Gustavo Noronha (kov)]

Using LOG sounds good too, thing is it will only show up when running on a debug build with WEBKIT_DEBUG set to InspectorServer, so we should not rely on it as the main means of letting the user know there was a problem.

[Anton Obzhirov]

(In reply to comment #6)
> Using LOG sounds good too, thing is it will only show up when running on a debug build with WEBKIT_DEBUG set to InspectorServer, so we should not rely on it as the main means of letting the user know there was a problem.
ok, I'll make change with g_warning.

[Anton Obzhirov]

Created attachment 206455 [details]
Patch

[Build Bot]

Comment on attachment 206455 [details]
Patch

Attachment 206455 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1052298

New failing tests:
svg/batik/filters/feTile.svg

[Build Bot]

Created attachment 206466 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.3

[WebKit Commit Bot]

Comment on attachment 206455 [details]
Patch

Clearing flags on attachment: 206455

Committed r152633: <http://trac.webkit.org/changeset/152633>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.