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.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@127497 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent a8106ec0
2012-09-04 Simon Fraser <simon.fraser@apple.com>
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 <roger_fong@apple.com>
Unreviewed. Removing accidentally added executable properties on test files.
<!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>
2012-09-04 Simon Fraser <simon.fraser@apple.com>
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 <dominik.rottsches@intel.com>
ResourceErrorBase needs to identify timeouts
......@@ -1440,9 +1440,7 @@ void FrameView::addFixedObject(RenderObject* object)
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())
......
......@@ -418,17 +418,6 @@ void RenderBoxModelObject::styleWillChange(StyleDifference diff, const RenderSty
}
}
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 @@ void RenderBoxModelObject::styleDidChange(StyleDifference diff, const RenderStyl
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()
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment