Clear pseudoElement from ElementRareData when FirstLetterPseudoElement doesn't have a LayoutObject

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 2044 matching lines @@
 void Element::rebuildShadowRootLayoutTree() {
   for (ShadowRoot* root = youngestShadowRoot(); root;
        root = root->olderShadowRoot()) {
     if (root->needsReattachLayoutTree() || root->childNeedsReattachLayoutTree())
       root->rebuildLayoutTree();
   }
 }
 
 void Element::reattachPseudoElementLayoutTree(PseudoId pseudoId) {
   if (PseudoElement* element = pseudoElement(pseudoId)) {
-    if (element->needsReattachLayoutTree() ||

[nainar, 2017/03/17 23:47:40]

Change as per discussed by esprehn@ and I in IRL: 

In the case of FirstLetter we need to mirror Element::updateFirstLetter(). We
have to delete the old first letter here to set the original text back on the
text layoutObject. If we dispose the old first letter after creating the new one
we will get incorrect results because we set the first letter back.

In the codepath before I landed codereview.chromium.org/2473743003 we would have
called updateFirstLetter but now we aren't. Calling reattachLayoutTree
unconditionally leads to bad state hence the call not having the correct
layoutObject information. This causes us to hit the DCHECK seen in the bug. 

Logging here for the record as I am OOO and will probably forget.

-        element->childNeedsReattachLayoutTree())
+    if (pseudoId == PseudoIdFirstLetter &&
+        !FirstLetterPseudoElement::firstLetterTextLayoutObject(*element))
+      elementRareData()->setPseudoElement(pseudoId, nullptr);
+    else if (element->needsReattachLayoutTree() ||

[esprehn, 2017/04/06 00:08:59]

I think we often add braces when things start to get cramped like this.

[nainar, 2017/04/06 00:20:45]

Done.

+             element->childNeedsReattachLayoutTree())
       element->rebuildLayoutTree();
   } else {
     createPseudoElementIfNeeded(pseudoId);
   }
 }
 
 void Element::updateCallbackSelectors(const ComputedStyle* oldStyle,
                                       const ComputedStyle* newStyle) {
   Vector<String> emptyVector;
   const Vector<String>& oldCallbackSelectors =
@@ skipping 2098 matching lines @@
 }
 
 DEFINE_TRACE_WRAPPERS(Element) {
   if (hasRareData()) {
     visitor->traceWrappers(elementRareData());
   }
   ContainerNode::traceWrappers(visitor);
 }
 
 }  // namespace blink