Bug 18402 - REGRESSION: visited element handling is incorrect in nested join/toString calls
Summary: REGRESSION: visited element handling is incorrect in nested join/toString calls
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P1 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-04-10 02:43 PDT by Alexey Proskuryakov
Modified: 2008-04-11 00:39 PDT (History)
1 user (show)

See Also:


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 Details | Formatted Diff | Diff
proposed fix (478.27 KB, patch)
2008-04-10 04:54 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
proposed fix (11.78 KB, patch)
2008-04-10 04:58 PDT, Alexey Proskuryakov
mrowe: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.