Bug 167084 – Web Inspector: capture async stack trace when workers/main context posts a message

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Blaze Burg]

This will require some UI work, since separate halves of the stack trace will be in two (or more, if posting a message while handling a dispatched message) contexts.

[Radar WebKit Bug Importer]

<rdar://problem/30033673>

[Matt Baker]

Created attachment 316266 [details]
Patch

[Matt Baker]

Created attachment 316268 [details]
[Image] postMessage stack trace

[Blaze Burg]

Comment on attachment 316266 [details]
Patch

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

I'm going to mark this as r- for now since the error cases are not tested. There may be some churn to address comments in dependent patches. Other than those two things, this change looks good.

> Source/JavaScriptCore/ChangeLog:8
> +

Nit: created

> Source/WebCore/inspector/InspectorInstrumentation.h:129
> +    static void didPostMessage(Frame*, TimerBase&, JSC::ExecState&);

Please make this name match the other method in the pair: didDispatchPostMessage

> Source/WebCore/inspector/PageDebuggerAgent.cpp:207
> +

I don't know much about this didFailPostMessage case. Does this only happen because of a security violation? What if you detach the iframe synchronously after posting a message?

> LayoutTests/inspector/debugger/async-stack-trace.html:51
> +    });

You need to test the case where the postMessage fails.

[Matt Baker]

Created attachment 316820 [details]
Patch

[Blaze Burg]

Comment on attachment 316820 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 316820 [details]
Patch

Rejecting attachment 316820 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 316820, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
debugger/async-stack-trace.html
Hunk #2 FAILED at 96.
1 out of 4 hunks FAILED -- saving rejects to file LayoutTests/inspector/debugger/async-stack-trace.html.rej
patching file LayoutTests/inspector/debugger/resources/postMessage-echo.html
patching file LayoutTests/inspector/dom-debugger/dom-breakpoints.html
Hunk #1 succeeded at 36 with fuzz 1.

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Brian Burg']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.webkit.org/results/4254271

[Matt Baker]

Created attachment 317264 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 317264 [details]
Patch

Clearing flags on attachment: 317264

Committed r220299: <http://trac.webkit.org/changeset/220299>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

(In reply to WebKit Commit Bot from comment #9)
> Comment on attachment 317264 [details]
> Patch
> 
> Clearing flags on attachment: 317264
> 
> Committed r220299: <http://trac.webkit.org/changeset/220299>

This change caused inspector/dom-debugger/dom-breakpoints.html to fail:

+!! EXCEPTION: Can't find variable: awaitEvaluateInPage
+Stack Trace: #0: (anonymous) (unknown)
+#1: promiseReactionJob [native code]

https://build.webkit.org/results/Apple%20El%20Capitan%20Debug%20WK1%20(Tests)/r220299%20(2825)/results.html

[Ryan Haddad]

Reverted r220299 for reason:

This change caused LayoutTest inspector/dom-debugger/dom-breakpoints.html to fail.

Committed r220342: <http://trac.webkit.org/changeset/220342>

[Matt Baker]

Created attachment 318282 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 318282 [details]
Patch

Clearing flags on attachment: 318282

Committed r220815: <http://trac.webkit.org/changeset/220815>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.