Closed Bug 1399603 Opened 4 years ago Closed 4 years ago
[intersection-observer] Stop observing a target when intersection root is deleted
Chapter 3.3 of the IntersectionObserver API spec says that an IntersectionObserver should stop observing (and eventually being deleted) a target when the intersection root is deleted. Atm, we hold a strong reference to a custom root element and keeping it alive as long as the IntersectionObserver is. This needs to be changed to the same way we unlink targets from observers when they are deleted. In worst case we keep root elements (which tend to contain a larger node tree) alive unnecessarily.
Comment on attachment 8912009 [details] [diff] [review] Stop observing a target when intersection root is deleted Blake, this patch is building up on https://bugzilla.mozilla.org/show_bug.cgi?id=1316277 which you already reviewed. So I figured it a good idea having you reviewing this as well. I discussed this change with dholbert on IRC and we agreed on using the same hashtable for keeping links between elements and intersection observers, whether they are observing the element or using it as the root element for intersections. We already have everything in place to unlink those pointers properly so it made sense to share this functionality rather then copy and pasting most of the code to introduce yet another list of intersection observer pointers.
Attachment #8912009 - Flags: review?(mrbkap)
Attachment #8912009 - Flags: review?(mrbkap) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f70abea8b810 [intersection-observer] Stop observing a target when intersection root is deleted. r=mrbkap
Try run before landing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1eab0601b12e4ceba20c5958d1bf1defd51aae1e
This by all means is wrong. IntersectionObserver has attribute root, and if that magically becomes null after GC, we have exposed GC behavior to the web. I filed https://github.com/w3c/IntersectionObserver/issues/275
This was backed out from all branches in bug 1422092.
You need to log in before you can comment on or make changes to this bug.