Bug 72063

Summary: [MutationObservers] V8 bindings don't properly wrap all calls into JS
Product: WebKit Reporter: Adam Klein <adamk>
Component: DOMAssignee: Adam Klein <adamk>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, antonm, fishd, japhet, pfeldman, rafaelw, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 72181, 72422, 72645    
Bug Blocks: 68729    
Attachments:
Description Flags
Patch none

Adam Klein
Reported 2011-11-10 14:52:03 PST
V8Proxy::didLeaveScriptContext claims to run whenever the outermost script invocation exits, but it's broken in two ways: - Not all script calls go through V8Proxy::callFunction (e.g., they call the v8::Function::Call, or V8Proxy::instrumentedCallFunction). - V8Proxy::m_recursion (used to count the number of nested invocations) is per-frame, which is incorrect. It should probably just be a static. So we need a replacement for m_recursion and didLeaveScriptContext. Where these live is up in the air (abarth suggests that V8Proxy is not the right place to live).
Attachments
Patch (7.88 KB, patch)
2011-11-29 15:42 PST, Rafael Weinstein 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Adam Klein
Comment 1 2011-11-11 14:46:56 PST
Per :dslomov, m_recursion should live in V8BindingPerIsolateData.
Rafael Weinstein
Comment 2 2011-11-29 15:42:02 PST
Created attachment 117062 [details] Patch
Rafael Weinstein
Comment 3 2011-11-29 15:49:14 PST
Note that this patch has the effect that old callers of callFunctionWithoutFrame now also check the max recursion depth, and invoke didLeaveScriptContext. Existing callers of instrumentedCallFunction get both of these PLUS (at no added cost) the V8GCController::checkMemoryUsage().
WebKit Review Bot
Comment 4 2011-11-30 04:57:27 PST
Comment on attachment 117062 [details] Patch Clearing flags on attachment: 117062 Committed r101490: <http://trac.webkit.org/changeset/101490>
WebKit Review Bot
Comment 5 2011-11-30 04:57:32 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.