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

third_party/WebKit/Source/core/html/HTMLOptionElement.cpp

Index: third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
diff --git a/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp b/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
index b0816b8f7424dee271ddc1c3f15beac0aa2ea9b6..7e624fe3a9e334f9275f2ab81170e22aff58a57f 100644
--- a/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
@@ -50,7 +50,6 @@ using namespace HTMLNames;
 
 HTMLOptionElement::HTMLOptionElement(Document& document)
     : HTMLElement(optionTag, document), m_isSelected(false) {
-  setHasCustomStyleCallbacks();
 }
 
 // An explicit empty destructor should be in HTMLOptionElement.cpp, because
@@ -91,18 +90,17 @@ HTMLOptionElement* HTMLOptionElement::createForJSConstructor(
 
 void HTMLOptionElement::attachLayoutTree(const AttachContext& context) {
   AttachContext optionContext(context);
-  if (context.resolvedStyle) {
-    DCHECK(!m_style || m_style == context.resolvedStyle);
-    m_style = context.resolvedStyle;
-  } else if (parentComputedStyle()) {
-    updateNonComputedStyle();
-    optionContext.resolvedStyle = m_style.get();
+  RefPtr<ComputedStyle> resolvedStyle;
+  if (!context.resolvedStyle && parentComputedStyle()) {

[esprehn, 2017/03/08 03:42:38]

This code doesn't look right, the call below to attachLayoutTree will compute
the style for you. There's no reason to call originalStyleForLayoutObject()
manually like this.

Why do we need to call updateListOnLayoutObject() when the resolvedStyle is
null?

Note that resolvedStyle is just a cache optimization, but you're using it to
call something with side effects on the ownerSelect which doesn't seem right. We
can enter this function with or without resolvedStyle, what state are you using
it as a proxy for?

[emilio, 2017/03/08 08:32:40]

Note that this chunk of the code is written to preserve the same behavior as
before (the updateListOnLayoutObject() call was just inside
updateNonComputedStyle()).

I think the intention of the old code may just be to avoid an unneeded style
recalc from calling updateNonComputedStyle.

I'll try to simplify this code and submit a CL for you to review.

+    if (HTMLSelectElement* select = ownerSelectElement())
+      select->updateListOnLayoutObject();
+    resolvedStyle = originalStyleForLayoutObject();
+    optionContext.resolvedStyle = resolvedStyle.get();
   }
   HTMLElement::attachLayoutTree(optionContext);
 }
 
 void HTMLOptionElement::detachLayoutTree(const AttachContext& context) {
-  m_style.clear();
   HTMLElement::detachLayoutTree(context);
 }
 
@@ -331,21 +329,6 @@ void HTMLOptionElement::setLabel(const AtomicString& label) {
   setAttribute(labelAttr, label);
 }
 
-void HTMLOptionElement::updateNonComputedStyle() {
-  m_style = originalStyleForLayoutObject();
-  if (HTMLSelectElement* select = ownerSelectElement())
-    select->updateListOnLayoutObject();
-}
-
-ComputedStyle* HTMLOptionElement::nonLayoutObjectComputedStyle() const {
-  return m_style.get();
-}
-
-PassRefPtr<ComputedStyle> HTMLOptionElement::customStyleForLayoutObject() {
-  updateNonComputedStyle();
-  return m_style;
-}
-
 String HTMLOptionElement::textIndentedToRespectGroupLabel() const {
   ContainerNode* parent = parentNode();
   if (parent && isHTMLOptGroupElement(*parent))
@@ -432,12 +415,13 @@ bool HTMLOptionElement::spatialNavigationFocused() const {
 }
 
 bool HTMLOptionElement::isDisplayNone() const {
-  // If m_style is not set, then the node is still unattached.
+  // If the style is not set, then the node is still unattached.
   // We have to wait till it gets attached to read the display property.
-  if (!m_style)
+  const ComputedStyle* style = nonLayoutObjectComputedStyle();
+  if (!style)
     return false;
 
-  if (m_style->display() != EDisplay::None) {
+  if (style->display() != EDisplay::None) {
     // We need to check the parent's display property.  Parent's
     // display:none doesn't override children's display properties in
     // ComputedStyle.
@@ -450,7 +434,7 @@ bool HTMLOptionElement::isDisplayNone() const {
       return !parentStyle || parentStyle->display() == EDisplay::None;
     }
   }
-  return m_style->display() == EDisplay::None;
+  return style->display() == EDisplay::None;
 }
 
 String HTMLOptionElement::innerText() {