- Revision
- 127301
- Author
- [email protected]
- Date
- 2012-08-31 12:38:40 -0700 (Fri, 31 Aug 2012)
Log Message
(Regression: r126774): Fix crash when scrolling after removing inline sticky element
https://bugs.webkit.org/show_bug.cgi?id=95584
Reviewed by Dave Hyatt.
Source/WebCore:
Move fixed/sticky registration and unregistration with the FrameView from
RenderBox and RenderInline into RenderBoxModelObject, which also
fixes the issue that inlines didn't unregister themselves on destruction.
Test: fast/css/sticky/remove-inline-sticky-crash.html
* rendering/RenderBox.cpp:
(WebCore::RenderBox::willBeDestroyed): Code moved to RenderBoxModelObject. No need
to null-check style.
(WebCore::RenderBox::styleWillChange): Code moved to RenderBoxModelObject.
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::willBeDestroyed): Remove fixed objects. Check
isPositioned() to avoid this work for most non-positioned renderers.
(WebCore::RenderBoxModelObject::styleWillChange): Register and unregister fixed
and sticky objects with the FrameView.
* rendering/RenderInline.cpp: styleWillChange() is no longer needed.
* rendering/RenderInline.h: Ditto.
LayoutTests:
Testcase removes an inline sticky, then scrolls.
* fast/css/sticky/remove-inline-sticky-crash-expected.txt: Added.
* fast/css/sticky/remove-inline-sticky-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (127300 => 127301)
--- trunk/LayoutTests/ChangeLog 2012-08-31 19:11:32 UTC (rev 127300)
+++ trunk/LayoutTests/ChangeLog 2012-08-31 19:38:40 UTC (rev 127301)
@@ -1,3 +1,15 @@
+2012-08-31 Simon Fraser <[email protected]>
+
+ (Regression: r126774): Fix crash when scrolling after removing inline sticky element
+ https://bugs.webkit.org/show_bug.cgi?id=95584
+
+ Reviewed by Dave Hyatt.
+
+ Testcase removes an inline sticky, then scrolls.
+
+ * fast/css/sticky/remove-inline-sticky-crash-expected.txt: Added.
+ * fast/css/sticky/remove-inline-sticky-crash.html: Added.
+
2012-08-31 Jon Lee <[email protected]>
[Tests] Add basic tests to http/tests/notifications
Added: trunk/LayoutTests/fast/css/sticky/remove-inline-sticky-crash-expected.txt (0 => 127301)
--- trunk/LayoutTests/fast/css/sticky/remove-inline-sticky-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/sticky/remove-inline-sticky-crash-expected.txt 2012-08-31 19:38:40 UTC (rev 127301)
@@ -0,0 +1,3 @@
+This test should not crash
+
+
Added: trunk/LayoutTests/fast/css/sticky/remove-inline-sticky-crash.html (0 => 127301)
--- trunk/LayoutTests/fast/css/sticky/remove-inline-sticky-crash.html (rev 0)
+++ trunk/LayoutTests/fast/css/sticky/remove-inline-sticky-crash.html 2012-08-31 19:38:40 UTC (rev 127301)
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+<style>
+ body {
+ margin: 0;
+ height: 2000px;
+ }
+
+ .box {
+ width: 200px;
+ height: 200px;
+ }
+
+ .sticky {
+ position: -webkit-sticky;
+ top: 100px;
+ background-color: green;
+ }
+</style>
+<script>
+ if (window.testRunner)
+ testRunner.dumpAsText();
+
+ function doTest()
+ {
+ var stickyBox = document.getElementById('sticky');
+ stickyBox.parentNode.removeChild(stickyBox);
+ window.scrollTo(0, 10);
+ }
+ window.addEventListener('load', doTest, false);
+</script>
+</head>
+<body>
+ <p>This test should not crash</p>
+ <span id="sticky" class="sticky box"></span>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (127300 => 127301)
--- trunk/Source/WebCore/ChangeLog 2012-08-31 19:11:32 UTC (rev 127300)
+++ trunk/Source/WebCore/ChangeLog 2012-08-31 19:38:40 UTC (rev 127301)
@@ -1,3 +1,28 @@
+2012-08-31 Simon Fraser <[email protected]>
+
+ (Regression: r126774): Fix crash when scrolling after removing inline sticky element
+ https://bugs.webkit.org/show_bug.cgi?id=95584
+
+ Reviewed by Dave Hyatt.
+
+ Move fixed/sticky registration and unregistration with the FrameView from
+ RenderBox and RenderInline into RenderBoxModelObject, which also
+ fixes the issue that inlines didn't unregister themselves on destruction.
+
+ Test: fast/css/sticky/remove-inline-sticky-crash.html
+
+ * rendering/RenderBox.cpp:
+ (WebCore::RenderBox::willBeDestroyed): Code moved to RenderBoxModelObject. No need
+ to null-check style.
+ (WebCore::RenderBox::styleWillChange): Code moved to RenderBoxModelObject.
+ * rendering/RenderBoxModelObject.cpp:
+ (WebCore::RenderBoxModelObject::willBeDestroyed): Remove fixed objects. Check
+ isPositioned() to avoid this work for most non-positioned renderers.
+ (WebCore::RenderBoxModelObject::styleWillChange): Register and unregister fixed
+ and sticky objects with the FrameView.
+ * rendering/RenderInline.cpp: styleWillChange() is no longer needed.
+ * rendering/RenderInline.h: Ditto.
+
2012-08-31 Nikhil Bhargava <[email protected]>
Remove extraneous includes (Node.h, Document.h, Element.h, RenderObject.h)
Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (127300 => 127301)
--- trunk/Source/WebCore/rendering/RenderBox.cpp 2012-08-31 19:11:32 UTC (rev 127300)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp 2012-08-31 19:38:40 UTC (rev 127301)
@@ -134,17 +134,8 @@
{
clearOverrideSize();
- if (style()) {
- RenderBlock::removePercentHeightDescendantIfNeeded(this);
+ RenderBlock::removePercentHeightDescendantIfNeeded(this);
- if (RenderView* view = this->view()) {
- if (FrameView* frameView = view->frameView()) {
- if (style()->hasViewportConstrainedPosition())
- frameView->removeFixedObject(this);
- }
- }
- }
-
RenderBoxModelObject::willBeDestroyed();
}
@@ -205,17 +196,6 @@
} else if (newStyle && isBody())
view()->repaint();
- if (FrameView *frameView = view()->frameView()) {
- bool newStyleIsViewportConstained = newStyle && newStyle->hasViewportConstrainedPosition();
- bool oldStyleIsViewportConstrained = oldStyle && oldStyle->hasViewportConstrainedPosition();
- if (newStyleIsViewportConstained != oldStyleIsViewportConstrained) {
- if (newStyleIsViewportConstained)
- frameView->addFixedObject(this);
- else
- frameView->removeFixedObject(this);
- }
- }
-
RenderBoxModelObject::styleWillChange(diff, newStyle);
}
Modified: trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp (127300 => 127301)
--- trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2012-08-31 19:11:32 UTC (rev 127300)
+++ trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2012-08-31 19:38:40 UTC (rev 127301)
@@ -348,6 +348,15 @@
// A continuation of this RenderObject should be destroyed at subclasses.
ASSERT(!continuation());
+ if (isPositioned()) {
+ if (RenderView* view = this->view()) {
+ if (FrameView* frameView = view->frameView()) {
+ if (style()->hasViewportConstrainedPosition())
+ frameView->removeFixedObject(this);
+ }
+ }
+ }
+
// If this is a first-letter object with a remaining text fragment then the
// entry needs to be cleared from the map.
if (firstLetterRemainingText())
@@ -409,6 +418,17 @@
}
}
+ if (FrameView *frameView = view()->frameView()) {
+ bool newStyleIsViewportConstained = newStyle && newStyle->hasViewportConstrainedPosition();
+ bool oldStyleIsViewportConstrained = oldStyle && oldStyle->hasViewportConstrainedPosition();
+ if (newStyleIsViewportConstained != oldStyleIsViewportConstrained) {
+ if (newStyleIsViewportConstained)
+ frameView->addFixedObject(this);
+ else
+ frameView->removeFixedObject(this);
+ }
+ }
+
RenderObject::styleWillChange(diff, newStyle);
}
Modified: trunk/Source/WebCore/rendering/RenderInline.cpp (127300 => 127301)
--- trunk/Source/WebCore/rendering/RenderInline.cpp 2012-08-31 19:11:32 UTC (rev 127300)
+++ trunk/Source/WebCore/rendering/RenderInline.cpp 2012-08-31 19:38:40 UTC (rev 127301)
@@ -155,23 +155,6 @@
}
}
-void RenderInline::styleWillChange(StyleDifference diff, const RenderStyle* newStyle)
-{
- if (FrameView *frameView = view()->frameView()) {
- RenderStyle* oldStyle = style();
- bool newStyleIsSticky = newStyle && newStyle->position() == StickyPosition;
- bool oldStyleIsSticky = oldStyle && oldStyle->position() == StickyPosition;
- if (newStyleIsSticky != oldStyleIsSticky) {
- if (newStyleIsSticky)
- frameView->addFixedObject(this);
- else
- frameView->removeFixedObject(this);
- }
- }
-
- RenderBoxModelObject::styleWillChange(diff, newStyle);
-}
-
void RenderInline::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
{
RenderBoxModelObject::styleDidChange(diff, oldStyle);
Modified: trunk/Source/WebCore/rendering/RenderInline.h (127300 => 127301)
--- trunk/Source/WebCore/rendering/RenderInline.h 2012-08-31 19:11:32 UTC (rev 127300)
+++ trunk/Source/WebCore/rendering/RenderInline.h 2012-08-31 19:38:40 UTC (rev 127301)
@@ -89,7 +89,6 @@
protected:
virtual void willBeDestroyed();
- virtual void styleWillChange(StyleDifference, const RenderStyle* newStyle) OVERRIDE;
virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle) OVERRIDE;
private:
