Title: [213967] trunk
- Revision
- 213967
- Author
- [email protected]
- Date
- 2017-03-14 18:20:57 -0700 (Tue, 14 Mar 2017)
Log Message
RenderElements should unregister for viewport visibility callbacks when they are destroyed
https://bugs.webkit.org/show_bug.cgi?id=169521
<rdar://problem/30959545>
Reviewed by Simon Fraser.
Source/WebCore:
When registering a RenderElement for viewport visibility callbacks, we always need to make sure that it is unregistered
before it is destroyed. While we account for this in the destructor of RenderElement, we only unregister in the destructor
if we are already registered for visibility callbacks. In the call to RenderObject::willBeDestroyed(), we clear out rare
data, which holds RenderElement's viewport callback registration state, so upon entering the destructor of RenderElement,
we skip unregistration because RenderElement thinks that it is not registered.
We can mitigate this by unregistering the RenderElement earlier, in RenderElement::willBeDestroyed, prior to clearing out
the rare data. However, we'd ideally want to move the cleanup logic out of the destructor altogether and into willBeDestroyed
(see https://bugs.webkit.org/show_bug.cgi?id=169650).
Test: fast/media/video-element-in-details-collapse.html
* rendering/RenderElement.cpp:
(WebCore::RenderElement::willBeDestroyed):
LayoutTests:
Adds a new layout test covering this regression. See WebCore ChangeLog for more details.
* fast/media/video-element-in-details-collapse-expected.txt: Added.
* fast/media/video-element-in-details-collapse.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (213966 => 213967)
--- trunk/LayoutTests/ChangeLog 2017-03-15 00:53:17 UTC (rev 213966)
+++ trunk/LayoutTests/ChangeLog 2017-03-15 01:20:57 UTC (rev 213967)
@@ -1,3 +1,16 @@
+2017-03-14 Wenson Hsieh <[email protected]>
+
+ RenderElements should unregister for viewport visibility callbacks when they are destroyed
+ https://bugs.webkit.org/show_bug.cgi?id=169521
+ <rdar://problem/30959545>
+
+ Reviewed by Simon Fraser.
+
+ Adds a new layout test covering this regression. See WebCore ChangeLog for more details.
+
+ * fast/media/video-element-in-details-collapse-expected.txt: Added.
+ * fast/media/video-element-in-details-collapse.html: Added.
+
2017-03-14 Andy Estes <[email protected]>
Update ApplePaySession.html after r213949
Added: trunk/LayoutTests/fast/media/video-element-in-details-collapse-expected.txt (0 => 213967)
--- trunk/LayoutTests/fast/media/video-element-in-details-collapse-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/media/video-element-in-details-collapse-expected.txt 2017-03-15 01:20:57 UTC (rev 213967)
@@ -0,0 +1 @@
+hi
Added: trunk/LayoutTests/fast/media/video-element-in-details-collapse.html (0 => 213967)
--- trunk/LayoutTests/fast/media/video-element-in-details-collapse.html (rev 0)
+++ trunk/LayoutTests/fast/media/video-element-in-details-collapse.html 2017-03-15 01:20:57 UTC (rev 213967)
@@ -0,0 +1,14 @@
+<details id="details" open>hi<video width=480 height=320></details>
+<script>
+details.focus();
+details.open = false;
+window._onclick_ = () => {
+ details.open = true;
+}
+if (window.testRunner && window.eventSender) {
+ testRunner.dumpAsText();
+ eventSender.mouseMoveTo(innerWidth / 2, innerHeight / 2);
+ eventSender.mouseDown();
+ eventSender.mouseUp();
+}
+</script>
Modified: trunk/Source/WebCore/ChangeLog (213966 => 213967)
--- trunk/Source/WebCore/ChangeLog 2017-03-15 00:53:17 UTC (rev 213966)
+++ trunk/Source/WebCore/ChangeLog 2017-03-15 01:20:57 UTC (rev 213967)
@@ -1,3 +1,26 @@
+2017-03-14 Wenson Hsieh <[email protected]>
+
+ RenderElements should unregister for viewport visibility callbacks when they are destroyed
+ https://bugs.webkit.org/show_bug.cgi?id=169521
+ <rdar://problem/30959545>
+
+ Reviewed by Simon Fraser.
+
+ When registering a RenderElement for viewport visibility callbacks, we always need to make sure that it is unregistered
+ before it is destroyed. While we account for this in the destructor of RenderElement, we only unregister in the destructor
+ if we are already registered for visibility callbacks. In the call to RenderObject::willBeDestroyed(), we clear out rare
+ data, which holds RenderElement's viewport callback registration state, so upon entering the destructor of RenderElement,
+ we skip unregistration because RenderElement thinks that it is not registered.
+
+ We can mitigate this by unregistering the RenderElement earlier, in RenderElement::willBeDestroyed, prior to clearing out
+ the rare data. However, we'd ideally want to move the cleanup logic out of the destructor altogether and into willBeDestroyed
+ (see https://bugs.webkit.org/show_bug.cgi?id=169650).
+
+ Test: fast/media/video-element-in-details-collapse.html
+
+ * rendering/RenderElement.cpp:
+ (WebCore::RenderElement::willBeDestroyed):
+
2017-03-14 Dean Jackson <[email protected]>
Rename LayerTypeWebGLLayer and use it for both WebGL and WebGPU
Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (213966 => 213967)
--- trunk/Source/WebCore/rendering/RenderElement.cpp 2017-03-15 00:53:17 UTC (rev 213966)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp 2017-03-15 01:20:57 UTC (rev 213967)
@@ -1132,6 +1132,9 @@
destroyLeftoverChildren();
+ if (isRegisteredForVisibleInViewportCallback())
+ unregisterForVisibleInViewportCallback();
+
if (hasCounterNodeMap())
RenderCounter::destroyCounterNodes(*this);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
