Bug 72063 - [MutationObservers] V8 bindings don't properly wrap all calls into JS
Summary: [MutationObservers] V8 bindings don't properly wrap all calls into JS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Klein
URL:
Keywords:
Depends on: 72181 72422 72645
Blocks: 68729
  Show dependency tree / graph
 
Reported: 2011-11-10 14:52 PST by Adam Klein
Modified: 2011-11-30 04:57 PST (History)
7 users (show)

See Also:

Attachments
Patch (7.88 KB, patch)
2011-11-29 15:42 PST, Rafael Weinstein 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Klein 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).
Comment 1 Adam Klein 2011-11-11 14:46:56 PST 
Per :dslomov, m_recursion should live in V8BindingPerIsolateData.
Comment 2 Rafael Weinstein 2011-11-29 15:42:02 PST 
Created attachment 117062 [details]
Patch
Comment 3 Rafael Weinstein 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().
Comment 4 WebKit Review Bot 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>
Comment 5 WebKit Review Bot 2011-11-30 04:57:32 PST 
All reviewed patches have been landed.  Closing bug.