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

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  * Copyright (C) 2004, 2005, 2006, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2010 Google Inc. All rights reserved.
  * Copyright (C) 2011 Motorola Mobility, Inc.  All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 32 matching lines @@
 #include "core/style/ComputedStyle.h"
 #include "wtf/Vector.h"
 #include "wtf/text/StringBuilder.h"
 
 namespace blink {
 
 using namespace HTMLNames;
 
 HTMLOptionElement::HTMLOptionElement(Document& document)
     : HTMLElement(optionTag, document), m_isSelected(false) {
-  setHasCustomStyleCallbacks();
 }
 
 // An explicit empty destructor should be in HTMLOptionElement.cpp, because
 // if an implicit destructor is used or an empty destructor is defined in
 // HTMLOptionElement.h, when including HTMLOptionElement.h,
 // msvc tries to expand the destructor and causes
 // a compile error because of lack of ComputedStyle definition.
 HTMLOptionElement::~HTMLOptionElement() {}
 
 HTMLOptionElement* HTMLOptionElement::create(Document& document) {
@@ skipping 20 matching lines @@
     element->setValue(value);
   if (defaultSelected)
     element->setAttribute(selectedAttr, emptyAtom);
   element->setSelected(selected);
 
   return element;
 }
 
 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);
 }
 
 bool HTMLOptionElement::supportsFocus() const {
   HTMLSelectElement* select = ownerSelectElement();
   if (select && select->usesMenuList())
     return false;
   return HTMLElement::supportsFocus();
 }
 
@@ skipping 208 matching lines @@
     return label;
   return collectOptionInnerText()
       .stripWhiteSpace(isHTMLSpace<UChar>)
       .simplifyWhiteSpace(isHTMLSpace<UChar>);
 }
 
 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))
     return "    " + displayLabel();
   return displayLabel();
 }
 
 bool HTMLOptionElement::ownElementDisabled() const {
   return fastHasAttribute(disabledAttr);
 }
@@ skipping 66 matching lines @@
 }
 
 bool HTMLOptionElement::spatialNavigationFocused() const {
   HTMLSelectElement* select = ownerSelectElement();
   if (!select || !select->isFocused())
     return false;
   return select->spatialNavigationFocusedOption() == this;
 }
 
 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.
     Element* parent = parentElement();
     DCHECK(parent);
     if (isHTMLOptGroupElement(*parent)) {
       const ComputedStyle* parentStyle = parent->computedStyle()
                                              ? parent->computedStyle()
                                              : parent->ensureComputedStyle();
       return !parentStyle || parentStyle->display() == EDisplay::None;
     }
   }
-  return m_style->display() == EDisplay::None;
+  return style->display() == EDisplay::None;
 }
 
 String HTMLOptionElement::innerText() {
   // A workaround for crbug.com/424578. We add ShadowRoot to an OPTION, but
   // innerText behavior for Shadow DOM is unclear.  We just return the same
   // string before adding ShadowRoot.
   return textContent();
 }
 
 }  // namespace blink