Bug 72645 - Move JS recursion counter from V8Proxy to V8BindingPerIsolateData
Summary: Move JS recursion counter from V8Proxy to V8BindingPerIsolateData
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Klein
URL:
Keywords:
Depends on:
Blocks: 72063
  Show dependency treegraph
 
Reported: 2011-11-17 13:12 PST by Adam Klein
Modified: 2011-11-17 21:41 PST (History)
5 users (show)

See Also:


Attachments
Patch (11.62 KB, patch)
2011-11-17 13:15 PST, Adam Klein
no flags Details | Formatted Diff | Diff
Use RAII (11.99 KB, patch)
2011-11-17 14:05 PST, Adam Klein
no flags Details | Formatted Diff | Diff
Avoid test DB name collision (12.01 KB, patch)
2011-11-17 14:11 PST, Adam Klein
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Klein 2011-11-17 13:12:07 PST
Move JS recursion counter from V8Proxy to V8BindingPerIsolateData
Comment 1 Adam Klein 2011-11-17 13:15:47 PST
Created attachment 115664 [details]
Patch
Comment 2 Adam Barth 2011-11-17 13:28:29 PST
Comment on attachment 115664 [details]
Patch

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

> Source/WebCore/bindings/v8/V8Proxy.cpp:468
> -        m_recursion++;
> +        incrementRecursionLevel();
>          result = V8Proxy::instrumentedCallFunction(m_frame->page(), function, receiver, argc, args);
> -        m_recursion--;
> +        decrementRecursionLevel();

Can we use a RAII to twiddle this state to avoid bugs?
Comment 3 Adam Klein 2011-11-17 13:35:09 PST
Comment on attachment 115664 [details]
Patch

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

>> Source/WebCore/bindings/v8/V8Proxy.cpp:468
>> +        decrementRecursionLevel();
> 
> Can we use a RAII to twiddle this state to avoid bugs?

My only reason for not using RAII was because I couldn't think of a good name for the RAII class that wouldn't immediately confused with v8::HandleScope or v8::Context::Scope (those two are confusing enough for me already). Any suggestions?
Comment 4 Adam Barth 2011-11-17 13:38:42 PST
Comment on attachment 115664 [details]
Patch

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

>>> Source/WebCore/bindings/v8/V8Proxy.cpp:468
>>> -        m_recursion++;
>>> +        incrementRecursionLevel();
>>>          result = V8Proxy::instrumentedCallFunction(m_frame->page(), function, receiver, argc, args);
>>> -        m_recursion--;
>>> +        decrementRecursionLevel();
>> 
>> Can we use a RAII to twiddle this state to avoid bugs?
> 
> My only reason for not using RAII was because I couldn't think of a good name for the RAII class that wouldn't immediately confused with v8::HandleScope or v8::Context::Scope (those two are confusing enough for me already). Any suggestions?

V8RecursionScope ?
Comment 5 Adam Klein 2011-11-17 14:05:16 PST
Created attachment 115672 [details]
Use RAII
Comment 6 Adam Klein 2011-11-17 14:11:51 PST
Created attachment 115676 [details]
Avoid test DB name collision
Comment 7 Adam Barth 2011-11-17 14:12:33 PST
Comment on attachment 115676 [details]
Avoid test DB name collision

Thanks!
Comment 8 WebKit Review Bot 2011-11-17 21:41:50 PST
Comment on attachment 115676 [details]
Avoid test DB name collision

Clearing flags on attachment: 115676

Committed r100721: <http://trac.webkit.org/changeset/100721>
Comment 9 WebKit Review Bot 2011-11-17 21:41:55 PST
All reviewed patches have been landed.  Closing bug.