[webkit-reviews] review canceled: [Bug 79131] Use V8 completion callback API to assert that V8RecursionScope is on the stack whenever invoking script : [Attachment 133957] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Mar 27 13:59:40 PDT 2012


Adam Barth <abarth at webkit.org> has canceled Rafael Weinstein
<rafaelw at chromium.org>'s request for review:
Bug 79131: Use V8 completion callback API to assert that V8RecursionScope is on
the stack whenever invoking script
https://bugs.webkit.org/show_bug.cgi?id=79131

Attachment 133957: Patch
https://bugs.webkit.org/attachment.cgi?id=133957&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=133957&action=review


> Source/WebCore/bindings/v8/DateExtension.cpp:93
> +    V8RecursionScope::BypassFinalizationPhase scope;

What does BypassFinalizationPhase mean?  Maybe V8RecursionScope::InternalScript
(to borrow a name from Comment #13 )?

> Source/WebKit/chromium/public/platform/WebScriptScope.h:36
> +namespace WebScriptScope {

This shouldn't be in platform.	It should be up one level in the directory
structure.  Platform doesn't have any notion of script.

> Source/WebKit/chromium/public/platform/WebScriptScope.h:38
> +class BypassFinalizationPhase {

I'm worried that clients of this API won't understand how to use it correctly. 
Why does the embedder know about "finalization phases" or "internal scripts"? 
Is this required whenever you call into V8, or only when you call via WebFrame?
 Perhaps and enum parameter on executeScriptAndReturnValue and friends would be
better?  It depends if you can name things in a way that makes sense to the
embedder.


More information about the webkit-reviews mailing list