Title: [127497] trunk
- Revision
- 127497
- Author
- [email protected]
- Date
- 2012-09-04 13:34:01 -0700 (Tue, 04 Sep 2012)
Log Message
Regression: Heap-use-after-free in WebCore::FrameView::scrollContentsFastPath
https://bugs.webkit.org/show_bug.cgi?id=95754
Reviewed by Dave Hyatt.
Source/WebCore:
It's possible to have a renderer with position:fixed or sticky style,
but no layer, for example a RenderScrollBarPart. Don't register such
renderers with the FrameView.
Moved the code that registers/unregisters with the FrameView from
styleWillChange() to styleDidChange(), since in the latter case
we can check if we have a RenderLayer. Only register renderers with layers.
We always unregister, which required removing an assertion in
FrameView::removeFixedObject(), and replacing it with a null check of m_fixedObjects.
Test: fast/css/remove-fixed-resizer-crash.html
* page/FrameView.cpp:
(WebCore::FrameView::removeFixedObject):
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::styleWillChange):
(WebCore::RenderBoxModelObject::styleDidChange):
LayoutTests:
Testcase with a position:fixed resizer and scrolling.
* fast/css/remove-fixed-resizer-crash-expected.txt: Added.
* fast/css/remove-fixed-resizer-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (127496 => 127497)
--- trunk/LayoutTests/ChangeLog 2012-09-04 20:30:38 UTC (rev 127496)
+++ trunk/LayoutTests/ChangeLog 2012-09-04 20:34:01 UTC (rev 127497)
@@ -1,3 +1,15 @@
+2012-09-04 Simon Fraser <[email protected]>
+
+ Regression: Heap-use-after-free in WebCore::FrameView::scrollContentsFastPath
+ https://bugs.webkit.org/show_bug.cgi?id=95754
+
+ Reviewed by Dave Hyatt.
+
+ Testcase with a position:fixed resizer and scrolling.
+
+ * fast/css/remove-fixed-resizer-crash-expected.txt: Added.
+ * fast/css/remove-fixed-resizer-crash.html: Added.
+
2012-09-04 Roger Fong <[email protected]>
Unreviewed. Removing accidentally added executable properties on test files.
Added: trunk/LayoutTests/fast/css/remove-fixed-resizer-crash-expected.txt (0 => 127497)
--- trunk/LayoutTests/fast/css/remove-fixed-resizer-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/remove-fixed-resizer-crash-expected.txt 2012-09-04 20:34:01 UTC (rev 127497)
@@ -0,0 +1,3 @@
+This test should not crash
+
+
Property changes on: trunk/LayoutTests/fast/css/remove-fixed-resizer-crash-expected.txt
___________________________________________________________________
Added: svn:mime-type
Added: svn:keywords
Added: svn:eol-style
Added: trunk/LayoutTests/fast/css/remove-fixed-resizer-crash.html (0 => 127497)
--- trunk/LayoutTests/fast/css/remove-fixed-resizer-crash.html (rev 0)
+++ trunk/LayoutTests/fast/css/remove-fixed-resizer-crash.html 2012-09-04 20:34:01 UTC (rev 127497)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+ body {
+ height: 2000px;
+ }
+
+ *::-webkit-resizer { position: fixed; }
+</style>
+<script>
+ if (window.testRunner)
+ testRunner.dumpAsText();
+
+ function doTest()
+ {
+ var svgElement = document.createElementNS("http://www.w3.org/2000/svg", "svg");
+ document.body.appendChild(svgElement);
+ window.scrollBy(0, 100);
+ }
+ window.addEventListener('load', doTest, false);
+</script>
+</head>
+<body>
+ <p>This test should not crash</p>
+ <input>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/css/remove-fixed-resizer-crash.html
___________________________________________________________________
Added: svn:mime-type
Added: svn:keywords
Added: svn:eol-style
Modified: trunk/Source/WebCore/ChangeLog (127496 => 127497)
--- trunk/Source/WebCore/ChangeLog 2012-09-04 20:30:38 UTC (rev 127496)
+++ trunk/Source/WebCore/ChangeLog 2012-09-04 20:34:01 UTC (rev 127497)
@@ -1,3 +1,28 @@
+2012-09-04 Simon Fraser <[email protected]>
+
+ Regression: Heap-use-after-free in WebCore::FrameView::scrollContentsFastPath
+ https://bugs.webkit.org/show_bug.cgi?id=95754
+
+ Reviewed by Dave Hyatt.
+
+ It's possible to have a renderer with position:fixed or sticky style,
+ but no layer, for example a RenderScrollBarPart. Don't register such
+ renderers with the FrameView.
+
+ Moved the code that registers/unregisters with the FrameView from
+ styleWillChange() to styleDidChange(), since in the latter case
+ we can check if we have a RenderLayer. Only register renderers with layers.
+ We always unregister, which required removing an assertion in
+ FrameView::removeFixedObject(), and replacing it with a null check of m_fixedObjects.
+
+ Test: fast/css/remove-fixed-resizer-crash.html
+
+ * page/FrameView.cpp:
+ (WebCore::FrameView::removeFixedObject):
+ * rendering/RenderBoxModelObject.cpp:
+ (WebCore::RenderBoxModelObject::styleWillChange):
+ (WebCore::RenderBoxModelObject::styleDidChange):
+
2012-09-04 Dominik Röttsches <[email protected]>
ResourceErrorBase needs to identify timeouts
Modified: trunk/Source/WebCore/page/FrameView.cpp (127496 => 127497)
--- trunk/Source/WebCore/page/FrameView.cpp 2012-09-04 20:30:38 UTC (rev 127496)
+++ trunk/Source/WebCore/page/FrameView.cpp 2012-09-04 20:34:01 UTC (rev 127497)
@@ -1440,9 +1440,7 @@
void FrameView::removeFixedObject(RenderObject* object)
{
- ASSERT(hasFixedObjects());
-
- if (m_fixedObjects->contains(object)) {
+ if (m_fixedObjects && m_fixedObjects->contains(object)) {
m_fixedObjects->remove(object);
if (Page* page = m_frame->page()) {
if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
Modified: trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp (127496 => 127497)
--- trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2012-09-04 20:30:38 UTC (rev 127496)
+++ trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2012-09-04 20:34:01 UTC (rev 127497)
@@ -418,17 +418,6 @@
}
}
- 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);
}
@@ -466,6 +455,17 @@
if (s_hadLayer && layer()->isSelfPaintingLayer() != s_layerWasSelfPainting)
setChildNeedsLayout(true);
}
+
+ if (FrameView *frameView = view()->frameView()) {
+ bool newStyleIsViewportConstained = style()->hasViewportConstrainedPosition();
+ bool oldStyleIsViewportConstrained = oldStyle && oldStyle->hasViewportConstrainedPosition();
+ if (newStyleIsViewportConstained != oldStyleIsViewportConstrained) {
+ if (newStyleIsViewportConstained && layer())
+ frameView->addFixedObject(this);
+ else
+ frameView->removeFixedObject(this);
+ }
+ }
}
void RenderBoxModelObject::updateBoxModelInfoFromStyle()
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
