Bug 189128 – IntersectionObserver leaks documents

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Simon Fraser (smfr)]

Testing with changes from bug 186214, I see that the following IntersectionObserver tests leak their documents:

imported/w3c/web-platform-tests/intersection-observer/client-rect.html
imported/w3c/web-platform-tests/intersection-observer/cross-origin-iframe.html
imported/w3c/web-platform-tests/intersection-observer/disconnect.html
imported/w3c/web-platform-tests/intersection-observer/isIntersecting-change-events.html
imported/w3c/web-platform-tests/intersection-observer/observer-attributes.html
imported/w3c/web-platform-tests/intersection-observer/shadow-content.html
imported/w3c/web-platform-tests/intersection-observer/text-target.html
imported/w3c/web-platform-tests/intersection-observer/timestamp.html
imported/w3c/web-platform-tests/intersection-observer/unclipped-root.html
imported/w3c/web-platform-tests/intersection-observer/zero-area-element-hidden.html
imported/w3c/web-platform-tests/intersection-observer/zero-area-element-visible.html

In manual testing, I see that the IntersectionObserver never goes away, which retains the callback, which retains the Document.

[Ali Juma]

Started looking into this. Using the approach from bug 186873 (making the callback a WeakCallback and using [CustomIsReachable] to add logic to keep the callback alive as long as we need it) seems to be the way to go here.

[Ali Juma]

Created attachment 348969 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 348969 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Currently,`Documents own IntersectionObservers while IntersectionObservers own callbacks

Errant backtick.

[Ali Juma]

Created attachment 349020 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 349020 [details]
Patch for landing

Clearing flags on attachment: 349020

Committed r235736: <https://trac.webkit.org/changeset/235736>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/44180008>

[Truitt Savell]

Looks like the new test added in: https://trac.webkit.org/changeset/235736/webkit

Is a flakey timeout: intersection-observer/no-document-leak.html

History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=intersection-observer%2Fno-document-leak.html

Diff:
https://build.webkit.org/results/Apple%20Sierra%20Release%20WK1%20(Tests)/r236149%20(12846)/intersection-observer/no-document-leak-pretty-diff.html

[Ali Juma]

(In reply to Truitt Savell from comment #8)
> Looks like the new test added in:
> https://trac.webkit.org/changeset/235736/webkit
> 
> Is a flakey timeout: intersection-observer/no-document-leak.html
> 
> History:
> https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=intersection-observer%2Fno-document-leak.html
> 
> Diff:
> https://build.webkit.org/results/Apple%20Sierra%20Release%20WK1%20(Tests)/
> r236149%20(12846)/intersection-observer/no-document-leak-pretty-diff.html

Tracking this in https://bugs.webkit.org/show_bug.cgi?id=189731.