Bug 169089 – Add WebKit2 hooks to notify the VM that the user has requested a debugger break.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

Patch coming.

[Mark Lam]

Created attachment 303243 [details]
proposed patch.

Let's try this on the EWS.

[Mark Lam]

Created attachment 303261 [details]
proposed patch.

[Joseph Pecoraro]

Comment on attachment 303261 [details]
proposed patch.

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

Patch looks fine to me. It should get a WebKit2 owner approval.

> Source/WebKit2/CMakeLists.txt:622
> +    UIProcess/WebInspectorInterruptDispatcher.messages.in

This should be down by: WebProcess/WebPage/WebInspector.messages.in it is not UIProcess. (Should fix GTK)

> Source/WebKit2/WebKit2.xcodeproj/project.pbxproj:6460
> +				FEE43FD21E67AFC60077D6D1 /* WebInspectorInterruptDispatcher.messages.in */,
> +				FEE43FD01E67AFC60077D6D1 /* WebInspectorInterruptDispatcher.cpp */,
> +				FEE43FD11E67AFC60077D6D1 /* WebInspectorInterruptDispatcher.h */,

Nit: Sort order, the messages file should be after the .h in the Xcode sidebar.

> Source/WebKit2/WebProcess/WebPage/WebInspectorInterruptDispatcher.messages.in:24
> +    NotifyNeedDebuggerBreak(uint64_t pageID)

pageID is not used and doesn't seem like it will be used anytime soon, so lets drop it.

[Tim Horton]

Comment on attachment 303261 [details]
proposed patch.

r+ if you fix joe's things

[Joseph Pecoraro]

Comment on attachment 303261 [details]
proposed patch.

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

> Source/JavaScriptCore/runtime/VM.cpp:960
> +#if OS(DARWIN)

Is there something special about this that needs the OS(DARWIN) guard?

[Joseph Pecoraro]

Comment on attachment 303261 [details]
proposed patch.

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

>> Source/JavaScriptCore/runtime/VM.cpp:960
>> +#if OS(DARWIN)
> 
> Is there something special about this that needs the OS(DARWIN) guard?

Oh, probably for the getpid(). WTF has getCurrentProcessID() which should work for all ports.

[Mark Lam]

Created attachment 303263 [details]
patch for landing.

Thanks for the reviews.  I've applied all the requested changes.  Will wait for the EWS bots to be happy before landing.

[Mark Lam]

Created attachment 303267 [details]
patch for landing: applied one missing change.

[WebKit Commit Bot]

Comment on attachment 303267 [details]
patch for landing: applied one missing change.

Clearing flags on attachment: 303267

Committed r213338: <http://trac.webkit.org/changeset/213338>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.