https://bugs.webkit.org/show_bug.cgi?id=88133
Reviewed by Julien Chaffraix and Abhishek Arya.
Source/WebCore:
When a fixed positioned element is collected into a named flow, we have to make sure
that such element has the RenderFlowThread as containing block instead of RenderView,
so that the fixed positioned element is laid out properly.
Making the RenderFlowThread the top most containing block for named flow elements required the
modification of RenderLayer::convertToLayerCoords so that the fixed positioned elements inside the
named flow take the same code path as the absolute positioned elements inside the named flow.
I also added a method, checkBlockPositionedObjectsNeedLayout, in order to verify that a block
that is ending its layout, setNeedsLayout(false), has all the positioned children laid out.
This way, we will hit an assertion if an out-of-flow positioned child inside a RenderFlowThread
is not laid out after the RenderFlowThread is laid out.
Tests: fast/regions/absolute-pos-elem-in-named-flow.html
fast/regions/absolute-pos-elem-in-region.html
fast/regions/fixed-pos-elem-in-named-flow.html
fast/regions/fixed-pos-elem-in-named-flow2.html
fast/regions/fixed-pos-elem-in-region.html
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::checkPositionedObjectsNeedLayout):
* rendering/RenderBlock.h:
(RenderBlock):
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::convertToLayerCoords):
* rendering/RenderObject.cpp:
(WebCore):
(WebCore::RenderObject::checkBlockPositionedObjectsNeedLayout):
(WebCore::RenderObject::containingBlock):
(WebCore::RenderObject::container):
* rendering/RenderObject.h:
(RenderObject):
(WebCore::RenderObject::setNeedsLayout):
LayoutTests:
When a fixed positioned element is collected into a named flow, we have to make sure
that such element has the RenderFlowThread as containing block instead of RenderView,
so that the fixed positioned element is laid out properly.
* fast/regions/absolute-pos-elem-in-named-flow-expected.txt: Added.
* fast/regions/absolute-pos-elem-in-named-flow.html: Added.
* fast/regions/absolute-pos-elem-in-region-expected.html: Added.
* fast/regions/absolute-pos-elem-in-region.html: Added.
* fast/regions/fixed-pos-elem-in-named-flow-expected.txt: Added.
* fast/regions/fixed-pos-elem-in-named-flow.html: Added.
* fast/regions/fixed-pos-elem-in-named-flow2-expected.txt: Added.
* fast/regions/fixed-pos-elem-in-named-flow2.html: Added.
* fast/regions/fixed-pos-elem-in-region-expected.html: Added.
* fast/regions/fixed-pos-elem-in-region.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@121557
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2012-06-29 Mihnea Ovidenie <mihnea@adobe.com>
+
+ Crash when flowing a fixed positioned element into a region.
+ https://bugs.webkit.org/show_bug.cgi?id=88133
+
+ Reviewed by Julien Chaffraix and Abhishek Arya.
+
+ When a fixed positioned element is collected into a named flow, we have to make sure
+ that such element has the RenderFlowThread as containing block instead of RenderView,
+ so that the fixed positioned element is laid out properly.
+
+ * fast/regions/absolute-pos-elem-in-named-flow-expected.txt: Added.
+ * fast/regions/absolute-pos-elem-in-named-flow.html: Added.
+ * fast/regions/absolute-pos-elem-in-region-expected.html: Added.
+ * fast/regions/absolute-pos-elem-in-region.html: Added.
+ * fast/regions/fixed-pos-elem-in-named-flow-expected.txt: Added.
+ * fast/regions/fixed-pos-elem-in-named-flow.html: Added.
+ * fast/regions/fixed-pos-elem-in-named-flow2-expected.txt: Added.
+ * fast/regions/fixed-pos-elem-in-named-flow2.html: Added.
+ * fast/regions/fixed-pos-elem-in-region-expected.html: Added.
+ * fast/regions/fixed-pos-elem-in-region.html: Added.
+
2012-06-29 Alexander Pavlov <apavlov@chromium.org>
Web Inspector: [Device Metrics] "Fit window" option inhibits adjusting the page zoom factor
--- /dev/null
+Test for WebKit Bug 88133 Collecting a fixed positioned element in a named flow causes WebKit to crash.
+
+This test collects an absolutely positioned object into a named flow without flowing it into a region.
+
+The test passes if it does not crash or assert.
+
+PASS
--- /dev/null
+<!doctype html>
+<html>
+ <head>
+ <style>
+ #absolute { position: absolute; -webkit-flow-into: flow; width: 50px; height: 50px; }
+ </style>
+ </head>
+ <body>
+ <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=88133">WebKit Bug 88133</a> Collecting a fixed positioned element in a named flow causes WebKit to crash.</p>
+ <p>This test collects an absolutely positioned object into a named flow without flowing it into a region.</p>
+ <p>The test passes if it does not crash or assert.</p>
+ <div id="absolute"></div>
+ <p>PASS</p>
+ <script>
+ if (window.testRunner)
+ window.testRunner.dumpAsText();
+ </script>
+ </body>
+</html>
--- /dev/null
+<!doctype html>
+<html>
+ <head>
+ <style>
+ #green { position: absolute; width: 50px; height: 50px; top: 125px; left: 75px; background-color: green; }
+ </style>
+ </head>
+ <body>
+ <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=88133">WebKit Bug 88133</a> Collecting a fixed positioned element in a named flow causes WebKit to crash.</p>
+ <p>This test collects an absolutely positioned element inside a named flow and flows it into a region.</p>
+ <p>The test passes if it does not crash or assert and no red rectangle is visible.</p>
+ <div id="green"></div>
+ </body>
+</html>
--- /dev/null
+<!doctype html>
+<html>
+ <head>
+ <style>
+ #red { position: absolute; width: 50px; height: 50px; top: 125px; left: 75px; background-color: red; }
+ #absolute { -webkit-flow-into: flow; position: absolute; width: 50px; height: 50px; background-color: green; top: 25px; left: 25px; }
+ #region { -webkit-flow-from: flow; width: 100px; height: 100px; position: absolute; top: 100px; left: 50px; }
+ </style>
+ </head>
+ <body>
+ <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=88133">WebKit Bug 88133</a> Collecting a fixed positioned element in a named flow causes WebKit to crash.</p>
+ <p>This test collects an absolutely positioned element inside a named flow and flows it into a region.</p>
+ <p>The test passes if it does not crash or assert and no red rectangle is visible.</p>
+ <div id="red"></div>
+ <div id="absolute"></div>
+ <div id="region"></div>
+ </body>
+</html>
--- /dev/null
+Test for WebKit Bug 88133 Collecting a fixed positioned element in a named flow causes WebKit to crash.
+
+This test collects a fixed positioned object into a named flow but without flowing it into a region.
+
+The test passes if it does not crash or assert.
+
+PASS
--- /dev/null
+<!doctype html>
+<html>
+ <head>
+ <style>
+ #fixed { position: fixed; -webkit-flow-into: flow; width: 50px; height: 50px; }
+ </style>
+ </head>
+ <body>
+ <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=88133">WebKit Bug 88133</a> Collecting a fixed positioned element in a named flow causes WebKit to crash.</p>
+ <p>This test collects a fixed positioned object into a named flow but without flowing it into a region.</p>
+ <p>The test passes if it does not crash or assert.</p>
+ <div id="fixed"></div>
+ <p>PASS</p>
+ <script>
+ if (window.testRunner)
+ window.testRunner.dumpAsText();
+ </script>
+ </body>
+</html>
--- /dev/null
+Test for WebKit Bug 88133 Collecting a fixed positioned element in a named flow causes WebKit to crash.
+
+This test collects a fixed positioned object into a named flow but without flowing it into a region. The fixed positioned element is the child of another element that has a transform, so that it requires a layer.
+
+The test passes if it does not crash or assert.
+
+PASS
--- /dev/null
+<!doctype html>
+<html>
+ <head>
+ <style>
+ #article { -webkit-flow-into: flow; }
+ #transform { width: 100px; height: 100px; -webkit-transform: translate3d(50px, 50px, 0px); }
+ #fixed { position: fixed; top: 25px; left: 25px; width: 50px; height: 50px; }
+ </style>
+ </head>
+ <body>
+ <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=88133">WebKit Bug 88133</a> Collecting a fixed positioned element in a named flow causes WebKit to crash.</p>
+ <p>This test collects a fixed positioned object into a named flow but without flowing it into a region. The fixed positioned element is the child of another element that has a transform, so that it requires a layer.</p>
+ <p>The test passes if it does not crash or assert.</p>
+ <div id="article">
+ <div id="transform">
+ <div id="fixed"></div>
+ </div>
+ </div>
+ <p>PASS</p>
+ <script>
+ if (window.testRunner)
+ window.testRunner.dumpAsText();
+ </script>
+ </body>
+</html>
--- /dev/null
+<!doctype html>
+<html>
+ <head>
+ <style>
+ #green { position: fixed; width: 50px; height: 50px; top: 125px; left: 75px; background-color: green; }
+ </style>
+ </head>
+ <body>
+ <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=88133">WebKit Bug 88133</a> Collecting a fixed positioned element in a named flow causes WebKit to crash.</p>
+ <p>This test collects a fixed position element inside a named flow and flows it into a region.</p>
+ <p>The test passes if it does not crash or assert and there is no red rectangle visible.</p>
+ <div id="green"></div>
+ </body>
+</html>
--- /dev/null
+<!doctype html>
+<html>
+ <head>
+ <style>
+ #red { position: fixed; width: 50px; height: 50px; top: 125px; left: 75px; background-color: red; }
+ #fixed { -webkit-flow-into: flow; position: fixed; width: 50px; height: 50px; background-color: green; top: 25px; left: 25px; }
+ #region { -webkit-flow-from: flow; width: 100px; height: 100px; position: absolute; top: 100px; left: 50px; }
+ </style>
+ </head>
+ <body>
+ <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=88133">WebKit Bug 88133</a> Collecting a fixed positioned element in a named flow causes WebKit to crash.</p>
+ <p>This test collects a fixed position element inside a named flow and flows it into a region.</p>
+ <p>The test passes if it does not crash or assert and there is no red rectangle visible.</p>
+ <div id="red"></div>
+ <div id="fixed"></div>
+ <div id="region"></div>
+ </body>
+</html>
+2012-06-29 Mihnea Ovidenie <mihnea@adobe.com>
+
+ Crash when flowing a fixed positioned element into a region.
+ https://bugs.webkit.org/show_bug.cgi?id=88133
+
+ Reviewed by Julien Chaffraix and Abhishek Arya.
+
+ When a fixed positioned element is collected into a named flow, we have to make sure
+ that such element has the RenderFlowThread as containing block instead of RenderView,
+ so that the fixed positioned element is laid out properly.
+ Making the RenderFlowThread the top most containing block for named flow elements required the
+ modification of RenderLayer::convertToLayerCoords so that the fixed positioned elements inside the
+ named flow take the same code path as the absolute positioned elements inside the named flow.
+ I also added a method, checkBlockPositionedObjectsNeedLayout, in order to verify that a block
+ that is ending its layout, setNeedsLayout(false), has all the positioned children laid out.
+ This way, we will hit an assertion if an out-of-flow positioned child inside a RenderFlowThread
+ is not laid out after the RenderFlowThread is laid out.
+
+ Tests: fast/regions/absolute-pos-elem-in-named-flow.html
+ fast/regions/absolute-pos-elem-in-region.html
+ fast/regions/fixed-pos-elem-in-named-flow.html
+ fast/regions/fixed-pos-elem-in-named-flow2.html
+ fast/regions/fixed-pos-elem-in-region.html
+
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::checkPositionedObjectsNeedLayout):
+ * rendering/RenderBlock.h:
+ (RenderBlock):
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::convertToLayerCoords):
+ * rendering/RenderObject.cpp:
+ (WebCore):
+ (WebCore::RenderObject::checkBlockPositionedObjectsNeedLayout):
+ (WebCore::RenderObject::containingBlock):
+ (WebCore::RenderObject::container):
+ * rendering/RenderObject.h:
+ (RenderObject):
+ (WebCore::RenderObject::setNeedsLayout):
+
2012-06-29 Konrad Piascik <kpiascik@rim.com>
Don't hardcode target dpi of 160 (it should be 96 on desktop)
}
#ifndef NDEBUG
+void RenderBlock::checkPositionedObjectsNeedLayout()
+{
+ if (PositionedObjectsListHashSet* positionedObjects = this->positionedObjects()) {
+ PositionedObjectsListHashSet::const_iterator end = positionedObjects->end();
+ for (PositionedObjectsListHashSet::const_iterator it = positionedObjects->begin(); it != end; ++it) {
+ RenderBox* currBox = *it;
+ ASSERT(!currBox->needsLayout());
+ }
+ }
+}
void RenderBlock::showLineTreeAndMark(const InlineBox* markedBox1, const char* markedLabel1, const InlineBox* markedBox2, const char* markedLabel2, const RenderObject* obj) const
{
bool runInIsPlacedIntoSiblingBlock(RenderObject* runIn);
#ifndef NDEBUG
+ void checkPositionedObjectsNeedLayout();
void showLineTreeAndMark(const InlineBox* = 0, const char* = 0, const InlineBox* = 0, const char* = 0, const RenderObject* = 0) const;
#endif
return;
EPosition position = renderer()->style()->position();
- if (position == FixedPosition && (!ancestorLayer || ancestorLayer == renderer()->view()->layer())) {
+
+ // FIXME: Positioning of out-of-flow(fixed, absolute) elements collected in a RenderFlowThread
+ // may need to be revisited in a future patch.
+ // If the fixed renderer is inside a RenderFlowThread, we should not compute location using localToAbsolute,
+ // since localToAbsolute maps the coordinates from named flow to regions coordinates and regions can be
+ // positioned in a completely different place in the viewport (RenderView).
+ if (position == FixedPosition && !renderer()->inRenderFlowThread() && (!ancestorLayer || ancestorLayer == renderer()->view()->layer())) {
// If the fixed layer's container is the root, just add in the offset of the view. We can obtain this by calling
// localToAbsolute() on the RenderView.
FloatPoint absPos = renderer()->localToAbsolute(FloatPoint(), true);
location += flooredLayoutSize(absPos);
return;
}
-
- if (position == FixedPosition) {
+
+ // For the fixed positioned elements inside a render flow thread, we should also skip the code path below
+ // Otherwise, for the case of ancestorLayer == rootLayer and fixed positioned element child of a transformed
+ // element in render flow thread, we will hit the fixed positioned container before hitting the ancestor layer.
+ if (position == FixedPosition && !renderer()->inRenderFlowThread()) {
// For a fixed layers, we need to walk up to the root to see if there's a fixed position container
// (e.g. a transformed layer). It's an error to call convertToLayerCoords() across a layer with a transform,
// so we should always find the ancestor at or before we find the fixed position container.
parentLayer = parent();
bool foundAncestorFirst = false;
while (parentLayer) {
+ // RenderFlowThread is a positioned container, child of RenderView, positioned at (0,0).
+ // This implies that, for out-of-flow positioned elements inside a RenderFlowThread,
+ // we are bailing out before reaching root layer.
if (isPositionedContainer(parentLayer))
break;
parentLayer = parentLayer->parent();
}
+ // We should not reach RenderView layer past the RenderFlowThread layer for any
+ // children of the RenderFlowThread.
+ if (renderer()->inRenderFlowThread() && !renderer()->isRenderFlowThread())
+ ASSERT(parentLayer != renderer()->view()->layer());
+
if (foundAncestorFirst) {
// Found ancestorLayer before the abs. positioned container, so compute offset of both relative
// to enclosingPositionedAncestor and subtract.
last->scheduleRelayout();
}
+#ifndef NDEBUG
+void RenderObject::checkBlockPositionedObjectsNeedLayout()
+{
+ ASSERT(!needsLayout());
+
+ if (isRenderBlock())
+ toRenderBlock(this)->checkPositionedObjectsNeedLayout();
+}
+#endif
+
void RenderObject::setPreferredLogicalWidthsDirty(bool shouldBeDirty, MarkingBehavior markParents)
{
bool alreadyDirty = preferredLogicalWidthsDirty();
break;
if (o->hasTransform() && o->isRenderBlock())
break;
+ // The render flow thread is the top most containing block
+ // for the fixed positioned elements.
+ if (o->isRenderFlowThread())
+ break;
#if ENABLE(SVG)
// foreignObject is the containing block for its contents.
if (o->isSVGForeignObject())
if (o->isSVGForeignObject())
break;
#endif
+ // The render flow thread is the top most containing block
+ // for the fixed positioned elements.
+ if (o->isRenderFlowThread())
+ break;
+
if (repaintContainerSkipped && o == repaintContainer)
*repaintContainerSkipped = true;
StyleDifference adjustStyleDifference(StyleDifference, unsigned contextSensitiveProperties) const;
Color selectionColor(int colorProperty) const;
-
+
+#ifndef NDEBUG
+ void checkBlockPositionedObjectsNeedLayout();
+#endif
+
RefPtr<RenderStyle> m_style;
Node* m_node;
setNormalChildNeedsLayout(false);
setNeedsPositionedMovementLayout(false);
setAncestorLineBoxDirty(false);
+#ifndef NDEBUG
+ checkBlockPositionedObjectsNeedLayout();
+#endif
}
}