Bug 191771

Summary: All users of ArrayBuffer should agree on the same max size
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, fpizlo, guijemont, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
the patch
none
the patch
mark.lam: review+
the patch none

Filip Pizlo
Reported 2018-11-16 14:15:46 PST
Patch forthcoming.
Attachments
the patch (9.61 KB, patch)
2018-11-16 14:17 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (9.76 KB, patch)
2018-11-16 14:22 PST, Filip Pizlo 		mark.lam: review+
DetailsFormatted DiffDiff
the patch (11.31 KB, patch)
2018-11-16 15:31 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2018-11-16 14:17:56 PST
Created attachment 355119 [details] the patch
Filip Pizlo
Comment 2 2018-11-16 14:22:12 PST
Created attachment 355121 [details] the patch Fixed some tests.
Mark Lam
Comment 3 2018-11-16 14:33:09 PST
<rdar://problem/46124570>
Mark Lam
Comment 4 2018-11-16 14:34:29 PST
Comment on attachment 355121 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=355121&action=review r=me > Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119 > + RELEASE_ASSERT(m_sizeInBytes <= MAX_ARRAY_BUFFER_SIZE); nit: This can be a debug ASSERT because we already have a size check above to ensure this.
Filip Pizlo
Comment 5 2018-11-16 15:31:51 PST
Created attachment 355143 [details] the patch It turns out that we have to allow large maximum and then fail when you try to grow. Otherwise we fail some spec test.
Filip Pizlo
Comment 6 2018-11-16 16:43:03 PST
Landed in https://trac.webkit.org/changeset/238326/webkit
Guillaume Emont
Comment 7 2018-11-19 06:03:49 PST
Skipped the new tests in https://trac.webkit.org/r238373 for memory limited devices, as they use 800M+ memory (which I assume is expected for these tests).
Note You need to log in before you can comment on or make changes to this bug.