[webkit-reviews] review granted: [Bug 117862] REGRESSION(r151808): fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html fails : [Attachment 205212] patch #2

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jun 21 15:58:57 PDT 2013


Geoffrey Garen <ggaren at apple.com> has granted Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 117862: REGRESSION(r151808):
fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html fails
https://bugs.webkit.org/show_bug.cgi?id=117862

Attachment 205212: patch #2
https://bugs.webkit.org/attachment.cgi?id=205212&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=205212&action=review


r=me

> Source/JavaScriptCore/interpreter/Interpreter.cpp:-130
> -    const size_t requiredStack = 32 * KB;
> -    const size_t errorModeRequiredStack = 16 * KB;

I didn't realize that these values had just shrunk in r151808. Can we make them
a little bigger? How about 128 required / 64 errorModeRequired? Would that work
on Windows? 256 => 32 shrank by 8X, which seems like a risky move.

> Source/JavaScriptCore/interpreter/Interpreter.cpp:794
> +    const VMStackBounds nativeStack(vm, wtfThreadData().stack());

Since the class is named VMStackBounds, the variable should be named
vmStackBounds.

> Source/JavaScriptCore/interpreter/Interpreter.cpp:959
> +    const VMStackBounds nativeStack(vm, wtfThreadData().stack());

Ditto.

> Source/JavaScriptCore/interpreter/Interpreter.cpp:1036
> +    const VMStackBounds nativeStack(vm, wtfThreadData().stack());

Ditto.

> Source/JavaScriptCore/interpreter/Interpreter.cpp:1116
> +    const VMStackBounds nativeStack(vm, wtfThreadData().stack());

Ditto.

> Source/JavaScriptCore/interpreter/Interpreter.cpp:1218
> +    const VMStackBounds nativeStack(vm, wtfThreadData().stack());

Ditto.


More information about the webkit-reviews mailing list