Bug 18402

Summary: REGRESSION: visited element handling is incorrect in nested join/toString calls
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: JavaScriptCoreAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: sam
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
test case
none
proposed fix
none
proposed fix
none
proposed fix mrowe: review+

Alexey Proskuryakov
Reported 2008-04-10 02:43:09 PDT
Array implementation used to have a single static HashSet for toString(), toLocalizedString() and join(). Now that these methods have been moved to separate functions, the HashSets are separate, too, which can be detected from JavaScript. AFAIK, ECMA-262 doesn't define error handling in this case, but I think that we'd like to maintain Firefox compatibility.
Attachments
test case (166 bytes, text/html)
2008-04-10 02:43 PDT, Alexey Proskuryakov 		no flags
Details
proposed fix (11.58 KB, patch)
2008-04-10 04:51 PDT, Alexey Proskuryakov 		no flags
DetailsFormatted DiffDiff
proposed fix (478.27 KB, patch)
2008-04-10 04:54 PDT, Alexey Proskuryakov 		no flags
DetailsFormatted DiffDiff
proposed fix (11.78 KB, patch)
2008-04-10 04:58 PDT, Alexey Proskuryakov 		mrowe: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2008-04-10 02:43:47 PDT
Created attachment 20450 [details] test case
Alexey Proskuryakov
Comment 2 2008-04-10 04:51:28 PDT
Created attachment 20453 [details] proposed fix
Alexey Proskuryakov
Comment 3 2008-04-10 04:54:19 PDT
Created attachment 20454 [details] proposed fix Forgot to save ChangeLog before making the patch.
Alexey Proskuryakov
Comment 4 2008-04-10 04:58:17 PDT
Created attachment 20455 [details] proposed fix Third time's the charm?
Geoffrey Garen
Comment 5 2008-04-10 15:07:32 PDT
+ OwnPtr<HashSet<JSObject*> > visitedElements; Since this variable is now visible outside of the array implementation file, I think it needs a more specific name. How about "arrayVisitedElements"? Maybe also add a comment that says "Global data shared by array prototype functions". Please land this patch with your testcase. r=me
Mark Rowe (bdash)
Comment 6 2008-04-10 17:51:01 PDT
Comment on attachment 20455 [details] proposed fix Setting r+ based on Geoff's r=me.
Alexey Proskuryakov
Comment 7 2008-04-11 00:39:52 PDT
Committed revision 31807.
Note You need to log in before you can comment on or make changes to this bug.