Title: [237929] trunk
- Revision
- 237929
- Author
- aj...@chromium.org
- Date
- 2018-11-07 10:30:30 -0800 (Wed, 07 Nov 2018)
Log Message
IntersectionObserverEntry doesn't keep JS wrappers of rects alive
https://bugs.webkit.org/show_bug.cgi?id=191330
Reviewed by Chris Dumez.
Source/WebCore:
Retain wrappers of each rect in an IntersectionObserverEntry as long as the entry's wrapper
is alive, by adding these wrappers as opaque roots.
Test: intersection-observer/intersection-observer-entry-keeps-js-wrappers-of-rects-alive.html
* bindings/js/JSIntersectionObserverEntryCustom.cpp:
(WebCore::JSIntersectionObserverEntry::visitAdditionalChildren):
* dom/DOMRectReadOnly.idl:
* page/IntersectionObserverEntry.h:
(WebCore::IntersectionObserverEntry::rootBounds const): Make this return a raw pointer instead of a RefPtr so that it
can be called in JSIntersectionObserverEntry::visitAdditionalChildren, which can be called from non-main threads.
(WebCore::IntersectionObserverEntry::boundingClientRect const): Ditto.
(WebCore::IntersectionObserverEntry::intersectionRect const): Ditto.
LayoutTests:
Add test coverage.
* intersection-observer/intersection-observer-entry-keeps-js-wrappers-of-rects-alive-expected.txt: Added.
* intersection-observer/intersection-observer-entry-keeps-js-wrappers-of-rects-alive.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (237928 => 237929)
--- trunk/LayoutTests/ChangeLog 2018-11-07 17:30:39 UTC (rev 237928)
+++ trunk/LayoutTests/ChangeLog 2018-11-07 18:30:30 UTC (rev 237929)
@@ -1,3 +1,15 @@
+2018-11-07 Ali Juma <aj...@chromium.org>
+
+ IntersectionObserverEntry doesn't keep JS wrappers of rects alive
+ https://bugs.webkit.org/show_bug.cgi?id=191330
+
+ Reviewed by Chris Dumez.
+
+ Add test coverage.
+
+ * intersection-observer/intersection-observer-entry-keeps-js-wrappers-of-rects-alive-expected.txt: Added.
+ * intersection-observer/intersection-observer-entry-keeps-js-wrappers-of-rects-alive.html: Added.
+
2018-11-07 Ryan Haddad <ryanhad...@apple.com>
Unreviewed, update test expectations for fast/events/pointer.
Added: trunk/LayoutTests/intersection-observer/intersection-observer-entry-keeps-js-wrappers-of-rects-alive-expected.txt (0 => 237929)
--- trunk/LayoutTests/intersection-observer/intersection-observer-entry-keeps-js-wrappers-of-rects-alive-expected.txt (rev 0)
+++ trunk/LayoutTests/intersection-observer/intersection-observer-entry-keeps-js-wrappers-of-rects-alive-expected.txt 2018-11-07 18:30:30 UTC (rev 237929)
@@ -0,0 +1,8 @@
+This tests that JS wrappers of rects in an IntersectionObserverEntry do not get collected.
+
+PASS
+PASS
+PASS
+PASS
+PASS
+
Added: trunk/LayoutTests/intersection-observer/intersection-observer-entry-keeps-js-wrappers-of-rects-alive.html (0 => 237929)
--- trunk/LayoutTests/intersection-observer/intersection-observer-entry-keeps-js-wrappers-of-rects-alive.html (rev 0)
+++ trunk/LayoutTests/intersection-observer/intersection-observer-entry-keeps-js-wrappers-of-rects-alive.html 2018-11-07 18:30:30 UTC (rev 237929)
@@ -0,0 +1,67 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests that JS wrappers of rects in an IntersectionObserverEntry do not get collected.</p>
+<pre id="log"></pre>
+<script src=""
+<script>
+
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+const targetCount = 5;
+const iterationCount = 5;
+
+for (let i = 0; i < targetCount; ++i) {
+ let target = document.createElement('div');
+ document.body.appendChild(target);
+}
+
+async function observe() {
+ return new Promise((resolve) => {
+ const observer = new IntersectionObserver(entries => {
+ for (const entry of entries) {
+ entry.rootBounds.myState = 'live';
+ entry.boundingClientRect.myState = 'live';
+ entry.intersectionRect.myState = 'live';
+ }
+ resolve(entries);
+ observer.disconnect();
+ });
+ document.querySelectorAll('div').forEach(target => { observer.observe(target); });
+ });
+}
+
+function check(entries) {
+ let deadCount = 0;
+ for (const entry of entries) {
+ if (entry.rootBounds.myState != 'live')
+ deadCount++;
+ if (entry.boundingClientRect.myState != 'live')
+ deadCount++;
+ if (entry.intersectionRect.myState != 'live')
+ deadCount++;
+ }
+ document.getElementById('log').textContent += (deadCount ? `FAIL - ${deadCount} rects lost JS wrappers` : 'PASS') + '\n';
+}
+
+async function runAll() {
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ for (let j = 0; j < iterationCount; ++j) {
+ const entries = await observe();
+ await Promise.resolve();
+ gc();
+ check(entries);
+ }
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+}
+
+runAll();
+
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (237928 => 237929)
--- trunk/Source/WebCore/ChangeLog 2018-11-07 17:30:39 UTC (rev 237928)
+++ trunk/Source/WebCore/ChangeLog 2018-11-07 18:30:30 UTC (rev 237929)
@@ -1,3 +1,24 @@
+2018-11-07 Ali Juma <aj...@chromium.org>
+
+ IntersectionObserverEntry doesn't keep JS wrappers of rects alive
+ https://bugs.webkit.org/show_bug.cgi?id=191330
+
+ Reviewed by Chris Dumez.
+
+ Retain wrappers of each rect in an IntersectionObserverEntry as long as the entry's wrapper
+ is alive, by adding these wrappers as opaque roots.
+
+ Test: intersection-observer/intersection-observer-entry-keeps-js-wrappers-of-rects-alive.html
+
+ * bindings/js/JSIntersectionObserverEntryCustom.cpp:
+ (WebCore::JSIntersectionObserverEntry::visitAdditionalChildren):
+ * dom/DOMRectReadOnly.idl:
+ * page/IntersectionObserverEntry.h:
+ (WebCore::IntersectionObserverEntry::rootBounds const): Make this return a raw pointer instead of a RefPtr so that it
+ can be called in JSIntersectionObserverEntry::visitAdditionalChildren, which can be called from non-main threads.
+ (WebCore::IntersectionObserverEntry::boundingClientRect const): Ditto.
+ (WebCore::IntersectionObserverEntry::intersectionRect const): Ditto.
+
2018-11-07 Simon Fraser <simon.fra...@apple.com>
TileController::tileSize() should not have side effects
Modified: trunk/Source/WebCore/bindings/js/JSIntersectionObserverEntryCustom.cpp (237928 => 237929)
--- trunk/Source/WebCore/bindings/js/JSIntersectionObserverEntryCustom.cpp 2018-11-07 17:30:39 UTC (rev 237928)
+++ trunk/Source/WebCore/bindings/js/JSIntersectionObserverEntryCustom.cpp 2018-11-07 18:30:30 UTC (rev 237929)
@@ -33,6 +33,9 @@
void JSIntersectionObserverEntry::visitAdditionalChildren(JSC::SlotVisitor& visitor)
{
visitor.addOpaqueRoot(root(wrapped().target()));
+ visitor.addOpaqueRoot(wrapped().boundingClientRect());
+ visitor.addOpaqueRoot(wrapped().intersectionRect());
+ visitor.addOpaqueRoot(wrapped().rootBounds());
}
}
Modified: trunk/Source/WebCore/dom/DOMRectReadOnly.idl (237928 => 237929)
--- trunk/Source/WebCore/dom/DOMRectReadOnly.idl 2018-11-07 17:30:39 UTC (rev 237928)
+++ trunk/Source/WebCore/dom/DOMRectReadOnly.idl 2018-11-07 18:30:30 UTC (rev 237929)
@@ -29,6 +29,7 @@
Constructor(optional unrestricted double x = 0, optional unrestricted double y = 0,
optional unrestricted double width = 0, optional unrestricted double height = 0),
Exposed=(Window,Worker),
+ GenerateIsReachable=Impl,
ImplementationLacksVTable
]
interface DOMRectReadOnly {
Modified: trunk/Source/WebCore/page/IntersectionObserverEntry.h (237928 => 237929)
--- trunk/Source/WebCore/page/IntersectionObserverEntry.h 2018-11-07 17:30:39 UTC (rev 237928)
+++ trunk/Source/WebCore/page/IntersectionObserverEntry.h 2018-11-07 18:30:30 UTC (rev 237929)
@@ -57,9 +57,9 @@
}
double time() const { return m_time; }
- RefPtr<DOMRectReadOnly> rootBounds() const { return m_rootBounds; }
- RefPtr<DOMRectReadOnly> boundingClientRect() const { return m_boundingClientRect; }
- RefPtr<DOMRectReadOnly> intersectionRect() const { return m_intersectionRect; }
+ DOMRectReadOnly* rootBounds() const { return m_rootBounds.get(); }
+ DOMRectReadOnly* boundingClientRect() const { return m_boundingClientRect.get(); }
+ DOMRectReadOnly* intersectionRect() const { return m_intersectionRect.get(); }
Element* target() const { return m_target.get(); }
bool isIntersecting() const { return m_isIntersecting; }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
