undefined/null this values should throw TypeErrors, not convert to the global object.
Created attachment 101129 [details] Preliminary patch
Created attachment 101202 [details] The patch
Comment on attachment 101202 [details] The patch Attachment 101202 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9112479 New failing tests: fast/js/object-prototype-properties.html fast/js/object-prototype-toLocaleString.html
Created attachment 101232 [details] Added missing updated results, fixed bug in harness.
Created attachment 101238 [details] Ooops, revert some debug code from last patch
Fixed in r91224
Filed crbug.com/89673 to track Chromium test failures.
Comment on attachment 101238 [details] Ooops, revert some debug code from last patch View in context: https://bugs.webkit.org/attachment.cgi?id=101238&action=review A couple of drive-by comments. > LayoutTests/fast/js/script-tests/object-prototype-properties.js:6 > +shouldThrow("toString()"); According the the ES5.1 spec this should not throw an exception and return "[object Undefined]": Section 15.2.4.2: 1. If the this value is undefined, return "[object Undefined]". > LayoutTests/fast/js/script-tests/object-prototype-properties.js:9 > +shouldThrow("hasProperty('hasProperty')"); I guess this should be hasOwnProperty?
(In reply to comment #8) > > +shouldThrow("toString()"); > > According the the ES5.1 spec this should not throw an exception and return "[object Undefined]": Good catch. I missed this coming out of draft, time to update my copy of the spec. :-) > > +shouldThrow("hasProperty('hasProperty')"); > > I guess this should be hasOwnProperty? Gah, yes it should. Thanks for the comments, will fix!
Created attachment 101409 [details] Fix ES5.1 correctness issues
Comment on attachment 101409 [details] Fix ES5.1 correctness issues View in context: https://bugs.webkit.org/attachment.cgi?id=101409&action=review > Source/JavaScriptCore/runtime/ObjectPrototype.cpp:199 > + if (thisValue.isUndefinedOrNull()) > + return jsNontrivialString(exec, thisValue.isUndefined() ? "[object Undefined]" : "[object Null]"); > return JSValue::encode(jsMakeNontrivialString(exec, "[object ", thisValue.toObject(exec)->className(), "]")); I worry a bit about the performance implications of allocating a string every time.
(In reply to comment #11) > I worry a bit about the performance implications of allocating a string every time. That's a fair concern, but I think we should probably stick with the simple solution for now & optimize if necessary. We don't cache the results of Object.prototype.toString for other classes of object, which I imagine is something we could do. Undefined/null are probably the least likely types to be converted (since behavior here is not going to be uniform across browsers right now), so any caching that is specific to undefined/null values is likely to be suboptimal.
ES5.1 corrections per Mads Ager's comments landed in r91344.