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

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

Issue 1964203004: Helper method for non-direct beforeChild in LayoutBlock::addChild(). (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fix broken test. Use RELEASE_ASSERT. Back out unnecessary change. Created 4 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: third_party/WebKit/Source/core/layout/LayoutBlock.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutBlock.cpp b/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
index 2ef3c37559e0d29a34041f50adb818f5bc5297d8..0a898a5bad19f29d0a3221d3952b3098f280dfd0 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
@@ -377,47 +377,56 @@ bool LayoutBlock::allowsOverflowClip() const
return node() != document().viewportDefiningElement();
}
-void LayoutBlock::addChild(LayoutObject* newChild, LayoutObject* beforeChild)
-{
- if (beforeChild && beforeChild->parent() != this) {
- LayoutObject* beforeChildContainer = beforeChild->parent();
- while (beforeChildContainer->parent() != this)
- beforeChildContainer = beforeChildContainer->parent();
- ASSERT(beforeChildContainer);
-
- if (beforeChildContainer->isAnonymous()) {
- // If the requested beforeChild is not one of our children, then this is because
- // there is an anonymous container within this object that contains the beforeChild.
- LayoutObject* beforeChildAnonymousContainer = beforeChildContainer;
- if (beforeChildAnonymousContainer->isAnonymousBlock()
- // Full screen layoutObjects and full screen placeholders act as anonymous blocks, not tables:
- || beforeChildAnonymousContainer->isLayoutFullScreen()
- || beforeChildAnonymousContainer->isLayoutFullScreenPlaceholder()
- ) {
- // Insert the child into the anonymous block box instead of here.
- if (newChild->isInline() || newChild->isFloatingOrOutOfFlowPositioned() || beforeChild->parent()->slowFirstChild() != beforeChild)
- beforeChild->parent()->addChild(newChild, beforeChild);
- else
- addChild(newChild, beforeChild->parent());
- return;
- }
+void LayoutBlock::addChildBeforeDescendant(LayoutObject* newChild, LayoutObject* beforeDescendant)
+{
+ ASSERT(beforeDescendant->parent() != this);
+ LayoutObject* beforeDescendantContainer = beforeDescendant->parent();
+ while (beforeDescendantContainer->parent() != this)
+ beforeDescendantContainer = beforeDescendantContainer->parent();
+ ASSERT(beforeDescendantContainer);
+
+ // We really can't go on if what we have found isn't anonymous. We're not supposed to use some
+ // random non-anonymous object and put the child there. That's a recipe for security issues.
+ RELEASE_ASSERT(beforeDescendantContainer->isAnonymous());
mstensho (USE GERRIT) 2016/05/11 21:06:00 Here's a difference. We previously allowed this si
eae 2016/05/11 21:10:11 Yay!
+
+ // If the requested insertion point is not one of our children, then this is because
+ // there is an anonymous container within this object that contains the beforeDescendant.
+ if (beforeDescendantContainer->isAnonymousBlock()
+ // Full screen layoutObjects and full screen placeholders act as anonymous blocks, not tables:
+ || beforeDescendantContainer->isLayoutFullScreen()
+ || beforeDescendantContainer->isLayoutFullScreenPlaceholder()) {
+ // Insert the child into the anonymous block box instead of here.
+ if (newChild->isInline() || newChild->isFloatingOrOutOfFlowPositioned() || beforeDescendant->parent()->slowFirstChild() != beforeDescendant)
+ beforeDescendant->parent()->addChild(newChild, beforeDescendant);
+ else
+ addChild(newChild, beforeDescendant->parent());
+ return;
+ }
- ASSERT(beforeChildAnonymousContainer->isTable());
- if (newChild->isTablePart()) {
- // Insert into the anonymous table.
- beforeChildAnonymousContainer->addChild(newChild, beforeChild);
- return;
- }
+ ASSERT(beforeDescendantContainer->isTable());
+ if (newChild->isTablePart()) {
+ // Insert into the anonymous table.
+ beforeDescendantContainer->addChild(newChild, beforeDescendant);
+ return;
+ }
- beforeChild = splitAnonymousBoxesAroundChild(beforeChild);
+ LayoutObject* beforeChild = splitAnonymousBoxesAroundChild(beforeDescendant);
- ASSERT(beforeChild->parent() == this);
- if (beforeChild->parent() != this) {
- // We should never reach here. If we do, we need to use the
- // safe fallback to use the topmost beforeChild container.
- beforeChild = beforeChildContainer;
- }
- }
+ ASSERT(beforeChild->parent() == this);
+ if (beforeChild->parent() != this) {
+ // We should never reach here. If we do, we need to use the
+ // safe fallback to use the topmost beforeChild container.
+ beforeChild = beforeDescendantContainer;
+ }
+
+ addChild(newChild, beforeChild);
+}
+
+void LayoutBlock::addChild(LayoutObject* newChild, LayoutObject* beforeChild)
+{
+ if (beforeChild && beforeChild->parent() != this) {
+ addChildBeforeDescendant(newChild, beforeChild);
+ return;
}
bool madeBoxesNonInline = false;

Powered by Google App Engine
This is Rietveld 408576698