Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1228)

Unified Diff: Source/core/layout/LayoutBlock.cpp

Issue 1128713004: Don't mark a positioned object for layout when we've just laid it out. (Closed) Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: Created 5 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: Source/core/layout/LayoutBlock.cpp
diff --git a/Source/core/layout/LayoutBlock.cpp b/Source/core/layout/LayoutBlock.cpp
index f9b493b9844ca95fb63757a302ec7634069e2145..f88a3e2491791bb1bda4057827d0a5a0af9f1982 100644
--- a/Source/core/layout/LayoutBlock.cpp
+++ b/Source/core/layout/LayoutBlock.cpp
@@ -1680,6 +1680,21 @@ LayoutUnit LayoutBlock::marginIntrinsicLogicalWidthForChild(LayoutBox& child) co
return margin;
}
+static bool needsLayoutDueToStaticPosition(LayoutObject* child, LayoutObject* containingBlock)
+{
+ // When a non-positioned block element moves, it may have positioned children that are implicitly positioned relative to the
+ // non-positioned block. In block flow we can detect these when we layout the non-positioned block, in line layout we always
+ // need to mark the positioned child for layout if its statically positioned in the direction in which the object lays out.
+ // TODO(rhogan): We may need to move the block layout case in here too, as marking a positioned object for layout while laying out an object's children
+ // invalidly assumes that our positionedObjects list is in DOM order and that we could never mark a positioned object for layout
+ // *after* we've laid it out in layoutPositionedObjects.
leviw_travelin_and_unemployed 2015/05/19 18:32:12 We should assert this. Can you file a bug and link
rhogan 2015/05/20 20:15:06 Added the bug, but couldn't think of a way of asse
+ if (!child->parent()->childrenInline())
+ return false;
+ const ComputedStyle* style = child->style();
+ bool isHorizontal = style->isHorizontalWritingMode();
+ return style->isDisplayInlineType() ? style->hasStaticInlinePosition(isHorizontal) : style->hasStaticBlockPosition(isHorizontal);
+}
+
void LayoutBlock::layoutPositionedObjects(bool relayoutChildren, PositionedLayoutBehavior info)
{
TrackedLayoutBoxListHashSet* positionedDescendants = positionedObjects();
@@ -1702,11 +1717,7 @@ void LayoutBlock::layoutPositionedObjects(bool relayoutChildren, PositionedLayou
continue;
}
- // When a non-positioned block element moves, it may have positioned children that are implicitly positioned relative to the
- // non-positioned block. Rather than trying to detect all of these movement cases, we just always lay out positioned
- // objects that are positioned implicitly like this. Such objects are rare, and so in typical DHTML menu usage (where everything is
- // positioned explicitly) this should not incur a performance penalty.
- if (relayoutChildren || (positionedObject->style()->hasStaticBlockPosition(isHorizontalWritingMode()) && positionedObject->parent() != this))
+ if (relayoutChildren || needsLayoutDueToStaticPosition(positionedObject, this))
layoutScope.setChildNeedsLayout(positionedObject);
// If relayoutChildren is set and the child has percentage padding or an embedded content box, we also need to invalidate the childs pref widths.

Powered by Google App Engine
This is Rietveld 408576698