Rename Node::childNodeCount() to countChildNodes() and avoid inefficient uses
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 14 Sep 2014 21:18:27 +0000 (21:18 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 14 Sep 2014 21:18:27 +0000 (21:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=136789

Reviewed by Darin Adler.

Source/WebCore:

Rename Node::childNodeCount() to countChildNodes() to make it clearer
that the method actually computes the result rather than returning a
cached value.

This patch also introduces a new Node::hasOneChild() method that is
used to check if a Node has a single child. This is much more efficient
than calling countChildNodes() == 1.

The patch also leverages Node::hasChildNodes() in a lot of places
instead of calling countChildNodes().

Finally, in a couple of places, we now use childrenOfType() to iterate
over children more efficient than using countChildNodes() and
childNode(index).

No new tests, no behavior change.

* WebCore.exp.in:
* WebCore.order:
Update the name of the exposed symbol for countChildNodes().

* dom/ContainerNode.cpp:
(WebCore::ContainerNode::removeChildren):
(WebCore::ContainerNode::countChildNodes):
(WebCore::ContainerNode::childNodeCount): Deleted.
Rename childNodeCount() to countChildNodes() to make it obvious it is
computing the result rather than returning a cached value.

* dom/ContainerNode.h:
(WebCore::ContainerNode::hasOneChild):
Introduce an efficient way to check in a ContainerNode has a single
child as "countChildNodes() == 1" calls were frequent and inefficient.

(WebCore::Node::countChildNodes):
(WebCore::Node::childNodeCount): Deleted.
* dom/Node.h:
Rename childNodeCount() to countChildNodes().

* dom/Position.cpp:
(WebCore::Position::parentAnchoredEquivalent):
* dom/Position.h:
(WebCore::lastOffsetInNode):
* dom/Range.cpp:
Mechanical renaming.

(WebCore::lengthOfContentsInNode):
* editing/ApplyStyleCommand.cpp:
(WebCore::ApplyStyleCommand::applyInlineStyleToNodeRange):
(WebCore::ApplyStyleCommand::shouldApplyInlineStyleToRun):
(WebCore::ApplyStyleCommand::applyInlineStyleToPushDown):
(WebCore::ApplyStyleCommand::mergeEndWithNextIfIdentical):
(WebCore::ApplyStyleCommand::applyInlineStyleChange):
Call hasChildNodes() rather than countChildNodes() as it is a lot more
efficient.

* editing/Editor.cpp:
(WebCore::Editor::setTextAsChildOfElement):
(WebCore::Editor::rangeOfString):
(WebCore::Editor::countMatchesForText):
* editing/FrameSelection.cpp:
(WebCore::FrameSelection::elementRangeContainingCaretSelection):
Mechanical renaming.

* editing/ReplaceSelectionCommand.cpp:
(WebCore::ReplaceSelectionCommand::insertAsListItems):
Call hasOneChild() instead of countChildNodes() == 1. Also remove
redundant listElement->hasChildNodes() check as hasOneChild() takes
care of this already.

* editing/TextIterator.cpp:
(WebCore::SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator):
* editing/VisibleUnits.cpp:
(WebCore::endOfDocument):
* editing/htmlediting.cpp:
(WebCore::lastOffsetForEditing):
Mechanical renaming.

(WebCore::visiblePositionBeforeNode):
(WebCore::visiblePositionAfterNode):
Call hasChildNodes() instead of converting countChildNodes() to a
boolean.

* editing/markup.cpp:
(WebCore::StyledMarkupAccumulator::traverseNodesForSerialization):
Call !hasChildNodes() instead of !countChildNodes().

(WebCore::isPlainTextMarkup):
- Drop !node->isElementNode() check as !node->hasTagName(divTag) takes
  care of discarding non-Element Nodes already.
- Cast the Node to an HTMLDivElement as soon as possible to avoid
  calling calling the slower Node APIs in several cases.
- Call hasOneChild() instead of countChildNodes() == 1.

* html/HTMLDivElement.h:
* html/HTMLTagNames.in:
Generate casting helpers as I use them in WebCore::isPlainTextMarkup().

* html/HTMLScriptElement.cpp:
(WebCore::HTMLScriptElement::setText):
* html/HTMLTitleElement.cpp:
(WebCore::HTMLTitleElement::setText):
Call hasOneChild() / hasChildNodes() instead of countChildNodes().

* html/shadow/MediaControlElements.cpp:
(WebCore::MediaControlTimelineContainerElement::setTimeDisplaysHidden):
(WebCore::MediaControlTextTrackContainerElement::updateDisplay):
Use childrenOfType<Element>() to iterate of child Elements instead of
slower childNodeCount() + childNode(index) + isElementNode(). Also,
childNodeCount() was not even cached before the loop.

* html/track/VTTRegion.cpp:
(WebCore::VTTRegion::displayLastTextTrackCueBox):
Use childrenOfType<Element>() to iterate of child Elements, thus
avoiding calling childNodeCount() + childNode(index).

* page/DOMSelection.cpp:
(WebCore::DOMSelection::extend):
(WebCore::DOMSelection::selectAllChildren):
* rendering/RenderObject.cpp:
(WebCore::RenderObject::caretMaxOffset):
* rendering/RenderReplaced.cpp:
(WebCore::RenderReplaced::isSelected):
* rendering/RenderView.cpp:
(WebCore::RenderView::splitSelectionBetweenSubtrees):
Mechanical renaming.

Source/WebKit/mac:

Rename childNodeCount() to countChildNodes().

* WebView/WebHTMLView.mm:
(-[WebHTMLView attributedString]):

Source/WebKit2:

Avoid calling slow Node::countChildNodes().

* WebProcess/WebPage/CoordinatedGraphics/WebPageCoordinatedGraphics.cpp:
(WebKit::WebPage::findZoomableAreaForPoint):
Replace call to "node->parentNode()->childNodeCount() != 1" by
"!node->parentNode()->hasOneChild()" which is equivalent but more
efficient.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@173606 268f45cc-cd09-0410-ab3c-d52691b4dbfc

31 files changed:
Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/WebCore.order
Source/WebCore/dom/ContainerNode.cpp
Source/WebCore/dom/ContainerNode.h
Source/WebCore/dom/Node.h
Source/WebCore/dom/Position.cpp
Source/WebCore/dom/Position.h
Source/WebCore/dom/Range.cpp
Source/WebCore/editing/ApplyStyleCommand.cpp
Source/WebCore/editing/Editor.cpp
Source/WebCore/editing/FrameSelection.cpp
Source/WebCore/editing/ReplaceSelectionCommand.cpp
Source/WebCore/editing/TextIterator.cpp
Source/WebCore/editing/VisibleUnits.cpp
Source/WebCore/editing/htmlediting.cpp
Source/WebCore/editing/markup.cpp
Source/WebCore/html/HTMLDivElement.h
Source/WebCore/html/HTMLScriptElement.cpp
Source/WebCore/html/HTMLTagNames.in
Source/WebCore/html/HTMLTitleElement.cpp
Source/WebCore/html/shadow/MediaControlElements.cpp
Source/WebCore/html/track/VTTRegion.cpp
Source/WebCore/page/DOMSelection.cpp
Source/WebCore/rendering/RenderObject.cpp
Source/WebCore/rendering/RenderReplaced.cpp
Source/WebCore/rendering/RenderView.cpp
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebView/WebHTMLView.mm
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/WebPageCoordinatedGraphics.cpp

index e658c3d..b0aa1bb 100644 (file)
@@ -1,3 +1,136 @@
+2014-09-14  Chris Dumez  <cdumez@apple.com>
+
+        Rename Node::childNodeCount() to countChildNodes() and avoid inefficient uses
+        https://bugs.webkit.org/show_bug.cgi?id=136789
+
+        Reviewed by Darin Adler.
+
+        Rename Node::childNodeCount() to countChildNodes() to make it clearer
+        that the method actually computes the result rather than returning a
+        cached value.
+
+        This patch also introduces a new Node::hasOneChild() method that is
+        used to check if a Node has a single child. This is much more efficient
+        than calling countChildNodes() == 1.
+
+        The patch also leverages Node::hasChildNodes() in a lot of places
+        instead of calling countChildNodes().
+
+        Finally, in a couple of places, we now use childrenOfType() to iterate
+        over children more efficient than using countChildNodes() and
+        childNode(index).
+
+        No new tests, no behavior change.
+
+        * WebCore.exp.in:
+        * WebCore.order:
+        Update the name of the exposed symbol for countChildNodes().
+
+        * dom/ContainerNode.cpp:
+        (WebCore::ContainerNode::removeChildren):
+        (WebCore::ContainerNode::countChildNodes):
+        (WebCore::ContainerNode::childNodeCount): Deleted.
+        Rename childNodeCount() to countChildNodes() to make it obvious it is
+        computing the result rather than returning a cached value.
+
+        * dom/ContainerNode.h:
+        (WebCore::ContainerNode::hasOneChild):
+        Introduce an efficient way to check in a ContainerNode has a single
+        child as "countChildNodes() == 1" calls were frequent and inefficient.
+
+        (WebCore::Node::countChildNodes):
+        (WebCore::Node::childNodeCount): Deleted.
+        * dom/Node.h:
+        Rename childNodeCount() to countChildNodes().
+
+        * dom/Position.cpp:
+        (WebCore::Position::parentAnchoredEquivalent):
+        * dom/Position.h:
+        (WebCore::lastOffsetInNode):
+        * dom/Range.cpp:
+        Mechanical renaming.
+
+        (WebCore::lengthOfContentsInNode):
+        * editing/ApplyStyleCommand.cpp:
+        (WebCore::ApplyStyleCommand::applyInlineStyleToNodeRange):
+        (WebCore::ApplyStyleCommand::shouldApplyInlineStyleToRun):
+        (WebCore::ApplyStyleCommand::applyInlineStyleToPushDown):
+        (WebCore::ApplyStyleCommand::mergeEndWithNextIfIdentical):
+        (WebCore::ApplyStyleCommand::applyInlineStyleChange):
+        Call hasChildNodes() rather than countChildNodes() as it is a lot more
+        efficient.
+
+        * editing/Editor.cpp:
+        (WebCore::Editor::setTextAsChildOfElement):
+        (WebCore::Editor::rangeOfString):
+        (WebCore::Editor::countMatchesForText):
+        * editing/FrameSelection.cpp:
+        (WebCore::FrameSelection::elementRangeContainingCaretSelection):
+        Mechanical renaming.
+
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::insertAsListItems):
+        Call hasOneChild() instead of countChildNodes() == 1. Also remove
+        redundant listElement->hasChildNodes() check as hasOneChild() takes
+        care of this already.
+
+        * editing/TextIterator.cpp:
+        (WebCore::SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator):
+        * editing/VisibleUnits.cpp:
+        (WebCore::endOfDocument):
+        * editing/htmlediting.cpp:
+        (WebCore::lastOffsetForEditing):
+        Mechanical renaming.
+
+        (WebCore::visiblePositionBeforeNode):
+        (WebCore::visiblePositionAfterNode):
+        Call hasChildNodes() instead of converting countChildNodes() to a
+        boolean.
+
+        * editing/markup.cpp:
+        (WebCore::StyledMarkupAccumulator::traverseNodesForSerialization):
+        Call !hasChildNodes() instead of !countChildNodes().
+
+        (WebCore::isPlainTextMarkup):
+        - Drop !node->isElementNode() check as !node->hasTagName(divTag) takes
+          care of discarding non-Element Nodes already.
+        - Cast the Node to an HTMLDivElement as soon as possible to avoid
+          calling calling the slower Node APIs in several cases.
+        - Call hasOneChild() instead of countChildNodes() == 1.
+
+        * html/HTMLDivElement.h:
+        * html/HTMLTagNames.in:
+        Generate casting helpers as I use them in WebCore::isPlainTextMarkup().
+
+        * html/HTMLScriptElement.cpp:
+        (WebCore::HTMLScriptElement::setText):
+        * html/HTMLTitleElement.cpp:
+        (WebCore::HTMLTitleElement::setText):
+        Call hasOneChild() / hasChildNodes() instead of countChildNodes().
+
+        * html/shadow/MediaControlElements.cpp:
+        (WebCore::MediaControlTimelineContainerElement::setTimeDisplaysHidden):
+        (WebCore::MediaControlTextTrackContainerElement::updateDisplay):
+        Use childrenOfType<Element>() to iterate of child Elements instead of
+        slower childNodeCount() + childNode(index) + isElementNode(). Also,
+        childNodeCount() was not even cached before the loop.
+
+        * html/track/VTTRegion.cpp:
+        (WebCore::VTTRegion::displayLastTextTrackCueBox):
+        Use childrenOfType<Element>() to iterate of child Elements, thus
+        avoiding calling childNodeCount() + childNode(index).
+
+        * page/DOMSelection.cpp:
+        (WebCore::DOMSelection::extend):
+        (WebCore::DOMSelection::selectAllChildren):
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::caretMaxOffset):
+        * rendering/RenderReplaced.cpp:
+        (WebCore::RenderReplaced::isSelected):
+        * rendering/RenderView.cpp:
+        (WebCore::RenderView::splitSelectionBetweenSubtrees):
+        Mechanical renaming.
+
 2014-09-13  Chris Dumez  <cdumez@apple.com>
 
         Un-inline Element constructor
index 8d1fd40..94d9b53 100644 (file)
@@ -1625,7 +1625,7 @@ __ZNK7WebCore12SharedBuffer4sizeEv
 __ZNK7WebCore12TextEncoding6decodeEPKcmbRb
 __ZNK7WebCore12TextIterator4nodeEv
 __ZNK7WebCore12TextIterator5rangeEv
-__ZNK7WebCore13ContainerNode14childNodeCountEv
+__ZNK7WebCore13ContainerNode15countChildNodesEv
 __ZNK7WebCore13ContainerNode9childNodeEj
 __ZNK7WebCore13GraphicsLayer18accumulatedOpacityEv
 __ZNK7WebCore13GraphicsLayer18getDebugBorderInfoERNS_5ColorERf
index a36002a..0b17796 100644 (file)
@@ -11622,7 +11622,7 @@ __ZN7WebCore21jsHTMLMetaElementNameEPN3JSC9ExecStateENS0_7JSValueENS0_12Property
 __ZN7WebCore24jsHTMLMetaElementContentEPN3JSC9ExecStateENS0_7JSValueENS0_12PropertyNameE
 __ZN7WebCore26setJSHTMLScriptElementTextEPN3JSC9ExecStateEPNS0_8JSObjectENS0_7JSValueE
 __ZN7WebCore17HTMLScriptElement7setTextERKN3WTF6StringE
-__ZNK7WebCore13ContainerNode14childNodeCountEv
+__ZNK7WebCore13ContainerNode15countChildNodesEv
 __ZN7WebCore20jsHTMLLinkElementRelEPN3JSC9ExecStateENS0_7JSValueENS0_12PropertyNameE
 __ZN7WebCore21jsHTMLLinkElementHrefEPN3JSC9ExecStateENS0_7JSValueENS0_12PropertyNameE
 __ZN7WebCore13NodeTraversal35nextIncludingPseudoSkippingChildrenEPKNS_4NodeES3_
index 1b11402..65b7ee8 100644 (file)
@@ -653,7 +653,7 @@ void ContainerNode::removeChildren()
         WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
         {
             NoEventDispatchAssertion assertNoEventDispatch;
-            removedChildren.reserveInitialCapacity(childNodeCount());
+            removedChildren.reserveInitialCapacity(countChildNodes());
             while (RefPtr<Node> n = m_firstChild) {
                 removedChildren.append(*m_firstChild);
                 removeBetween(0, m_firstChild->nextSibling(), *m_firstChild);
@@ -931,12 +931,11 @@ LayoutRect ContainerNode::boundingBox() const
     return enclosingLayoutRect(FloatRect(upperLeft, lowerRight.expandedTo(upperLeft) - upperLeft));
 }
 
-unsigned ContainerNode::childNodeCount() const
+unsigned ContainerNode::countChildNodes() const
 {
     unsigned count = 0;
-    Node *n;
-    for (n = firstChild(); n; n = n->nextSibling())
-        count++;
+    for (Node* child = firstChild(); child; child = child->nextSibling())
+        ++count;
     return count;
 }
 
index 639ed24..510fa04 100644 (file)
@@ -87,11 +87,12 @@ public:
     static ptrdiff_t firstChildMemoryOffset() { return OBJECT_OFFSETOF(ContainerNode, m_firstChild); }
     Node* lastChild() const { return m_lastChild; }
     bool hasChildNodes() const { return m_firstChild; }
+    bool hasOneChild() const { return m_firstChild && !m_firstChild->nextSibling(); }
 
     bool directChildNeedsStyleRecalc() const { return getFlag(DirectChildNeedsStyleRecalcFlag); }
     void setDirectChildNeedsStyleRecalc() { setFlag(DirectChildNeedsStyleRecalcFlag); }
 
-    WEBCORE_EXPORT unsigned childNodeCount() const;
+    WEBCORE_EXPORT unsigned countChildNodes() const;
     WEBCORE_EXPORT Node* childNode(unsigned index) const;
 
     bool insertBefore(PassRefPtr<Node> newChild, Node* refChild, ExceptionCode& = ASSERT_NO_EXCEPTION);
@@ -184,11 +185,11 @@ inline ContainerNode::ContainerNode(Document& document, ConstructionType type)
 {
 }
 
-inline unsigned Node::childNodeCount() const
+inline unsigned Node::countChildNodes() const
 {
     if (!isContainerNode())
         return 0;
-    return toContainerNode(this)->childNodeCount();
+    return toContainerNode(this)->countChildNodes();
 }
 
 inline Node* Node::childNode(unsigned index) const
index ebbd8a9..f477ff3 100644 (file)
@@ -416,7 +416,7 @@ public:
     bool isReadOnlyNode() const { return nodeType() == ENTITY_REFERENCE_NODE; }
     bool isDocumentTypeNode() const { return nodeType() == DOCUMENT_TYPE_NODE; }
     virtual bool childTypeAllowed(NodeType) const { return false; }
-    unsigned childNodeCount() const;
+    unsigned countChildNodes() const;
     Node* childNode(unsigned index) const;
 
     void checkSetPrefix(const AtomicString& prefix, ExceptionCode&);
index 3ad8eda..f2c691e 100644 (file)
@@ -226,7 +226,7 @@ Position Position::parentAnchoredEquivalent() const
     }
 
     if (!m_anchorNode->offsetInCharacters()
-        && (m_anchorType == PositionIsAfterAnchor || m_anchorType == PositionIsAfterChildren || static_cast<unsigned>(m_offset) == m_anchorNode->childNodeCount())
+        && (m_anchorType == PositionIsAfterAnchor || m_anchorType == PositionIsAfterChildren || static_cast<unsigned>(m_offset) == m_anchorNode->countChildNodes())
         && (editingIgnoresContent(m_anchorNode.get()) || isRenderedTable(m_anchorNode.get()))
         && containerNode()) {
         return positionInParentAfterNode(m_anchorNode.get());
index 298117f..51bf472 100644 (file)
@@ -297,10 +297,10 @@ inline Position positionAfterNode(Node* anchorNode)
 
 inline int lastOffsetInNode(Node* node)
 {
-    return node->offsetInCharacters() ? node->maxCharacterOffset() : static_cast<int>(node->childNodeCount());
+    return node->offsetInCharacters() ? node->maxCharacterOffset() : static_cast<int>(node->countChildNodes());
 }
 
-// firstPositionInNode and lastPositionInNode return parent-anchored positions, lastPositionInNode construction is O(n) due to childNodeCount()
+// firstPositionInNode and lastPositionInNode return parent-anchored positions, lastPositionInNode construction is O(n) due to countChildNodes()
 inline Position firstPositionInNode(Node* anchorNode)
 {
     if (anchorNode->isTextNode())
index b0bc320..d68484f 100644 (file)
@@ -672,7 +672,7 @@ static inline unsigned lengthOfContentsInNode(Node* node)
     case Node::DOCUMENT_FRAGMENT_NODE:
     case Node::NOTATION_NODE:
     case Node::XPATH_NAMESPACE_NODE:
-        return node->childNodeCount();
+        return node->countChildNodes();
     }
     ASSERT_NOT_REACHED();
     return 0;
index 5384b6f..4d67dc9 100644 (file)
@@ -777,7 +777,7 @@ void ApplyStyleCommand::applyInlineStyleToNodeRange(EditingStyle* style, PassRef
         if (isBlock(node.get()))
             continue;
         
-        if (node->childNodeCount()) {
+        if (node->hasChildNodes()) {
             if (node->contains(pastEndNode.get()) || containsNonEditableRegion(node.get()) || !node->parentNode()->hasEditableStyle())
                 continue;
             if (editingIgnoresContent(node.get())) {
@@ -835,7 +835,7 @@ bool ApplyStyleCommand::shouldApplyInlineStyleToRun(EditingStyle* style, Node* r
     ASSERT(style && runStart);
 
     for (Node* node = runStart; node && node != pastEndNode; node = NodeTraversal::next(node)) {
-        if (node->childNodeCount())
+        if (node->hasChildNodes())
             continue;
         // We don't consider m_isInlineElementToRemoveFunction here because we never apply style when m_isInlineElementToRemoveFunction is specified
         if (!style->styleIsPresentInComputedStyleOfNode(node))
@@ -1007,7 +1007,7 @@ void ApplyStyleCommand::applyInlineStyleToPushDown(Node* node, EditingStyle* sty
 
     // Since addInlineStyleIfNeeded can't add styles to block-flow render objects, add style attribute instead.
     // FIXME: applyInlineStyleToRange should be used here instead.
-    if ((node->renderer()->isRenderBlockFlow() || node->childNodeCount()) && node->isHTMLElement()) {
+    if ((node->renderer()->isRenderBlockFlow() || node->hasChildNodes()) && node->isHTMLElement()) {
         setNodeAttribute(toHTMLElement(node), styleAttr, newInlineStyle->style()->asText());
         return;
     }
@@ -1325,7 +1325,7 @@ bool ApplyStyleCommand::mergeEndWithNextIfIdentical(const Position& start, const
         mergeIdenticalElements(element, nextElement);
 
         bool shouldUpdateStart = start.containerNode() == endNode;
-        int endOffset = nextChild ? nextChild->nodeIndex() : nextElement->childNodeCount();
+        int endOffset = nextChild ? nextChild->nodeIndex() : nextElement->countChildNodes();
         updateStartEnd(shouldUpdateStart ? Position(nextElement, start.offsetInContainerNode(), Position::PositionIsOffsetInAnchor) : start,
                        Position(nextElement, endOffset, Position::PositionIsOffsetInAnchor));
         return true;
@@ -1433,7 +1433,7 @@ void ApplyStyleCommand::applyInlineStyleChange(PassRefPtr<Node> passedStart, Pas
         if (container->isHTMLElement() && container->hasTagName(fontTag))
             fontContainer = toHTMLElement(container);
         bool styleContainerIsNotSpan = !styleContainer || !styleContainer->hasTagName(spanTag);
-        if (container->isHTMLElement() && (container->hasTagName(spanTag) || (styleContainerIsNotSpan && container->childNodeCount())))
+        if (container->isHTMLElement() && (container->hasTagName(spanTag) || (styleContainerIsNotSpan && container->hasChildNodes())))
             styleContainer = toHTMLElement(container);
         if (!container->firstChild())
             break;
index f41065b..8c8dadc 100644 (file)
@@ -704,7 +704,7 @@ void Editor::setTextAsChildOfElement(const String& text, Element* elem)
     // set the selection to the end
     VisibleSelection selection;
 
-    Position pos = createLegacyEditingPosition(elem, elem->childNodeCount());
+    Position pos = createLegacyEditingPosition(elem, elem->countChildNodes());
 
     VisiblePosition visiblePos(pos, VP_DEFAULT_AFFINITY);
     if (visiblePos.isNull())
@@ -3168,7 +3168,7 @@ PassRefPtr<Range> Editor::rangeOfString(const String& target, Range* referenceRa
     RefPtr<Node> shadowTreeRoot = referenceRange && referenceRange->startContainer() ? referenceRange->startContainer()->nonBoundaryShadowTreeRootNode() : 0;
     if (shadowTreeRoot) {
         if (forward)
-            searchRange->setEnd(shadowTreeRoot.get(), shadowTreeRoot->childNodeCount());
+            searchRange->setEnd(shadowTreeRoot.get(), shadowTreeRoot->countChildNodes());
         else
             searchRange->setStart(shadowTreeRoot.get(), 0);
     }
@@ -3186,7 +3186,7 @@ PassRefPtr<Range> Editor::rangeOfString(const String& target, Range* referenceRa
 
         if (shadowTreeRoot) {
             if (forward)
-                searchRange->setEnd(shadowTreeRoot.get(), shadowTreeRoot->childNodeCount());
+                searchRange->setEnd(shadowTreeRoot.get(), shadowTreeRoot->countChildNodes());
             else
                 searchRange->setStart(shadowTreeRoot.get(), 0);
         }
@@ -3279,7 +3279,7 @@ unsigned Editor::countMatchesForText(const String& target, Range* range, FindOpt
 
         Node* shadowTreeRoot = searchRange->shadowRoot();
         if (searchRange->collapsed(IGNORE_EXCEPTION) && shadowTreeRoot)
-            searchRange->setEnd(shadowTreeRoot, shadowTreeRoot->childNodeCount(), IGNORE_EXCEPTION);
+            searchRange->setEnd(shadowTreeRoot, shadowTreeRoot->countChildNodes(), IGNORE_EXCEPTION);
     } while (true);
 
     if (markMatches || matches) {
index f0ac279..7b88151 100644 (file)
@@ -2196,7 +2196,7 @@ PassRefPtr<Range> FrameSelection::elementRangeContainingCaretSelection() const
         return nullptr;
 
     Position startPos = createLegacyEditingPosition(element, 0);
-    Position endPos = createLegacyEditingPosition(element, element->childNodeCount());
+    Position endPos = createLegacyEditingPosition(element, element->countChildNodes());
     
     VisiblePosition startVisiblePos(startPos, VP_DEFAULT_AFFINITY);
     VisiblePosition endVisiblePos(endPos, VP_DEFAULT_AFFINITY);
index 0eb89bb..b45be5d 100644 (file)
@@ -1437,7 +1437,7 @@ Node* ReplaceSelectionCommand::insertAsListItems(PassRefPtr<HTMLElement> prpList
 {
     RefPtr<HTMLElement> listElement = prpListElement;
 
-    while (listElement->hasChildNodes() && isListElement(listElement->firstChild()) && listElement->childNodeCount() == 1)
+    while (listElement->hasOneChild() && isListElement(listElement->firstChild()))
         listElement = toHTMLElement(listElement->firstChild());
 
     bool isStart = isStartOfParagraph(insertPos);
index f422148..093c548 100644 (file)
@@ -1146,13 +1146,13 @@ SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator(const Range& ra
     int endOffset = range.endOffset();
 
     if (!startNode->offsetInCharacters()) {
-        if (startOffset >= 0 && startOffset < static_cast<int>(startNode->childNodeCount())) {
+        if (startOffset >= 0 && startOffset < static_cast<int>(startNode->countChildNodes())) {
             startNode = startNode->childNode(startOffset);
             startOffset = 0;
         }
     }
     if (!endNode->offsetInCharacters()) {
-        if (endOffset > 0 && endOffset <= static_cast<int>(endNode->childNodeCount())) {
+        if (endOffset > 0 && endOffset <= static_cast<int>(endNode->countChildNodes())) {
             endNode = endNode->childNode(endOffset - 1);
             endOffset = lastOffsetInNode(endNode);
         }
index 20f93cc..64ce390 100644 (file)
@@ -1364,7 +1364,7 @@ VisiblePosition endOfDocument(const Node* node)
     // (As above, in startOfDocument.)  The canonicalization can reject valid visible positions
     // when descending from the root element, so we construct the visible position directly from a
     // valid candidate.
-    Position lastPosition = createLegacyEditingPosition(node->document().documentElement(), node->document().documentElement()->childNodeCount());
+    Position lastPosition = createLegacyEditingPosition(node->document().documentElement(), node->document().documentElement()->countChildNodes());
     Position lastCandidate = previousCandidate(lastPosition);
     if (lastCandidate.isNull())
         return VisiblePosition();
index 555d7ee..e645eca 100644 (file)
@@ -343,7 +343,7 @@ int lastOffsetForEditing(const Node* node)
         return node->maxCharacterOffset();
 
     if (node->hasChildNodes())
-        return node->childNodeCount();
+        return node->countChildNodes();
 
     // NOTE: This should preempt the childNodeCount for, e.g., select nodes
     if (editingIgnoresContent(node))
@@ -513,7 +513,7 @@ Node* isLastPositionBeforeTable(const VisiblePosition& visiblePosition)
 VisiblePosition visiblePositionBeforeNode(Node* node)
 {
     ASSERT(node);
-    if (node->childNodeCount())
+    if (node->hasChildNodes())
         return VisiblePosition(firstPositionInOrBeforeNode(node), DOWNSTREAM);
     ASSERT(node->parentNode());
     ASSERT(!node->parentNode()->isShadowRoot());
@@ -524,7 +524,7 @@ VisiblePosition visiblePositionBeforeNode(Node* node)
 VisiblePosition visiblePositionAfterNode(Node* node)
 {
     ASSERT(node);
-    if (node->childNodeCount())
+    if (node->hasChildNodes())
         return VisiblePosition(lastPositionInOrAfterNode(node), DOWNSTREAM);
     ASSERT(node->parentNode());
     ASSERT(!node->parentNode()->isShadowRoot());
index af7ff4e..6c22bfe 100644 (file)
@@ -43,6 +43,7 @@
 #include "ExceptionCodePlaceholder.h"
 #include "Frame.h"
 #include "HTMLBodyElement.h"
+#include "HTMLDivElement.h"
 #include "HTMLElement.h"
 #include "HTMLNames.h"
 #include "HTMLTableElement.h"
@@ -398,7 +399,7 @@ Node* StyledMarkupAccumulator::traverseNodesForSerialization(Node* startNode, No
                 appendStartTag(*n);
 
             // If node has no children, close the tag now.
-            if (!n->childNodeCount()) {
+            if (!n->hasChildNodes()) {
                 if (shouldEmit)
                     appendEndTag(*n);
                 lastClosed = n;
@@ -740,15 +741,27 @@ static void fillContainerFromString(ContainerNode* paragraph, const String& stri
     }
 }
 
-bool isPlainTextMarkup(Node *node)
+bool isPlainTextMarkup(Nodenode)
 {
-    if (!node->isElementNode() || !node->hasTagName(divTag) || toElement(node)->hasAttributes())
+    if (!isHTMLDivElement(node))
         return false;
+
+    HTMLDivElement& element = toHTMLDivElement(*node);
+    if (element.hasAttributes())
+        return false;
+
+    Node* firstChild = element.firstChild();
+    if (!firstChild)
+        return false;
+
+    Node* secondChild = firstChild->nextSibling();
+    if (!secondChild)
+        return firstChild->isTextNode() || firstChild->firstChild();
     
-    if (node->childNodeCount() == 1 && (node->firstChild()->isTextNode() || (node->firstChild()->firstChild())))
-        return true;
+    if (secondChild->nextSibling())
+        return false;
     
-    return (node->childNodeCount() == 2 && isTabSpanTextNode(node->firstChild()->firstChild()) && node->firstChild()->nextSibling()->isTextNode());
+    return isTabSpanTextNode(firstChild->firstChild()) && secondChild->isTextNode();
 }
 
 static bool contextPreservesNewline(const Range& context)
index ff81b9b..1b5d20a 100644 (file)
@@ -40,6 +40,8 @@ private:
     virtual void collectStyleForPresentationAttribute(const QualifiedName&, const AtomicString&, MutableStyleProperties&) override;
 };
 
+NODE_TYPE_CASTS(HTMLDivElement)
+
 } // namespace WebCore
 
 #endif // HTMLDivElement_h
index 580e14d..30e3d46 100644 (file)
@@ -81,14 +81,12 @@ void HTMLScriptElement::setText(const String &value)
 {
     Ref<HTMLScriptElement> protectFromMutationEvents(*this);
 
-    int numChildren = childNodeCount();
-
-    if (numChildren == 1 && firstChild()->isTextNode()) {
+    if (hasOneChild() && firstChild()->isTextNode()) {
         toText(firstChild())->setData(value, IGNORE_EXCEPTION);
         return;
     }
 
-    if (numChildren > 0)
+    if (hasChildNodes())
         removeChildren();
 
     appendChild(document().createTextNode(value.impl()), IGNORE_EXCEPTION);
index 74d4b1c..eca25ee 100644 (file)
@@ -38,7 +38,7 @@ del interfaceName=HTMLModElement
 details conditional=DETAILS_ELEMENT, generateTypeHelpers 
 dfn interfaceName=HTMLElement
 dir interfaceName=HTMLDirectoryElement
-div
+div generateTypeHelpers
 dl interfaceName=HTMLDListElement
 dt interfaceName=HTMLElement
 em interfaceName=HTMLElement
index 14aee37..00e67b3 100644 (file)
@@ -96,10 +96,8 @@ StringWithDirection HTMLTitleElement::textWithDirection()
 void HTMLTitleElement::setText(const String &value)
 {
     Ref<HTMLTitleElement> protectFromMutationEvents(*this);
-
-    int numChildren = childNodeCount();
     
-    if (numChildren == 1 && firstChild()->isTextNode())
+    if (hasOneChild() && firstChild()->isTextNode())
         toText(firstChild())->setData(value, IGNORE_EXCEPTION);
     else {
         // We make a copy here because entity of "value" argument can be Document::m_title,
@@ -107,7 +105,7 @@ void HTMLTitleElement::setText(const String &value)
         // which causes HTMLTitleElement::childrenChanged(), which ends up Document::setTitle().
         String valueCopy(value);
 
-        if (numChildren > 0)
+        if (hasChildNodes())
             removeChildren();
 
         appendChild(document().createTextNode(valueCopy.impl()), IGNORE_EXCEPTION);
index b09f38a..366b95a 100644 (file)
@@ -33,6 +33,7 @@
 #include "MediaControlElements.h"
 
 #include "DOMTokenList.h"
+#include "ElementChildIterator.h"
 #include "EventHandler.h"
 #include "EventNames.h"
 #include "ExceptionCodePlaceholder.h"
@@ -296,20 +297,16 @@ PassRefPtr<MediaControlTimelineContainerElement> MediaControlTimelineContainerEl
 
 void MediaControlTimelineContainerElement::setTimeDisplaysHidden(bool hidden)
 {
-    for (unsigned i = 0; i < childNodeCount(); ++i) {
-        Node* child = childNode(i);
-        if (!child || !child->isElementNode())
-            continue;
-        Element* element = toElement(child);
-        if (element->shadowPseudoId() != getMediaControlTimeRemainingDisplayElementShadowPseudoId()
-            && element->shadowPseudoId() != getMediaControlCurrentTimeDisplayElementShadowPseudoId())
+    for (auto& element : childrenOfType<Element>(*this)) {
+        if (element.shadowPseudoId() != getMediaControlTimeRemainingDisplayElementShadowPseudoId()
+            && element.shadowPseudoId() != getMediaControlCurrentTimeDisplayElementShadowPseudoId())
             continue;
 
-        MediaControlTimeDisplayElement* timeDisplay = static_cast<MediaControlTimeDisplayElement*>(element);
+        MediaControlTimeDisplayElement& timeDisplay = static_cast<MediaControlTimeDisplayElement&>(element);
         if (hidden)
-            timeDisplay->hide();
+            timeDisplay.hide();
         else
-            timeDisplay->show();
+            timeDisplay.show();
     }
 }
 
@@ -1152,7 +1149,9 @@ void MediaControlTextTrackContainerElement::updateDisplay()
     // we wish to render (e.g., we are adding another cue in a set of roll-up
     // cues), remove all the existing CSS boxes representing the cues and re-add
     // them so that the new cue is at the bottom.
-    if (childNodeCount() < activeCues.size())
+    // FIXME: Calling countChildNodes() here is inefficient. We don't need to
+    // traverse all children just to check if there are less children than cues.
+    if (countChildNodes() < activeCues.size())
         removeChildren();
 
     // Sort the active cues for the appropriate display order. For example, for roll-up
index 07270c2..e09bbe5 100644 (file)
@@ -36,6 +36,7 @@
 
 #include "ClientRect.h"
 #include "DOMTokenList.h"
+#include "ElementChildIterator.h"
 #include "ExceptionCodePlaceholder.h"
 #include "HTMLDivElement.h"
 #include "Logging.h"
@@ -361,7 +362,7 @@ void VTTRegion::displayLastTextTrackCueBox()
     ASSERT(m_cueContainer);
 
     // The container needs to be rendered, if it is not empty and the region is not currently scrolling.
-    if (!m_cueContainer->renderer() || !m_cueContainer->childNodeCount() || m_scrollTimer.isActive())
+    if (!m_cueContainer->renderer() || !m_cueContainer->hasChildNodes() || m_scrollTimer.isActive())
         return;
 
     // If it's a scrolling region, add the scrolling class.
@@ -371,9 +372,10 @@ void VTTRegion::displayLastTextTrackCueBox()
     float regionBottom = m_regionDisplayTree->getBoundingClientRect()->bottom();
 
     // Find first cue that is not entirely displayed and scroll it upwards.
-    for (size_t i = 0; i < m_cueContainer->childNodeCount() && !m_scrollTimer.isActive(); ++i) {
-        float childTop = static_cast<HTMLDivElement*>(m_cueContainer->childNode(i))->getBoundingClientRect()->top();
-        float childBottom = static_cast<HTMLDivElement*>(m_cueContainer->childNode(i))->getBoundingClientRect()->bottom();
+    for (auto& child : childrenOfType<Element>(*m_cueContainer)) {
+        RefPtr<ClientRect> rect = child.getBoundingClientRect();
+        float childTop = rect->top();
+        float childBottom = rect->bottom();
 
         if (regionBottom >= childBottom)
             continue;
@@ -384,6 +386,7 @@ void VTTRegion::displayLastTextTrackCueBox()
         m_cueContainer->setInlineStyleProperty(CSSPropertyTop, m_currentTop, CSSPrimitiveValue::CSS_PX);
 
         startTimer();
+        break;
     }
 }
 
index 18cba88..73d9795 100644 (file)
@@ -337,7 +337,7 @@ void DOMSelection::extend(Node* node, int offset, ExceptionCode& ec)
         return;
     }
 
-    if (offset < 0 || offset > (node->offsetInCharacters() ? caretMaxOffset(node) : (int)node->childNodeCount())) {
+    if (offset < 0 || offset > (node->offsetInCharacters() ? caretMaxOffset(node) : static_cast<int>(node->countChildNodes()))) {
         ec = INDEX_SIZE_ERR;
         return;
     }
@@ -481,7 +481,7 @@ void DOMSelection::selectAllChildren(Node* n, ExceptionCode& ec)
         return;
 
     // This doesn't (and shouldn't) select text node characters.
-    setBaseAndExtent(n, 0, n, n->childNodeCount(), ec);
+    setBaseAndExtent(n, 0, n, n->countChildNodes(), ec);
 }
 
 String DOMSelection::toString()
index 0723fd7..7d14566 100644 (file)
@@ -2380,7 +2380,7 @@ int RenderObject::caretMinOffset() const
 int RenderObject::caretMaxOffset() const
 {
     if (isReplaced())
-        return node() ? std::max(1U, node()->childNodeCount()) : 1;
+        return node() ? std::max(1U, node()->countChildNodes()) : 1;
     if (isHR())
         return 1;
     return 0;
index 45e171a..a8276d7 100644 (file)
@@ -585,7 +585,7 @@ bool RenderReplaced::isSelected() const
     if (s == SelectionStart)
         return selectionStart == 0;
         
-    int end = element()->hasChildNodes() ? element()->childNodeCount() : 1;
+    int end = element()->hasChildNodes() ? element()->countChildNodes() : 1;
     if (s == SelectionEnd)
         return selectionEnd == end;
     if (s == SelectionBoth)
index 5969eb4..60c73a4 100644 (file)
@@ -905,7 +905,7 @@ void RenderView::splitSelectionBetweenSubtrees(RenderObject* start, int startPos
             if (node == endNode)
                 selectionData.setSelectionEndPos(endPos);
             else
-                selectionData.setSelectionEndPos(node->offsetInCharacters() ? node->maxCharacterOffset() : node->childNodeCount());
+                selectionData.setSelectionEndPos(node->offsetInCharacters() ? node->maxCharacterOffset() : node->countChildNodes());
 
             renderSubtreesMap.set(&root, selectionData);
         }
index da56031..ac38bec 100644 (file)
@@ -1,3 +1,15 @@
+2014-09-14  Chris Dumez  <cdumez@apple.com>
+
+        Rename Node::childNodeCount() to countChildNodes() and avoid inefficient uses
+        https://bugs.webkit.org/show_bug.cgi?id=136789
+
+        Reviewed by Darin Adler.
+
+        Rename childNodeCount() to countChildNodes().
+
+        * WebView/WebHTMLView.mm:
+        (-[WebHTMLView attributedString]):
+
 2014-09-10  Jon Honeycutt  <jhoneycutt@apple.com>
 
         Re-add the request autocomplete feature
index f9eac89..532dccc 100644 (file)
@@ -6686,7 +6686,7 @@ static CGImageRef selectionImage(Frame* frame, bool forceBlackText)
     NSAttributedString *attributedString = [self _attributeStringFromDOMRange:[document _documentRange]];
     if (!attributedString) {
         Document* coreDocument = core(document);
-        attributedString = editingAttributedStringFromRange(*Range::create(*coreDocument, coreDocument, 0, coreDocument, coreDocument->childNodeCount()));
+        attributedString = editingAttributedStringFromRange(*Range::create(*coreDocument, coreDocument, 0, coreDocument, coreDocument->countChildNodes()));
     }
     return attributedString;
 }
index ec5a29d..aa5bf51 100644 (file)
@@ -1,3 +1,18 @@
+2014-09-14  Chris Dumez  <cdumez@apple.com>
+
+        Rename Node::childNodeCount() to countChildNodes() and avoid inefficient uses
+        https://bugs.webkit.org/show_bug.cgi?id=136789
+
+        Reviewed by Darin Adler.
+
+        Avoid calling slow Node::countChildNodes().
+
+        * WebProcess/WebPage/CoordinatedGraphics/WebPageCoordinatedGraphics.cpp:
+        (WebKit::WebPage::findZoomableAreaForPoint):
+        Replace call to "node->parentNode()->childNodeCount() != 1" by
+        "!node->parentNode()->hasOneChild()" which is equivalent but more
+        efficient.
+
 2014-09-10  Jon Honeycutt  <jhoneycutt@apple.com>
 
         Re-add the request autocomplete feature
index 4efec58..d8bfe88 100644 (file)
@@ -64,9 +64,9 @@ void WebPage::findZoomableAreaForPoint(const IntPoint& point, const IntSize& are
             return;
 
         // Candidate found, and it is a better candidate than its parent.
-        // NB: A parent is considered a better candidate iff the node is
+        // NB: A parent is considered a better candidate if the node is
         // contained by it and it is the only child.
-        if (found && (!node->parentNode() || node->parentNode()->childNodeCount() != 1))
+        if (found && (!node->parentNode() || !node->parentNode()->hasOneChild()))
             break;
 
         node = node->parentNode();