Diff
Modified: trunk/Source/WebCore/ChangeLog (135667 => 135668)
--- trunk/Source/WebCore/ChangeLog 2012-11-25 08:01:28 UTC (rev 135667)
+++ trunk/Source/WebCore/ChangeLog 2012-11-25 16:53:44 UTC (rev 135668)
@@ -1,3 +1,75 @@
+2012-11-24 Antti Koivisto <[email protected]>
+
+ Make renderer construction less generic
+ https://bugs.webkit.org/show_bug.cgi?id=103175
+
+ Reviewed by Ojan Vafai.
+
+ The renderer construction code currently operates on Nodes and is very generic. In reality
+ only Element and Text nodes can have renderers and the Text case is much simpler.
+
+ This patch separates the Text and Element renderer construction paths and makes it statically
+ known that other Node types can't have renderers. Less generic code is less branchy and enables
+ further optimizations.
+
+ * dom/CharacterData.cpp:
+ (WebCore::CharacterData::parserAppendData):
+ (WebCore::CharacterData::setDataAndUpdate):
+ (WebCore):
+ (WebCore::CharacterData::rendererIsNeeded):
+ (WebCore::CharacterData::createRenderer):
+
+ Only Text subclass of CharacterData can have renderers.
+
+ * dom/CharacterData.h:
+ (CharacterData):
+ * dom/ContainerNode.h:
+ (WebCore::ContainerNode::childShouldCreateRenderer):
+
+ Move childShouldCreateRenderer from Node to ContainerNode.
+
+ (ContainerNode):
+ * dom/Element.cpp:
+ (WebCore::Element::rendererIsNeeded):
+ (WebCore):
+ (WebCore::Element::attach):
+ (WebCore::Element::childShouldCreateRenderer):
+ * dom/Element.h:
+
+ Move rendererIsNeeded and createRenderer from Node to Element.
+
+ (Element):
+ * dom/Node.cpp:
+ (WebCore::Node::attach):
+ (WebCore):
+ * dom/Node.h:
+ (Node):
+ * dom/NodeRenderingContext.cpp:
+ (WebCore::NodeRenderingContext::createRendererForElementIfNeeded):
+ (WebCore::NodeRenderingContext::createRendererForTextIfNeeded):
+
+ Separate the Element and Text renderer creation paths. Both are less branchy.
+ The Text path is much simpler and avoids a bunch of virtual calls.
+
+ (WebCore):
+ * dom/NodeRenderingContext.h:
+ (NodeRenderingContext):
+ * dom/Text.cpp:
+ (WebCore::Text::textRendererIsNeeded):
+ (WebCore::Text::createTextRendererIfNeeded):
+ (WebCore):
+ (WebCore::Text::createTextRenderer):
+ (WebCore::Text::attach):
+ (WebCore::Text::updateTextRenderer):
+ * dom/Text.h:
+
+ Add non-virtual Text specific functions.
+
+ (WebCore):
+ (Text):
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::clone):
+
2012-11-22 Ryosuke Niwa <[email protected]>
REGRESSION(r135493): HTMLCollection and DynamicNodeList have two vtable pointers
Modified: trunk/Source/WebCore/dom/CharacterData.cpp (135667 => 135668)
--- trunk/Source/WebCore/dom/CharacterData.cpp 2012-11-25 08:01:28 UTC (rev 135667)
+++ trunk/Source/WebCore/dom/CharacterData.cpp 2012-11-25 16:53:44 UTC (rev 135668)
@@ -32,6 +32,7 @@
#include "NodeRenderingContext.h"
#include "RenderText.h"
#include "StyleInheritedData.h"
+#include "Text.h"
#include "TextBreakIterator.h"
#include "WebCoreMemoryInstrumentation.h"
@@ -89,7 +90,10 @@
else
m_data.append(string.characters16() + offset, characterLengthLimit);
- updateRenderer(oldLength, 0);
+ ASSERT(!renderer() || isTextNode());
+ if (isTextNode())
+ toText(this)->updateTextRenderer(oldLength, 0);
+
document()->incDOMTreeVersion();
// We don't call dispatchModifiedEvent here because we don't want the
// parser to dispatch DOM mutation events.
@@ -193,7 +197,9 @@
String oldData = m_data;
m_data = newData;
- updateRenderer(offsetOfReplacedData, oldLength);
+ ASSERT(!renderer() || isTextNode());
+ if (isTextNode())
+ toText(this)->updateTextRenderer(offsetOfReplacedData, oldLength);
if (document()->frame())
document()->frame()->selection()->textWasReplaced(this, offsetOfReplacedData, oldLength, newLength);
@@ -202,14 +208,6 @@
dispatchModifiedEvent(oldData);
}
-void CharacterData::updateRenderer(unsigned offsetOfReplacedData, unsigned lengthOfReplacedData)
-{
- if ((!renderer() || !rendererIsNeeded(NodeRenderingContext(this, renderer()->style()))) && attached())
- reattach();
- else if (renderer())
- toRenderText(renderer())->setTextWithOffset(m_data.impl(), offsetOfReplacedData, lengthOfReplacedData);
-}
-
void CharacterData::dispatchModifiedEvent(const String& oldData)
{
#if ENABLE(MUTATION_OBSERVERS)
@@ -245,13 +243,6 @@
return static_cast<int>(length());
}
-bool CharacterData::rendererIsNeeded(const NodeRenderingContext& context)
-{
- if (!m_data || !length())
- return false;
- return Node::rendererIsNeeded(context);
-}
-
bool CharacterData::offsetInCharacters() const
{
return true;
Modified: trunk/Source/WebCore/dom/CharacterData.h (135667 => 135668)
--- trunk/Source/WebCore/dom/CharacterData.h 2012-11-25 08:01:28 UTC (rev 135667)
+++ trunk/Source/WebCore/dom/CharacterData.h 2012-11-25 16:53:44 UTC (rev 135668)
@@ -57,8 +57,6 @@
ASSERT(type == CreateOther || type == CreateText || type == CreateEditingText);
}
- virtual bool rendererIsNeeded(const NodeRenderingContext&);
-
void setDataWithoutUpdate(const String& data)
{
ASSERT(!data.isNull());
@@ -73,7 +71,6 @@
virtual int maxCharacterOffset() const;
virtual bool offsetInCharacters() const;
void setDataAndUpdate(const String&, unsigned offsetOfReplacedData, unsigned oldLength, unsigned newLength);
- void updateRenderer(unsigned offsetOfReplacedData, unsigned lengthOfReplacedData);
void checkCharDataOperation(unsigned offset, ExceptionCode&);
String m_data;
Modified: trunk/Source/WebCore/dom/ContainerNode.h (135667 => 135668)
--- trunk/Source/WebCore/dom/ContainerNode.h 2012-11-25 08:01:28 UTC (rev 135667)
+++ trunk/Source/WebCore/dom/ContainerNode.h 2012-11-25 16:53:44 UTC (rev 135668)
@@ -131,6 +131,8 @@
void disconnectDescendantFrames();
+ virtual bool childShouldCreateRenderer(const NodeRenderingContext&) const { return true; }
+
// More efficient versions of these two functions for the case where we are starting with a ContainerNode.
Node* traverseNextNode() const;
Node* traverseNextNode(const Node* stayWithin) const;
Modified: trunk/Source/WebCore/dom/Element.cpp (135667 => 135668)
--- trunk/Source/WebCore/dom/Element.cpp 2012-11-25 08:01:28 UTC (rev 135667)
+++ trunk/Source/WebCore/dom/Element.cpp 2012-11-25 16:53:44 UTC (rev 135668)
@@ -1097,6 +1097,11 @@
return srcAttr;
}
+bool Element::rendererIsNeeded(const NodeRenderingContext& context)
+{
+ return (document()->documentElement() == this) || (context.style()->display() != NONE);
+}
+
RenderObject* Element::createRenderer(RenderArena* arena, RenderStyle* style)
{
if (document()->documentElement() == this && style->display() == NONE) {
@@ -1184,12 +1189,18 @@
ContainerNode::removedFrom(insertionPoint);
}
+void Element::createRendererIfNeeded()
+{
+ NodeRenderingContext(this).createRendererForElementIfNeeded();
+}
+
void Element::attach()
{
suspendPostAttachCallbacks();
WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
createRendererIfNeeded();
+
StyleResolverParentPusher parentPusher(this);
if (parentElement() && parentElement()->isInCanvasSubtree())
@@ -2140,7 +2151,7 @@
if (childContext.node()->isSVGElement())
return childContext.node()->hasTagName(SVGNames::svgTag) || isSVGElement();
- return Node::childShouldCreateRenderer(childContext);
+ return ContainerNode::childShouldCreateRenderer(childContext);
}
#endif
@@ -2530,7 +2541,6 @@
return 0;
}
-
void Element::cloneAttributesFromElement(const Element& other)
{
if (hasSyntheticAttrChildNodes())
Modified: trunk/Source/WebCore/dom/Element.h (135667 => 135668)
--- trunk/Source/WebCore/dom/Element.h 2012-11-25 08:01:28 UTC (rev 135667)
+++ trunk/Source/WebCore/dom/Element.h 2012-11-25 16:53:44 UTC (rev 135668)
@@ -270,6 +270,7 @@
virtual void attach();
virtual void detach();
virtual RenderObject* createRenderer(RenderArena*, RenderStyle*);
+ virtual bool rendererIsNeeded(const NodeRenderingContext&);
void recalcStyle(StyleChange = NoChange);
ElementShadow* shadow() const;
@@ -425,7 +426,7 @@
virtual bool isSpellCheckingEnabled() const;
PassRefPtr<WebKitAnimationList> webkitGetAnimations() const;
-
+
PassRefPtr<RenderStyle> styleForRenderer();
RenderRegion* renderRegion() const;
@@ -530,13 +531,14 @@
bool shouldInvalidateDistributionWhenAttributeChanged(ElementShadow*, const QualifiedName&, const AtomicString&);
-private:
ElementRareData* elementRareData() const;
ElementRareData* ensureElementRareData();
void detachAllAttrNodesFromElement();
void detachAttrNodeFromElementWithValue(Attr*, const AtomicString& value);
+ void createRendererIfNeeded();
+
RefPtr<ElementAttributeData> m_attributeData;
};
Modified: trunk/Source/WebCore/dom/Node.cpp (135667 => 135668)
--- trunk/Source/WebCore/dom/Node.cpp 2012-11-25 08:01:28 UTC (rev 135667)
+++ trunk/Source/WebCore/dom/Node.cpp 2012-11-25 16:53:44 UTC (rev 135668)
@@ -1267,7 +1267,7 @@
// FIXME: This is O(N^2) for the innerHTML case, where all children are replaced at once (and not attached).
// If this node got a renderer it may be the previousRenderer() of sibling text nodes and thus affect the
- // result of Text::rendererIsNeeded() for those nodes.
+ // result of Text::textRendererIsNeeded() for those nodes.
if (renderer()) {
for (Node* next = nextSibling(); next; next = next->nextSibling()) {
if (next->renderer())
@@ -1275,7 +1275,7 @@
if (!next->attached())
break; // Assume this means none of the following siblings are attached.
if (next->isTextNode())
- next->createRendererIfNeeded();
+ toText(next)->createTextRendererIfNeeded();
}
}
@@ -1395,22 +1395,6 @@
return NodeRenderingContext(this).parentNodeForRenderingAndStyle();
}
-void Node::createRendererIfNeeded()
-{
- NodeRenderingContext(this).createRendererIfNeeded();
-}
-
-bool Node::rendererIsNeeded(const NodeRenderingContext& context)
-{
- return (document()->documentElement() == this) || (context.style()->display() != NONE);
-}
-
-RenderObject* Node::createRenderer(RenderArena*, RenderStyle*)
-{
- ASSERT_NOT_REACHED();
- return 0;
-}
-
RenderStyle* Node::virtualComputedStyle(PseudoId pseudoElementSpecifier)
{
return parentOrHostNode() ? parentOrHostNode()->computedStyle(pseudoElementSpecifier) : 0;
Modified: trunk/Source/WebCore/dom/Node.h (135667 => 135668)
--- trunk/Source/WebCore/dom/Node.h 2012-11-25 08:01:28 UTC (rev 135667)
+++ trunk/Source/WebCore/dom/Node.h 2012-11-25 16:53:44 UTC (rev 135668)
@@ -539,10 +539,6 @@
void reattach();
void reattachIfAttached();
- void createRendererIfNeeded();
- virtual bool rendererIsNeeded(const NodeRenderingContext&);
- virtual bool childShouldCreateRenderer(const NodeRenderingContext&) const { return true; }
- virtual RenderObject* createRenderer(RenderArena*, RenderStyle*);
ContainerNode* parentNodeForRenderingAndStyle();
// Wrapper for nodes that don't have a renderer, but still cache the style (like HTMLOptionElement).
Modified: trunk/Source/WebCore/dom/NodeRenderingContext.cpp (135667 => 135668)
--- trunk/Source/WebCore/dom/NodeRenderingContext.cpp 2012-11-25 08:01:28 UTC (rev 135667)
+++ trunk/Source/WebCore/dom/NodeRenderingContext.cpp 2012-11-25 16:53:44 UTC (rev 135668)
@@ -38,9 +38,11 @@
#include "RenderFullScreen.h"
#include "RenderNamedFlowThread.h"
#include "RenderObject.h"
+#include "RenderText.h"
#include "RenderView.h"
#include "ShadowRoot.h"
#include "StyleInheritedData.h"
+#include "Text.h"
#if ENABLE(SVG)
#include "SVGNames.h"
@@ -146,11 +148,12 @@
void NodeRenderingContext::moveToFlowThreadIfNeeded()
{
+ ASSERT(m_node->isElementNode());
ASSERT(m_style);
if (!m_node->document()->cssRegionsEnabled())
return;
- if (!m_node->isElementNode() || m_style->flowThread().isEmpty())
+ if (m_style->flowThread().isEmpty())
return;
// FIXME: Do not collect elements if they are in shadow tree.
@@ -201,21 +204,21 @@
}
#endif
-void NodeRenderingContext::createRendererIfNeeded()
+void NodeRenderingContext::createRendererForElementIfNeeded()
{
ASSERT(!m_node->renderer());
+ Element* element = toElement(m_node);
+
if (!shouldCreateRenderer())
return;
- Element* element = m_node->isElementNode() ? toElement(m_node) : 0;
-
- m_style = element ? element->styleForRenderer() : parentRenderer()->style();
+ m_style = element->styleForRenderer();
ASSERT(m_style);
moveToFlowThreadIfNeeded();
- if (!m_node->rendererIsNeeded(*this)) {
- if (element && m_style->affectedByEmpty())
+ if (!element->rendererIsNeeded(*this)) {
+ if (m_style->affectedByEmpty())
element->setStyleAffectedByEmpty();
return;
}
@@ -223,23 +226,23 @@
RenderObject* nextRenderer = this->nextRenderer();
#if ENABLE(DIALOG_ELEMENT)
- if (element && element->isInTopLayer())
+ if (element->isInTopLayer())
adjustInsertionPointForTopLayerElement(element, parentRenderer, nextRenderer);
#endif
- Document* document = m_node->document();
- RenderObject* newRenderer = m_node->createRenderer(document->renderArena(), m_style.get());
+ Document* document = element->document();
+ RenderObject* newRenderer = element->createRenderer(document->renderArena(), m_style.get());
if (!newRenderer)
return;
if (!parentRenderer->isChildAllowed(newRenderer, m_style.get())) {
newRenderer->destroy();
return;
}
- m_node->setRenderer(newRenderer);
+ element->setRenderer(newRenderer);
newRenderer->setAnimatableStyle(m_style.release()); // setAnimatableStyle() can depend on renderer() already being set.
#if ENABLE(FULLSCREEN_API)
- if (document->webkitIsFullScreen() && document->webkitCurrentFullScreenElement() == m_node) {
+ if (document->webkitIsFullScreen() && document->webkitCurrentFullScreenElement() == element) {
newRenderer = RenderFullScreen::wrapRenderer(newRenderer, parentRenderer, document);
if (!newRenderer)
return;
@@ -249,4 +252,33 @@
parentRenderer->addChild(newRenderer, nextRenderer);
}
+void NodeRenderingContext::createRendererForTextIfNeeded()
+{
+ ASSERT(!m_node->renderer());
+
+ Text* textNode = toText(m_node);
+
+ if (!shouldCreateRenderer())
+ return;
+ RenderObject* parentRenderer = this->parentRenderer();
+ ASSERT(parentRenderer);
+ m_style = parentRenderer->style();
+
+ if (!textNode->textRendererIsNeeded(*this))
+ return;
+ Document* document = textNode->document();
+ RenderText* newRenderer = textNode->createTextRenderer(document->renderArena(), m_style.get());
+ if (!newRenderer)
+ return;
+ if (!parentRenderer->isChildAllowed(newRenderer, m_style.get())) {
+ newRenderer->destroy();
+ return;
+ }
+ RenderObject* nextRenderer = this->nextRenderer();
+ textNode->setRenderer(newRenderer);
+ // Parent takes care of the animations, no need to call setAnimatableStyle.
+ newRenderer->setStyle(m_style.release());
+ parentRenderer->addChild(newRenderer, nextRenderer);
}
+
+}
Modified: trunk/Source/WebCore/dom/NodeRenderingContext.h (135667 => 135668)
--- trunk/Source/WebCore/dom/NodeRenderingContext.h 2012-11-25 08:01:28 UTC (rev 135667)
+++ trunk/Source/WebCore/dom/NodeRenderingContext.h 2012-11-25 16:53:44 UTC (rev 135668)
@@ -48,7 +48,8 @@
NodeRenderingContext(Node*, RenderStyle*);
~NodeRenderingContext();
- void createRendererIfNeeded();
+ void createRendererForTextIfNeeded();
+ void createRendererForElementIfNeeded();
Node* node() const;
ContainerNode* parentNodeForRenderingAndStyle() const;
Modified: trunk/Source/WebCore/dom/Text.cpp (135667 => 135668)
--- trunk/Source/WebCore/dom/Text.cpp 2012-11-25 08:01:28 UTC (rev 135667)
+++ trunk/Source/WebCore/dom/Text.cpp 2012-11-25 16:53:44 UTC (rev 135668)
@@ -195,13 +195,17 @@
return create(document(), data());
}
-bool Text::rendererIsNeeded(const NodeRenderingContext& context)
+bool Text::textRendererIsNeeded(const NodeRenderingContext& context)
{
if (isEditingText())
return true;
- if (!CharacterData::rendererIsNeeded(context))
+
+ if (!length())
return false;
+ if (context.style()->display() == NONE)
+ return false;
+
bool _onlyWS_ = containsOnlyWhitespace();
if (!onlyWS)
return true;
@@ -252,13 +256,17 @@
}
#endif
-RenderObject* Text::createRenderer(RenderArena* arena, RenderStyle* style)
+void Text::createTextRendererIfNeeded()
{
+ NodeRenderingContext(this).createRendererForTextIfNeeded();
+}
+
+RenderText* Text::createTextRenderer(RenderArena* arena, RenderStyle* style)
+{
#if ENABLE(SVG)
if (isSVGText(this) || isSVGShadowText(this))
return new (arena) RenderSVGInlineText(this, dataImpl());
#endif
-
if (style->hasTextCombine())
return new (arena) RenderCombineText(this, dataImpl());
@@ -267,14 +275,13 @@
void Text::attach()
{
- createRendererIfNeeded();
+ createTextRendererIfNeeded();
CharacterData::attach();
}
void Text::recalcTextStyle(StyleChange change)
{
- RenderObject* renderer = this->renderer();
-
+ RenderText* renderer = toRenderText(this->renderer());
// The only time we have a renderer and our parent doesn't is if our parent
// is a shadow root.
if (change != NoChange && renderer) {
@@ -287,15 +294,26 @@
}
if (needsStyleRecalc()) {
- if (renderer) {
- if (renderer->isText())
- toRenderText(renderer)->setText(dataImpl());
- } else
+ if (renderer)
+ renderer->setText(dataImpl());
+ else
reattach();
}
clearNeedsStyleRecalc();
}
+void Text::updateTextRenderer(unsigned offsetOfReplacedData, unsigned lengthOfReplacedData)
+{
+ if (!attached())
+ return;
+ RenderText* textRenderer = toRenderText(renderer());
+ if (!textRenderer || !textRendererIsNeeded(NodeRenderingContext(this, textRenderer->style()))) {
+ reattach();
+ return;
+ }
+ textRenderer->setTextWithOffset(dataImpl(), offsetOfReplacedData, lengthOfReplacedData);
+}
+
bool Text::childTypeAllowed(NodeType) const
{
return false;
Modified: trunk/Source/WebCore/dom/Text.h (135667 => 135668)
--- trunk/Source/WebCore/dom/Text.h 2012-11-25 08:01:28 UTC (rev 135667)
+++ trunk/Source/WebCore/dom/Text.h 2012-11-25 16:53:44 UTC (rev 135668)
@@ -26,7 +26,9 @@
#include "CharacterData.h"
namespace WebCore {
-
+
+class RenderText;
+
class Text : public CharacterData {
public:
static const unsigned defaultLengthLimit = 1 << 16;
@@ -43,6 +45,10 @@
PassRefPtr<Text> replaceWholeText(const String&, ExceptionCode&);
void recalcTextStyle(StyleChange);
+ void createTextRendererIfNeeded();
+ bool textRendererIsNeeded(const NodeRenderingContext&);
+ RenderText* createTextRenderer(RenderArena*, RenderStyle*);
+ void updateTextRenderer(unsigned offsetOfReplacedData, unsigned lengthOfReplacedData);
virtual void attach();
@@ -58,8 +64,6 @@
virtual String nodeName() const;
virtual NodeType nodeType() const;
virtual PassRefPtr<Node> cloneNode(bool deep);
- virtual bool rendererIsNeeded(const NodeRenderingContext&);
- virtual RenderObject* createRenderer(RenderArena*, RenderStyle*);
virtual bool childTypeAllowed(NodeType) const;
virtual PassRefPtr<Text> virtualCreate(const String&);
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (135667 => 135668)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-11-25 08:01:28 UTC (rev 135667)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-11-25 16:53:44 UTC (rev 135668)
@@ -560,7 +560,7 @@
cloneBlock->setChildrenInline(childrenInline());
}
else {
- RenderObject* cloneRenderer = node()->createRenderer(renderArena(), style());
+ RenderObject* cloneRenderer = toElement(node())->createRenderer(renderArena(), style());
cloneBlock = toRenderBlock(cloneRenderer);
cloneBlock->setStyle(style());
