- Revision
- 132684
- Author
- [email protected]
- Date
- 2012-10-26 12:28:43 -0700 (Fri, 26 Oct 2012)
Log Message
Remove setRenderStyle in favor of callbacks on HTMLOptionElement and HTMLOptGroupElement
https://bugs.webkit.org/show_bug.cgi?id=100397
Patch by Elliott Sprehn <[email protected]> on 2012-10-26
Reviewed by Ojan Vafai.
Use Node custom callbacks to support the non-renderer style caching for option
and optgroup eliminating the need for setRenderStyle.
setRenderStyle only existed to support HTMLOptionElement and HTMLOptGroupElement
so they could store their RenderStyle even though they have no renderer. This
means all style setting went through the virtual call to setRenderStyle, and it
also hid the if statement protecting against null renderers meaning we end up
checking if the renderer is null repeatedly in recalcStyle. This refactor cleans
up recalcStyle to be more clear about what's going on.
No new tests needed, this is just a refactor.
* dom/Element.cpp:
(WebCore::Element::pseudoStyleCacheIsInvalid):
(WebCore::Element::recalcStyle):
* dom/Node.cpp:
(WebCore::Node::createRenderer):
(WebCore::Node::setRenderStyle):
Removed this method because it was only here to support HTMLOptionElement
and HTMLOptGroupElement. Instead we can use node custom callbacks.
* dom/Node.h:
(WebCore::Node::nonRendererStyle): Renamed from nonRendererRenderStyle to match other style methods.
* dom/NodeRenderStyle.h:
(WebCore::Node::renderStyle):
* html/HTMLOptGroupElement.cpp:
(WebCore::HTMLOptGroupElement::HTMLOptGroupElement):
(WebCore::HTMLOptGroupElement::attach): Reorder the logic to avoid calling styleForRenderer twice.
(WebCore::HTMLOptGroupElement::updateNonRenderStyle): Updates the cached non-renderer style.
(WebCore::HTMLOptGroupElement::nonRendererStyle):
(WebCore::HTMLOptGroupElement::customStyleForRenderer):
* html/HTMLOptGroupElement.h:
* html/HTMLOptionElement.cpp:
(WebCore::HTMLOptionElement::HTMLOptionElement):
(WebCore::HTMLOptionElement::attach): Reorder the logic to avoid calling styleForRenderer twice.
(WebCore::HTMLOptionElement::updateNonRenderStyle): Updates the cached non-renderer style.
(WebCore::HTMLOptionElement::nonRendererStyle):
(WebCore::HTMLOptionElement::customStyleForRenderer):
(WebCore::HTMLOptionElement::didRecalcStyle): Requests the repaint of the select like setRenderStyle used to.
* html/HTMLOptionElement.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (132683 => 132684)
--- trunk/Source/WebCore/ChangeLog 2012-10-26 19:15:39 UTC (rev 132683)
+++ trunk/Source/WebCore/ChangeLog 2012-10-26 19:28:43 UTC (rev 132684)
@@ -1,3 +1,50 @@
+2012-10-26 Elliott Sprehn <[email protected]>
+
+ Remove setRenderStyle in favor of callbacks on HTMLOptionElement and HTMLOptGroupElement
+ https://bugs.webkit.org/show_bug.cgi?id=100397
+
+ Reviewed by Ojan Vafai.
+
+ Use Node custom callbacks to support the non-renderer style caching for option
+ and optgroup eliminating the need for setRenderStyle.
+
+ setRenderStyle only existed to support HTMLOptionElement and HTMLOptGroupElement
+ so they could store their RenderStyle even though they have no renderer. This
+ means all style setting went through the virtual call to setRenderStyle, and it
+ also hid the if statement protecting against null renderers meaning we end up
+ checking if the renderer is null repeatedly in recalcStyle. This refactor cleans
+ up recalcStyle to be more clear about what's going on.
+
+ No new tests needed, this is just a refactor.
+
+ * dom/Element.cpp:
+ (WebCore::Element::pseudoStyleCacheIsInvalid):
+ (WebCore::Element::recalcStyle):
+ * dom/Node.cpp:
+ (WebCore::Node::createRenderer):
+ (WebCore::Node::setRenderStyle):
+ Removed this method because it was only here to support HTMLOptionElement
+ and HTMLOptGroupElement. Instead we can use node custom callbacks.
+ * dom/Node.h:
+ (WebCore::Node::nonRendererStyle): Renamed from nonRendererRenderStyle to match other style methods.
+ * dom/NodeRenderStyle.h:
+ (WebCore::Node::renderStyle):
+ * html/HTMLOptGroupElement.cpp:
+ (WebCore::HTMLOptGroupElement::HTMLOptGroupElement):
+ (WebCore::HTMLOptGroupElement::attach): Reorder the logic to avoid calling styleForRenderer twice.
+ (WebCore::HTMLOptGroupElement::updateNonRenderStyle): Updates the cached non-renderer style.
+ (WebCore::HTMLOptGroupElement::nonRendererStyle):
+ (WebCore::HTMLOptGroupElement::customStyleForRenderer):
+ * html/HTMLOptGroupElement.h:
+ * html/HTMLOptionElement.cpp:
+ (WebCore::HTMLOptionElement::HTMLOptionElement):
+ (WebCore::HTMLOptionElement::attach): Reorder the logic to avoid calling styleForRenderer twice.
+ (WebCore::HTMLOptionElement::updateNonRenderStyle): Updates the cached non-renderer style.
+ (WebCore::HTMLOptionElement::nonRendererStyle):
+ (WebCore::HTMLOptionElement::customStyleForRenderer):
+ (WebCore::HTMLOptionElement::didRecalcStyle): Requests the repaint of the select like setRenderStyle used to.
+ * html/HTMLOptionElement.h:
+
2012-10-26 Dirk Schulze <[email protected]>
-webkit-clip-path property should just reference clipPath
Modified: trunk/Source/WebCore/dom/Element.cpp (132683 => 132684)
--- trunk/Source/WebCore/dom/Element.cpp 2012-10-26 19:15:39 UTC (rev 132683)
+++ trunk/Source/WebCore/dom/Element.cpp 2012-10-26 19:28:43 UTC (rev 132684)
@@ -1068,8 +1068,9 @@
bool Element::pseudoStyleCacheIsInvalid(const RenderStyle* currentStyle, RenderStyle* newStyle)
{
ASSERT(currentStyle == renderStyle());
+ ASSERT(renderer());
- if (!renderer() || !currentStyle)
+ if (!currentStyle)
return false;
const PseudoStyleCache* pseudoStyleCache = currentStyle->cachedPseudoStyles();
@@ -1119,7 +1120,7 @@
return;
}
- // Ref currentStyle in case it would otherwise be deleted when setRenderStyle() is called.
+ // Ref currentStyle in case it would otherwise be deleted when setting the new style in the renderer.
RefPtr<RenderStyle> currentStyle(renderStyle());
bool hasParentStyle = parentNodeForRenderingAndStyle() ? static_cast<bool>(parentNodeForRenderingAndStyle()->renderStyle()) : false;
bool hasDirectAdjacentRules = currentStyle && currentStyle->childrenAffectedByDirectAdjacentRules();
@@ -1168,17 +1169,15 @@
newStyle->setChildrenAffectedByDirectAdjacentRules();
}
- if (ch != NoChange || pseudoStyleCacheIsInvalid(currentStyle.get(), newStyle.get()) || (change == Force && renderer() && renderer()->requiresForcedStyleRecalcPropagation())) {
- setRenderStyle(newStyle);
- } else if (needsStyleRecalc() && styleChangeType() != SyntheticStyleChange) {
- // 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.
- if (renderer())
- renderer()->setStyleInternal(newStyle.get());
- else
- setRenderStyle(newStyle);
- } else if (styleChangeType() == SyntheticStyleChange)
- setRenderStyle(newStyle);
+ if (RenderObject* renderer = this->renderer()) {
+ if (ch != NoChange || pseudoStyleCacheIsInvalid(currentStyle.get(), newStyle.get()) || (change == Force && renderer->requiresForcedStyleRecalcPropagation()) || styleChangeType() == SyntheticStyleChange)
+ renderer->setAnimatableStyle(newStyle.get());
+ else if (needsStyleRecalc()) {
+ // 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.
+ renderer->setStyleInternal(newStyle.get());
+ }
+ }
// If "rem" units are used anywhere in the document, and if the document element's font size changes, then go ahead and force font updating
// all the way down the tree. This is simpler than having to maintain a cache of objects (and such font size changes should be rare anyway).
Modified: trunk/Source/WebCore/dom/Node.cpp (132683 => 132684)
--- trunk/Source/WebCore/dom/Node.cpp 2012-10-26 19:15:39 UTC (rev 132683)
+++ trunk/Source/WebCore/dom/Node.cpp 2012-10-26 19:28:43 UTC (rev 132684)
@@ -1416,18 +1416,7 @@
ASSERT_NOT_REACHED();
return 0;
}
-
-RenderStyle* Node::nonRendererRenderStyle() const
-{
- return 0;
-}
-void Node::setRenderStyle(PassRefPtr<RenderStyle> s)
-{
- if (m_renderer)
- m_renderer->setAnimatableStyle(s);
-}
-
RenderStyle* Node::virtualComputedStyle(PseudoId pseudoElementSpecifier)
{
return parentOrHostNode() ? parentOrHostNode()->computedStyle(pseudoElementSpecifier) : 0;
Modified: trunk/Source/WebCore/dom/Node.h (132683 => 132684)
--- trunk/Source/WebCore/dom/Node.h 2012-10-26 19:15:39 UTC (rev 132683)
+++ trunk/Source/WebCore/dom/Node.h 2012-10-26 19:28:43 UTC (rev 132684)
@@ -529,7 +529,6 @@
// Wrapper for nodes that don't have a renderer, but still cache the style (like HTMLOptionElement).
RenderStyle* renderStyle() const;
- virtual void setRenderStyle(PassRefPtr<RenderStyle>);
RenderStyle* computedStyle(PseudoId pseudoElementSpecifier = NOPSEUDO) { return virtualComputedStyle(pseudoElementSpecifier); }
@@ -783,7 +782,7 @@
virtual OwnPtr<NodeRareData> createRareData();
bool rareDataFocused() const;
- virtual RenderStyle* nonRendererRenderStyle() const;
+ virtual RenderStyle* nonRendererStyle() const { return 0; }
virtual const AtomicString& virtualPrefix() const;
virtual const AtomicString& virtualLocalName() const;
Modified: trunk/Source/WebCore/dom/NodeRenderStyle.h (132683 => 132684)
--- trunk/Source/WebCore/dom/NodeRenderStyle.h 2012-10-26 19:15:39 UTC (rev 132683)
+++ trunk/Source/WebCore/dom/NodeRenderStyle.h 2012-10-26 19:28:43 UTC (rev 132684)
@@ -35,9 +35,9 @@
{
// Using a ternary here confuses the Solaris Studio 12/12.1/12.2 compilers:
// Bug is CR 6569194, "Problem with question operator binding in inline function"
- if (m_renderer)
+ if (m_renderer)
return m_renderer->style();
- return nonRendererRenderStyle();
+ return nonRendererStyle();
}
}
Modified: trunk/Source/WebCore/html/HTMLOptGroupElement.cpp (132683 => 132684)
--- trunk/Source/WebCore/html/HTMLOptGroupElement.cpp 2012-10-26 19:15:39 UTC (rev 132683)
+++ trunk/Source/WebCore/html/HTMLOptGroupElement.cpp 2012-10-26 19:28:43 UTC (rev 132684)
@@ -42,6 +42,7 @@
: HTMLElement(tagName, document)
{
ASSERT(hasTagName(optgroupTag));
+ setHasCustomCallbacks();
}
PassRefPtr<HTMLOptGroupElement> HTMLOptGroupElement::create(const QualifiedName& tagName, Document* document)
@@ -94,9 +95,12 @@
void HTMLOptGroupElement::attach()
{
- if (parentNode()->renderStyle())
- setRenderStyle(styleForRenderer());
HTMLElement::attach();
+ // If after attaching nothing called styleForRenderer() on this node we
+ // manually cache the value. This happens if our parent doesn't have a
+ // renderer like <optgroup> or if it doesn't allow children like <select>.
+ if (!m_style && parentNode()->renderStyle())
+ updateNonRenderStyle();
}
void HTMLOptGroupElement::detach()
@@ -105,16 +109,24 @@
HTMLElement::detach();
}
-void HTMLOptGroupElement::setRenderStyle(PassRefPtr<RenderStyle> newStyle)
+void HTMLOptGroupElement::updateNonRenderStyle()
{
- m_style = newStyle;
+ m_style = document()->styleResolver()->styleForElement(this);
}
-
-RenderStyle* HTMLOptGroupElement::nonRendererRenderStyle() const
-{
- return m_style.get();
+
+RenderStyle* HTMLOptGroupElement::nonRendererStyle() const
+{
+ return m_style.get();
}
+PassRefPtr<RenderStyle> HTMLOptGroupElement::customStyleForRenderer()
+{
+ // styleForRenderer is called whenever a new style should be associated
+ // with an Element so now is a good time to update our cached style.
+ updateNonRenderStyle();
+ return m_style;
+}
+
String HTMLOptGroupElement::groupLabelText() const
{
String itemText = document()->displayStringModifiedByEncoding(getAttribute(labelAttr));
Modified: trunk/Source/WebCore/html/HTMLOptGroupElement.h (132683 => 132684)
--- trunk/Source/WebCore/html/HTMLOptGroupElement.h 2012-10-26 19:15:39 UTC (rev 132683)
+++ trunk/Source/WebCore/html/HTMLOptGroupElement.h 2012-10-26 19:28:43 UTC (rev 132684)
@@ -50,14 +50,16 @@
virtual bool rendererIsNeeded(const NodeRenderingContext&) { return false; }
virtual void attach();
virtual void detach();
- virtual void setRenderStyle(PassRefPtr<RenderStyle>);
virtual void childrenChanged(bool changedByParser = false, Node* beforeChange = 0, Node* afterChange = 0, int childCountDelta = 0);
virtual void accessKeyAction(bool sendMouseEvents);
-
- virtual RenderStyle* nonRendererRenderStyle() const;
-
+
+ // <optgroup> never has a renderer so we manually manage a cached style.
+ void updateNonRenderStyle();
+ virtual RenderStyle* nonRendererStyle() const OVERRIDE;
+ virtual PassRefPtr<RenderStyle> customStyleForRenderer() OVERRIDE;
+
void recalcSelectOptions();
RefPtr<RenderStyle> m_style;
Modified: trunk/Source/WebCore/html/HTMLOptionElement.cpp (132683 => 132684)
--- trunk/Source/WebCore/html/HTMLOptionElement.cpp 2012-10-26 19:15:39 UTC (rev 132683)
+++ trunk/Source/WebCore/html/HTMLOptionElement.cpp 2012-10-26 19:28:43 UTC (rev 132684)
@@ -55,6 +55,7 @@
, m_isSelected(false)
{
ASSERT(hasTagName(optionTag));
+ setHasCustomCallbacks();
}
PassRefPtr<HTMLOptionElement> HTMLOptionElement::create(Document* document)
@@ -90,9 +91,12 @@
void HTMLOptionElement::attach()
{
- if (parentNode()->renderStyle())
- setRenderStyle(styleForRenderer());
HTMLElement::attach();
+ // If after attaching nothing called styleForRenderer() on this node we
+ // manually cache the value. This happens if our parent doesn't have a
+ // renderer like <optgroup> or if it doesn't allow children like <select>.
+ if (!m_style && parentNode()->renderStyle())
+ updateNonRenderStyle();
}
void HTMLOptionElement::detach()
@@ -305,20 +309,34 @@
setAttribute(labelAttr, label);
}
-void HTMLOptionElement::setRenderStyle(PassRefPtr<RenderStyle> newStyle)
+void HTMLOptionElement::updateNonRenderStyle()
{
- m_style = newStyle;
+ m_style = document()->styleResolver()->styleForElement(this);
+}
+
+RenderStyle* HTMLOptionElement::nonRendererStyle() const
+{
+ return m_style.get();
+}
+
+PassRefPtr<RenderStyle> HTMLOptionElement::customStyleForRenderer()
+{
+ // styleForRenderer is called whenever a new style should be associated
+ // with an Element so now is a good time to update our cached style.
+ updateNonRenderStyle();
+ return m_style;
+}
+
+void HTMLOptionElement::didRecalcStyle(StyleChange)
+{
+ // FIXME: This is nasty, we ask our owner select to repaint even if the new
+ // style is exactly the same.
if (HTMLSelectElement* select = ownerSelectElement()) {
if (RenderObject* renderer = select->renderer())
renderer->repaint();
}
}
-RenderStyle* HTMLOptionElement::nonRendererRenderStyle() const
-{
- return m_style.get();
-}
-
String HTMLOptionElement::textIndentedToRespectGroupLabel() const
{
ContainerNode* parent = parentNode();
Modified: trunk/Source/WebCore/html/HTMLOptionElement.h (132683 => 132684)
--- trunk/Source/WebCore/html/HTMLOptionElement.h 2012-10-26 19:15:39 UTC (rev 132683)
+++ trunk/Source/WebCore/html/HTMLOptionElement.h 2012-10-26 19:28:43 UTC (rev 132684)
@@ -75,7 +75,6 @@
virtual bool rendererIsNeeded(const NodeRenderingContext&) { return false; }
virtual void attach();
virtual void detach();
- virtual void setRenderStyle(PassRefPtr<RenderStyle>);
virtual void parseAttribute(const Attribute&) OVERRIDE;
@@ -84,8 +83,13 @@
virtual void childrenChanged(bool changedByParser = false, Node* beforeChange = 0, Node* afterChange = 0, int childCountDelta = 0);
- virtual RenderStyle* nonRendererRenderStyle() const;
+ // <option> never has a renderer so we manually manage a cached style.
+ void updateNonRenderStyle();
+ virtual RenderStyle* nonRendererStyle() const OVERRIDE;
+ virtual PassRefPtr<RenderStyle> customStyleForRenderer() OVERRIDE;
+ void didRecalcStyle(StyleChange) OVERRIDE;
+
String collectOptionInnerText() const;
String m_value;
