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+

Description Alexey Proskuryakov 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.
Comment 1 Alexey Proskuryakov 2008-04-10 02:43:47 PDT 
Created attachment 20450 [details]
test case
Comment 2 Alexey Proskuryakov 2008-04-10 04:51:28 PDT 
Created attachment 20453 [details]
proposed fix
Comment 3 Alexey Proskuryakov 2008-04-10 04:54:19 PDT 
Created attachment 20454 [details]
proposed fix

Forgot to save ChangeLog before making the patch.
Comment 4 Alexey Proskuryakov 2008-04-10 04:58:17 PDT 
Created attachment 20455 [details]
proposed fix

Third time's the charm?
Comment 5 Geoffrey Garen 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
Comment 6 Mark Rowe (bdash) 2008-04-10 17:51:01 PDT 
Comment on attachment 20455 [details]
proposed fix

Setting r+ based on Geoff's r=me.
Comment 7 Alexey Proskuryakov 2008-04-11 00:39:52 PDT 
Committed revision 31807.