[webkit-reviews] review granted: [Bug 190235] IntersectionObserver doesn't keep target's JS wrapper alive : [Attachment 353889] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Ali Juma <ajuma at chromium.org>'s
request for review:
Bug 190235: IntersectionObserver doesn't keep target's JS wrapper alive
https://bugs.webkit.org/show_bug.cgi?id=190235

Attachment 353889: Patch

https://bugs.webkit.org/attachment.cgi?id=353889&action=review




--- Comment #13 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 353889
  --> https://bugs.webkit.org/attachment.cgi?id=353889
Patch

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

r=me with one additional test.

> Source/WebCore/page/IntersectionObserverEntry.h:62
>      RefPtr<DOMRectReadOnly> rootBounds() const { return m_rootBounds; }
>      RefPtr<DOMRectReadOnly> boundingClientRect() const { return
m_boundingClientRect; }
>      RefPtr<DOMRectReadOnly> intersectionRect() const { return
m_intersectionRect; }

DOMRectReadOnly have the same issue.
We need to visit these objects' wrappers as well.
Otherwise, properties added on DOMRectReadOnly would be collected.
But that could be addressed in a separate patch.

> LayoutTests/intersection-observer/root-element-deleted.html:23
>	   requestAnimationFrame(t.step_func_done(function() {
> +	       observer.takeRecords();

We should add another test which doesn't rely on takeRecords to make sure
deliver() path results in the clearing of GCReachableRef as well.