[webkit-reviews] review granted: [Bug 169904] Add support for Error.stackTraceLimit. : [Attachment 305064] proposed patch: rebased.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Mar 22 12:19:30 PDT 2017

Saam Barati <sbarati at apple.com> has granted Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 169904: Add support for Error.stackTraceLimit.

Attachment 305064: proposed patch: rebased.


--- Comment #13 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 305064
  --> https://bugs.webkit.org/attachment.cgi?id=305064
proposed patch: rebased.

View in context: https://bugs.webkit.org/attachment.cgi?id=305064&action=review

Thanks for you response.
r=me with a comment.

>>> Source/JavaScriptCore/runtime/ErrorConstructor.cpp:50
>>> +	 unsigned defaultStackTraceLimit =
>>> +	 globalObject()->setErrorStackTraceLimit(defaultStackTraceLimit);
>>> +	 putDirectWithoutTransition(vm, vm.propertyNames->stackTraceLimit,
jsNumber(defaultStackTraceLimit), None);
>> Why not just make this a getter/setter? What does V8 do? If we were to
actually propose this as spec, I think a getter/setter is cleaner.
> Because then Object.getOwnPropertyDescriptor(Error, "stackTraceLimit") will
return { value; undefined, ..., get: function ...., set: function ... }. 
Chrome's impl returns { value: 10, ... } with no get and set.

Style nit:
It's weird that this code lives here. What I'd do is this:
1. Make global object set its own value via the option during JSGlobalObject()
2. Make this grab the value from the global object, and use that in putDirect,
so something like: putDirect(..., globalObject->errorLimit() ?
globalObject.errorLimit.get() : Options::defaultErrorStackTraceLimit(), ...)

More information about the webkit-reviews mailing list