Summary: | ASSERT when paused in debugger and console evaluation causes exception | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | NEW --- | ||||||||
Severity: | Normal | CC: | commit-queue, ews-watchlist, joepeck, keith_miller, mark.lam, msaboff, saam | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Joseph Pecoraro
2019-01-08 11:05:05 PST
This is the inspector evaluating JavaScript when paused, so there are a few things. 1. The page is paused in JavaScript (nested event loop) 2. The inspector then evaluates script in its InjectedScript (InjectedScript::evaluateOnCallFrame) 3. The evaluate on call frame performs an eval on the call frame (DebuggerCallFrame::evaluateWithScopeExtension) So the "topCallFrame" may be a little tricky here. Worked through this with Mark. The Debugger is evaluating on a specific call frame (the selected one in Web Inspector) so the target call frame will not be the topCallFrame. We will enhance the assertion for the inspector's case. Created attachment 358634 [details]
[PATCH] Proposed Fix
Comment on attachment 358634 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=358634&action=review r=me with some suggestions. > Source/JavaScriptCore/debugger/DebuggerEvalEnabler.h:37 > + EvalOnCurrentFrame, > + EvalOnDebuggerCallFrame, Now that I have more time to think about it, how about naming these as: EvalOnCurrentFrame => EvalOnCurrentCallFrame EvalOnDebuggerCallFrame => EvalOnCallFrameAtDebuggerEntry I think these names would be clearer. What do you think? > Source/JavaScriptCore/runtime/JSGlobalObject.h:494 > + const ExecState* m_evalDebuggerCallFrame { nullptr }; I suggest naming this as "m_callFrameAtDebuggerEntry" instead. > Source/JavaScriptCore/runtime/JSGlobalObject.h:904 > + const ExecState* evalDebuggerCallFrame() const { return m_evalDebuggerCallFrame; } > + void setEvalDebuggerCallFrame(const ExecState* callFrame) { m_evalDebuggerCallFrame = callFrame; } I suggest renaming these as callFrameAtDebuggerEntry() and setCallFrameAtDebuggerEntry() instead. I like those suggestions. Created attachment 358636 [details]
[PATCH] For Landing
Comment on attachment 358636 [details] [PATCH] For Landing Clearing flags on attachment: 358636 Committed r239753: <https://trac.webkit.org/changeset/239753> |