Eliminate the use of lastChild in TextIterator
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Dec 2016 09:11:32 +0000 (09:11 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Dec 2016 09:11:32 +0000 (09:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=166456

Reviewed by Antti Koivisto.

Just use the node we just existed in TextIterator::exitNode and in emitting additional new line
to eliminate the use of Node::lastChild.

Also initialize member variables in the declaration instead of the constructor to modernize the code.

* editing/TextIterator.cpp:
(WebCore::TextIterator::TextIterator):
(WebCore::TextIterator::advance):
(WebCore::TextIterator::exitNode):
* editing/TextIterator.h:

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

Source/WebCore/ChangeLog
Source/WebCore/editing/TextIterator.cpp
Source/WebCore/editing/TextIterator.h

index bd11736..7b90109 100644 (file)
@@ -1,3 +1,21 @@
+2016-12-23  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Eliminate the use of lastChild in TextIterator
+        https://bugs.webkit.org/show_bug.cgi?id=166456
+
+        Reviewed by Antti Koivisto.
+
+        Just use the node we just existed in TextIterator::exitNode and in emitting additional new line
+        to eliminate the use of Node::lastChild.
+
+        Also initialize member variables in the declaration instead of the constructor to modernize the code.
+
+        * editing/TextIterator.cpp:
+        (WebCore::TextIterator::TextIterator):
+        (WebCore::TextIterator::advance):
+        (WebCore::TextIterator::exitNode):
+        * editing/TextIterator.h:
+
 2016-12-22  Andy Estes  <aestes@apple.com>
 
         Reduce QuickLook.h include overhead
index 3817fd2..7509060 100644 (file)
@@ -39,6 +39,7 @@
 #include "HTMLNames.h"
 #include "HTMLParagraphElement.h"
 #include "HTMLProgressElement.h"
+#include "HTMLSlotElement.h"
 #include "HTMLTextAreaElement.h"
 #include "HTMLTextFormControlElement.h"
 #include "InlineTextBox.h"
@@ -340,23 +341,6 @@ void TextIteratorCopyableText::appendToStringBuilder(StringBuilder& builder) con
 
 TextIterator::TextIterator(const Range* range, TextIteratorBehavior behavior)
     : m_behavior(behavior)
-    , m_handledNode(false)
-    , m_handledChildren(false)
-    , m_startContainer(nullptr)
-    , m_startOffset(0)
-    , m_endContainer(nullptr)
-    , m_endOffset(0)
-    , m_positionNode(nullptr)
-    , m_needsAnotherNewline(false)
-    , m_textBox(nullptr)
-    , m_remainingTextBox(nullptr)
-    , m_firstLetterText(nullptr)
-    , m_lastTextNode(nullptr)
-    , m_lastTextNodeEndedWithCollapsedSpace(false)
-    , m_lastCharacter(0)
-    , m_sortedTextBoxesPosition(0)
-    , m_hasEmitted(false)
-    , m_handledFirstLetter(false)
 {
     // FIXME: Only m_positionNode above needs to be initialized if range is null.
     if (!range)
@@ -407,16 +391,15 @@ void TextIterator::advance()
     m_text = StringView();
 
     // handle remembered node that needed a newline after the text node's newline
-    if (m_needsAnotherNewline) {
+    if (m_nodeForAdditionalNewline) {
         // Emit the extra newline, and position it *inside* m_node, after m_node's 
         // contents, in case it's a block, in the same way that we position the first 
         // newline. The range for the emitted newline should start where the line
         // break begins.
         // FIXME: It would be cleaner if we emitted two newlines during the last 
         // iteration, instead of using m_needsAnotherNewline.
-        Node& baseNode = m_node->lastChild() ? *m_node->lastChild() : *m_node;
-        emitCharacter('\n', *baseNode.parentNode(), &baseNode, 1, 1);
-        m_needsAnotherNewline = false;
+        emitCharacter('\n', *m_nodeForAdditionalNewline->parentNode(), m_nodeForAdditionalNewline, 1, 1);
+        m_nodeForAdditionalNewline = nullptr;
         return;
     }
 
@@ -475,11 +458,12 @@ void TextIterator::advance()
                     if ((pastEnd && parentNode == m_endContainer) || m_endContainer->isDescendantOf(*parentNode))
                         return;
                     bool haveRenderer = m_node->renderer();
+                    Node* exitedNode = m_node;
                     m_node = parentNode;
                     m_fullyClippedStack.pop();
                     parentNode = m_node->parentOrShadowHostNode();
                     if (haveRenderer)
-                        exitNode();
+                        exitNode(exitedNode);
                     if (m_positionNode) {
                         m_handledNode = true;
                         m_handledChildren = true;
@@ -1096,7 +1080,7 @@ bool TextIterator::handleNonTextNode()
     return true;
 }
 
-void TextIterator::exitNode()
+void TextIterator::exitNode(Node* exitedNode)
 {
     // prevent emitting a newline when exiting a collapsed block at beginning of the range
     // FIXME: !m_hasEmitted does not necessarily mean there was a collapsed block... it could
@@ -1108,7 +1092,7 @@ void TextIterator::exitNode()
     // Emit with a position *inside* m_node, after m_node's contents, in 
     // case it is a block, because the run should start where the 
     // emitted character is positioned visually.
-    Node* baseNode = m_node->lastChild() ? m_node->lastChild() : m_node;
+    Node* baseNode = exitedNode;
     // FIXME: This shouldn't require the m_lastTextNode to be true, but we can't change that without making
     // the logic in _web_attributedStringFromRange match. We'll get that for free when we switch to use
     // TextIterator in _web_attributedStringFromRange.
@@ -1123,8 +1107,9 @@ void TextIterator::exitNode()
             // insert a newline with a position following this block's contents.
             emitCharacter('\n', *baseNode->parentNode(), baseNode, 1, 1);
             // remember whether to later add a newline for the current node
-            ASSERT(!m_needsAnotherNewline);
-            m_needsAnotherNewline = addNewline;
+            ASSERT(!m_nodeForAdditionalNewline);
+            if (addNewline)
+                m_nodeForAdditionalNewline = baseNode;
         } else if (addNewline)
             // insert a newline with a position following this block's contents.
             emitCharacter('\n', *baseNode->parentNode(), baseNode, 1, 1);
index 9b0c811..1b78d67 100644 (file)
@@ -114,7 +114,7 @@ public:
     WEBCORE_EXPORT static Ref<Range> subrange(Range* entireRange, int characterOffset, int characterCount);
 
 private:
-    void exitNode();
+    void exitNode(Node*);
     bool shouldRepresentNodeOffsetZero();
     bool shouldEmitSpaceBeforeAndAfterNode(Node&);
     void representNodeOffsetZero();
@@ -126,44 +126,46 @@ private:
     void emitCharacter(UChar, Node& characterNode, Node* offsetBaseNode, int textStartOffset, int textEndOffset);
     void emitText(Text& textNode, RenderText&, int textStartOffset, int textEndOffset);
 
-    const TextIteratorBehavior m_behavior;
+    Node* baseNodeForEmittingNewLine() const;
+
+    const TextIteratorBehavior m_behavior { TextIteratorDefaultBehavior };
 
     // Current position, not necessarily of the text being returned, but position as we walk through the DOM tree.
-    Node* m_node;
-    int m_offset;
-    bool m_handledNode;
-    bool m_handledChildren;
+    Node* m_node { nullptr };
+    int m_offset { 0 };
+    bool m_handledNode { false };
+    bool m_handledChildren { false };
     BitStack m_fullyClippedStack;
 
     // The range.
-    Node* m_startContainer;
-    int m_startOffset;
-    Node* m_endContainer;
-    int m_endOffset;
-    Node* m_pastEndNode;
+    Node* m_startContainer { nullptr };
+    int m_startOffset { 0 };
+    Node* m_endContainer { nullptr };
+    int m_endOffset { 0 };
+    Node* m_pastEndNode { nullptr };
 
     // The current text and its position, in the form to be returned from the iterator.
-    Node* m_positionNode;
-    mutable Node* m_positionOffsetBaseNode;
-    mutable int m_positionStartOffset;
-    mutable int m_positionEndOffset;
+    Node* m_positionNode { nullptr };
+    mutable Node* m_positionOffsetBaseNode { nullptr };
+    mutable int m_positionStartOffset { 0 };
+    mutable int m_positionEndOffset { 0 };
     TextIteratorCopyableText m_copyableText;
     StringView m_text;
 
     // Used when there is still some pending text from the current node; when these are false and null, we go back to normal iterating.
-    bool m_needsAnotherNewline;
-    InlineTextBox* m_textBox;
+    Node* m_nodeForAdditionalNewline { nullptr };
+    InlineTextBox* m_textBox { nullptr };
 
     // Used when iterating over :first-letter text to save pointer to remaining text box.
-    InlineTextBox* m_remainingTextBox;
+    InlineTextBox* m_remainingTextBox { nullptr };
 
     // Used to point to RenderText object for :first-letter.
-    RenderText* m_firstLetterText;
+    RenderText* m_firstLetterText { nullptr };
 
     // Used to do the whitespace collapsing logic.
-    Text* m_lastTextNode;
-    bool m_lastTextNodeEndedWithCollapsedSpace;
-    UChar m_lastCharacter;
+    Text* m_lastTextNode { nullptr };
+    bool m_lastTextNodeEndedWithCollapsedSpace { false };
+    UChar m_lastCharacter { 0 };
 
     // Used to do simple line layout run logic.
     bool m_nextRunNeedsWhitespace { false };
@@ -173,13 +175,13 @@ private:
 
     // Used when text boxes are out of order (Hebrew/Arabic with embedded LTR text)
     Vector<InlineTextBox*> m_sortedTextBoxes;
-    size_t m_sortedTextBoxesPosition;
+    size_t m_sortedTextBoxesPosition { 0 };
 
     // Used when deciding whether to emit a "positioning" (e.g. newline) before any other content
-    bool m_hasEmitted;
+    bool m_hasEmitted { false };
 
     // Used when deciding text fragment created by :first-letter should be looked into.
-    bool m_handledFirstLetter;
+    bool m_handledFirstLetter { false };
 };
 
 // Iterates through the DOM range, returning all the text, and 0-length boundaries