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

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Peter Kelly (pmk@post.com)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2007 David Smith (catfish.man@gmail.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2012, 2013 Apple Inc.
  * All rights reserved.
  *           (C) 2007 Eric Seidel (eric@webkit.org)
  *
@@ skipping 1511 matching lines @@
     return parent->locateNamespacePrefix(namespaceToLocate);
 
   return nullAtom;
 }
 
 const AtomicString Element::imageSourceURL() const {
   return getAttribute(srcAttr);
 }
 
 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) {
   return LayoutObject::createObject(this, style);
 }
 
 Node::InsertionNotificationRequest Element::insertedInto(
     ContainerNode* insertionPoint) {
   // need to do superclass processing first so isConnected() is true
   // by the time we reach updateId
@@ skipping 117 matching lines @@
 void Element::attachLayoutTree(const AttachContext& context) {
   DCHECK(document().inStyleRecalc());
 
   // We've already been through detach when doing an attach, but we might
   // need to clear any state that's been added since then.
   if (hasRareData() && getStyleChangeType() == NeedsReattachStyleChange) {
     ElementRareData* data = elementRareData();
     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();
 
   if (hasRareData() && !layoutObject()) {
     if (ElementAnimations* elementAnimations =
             elementRareData()->elementAnimations()) {
       elementAnimations->cssAnimations().cancel();
       elementAnimations->setAnimationStyleChange(false);
     }
   }
 
   SelectorFilterParentScope filterScope(*this);
   StyleSharingDepthScope sharingScope(*this);
 
   createPseudoElementIfNeeded(PseudoIdBefore);
 
   // When a shadow root exists, it does the work of attaching the children.
   if (ElementShadow* shadow = this->shadow())
     shadow->attach(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);
 
   // We create the first-letter element after the :before, :after and
   // children are attached because the first letter text could come
   // from any of them.
   createPseudoElementIfNeeded(PseudoIdFirstLetter);
 }
 
 void Element::detachLayoutTree(const AttachContext& context) {
@@ skipping 252 matching lines @@
         pseudoStyleCacheIsInvalid(oldStyle.get(), newStyle.get())) {
       layoutObject->setStyle(newStyle.get());
     } else {
       // Although no change occurred, we use the new style so that the cousin
       // style sharing code won't get fooled into believing this style is the
       // same.
       // FIXME: We may be able to remove this hack, see discussion in
       // https://codereview.chromium.org/30453002/
       layoutObject->setStyleInternal(newStyle.get());
     }
+  } else if (localChange != NoChange &&
+             shouldStoreNonLayoutObjectComputedStyle(*newStyle)) {
+    storeNonLayoutObjectComputedStyle(newStyle);
   }
 
   if (getStyleChangeType() >= SubtreeStyleChange)
     return Force;
 
   if (change > Inherit || localChange > Inherit)
     return max(localChange, change);
 
   if (localChange < IndependentInherit) {
     if (oldStyle->hasChildDependentFlags()) {
@@ skipping 1142 matching lines @@
 
   RefPtr<ComputedStyle> result =
       document().ensureStyleResolver().pseudoStyleForElement(
           this, PseudoStyleRequest(pseudoElementSpecifier,
                                    PseudoStyleRequest::ForComputedStyle),
           elementStyle);
   DCHECK(result);
   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;
   // The language property is inherited, so we iterate over the parents to find
   // the first language.
   do {
     if (n->isElementNode()) {
       if (const ElementData* elementData = toElement(n)->elementData()) {
         AttributeCollection attributes = elementData->attributes();
         // Spec: xml:lang takes precedence -- http://www.w3.org/TR/xhtml1/#C_7
@@ skipping 713 matching lines @@
       StyleChangeReasonForTracing::create(StyleChangeReason::StyleSheetChange));
   InspectorInstrumentation::didInvalidateStyleAttr(this);
 }
 
 void Element::inlineStyleChanged() {
   DCHECK(isStyledElement());
   setNeedsStyleRecalc(LocalStyleChange, StyleChangeReasonForTracing::create(
                                             StyleChangeReason::Inline));
   DCHECK(elementData());
   elementData()->m_styleAttributeIsDirty = true;
+
   InspectorInstrumentation::didInvalidateStyleAttr(this);
 
   if (MutationObserverInterestGroup* recipients =
           MutationObserverInterestGroup::createForAttributesMutation(
               *this, styleAttr)) {
     // We don't use getAttribute() here to get a style attribute value
     // before the change.
     AtomicString oldValue;
     if (const Attribute* attribute =
             elementData()->attributes().find(styleAttr))
@@ skipping 212 matching lines @@
 }
 
 DEFINE_TRACE_WRAPPERS(Element) {
   if (hasRareData()) {
     visitor->traceWrappers(elementRareData());
   }
   ContainerNode::traceWrappers(visitor);
 }
 
 }  // namespace blink