Bug 117862

Summary: REGRESSION(r151808): fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html fails
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren, mark.lam
Priority: P2 Keywords: LayoutTestFailure, Regression
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 117801    
Bug Blocks:    
Attachments:
Description Flags
the fix
none
patch #2 ggaren: review+

Ryosuke Niwa
Reported 2013-06-20 20:58:38 PDT
fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html has been failing since http://trac.webkit.org/changeset/151808
Attachments
the fix (19.15 KB, patch)
2013-06-21 13:22 PDT, Mark Lam 		no flags
DetailsFormatted DiffDiff
patch #2 (20.08 KB, patch)
2013-06-21 13:32 PDT, Mark Lam 		ggaren: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2013-06-20 21:01:08 PDT
Committed r151823: <http://trac.webkit.org/changeset/151823>
Mark Lam
Comment 2 2013-06-21 13:22:48 PDT
Created attachment 205210 [details] the fix
Mark Lam
Comment 3 2013-06-21 13:32:07 PDT
Created attachment 205212 [details] patch #2
Geoffrey Garen
Comment 4 2013-06-21 15:57:37 PDT
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
Comment 5 2013-06-21 16:15:29 PDT
(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
Comment 6 2013-06-21 16:59:59 PDT
Landed in r151869: <http://trac.webkit.org/changeset/151869>.
Note You need to log in before you can comment on or make changes to this bug.