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

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights
  * reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
@@ skipping 236 matching lines @@
       parent.isLayoutTableCol() || parent.isFrameSet() ||
       parent.isFlexibleBox() || parent.isLayoutGrid() || parent.isSVGRoot() ||
       parent.isSVGContainer() || parent.isSVGImage() || parent.isSVGShape()) {
     return false;
   }
   return true;
 }
 
 bool Text::textLayoutObjectIsNeeded(const ComputedStyle& style,
                                     const LayoutObject& parent) const {
+  const LayoutObject* prev = nullptr;
+  bool lookedForPrev = false;
+
   if (!parent.canHaveChildren())
     return false;
 
   if (isEditingText())
     return true;
 
   if (!length())
     return false;
 
   if (style.display() == EDisplay::None)
     return false;
 
   if (!containsOnlyWhitespace())
     return true;
 
+  // We may have multiple text layout object siblings as part of a layout tree
+  // affected by display: contents. In the case we have something like:
+  //
+  //   <div>Foo<div style="display: contents"> </div>Bar</div>
+  //
+  // We can't just bail out and don't create a text layout object for the
+  // whitespace text node inside the display: contents element.
+  //
+  // We can check whether our style parent or our previous light-tree sibling
+  // has display contents to to avoid doing the previous layout object lookup

[rune, 2017/01/27 13:09:23]

double "to"

+  // more than necessary.
+  if (style.display() == EDisplay::Contents ||
+      (previousSibling() && previousSibling()->isElementNode() &&
+       toElement(previousSibling())->hasDisplayContentsStyle())) {
+    prev = LayoutTreeBuilderTraversal::previousSiblingLayoutObject(*this);
+    lookedForPrev = true;
+    if (prev && prev->node() && prev->node()->isTextNode())

[rune, 2017/01/27 13:09:23]

Not strictly true if the previous sibling is also a whitespace layout object?

We're already sub-optimal without display:content. I've reported
crbug.com/686016.

Did you fix whitespace attachment partially here to make more of the csswg tests
pass? It would probably be good to contain whitespace attachment fixes in a
separate CL?

[emilio, 2017/01/27 22:13:15]

Right, I guess we could avoid creating it if the previous sibling was also a
whitespace-only text node, if this is what you meant.
Yes, and yes, I can split this out into another CL.

+      return true;
+  }
+
   if (!canHaveWhitespaceChildren(parent))
     return false;
 
   // pre-wrap in SVG never makes layoutObject.
   if (style.whiteSpace() == EWhiteSpace::kPreWrap && parent.isSVG())
     return false;
 
   // pre/pre-wrap/pre-line always make layoutObjects.
   if (style.preserveNewline())
     return true;
 
   // childNeedsDistributionRecalc() here is rare, only happens JS calling
   // surroundContents() etc. from DOMNodeInsertedIntoDocument etc.
   if (document().childNeedsDistributionRecalc())
     return true;
 
   // Avoiding creation of a layoutObject for the text node is a non-essential
   // memory optimization.  So to avoid blowing up on very wide DOMs, we limit
   // the number of siblings to visit.
   unsigned maxSiblingsToVisit = 50;
 
-  const LayoutObject* prev =
-      LayoutTreeBuilderTraversal::previousSiblingLayoutObject(
-          *this, maxSiblingsToVisit);
+  if (!lookedForPrev) {
+    lookedForPrev = true;
+    prev = LayoutTreeBuilderTraversal::previousSiblingLayoutObject(
+        *this, maxSiblingsToVisit);
+  }
+
   if (prev && prev->isBR())  // <span><br/> <br/></span>
     return false;
 
   if (parent.isLayoutInline()) {
     // <span><div/> <div/></span>
     if (prev && !prev->isInline() && !prev->isOutOfFlowPositioned())
       return false;
   } else {
     if (parent.isLayoutBlock() && !parent.childrenInline() &&
         (!prev || !prev->isInline()))
@@ skipping 36 matching lines @@
   if (isSVGText(this))
     return new LayoutSVGInlineText(this, dataImpl());
 
   if (style.hasTextCombine())
     return new LayoutTextCombine(this, dataImpl());
 
   return new LayoutText(this, dataImpl());
 }
 
 void Text::attachLayoutTree(const AttachContext& context) {
-  if (ContainerNode* layoutParent = LayoutTreeBuilderTraversal::parent(*this)) {
-    if (LayoutObject* parentLayoutObject = layoutParent->layoutObject()) {
-      if (textLayoutObjectIsNeeded(*parentLayoutObject->style(),
-                                   *parentLayoutObject))
-        LayoutTreeBuilderForText(*this, parentLayoutObject)
-            .createLayoutObject();
+  ContainerNode* styleParent = LayoutTreeBuilderTraversal::parent(*this);
+  LayoutObject* parentLayoutObject =
+      LayoutTreeBuilderTraversal::parentLayoutObject(*this);
+
+  if (styleParent && parentLayoutObject) {
+    DCHECK(styleParent->computedStyle());
+    if (textLayoutObjectIsNeeded(*styleParent->computedStyle(),
+                                 *parentLayoutObject)) {
+      LayoutTreeBuilderForText(*this, parentLayoutObject,
+                               styleParent->mutableComputedStyle())

[rune, 2017/01/27 13:09:23]

Using mutableComputedStyle() here looks worrying, but I see that we need it to
be non-const to share ComputedStyle with the parent element.

[emilio, 2017/01/27 22:13:15]

Yes, I agree, though this was pre-existing (the previous code used
layoutObject->mutableStyleRef()).

+          .createLayoutObject();
     }
   }
   CharacterData::attachLayoutTree(context);
 }
 
 void Text::reattachLayoutTreeIfNeeded(const AttachContext& context) {
   bool layoutObjectIsNeeded = false;
-  ContainerNode* layoutParent = LayoutTreeBuilderTraversal::parent(*this);
-  if (layoutParent) {
-    if (LayoutObject* parentLayoutObject = layoutParent->layoutObject()) {
-      if (textLayoutObjectIsNeeded(*parentLayoutObject->style(),
-                                   *parentLayoutObject))
-        layoutObjectIsNeeded = true;
-    }
+  ContainerNode* styleParent = LayoutTreeBuilderTraversal::parent(*this);
+  LayoutObject* parentLayoutObject =
+      LayoutTreeBuilderTraversal::parentLayoutObject(*this);
+  if (styleParent && parentLayoutObject) {
+    DCHECK(styleParent->computedStyle());
+    layoutObjectIsNeeded = textLayoutObjectIsNeeded(
+        *styleParent->computedStyle(), *parentLayoutObject);
   }
 
   if (layoutObjectIsNeeded == !!layoutObject())
     return;
 
   // The following is almost the same as Node::reattachLayoutTree() except that
   // we create a layoutObject only if needed.  Not calling reattachLayoutTree()
   // to avoid repeated calls to Text::textLayoutObjectIsNeeded().
   AttachContext reattachContext(context);
   reattachContext.performingReattach = true;
 
   if (getStyleChangeType() < NeedsReattachStyleChange)
     detachLayoutTree(reattachContext);
-  if (layoutObjectIsNeeded)
-    LayoutTreeBuilderForText(*this, layoutParent->layoutObject())
+  if (layoutObjectIsNeeded) {
+    LayoutTreeBuilderForText(*this, parentLayoutObject,
+                             styleParent->mutableComputedStyle())
         .createLayoutObject();
+  }
   CharacterData::attachLayoutTree(reattachContext);
 }
 
 void Text::recalcTextStyle(StyleRecalcChange change, Text* nextTextSibling) {
   if (LayoutTextItem layoutItem = LayoutTextItem(this->layoutObject())) {
     if (change != NoChange || needsStyleRecalc())
       layoutItem.setStyle(document().ensureStyleResolver().styleForText(this));
     if (needsStyleRecalc())
       layoutItem.setText(dataImpl());
     clearNeedsStyleRecalc();
@@ skipping 51 matching lines @@
 
 Text* Text::cloneWithData(const String& data) {
   return create(document(), data);
 }
 
 DEFINE_TRACE(Text) {
   CharacterData::trace(visitor);
 }
 
 }  // namespace blink