Bug 117862 – REGRESSION(r151808): fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html fails

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Ryosuke Niwa]

fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html has been failing since http://trac.webkit.org/changeset/151808

[Ryosuke Niwa]

Committed r151823: <http://trac.webkit.org/changeset/151823>

[Mark Lam]

Created attachment 205210 [details]
the fix

[Mark Lam]

Created attachment 205212 [details]
patch #2

[Geoffrey Garen]

Comment on attachment 205212 [details]
patch #2

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.

[Mark Lam]

(In reply to comment #4)
> 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.

128/64 checks out fine for run-javascript-tests on Windows, where the256/64 values failed previously.  I will adopt these values and rerun the full tests on Mac.

[Mark Lam]

Landed in r151869: <http://trac.webkit.org/changeset/151869>.