Support display: contents for elements, first-line and first-letter pseudos.

third_party/WebKit/Source/core/dom/Element.cpp

Index: third_party/WebKit/Source/core/dom/Element.cpp
diff --git a/third_party/WebKit/Source/core/dom/Element.cpp b/third_party/WebKit/Source/core/dom/Element.cpp
index 741ab09deb5911caef748d874430ff864557ec97..913df166d94bd8bcf94ec2d50f10d93f3ea13abb 100644
--- a/third_party/WebKit/Source/core/dom/Element.cpp
+++ b/third_party/WebKit/Source/core/dom/Element.cpp
@@ -1529,7 +1529,8 @@ const AtomicString Element::imageSourceURL() const {
 }
 
 bool Element::layoutObjectIsNeeded(const ComputedStyle& style) {
-  return style.display() != EDisplay::None;
+  return style.display() != EDisplay::None &&
+         style.display() != EDisplay::Contents;
 }
 
 LayoutObject* Element::createLayoutObject(const ComputedStyle& style) {
@@ -1667,9 +1668,15 @@ void Element::attachLayoutTree(const AttachContext& context) {
     data->clearComputedStyle();
   }
 
-  if (!isActiveSlotOrActiveInsertionPoint())
-    LayoutTreeBuilderForElement(*this, context.resolvedStyle)
-        .createLayoutObjectIfNeeded();
+  if (!isActiveSlotOrActiveInsertionPoint()) {
+    LayoutTreeBuilderForElement builder(*this, context.resolvedStyle);
+    builder.createLayoutObjectIfNeeded();
+
+    if (ComputedStyle* style = builder.resolvedStyle()) {
+      if (!layoutObject() && shouldStoreNonLayoutObjectComputedStyle(*style))
+        storeNonLayoutObjectComputedStyle(style);
+    }
+  }
 
   addCallbackSelectors();
 
@@ -1692,6 +1699,25 @@ void Element::attachLayoutTree(const AttachContext& context) {
 
   ContainerNode::attachLayoutTree(context);
 
+  // Quite unfortunately, if we're a display: contents node reattaching our
+  // subtree we need to reattach the parent's *after* attaching all our
+  // children. We might be able to be more granular here, but otherwise we don't
+  // always properly rebuild the layout tree due to anonymous boxes, for
+  // example. See css-display-3/display-contents-reattachment.html, which fails
+  // without this, for example, due to misplaced flex anonymous blocks.
+  //
+  // Also, don't do it if we're a pseudo-element, because it's pointless and
+  // causes infinite recursion if the parent doesn't have display: contents.
+  //
+  // Finally, don't do it also if our parent is the document, since it's also
+  // pointless, and it asserts when detached.
+  if (hasDisplayContentsStyle() && !isPseudoElement() && parentNode() &&
+      parentNode() != &document() && !parentNode()->needsAttach()) {
+    parentNode()->reattachLayoutTree();

[rune, 2016/11/18 10:35:40]

This is not good. So if you have N display:contents siblings needing
reattachment you reattach the parent and all of the other descendants N times as
well? Also, going back to the parent like this breaks contracts. There must be a
better way.

I think you should go back to writing a design document and distribute that to a
wider audience (style-dev@).

[emilio, 2016/11/18 10:54:35]

Yes, this is the single most nasty thing I've had to write, and it's almost
never necessary (I've only identified the issue with that single flex test
case).

Couldn't think about anything better at the moment, but yeah, I'll try to
diagnose it a bit better and send a mail to see if other people can come up with
anything better.

[emilio, 2016/11/18 11:04:43]

Also note that this is not as bad as you said (sorry for eagerly replying
before).

Note that we only do this if the display: contents sibling is the _root_ of the
attachment, so if you have n display: contents siblings and you are reattaching
the parent, you attach each one only once.

If you're attaching a display: contents node that has only display: contents
siblings you're reattaching the first node's children once, then everyone else
just once.

The worst case for this is when there are multiple nested display: contents
nodes, where, with this code, we go up in the tree re-attaching the world, but
that's avoidable too if we just re-attach from the non-display: contents parent
directly once.

+    DCHECK(!needsAttach());
+    return;
+  }
+
   createPseudoElementIfNeeded(PseudoIdAfter);
   createPseudoElementIfNeeded(PseudoIdBackdrop);
 
@@ -1964,6 +1990,9 @@ StyleRecalcChange Element::recalcOwnStyle(StyleRecalcChange change) {
       // https://codereview.chromium.org/30453002/
       layoutObject->setStyleInternal(newStyle.get());
     }
+  } else if (localChange != NoChange &&
+             shouldStoreNonLayoutObjectComputedStyle(*newStyle)) {
+    storeNonLayoutObjectComputedStyle(newStyle);
   }
 
   if (getStyleChangeType() >= SubtreeStyleChange)
@@ -3126,6 +3155,37 @@ const ComputedStyle* Element::ensureComputedStyle(
   return elementStyle->addCachedPseudoStyle(result.release());
 }
 
+const ComputedStyle* Element::nonLayoutObjectComputedStyle() const {
+  if (layoutObject() || !hasRareData())
+    return nullptr;
+
+  return elementRareData()->computedStyle();
+}
+
+bool Element::hasDisplayContentsStyle() const {
+  if (const ComputedStyle* style = nonLayoutObjectComputedStyle())
+    return style->display() == EDisplay::Contents;
+  return false;
+}
+
+bool Element::shouldStoreNonLayoutObjectComputedStyle(
+    const ComputedStyle& style) const {
+#if DCHECK_IS_ON()
+  if (style.display() == EDisplay::Contents)
+    DCHECK(!layoutObject());
+#endif
+
+  return style.display() == EDisplay::Contents ||
+         isHTMLOptGroupElement(*this) || isHTMLOptionElement(*this);
+}
+
+void Element::storeNonLayoutObjectComputedStyle(
+    PassRefPtr<ComputedStyle> style) {
+  DCHECK(style);
+  DCHECK(shouldStoreNonLayoutObjectComputedStyle(*style));
+  ensureElementRareData().setComputedStyle(std::move(style));
+}
+
 AtomicString Element::computeInheritedLanguage() const {
   const Node* n = this;
   AtomicString value;
@@ -3859,6 +3919,7 @@ void Element::inlineStyleChanged() {
                                             StyleChangeReason::Inline));
   DCHECK(elementData());
   elementData()->m_styleAttributeIsDirty = true;
+
   InspectorInstrumentation::didInvalidateStyleAttr(this);
 
   if (MutationObserverInterestGroup* recipients =