[webkit-reviews] review requested: [Bug 33053] JSON.stringify and JSON.parse implementations needlessly process properties in the prototype chain : [Attachment 46130] Revised patch (adds tests)

[bugzilla-daemon at webkit.org]

Kent Hansen <kent.hansen at nokia.com> has asked  for review:
Bug 33053: JSON.stringify and JSON.parse implementations needlessly process
properties in the prototype chain
https://bugs.webkit.org/show_bug.cgi?id=33053

Attachment 46130: Revised patch (adds tests)
https://bugs.webkit.org/attachment.cgi?id=46130&action=review

------- Additional Comments from Kent Hansen <kent.hansen at nokia.com>
I haven't added the checks for overridesGetPropertyNames() as I mentioned in
comment #3, because that would still mean the behavior would be broken for
classes that do override it (and there are quite a few). If a class
reimplements getPropertyNames(), it should reimplement getOwnPropertyNames()
too (i.e. the OverridesGetPropertyNames should imply that both of the property
name functions are reimplemented). I can't think of a use case where it's
meaningful to only reimplement getPropertyNames(), at least.

The difference between the result created by json2.js (subject to the for-in
issue) and the native implementation has been marked as FAIL.
I'm not sure how to best proceed:
1) Acknowledge that the for-in behavior is a bug in JSC, and make it a blocker
for this bug; or
2) Try to convince Crockford that json2.js should be updated to work around
this behavior (i.e. detect for-in behavior for injected properties and fall
back to a slower codepath); or
3) Accept that the native JSON and json2.js results will differ in this case.