Bug 90768 – [WK2][EFL] Facilitate debugging of the Web Process

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

WebKitTestRunner should provide a way to pause the UIProcess on start to allow attaching to the WebProcess with gdb for debugging purposes.

This could be easily achieved via an environment variable.

[Chris Dumez]

I meant the WebProcess, not UIProcess.

[Chris Dumez]

Created attachment 151230 [details]
Patch

[Thiago Marcos P. Santos]

Honestly I like more the approach of adding a mechanism to ProcessLauncherEfl.cpp to wrap the new process around something. What I'm suggesting is similar to the way how it is done in chromium:

http://code.google.com/p/chromium/wiki/LinuxDebugging

[Chris Dumez]

Comment on attachment 151230 [details]
Patch

Ok, I agree with Thiago, I'll implement the "WebProcessCmdPrefix" approach then.

[Chris Dumez]

Created attachment 151298 [details]
Patch

Here is an updated proposal. It is now possible to debug WebProcess using something like:
./Tools/Scripts/run-webkit-tests -2 --efl --webprocess-cmd-prefix="xterm -title WebProcess -e gdb --args" LayoutTests/batterystatus/add-listener-from-callback.html

2 things to keep in mind:
a) This looks like this may introduce a security issue as it is since anyone can mess with the WebProcess loading
b) Should we allow this only in debug mode?

[Chris Dumez]

Created attachment 151435 [details]
Patch

Enable functionality in debug mode only.

[Chris Dumez]

Created attachment 151439 [details]
Patch

Fix compilation error.

[Ojan Vafai]

Comment on attachment 151439 [details]
Patch

The webkitpy changes look good to me. I'd prefer if someone more familiar with WebKit2 reviewed that part though. I don't know the WebKit2 code at all.

[Gyuyoung Kim]

Comment on attachment 151439 [details]
Patch

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

> Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:79
> +            String cmd = m_launchOptions.processCmdPrefix + " " + fullPath + " " + socket;

I would prefer to use string utility function. For example, makeString().

[Chris Dumez]

Created attachment 151614 [details]
Patch

- Use makeString()
- Remove PLATFORM(EFL) #ifdef

[Kenneth Rohde Christiansen]

Comment on attachment 151614 [details]
Patch

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

> Source/WebKit2/ChangeLog:12
> +        for debugging purposes with prefixes such as:
> +        "xterm -title renderer -e gdb --args".

How is this much better than using gdb --args ... and then set follow-fork-mode child ?

> Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:261
> +    option_group_definitions.append(("EFL-specific Options", [
> +        optparse.make_option("--webprocess-cmd-prefix", type="string",
> +            default=False, help="Prefix used when spawning the Web process (Debug mode only)"),
> +    ]))
> +

I guess Qt could use this as well? or actually all webkit2 ports

[Chris Dumez]

Comment on attachment 151614 [details]
Patch

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

>> Source/WebKit2/ChangeLog:12
>> +        "xterm -title renderer -e gdb --args".
> 
> How is this much better than using gdb --args ... and then set follow-fork-mode child ?

In the gdb case, you can achieve pretty much the same thing with set follow-fork-mode child, indeed. But our approach is more generic, you can for example run valgrind on the Web process.

>> Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:261
>> +
> 
> I guess Qt could use this as well? or actually all webkit2 ports

Yes, I do believe this would be useful to other ports as well. However, this require some work in their port-specific ProcessLauncher to support it. As a consequence, this is an EFL-specific Option for now. As soon as we bring support for other ports, we can get rid of the "EFL-specific" mention.

[Thiago Marcos P. Santos]

Valgrind can trace children as well but...

In both cases, I still think this is a valid approach because you have much less debugging overhead by ptrace'ing only the WebProcess.

It is specially important to reduce your tracing scope when you are using tools that does CPU sampling.

[Chris Dumez]

@Kenneth: Is it OK to land as it is then?

@Thiago: Good to know. I had no idea Valgrind could follow children. I knew for sure it could not attach to a running process though.

[Kenneth Rohde Christiansen]

sure, go ahead

[Gyuyoung Kim]

Comment on attachment 151614 [details]
Patch

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

> Source/WebKit2/UIProcess/WebProcessProxy.cpp:114
> +        const char* webProcessCmdPrefix = getenv("WEB_PROCESS_CMD_PREFIX");

Could you add an article to WebKit EFL wiki page how to use this environment ?

[WebKit Review Bot]

Comment on attachment 151614 [details]
Patch

Clearing flags on attachment: 151614

Committed r122542: <http://trac.webkit.org/changeset/122542>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

(In reply to comment #16)
> (From update of attachment 151614 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=151614&action=review
> 
> > Source/WebKit2/UIProcess/WebProcessProxy.cpp:114
> > +        const char* webProcessCmdPrefix = getenv("WEB_PROCESS_CMD_PREFIX");
> 
> Could you add an article to WebKit EFL wiki page how to use this environment ?

I updated the wiki page:
http://trac.webkit.org/wiki/WebKitEFLLayoutTest#Debuggingcrashes