[webkit-reviews] review granted: [Bug 185996] Error instances should not strongly hold onto StackFrames : [Attachment 341329] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri May 25 15:10:26 PDT 2018
Mark Lam <mark.lam at apple.com> has granted Keith Miller
<keith_miller at apple.com>'s request for review:
Bug 185996: Error instances should not strongly hold onto StackFrames
https://bugs.webkit.org/show_bug.cgi?id=185996
Attachment 341329: Patch
https://bugs.webkit.org/attachment.cgi?id=341329&action=review
--- Comment #3 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 341329
--> https://bugs.webkit.org/attachment.cgi?id=341329
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=341329&action=review
r=me with fixes.
> Source/JavaScriptCore/runtime/ErrorInstance.cpp:216
> + for (const auto& frame : *m_stackTrace.get()) {
> + if (!frame.isMarked()) {
> + computeErrorInfo(vm);
> + return;
> + }
> + }
Technically, one could infer backwards using svn blame to figure out why you're
doing this here, but I think this action is sufficiently detached from the
motivation behind it that it's worth adding a comment here to explain why
you're doing this.
> Source/JavaScriptCore/tools/JSDollarVM.cpp:1693
> + {
> + return
JSValue::encode(jsNumber(exec->vm().heap.globalObjectCount()));
> + }
Please fix indentation.
> Source/JavaScriptCore/tools/JSDollarVM.cpp:1851
> + addFunction(vm, "globalObjectCount", functionGlobalObjectCount, 0);
This functionality is not documented in the ChangeLog. Please add a short
ChangeLog comment for this.
More information about the webkit-reviews
mailing list