Bug 169089

Summary: Add WebKit2 hooks to notify the VM that the user has requested a debugger break.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, ggaren, jfbastien, joepeck, keith_miller, msaboff, saam
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch.
none
proposed patch.
thorton: review+
patch for landing.
none
patch for landing: applied one missing change. none

Mark Lam
Reported 2017-03-02 12:19:02 PST
Patch coming.
Attachments
proposed patch. (20.83 KB, patch)
2017-03-02 14:35 PST, Mark Lam 		no flags
DetailsFormatted DiffDiff
proposed patch. (21.49 KB, patch)
2017-03-02 15:47 PST, Mark Lam 		thorton: review+
DetailsFormatted DiffDiff
patch for landing. (21.80 KB, patch)
2017-03-02 16:14 PST, Mark Lam 		no flags
DetailsFormatted DiffDiff
patch for landing: applied one missing change. (21.78 KB, patch)
2017-03-02 16:24 PST, Mark Lam 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Mark Lam
Comment 1 2017-03-02 14:35:44 PST
Created attachment 303243 [details] proposed patch. Let's try this on the EWS.
Mark Lam
Comment 2 2017-03-02 15:47:31 PST
Created attachment 303261 [details] proposed patch.
Joseph Pecoraro
Comment 3 2017-03-02 16:00:40 PST
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 4 2017-03-02 16:04:02 PST
Comment on attachment 303261 [details] proposed patch. r+ if you fix joe's things
Joseph Pecoraro
Comment 5 2017-03-02 16:08:58 PST
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 6 2017-03-02 16:10:46 PST
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
Comment 7 2017-03-02 16:14:09 PST
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
Comment 8 2017-03-02 16:24:57 PST
Created attachment 303267 [details] patch for landing: applied one missing change.
WebKit Commit Bot
Comment 9 2017-03-02 18:41:48 PST
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
Comment 10 2017-03-02 18:41:53 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.