[webkit-reviews] review denied: [Bug 59571] [JSC] Nodes in shadow DOM should not be GCed while their hosts are alive : [Attachment 91208] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has denied Dominic Cooney
<dominicc at chromium.org>'s request for review:
Bug 59571: [JSC] Nodes in shadow DOM should not be GCed while their hosts are
alive
https://bugs.webkit.org/show_bug.cgi?id=59571

Attachment 91208: Patch
https://bugs.webkit.org/attachment.cgi?id=91208&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=91208&action=review

It looks like your patch doesn't include any new test results. If we're going
to make this change, I think we need a test with results that vary depending on
whether this change is in the tree.

> LayoutTests/fast/dom/shadow/gc-shadow.html:19
> +    // Some ports (unfortunately) take GCController.collect as advisory.
> +    for (var i = 0; i < 10000; i++)
> +	   var s = new String(i);

Which ports take GCController.collect as advisory?

If there is a port like that, it's broken. I don't think we should make this
test 2-3X slower on all ports just because some ports are broken. If
window.GCController is defined, we should call collect() and return early.