Use more const references in MarkupAccumulator and its subclasses

Source/core/editing/markup.cpp

Index: Source/core/editing/markup.cpp
diff --git a/Source/core/editing/markup.cpp b/Source/core/editing/markup.cpp
index e1cdca63369f0b8b202af89dac1b45ecca333557..681b2400180eaffc79776457a197d47d40cf9b2c 100755
--- a/Source/core/editing/markup.cpp
+++ b/Source/core/editing/markup.cpp
@@ -116,29 +116,29 @@ class StyledMarkupAccumulator FINAL : public MarkupAccumulator {
 public:
     enum RangeFullySelectsNode { DoesFullySelectNode, DoesNotFullySelectNode };
 
-    StyledMarkupAccumulator(Vector<Node*>* nodes, EAbsoluteURLs, EAnnotateForInterchange, const Range*, Node* highestNodeToBeSerialized = 0);
+    StyledMarkupAccumulator(Vector<const Node*>* nodes, EAbsoluteURLs, EAnnotateForInterchange, const Range*, Node* highestNodeToBeSerialized = 0);
     Node* serializeNodes(Node* startNode, Node* pastEnd);
     void appendString(const String& s) { return MarkupAccumulator::appendString(s); }
-    void wrapWithNode(Node*, bool convertBlocksToInlines = false, RangeFullySelectsNode = DoesFullySelectNode);
-    void wrapWithStyleNode(StylePropertySet*, Document*, bool isBlock = false);
+    void wrapWithNode(const Node&, bool convertBlocksToInlines = false, RangeFullySelectsNode = DoesFullySelectNode);
+    void wrapWithStyleNode(StylePropertySet*, const Document&, bool isBlock = false);
     String takeResults();
 
 private:
-    void appendStyleNodeOpenTag(StringBuilder&, StylePropertySet*, Document*, bool isBlock = false);
+    void appendStyleNodeOpenTag(StringBuilder&, StylePropertySet*, const Document&, bool isBlock = false);
     const String& styleNodeCloseTag(bool isBlock = false);
-    virtual void appendText(StringBuilder& out, Text*) OVERRIDE;
-    String renderedText(const Node*, const Range*);
-    String stringValueForRange(const Node*, const Range*);
-    void appendElement(StringBuilder& out, Element*, bool addDisplayInline, RangeFullySelectsNode);
-    virtual void appendElement(StringBuilder& out, Element* element, Namespaces*) OVERRIDE { appendElement(out, element, false, DoesFullySelectNode); }
+    virtual void appendText(StringBuilder& out, const Text&) OVERRIDE;
+    String renderedText(const Node&, const Range*);
+    String stringValueForRange(const Node&, const Range*);
+    void appendElement(StringBuilder& out, const Element&, bool addDisplayInline, RangeFullySelectsNode);
+    virtual void appendElement(StringBuilder& out, const Element& element, Namespaces*) OVERRIDE { appendElement(out, element, false, DoesFullySelectNode); }
 
     enum NodeTraversalMode { EmitString, DoNotEmitString };
     Node* traverseNodesForSerialization(Node* startNode, Node* pastEnd, NodeTraversalMode);
 
     bool shouldAnnotate() { return m_shouldAnnotate == AnnotateForInterchange; }
-    bool shouldApplyWrappingStyle(Node* node) const
+    bool shouldApplyWrappingStyle(const Node& node) const
     {
-        return m_highestNodeToBeSerialized && m_highestNodeToBeSerialized->parentNode() == node->parentNode()
+        return m_highestNodeToBeSerialized && m_highestNodeToBeSerialized->parentNode() == node.parentNode()
             && m_wrappingStyle && m_wrappingStyle->style();
     }
 
@@ -148,7 +148,7 @@ private:
     RefPtr<EditingStyle> m_wrappingStyle;
 };
 
-inline StyledMarkupAccumulator::StyledMarkupAccumulator(Vector<Node*>* nodes, EAbsoluteURLs shouldResolveURLs, EAnnotateForInterchange shouldAnnotate,
+inline StyledMarkupAccumulator::StyledMarkupAccumulator(Vector<const Node*>* nodes, EAbsoluteURLs shouldResolveURLs, EAnnotateForInterchange shouldAnnotate,
     const Range* range, Node* highestNodeToBeSerialized)
     : MarkupAccumulator(nodes, shouldResolveURLs, range)
     , m_shouldAnnotate(shouldAnnotate)
@@ -156,20 +156,20 @@ inline StyledMarkupAccumulator::StyledMarkupAccumulator(Vector<Node*>* nodes, EA
 {
 }
 
-void StyledMarkupAccumulator::wrapWithNode(Node* node, bool convertBlocksToInlines, RangeFullySelectsNode rangeFullySelectsNode)
+void StyledMarkupAccumulator::wrapWithNode(const Node& node, bool convertBlocksToInlines, RangeFullySelectsNode rangeFullySelectsNode)
 {
     StringBuilder markup;
-    if (node->isElementNode())
-        appendElement(markup, toElement(node), convertBlocksToInlines && isBlock(const_cast<Node*>(node)), rangeFullySelectsNode);
+    if (node.isElementNode())
+        appendElement(markup, toElement(node), convertBlocksToInlines && isBlock(&node), rangeFullySelectsNode);
     else
         appendStartMarkup(markup, node, 0);
     m_reversedPrecedingMarkup.append(markup.toString());
     appendEndTag(node);
     if (m_nodes)
-        m_nodes->append(node);
+        m_nodes->append(&node);
 }
 
-void StyledMarkupAccumulator::wrapWithStyleNode(StylePropertySet* style, Document* document, bool isBlock)
+void StyledMarkupAccumulator::wrapWithStyleNode(StylePropertySet* style, const Document& document, bool isBlock)
 {
     StringBuilder openTag;
     appendStyleNodeOpenTag(openTag, style, document, isBlock);
@@ -177,7 +177,7 @@ void StyledMarkupAccumulator::wrapWithStyleNode(StylePropertySet* style, Documen
     appendString(styleNodeCloseTag(isBlock));
 }
 
-void StyledMarkupAccumulator::appendStyleNodeOpenTag(StringBuilder& out, StylePropertySet* style, Document* document, bool isBlock)
+void StyledMarkupAccumulator::appendStyleNodeOpenTag(StringBuilder& out, StylePropertySet* style, const Document& document, bool isBlock)
 {
     // wrappingStyleForSerialization should have removed -webkit-text-decorations-in-effect
     ASSERT(propertyMissingOrEqualToNone(style, CSSPropertyWebkitTextDecorationsInEffect));
@@ -185,7 +185,7 @@ void StyledMarkupAccumulator::appendStyleNodeOpenTag(StringBuilder& out, StylePr
         out.appendLiteral("<div style=\"");
     else
         out.appendLiteral("<span style=\"");
-    appendAttributeValue(out, style->asText(), document->isHTMLDocument());
+    appendAttributeValue(out, style->asText(), document.isHTMLDocument());
     out.appendLiteral("\">");
 }
 
@@ -210,9 +210,9 @@ String StyledMarkupAccumulator::takeResults()
     return result.toString().replace(0, "");
 }
 
-void StyledMarkupAccumulator::appendText(StringBuilder& out, Text* text)
+void StyledMarkupAccumulator::appendText(StringBuilder& out, const Text& text)
 {
-    const bool parentIsTextarea = text->parentElement() && text->parentElement()->tagQName() == textareaTag;
+    const bool parentIsTextarea = text.parentElement() && text.parentElement()->tagQName() == textareaTag;
     const bool wrappingSpan = shouldApplyWrappingStyle(text) && !parentIsTextarea;
     if (wrappingSpan) {
         RefPtr<EditingStyle> wrappingStyle = m_wrappingStyle->copy();
@@ -222,13 +222,13 @@ void StyledMarkupAccumulator::appendText(StringBuilder& out, Text* text)
         // FIXME: Should this be included in forceInline?
         wrappingStyle->style()->setProperty(CSSPropertyFloat, CSSValueNone);
 
-        appendStyleNodeOpenTag(out, wrappingStyle->style(), &text->document());
+        appendStyleNodeOpenTag(out, wrappingStyle->style(), text.document());
     }
 
     if (!shouldAnnotate() || parentIsTextarea)
         MarkupAccumulator::appendText(out, text);
     else {
-        const bool useRenderedText = !enclosingNodeWithTag(firstPositionInNode(text), selectTag);
+        const bool useRenderedText = !enclosingNodeWithTag(firstPositionInNode(const_cast<Text*>(&text)), selectTag);

[adamk, 2014/01/28 22:12:35]

This and the various const_casts below make me a bit sad. Are these just cases
where const-correctness needs to be pushed down further? Or is there actual
mutation happening in the code these things are calling? 

[Inactive, 2014/01/29 17:13:21]

Yes, this is unfortunate. I will be less agressive on the constness to avoid
this in the next iteration.

         String content = useRenderedText ? renderedText(text, m_range) : stringValueForRange(text, m_range);
         StringBuilder buffer;
         appendCharactersReplacingEntities(buffer, content, 0, content.length(), EntityMaskInPCDATA);
@@ -239,31 +239,31 @@ void StyledMarkupAccumulator::appendText(StringBuilder& out, Text* text)
         out.append(styleNodeCloseTag());
 }
 
-String StyledMarkupAccumulator::renderedText(const Node* node, const Range* range)
+String StyledMarkupAccumulator::renderedText(const Node& node, const Range* range)
 {
-    if (!node->isTextNode())
+    if (!node.isTextNode())
         return String();
 
-    const Text* textNode = toText(node);
+    const Text& textNode = toText(node);
     unsigned startOffset = 0;
-    unsigned endOffset = textNode->length();
+    unsigned endOffset = textNode.length();
 
     if (range && node == range->startContainer())
         startOffset = range->startOffset();
     if (range && node == range->endContainer())
         endOffset = range->endOffset();
 
-    Position start = createLegacyEditingPosition(const_cast<Node*>(node), startOffset);
-    Position end = createLegacyEditingPosition(const_cast<Node*>(node), endOffset);
-    return plainText(Range::create(node->document(), start, end).get());
+    Position start = createLegacyEditingPosition(const_cast<Node*>(&node), startOffset);
+    Position end = createLegacyEditingPosition(const_cast<Node*>(&node), endOffset);
+    return plainText(Range::create(node.document(), start, end).get());
 }
 
-String StyledMarkupAccumulator::stringValueForRange(const Node* node, const Range* range)
+String StyledMarkupAccumulator::stringValueForRange(const Node& node, const Range* range)
 {
     if (!range)
-        return node->nodeValue();
+        return node.nodeValue();
 
-    String str = node->nodeValue();
+    String str = node.nodeValue();
     if (node == range->endContainer())
         str.truncate(range->endOffset());
     if (node == range->startContainer())
@@ -271,16 +271,16 @@ String StyledMarkupAccumulator::stringValueForRange(const Node* node, const Rang
     return str;
 }
 
-void StyledMarkupAccumulator::appendElement(StringBuilder& out, Element* element, bool addDisplayInline, RangeFullySelectsNode rangeFullySelectsNode)
+void StyledMarkupAccumulator::appendElement(StringBuilder& out, const Element& element, bool addDisplayInline, RangeFullySelectsNode rangeFullySelectsNode)
 {
-    const bool documentIsHTML = element->document().isHTMLDocument();
+    const bool documentIsHTML = element.document().isHTMLDocument();
     appendOpenTag(out, element, 0);
 
-    const unsigned length = element->hasAttributes() ? element->attributeCount() : 0;
-    const bool shouldAnnotateOrForceInline = element->isHTMLElement() && (shouldAnnotate() || addDisplayInline);
+    const unsigned length = element.hasAttributes() ? element.attributeCount() : 0;
+    const bool shouldAnnotateOrForceInline = element.isHTMLElement() && (shouldAnnotate() || addDisplayInline);
     const bool shouldOverrideStyleAttr = shouldAnnotateOrForceInline || shouldApplyWrappingStyle(element);
     for (unsigned i = 0; i < length; ++i) {
-        const Attribute* attribute = element->attributeItem(i);
+        const Attribute* attribute = element.attributeItem(i);
         // We'll handle the style attribute separately, below.
         if (attribute->name() == styleAttr && shouldOverrideStyleAttr)
             continue;
@@ -292,17 +292,17 @@ void StyledMarkupAccumulator::appendElement(StringBuilder& out, Element* element
 
         if (shouldApplyWrappingStyle(element)) {
             newInlineStyle = m_wrappingStyle->copy();
-            newInlineStyle->removePropertiesInElementDefaultStyle(element);
-            newInlineStyle->removeStyleConflictingWithStyleOfNode(element);
+            newInlineStyle->removePropertiesInElementDefaultStyle(const_cast<Element*>(&element));
+            newInlineStyle->removeStyleConflictingWithStyleOfNode(const_cast<Element*>(&element));
         } else
             newInlineStyle = EditingStyle::create();
 
-        if (element->isStyledElement() && element->inlineStyle())
-            newInlineStyle->overrideWithStyle(element->inlineStyle());
+        if (element.isStyledElement() && element.inlineStyle())
+            newInlineStyle->overrideWithStyle(element.inlineStyle());
 
         if (shouldAnnotateOrForceInline) {
             if (shouldAnnotate())
-                newInlineStyle->mergeStyleFromRulesForSerialization(toHTMLElement(element));
+                newInlineStyle->mergeStyleFromRulesForSerialization(const_cast<HTMLElement*>(&toHTMLElement(element)));
 
             if (addDisplayInline)
                 newInlineStyle->forceInline();
@@ -366,12 +366,12 @@ Node* StyledMarkupAccumulator::traverseNodesForSerialization(Node* startNode, No
         } else {
             // Add the node to the markup if we're not skipping the descendants
             if (shouldEmit)
-                appendStartTag(n);
+                appendStartTag(*n);
 
             // If node has no children, close the tag now.
             if (!n->childNodeCount()) {
                 if (shouldEmit)
-                    appendEndTag(n);
+                    appendEndTag(*n);
                 lastClosed = n;
             } else {
                 openedTag = true;
@@ -385,11 +385,12 @@ Node* StyledMarkupAccumulator::traverseNodesForSerialization(Node* startNode, No
             // Close up the ancestors.
             while (!ancestorsToClose.isEmpty()) {
                 Node* ancestor = ancestorsToClose.last();
+                ASSERT(ancestor);
                 if (next != pastEnd && next->isDescendantOf(ancestor))
                     break;
                 // Not at the end of the range, close ancestors up to sibling of next node.
                 if (shouldEmit)
-                    appendEndTag(ancestor);
+                    appendEndTag(*ancestor);
                 lastClosed = ancestor;
                 ancestorsToClose.removeLast();
             }
@@ -405,7 +406,7 @@ Node* StyledMarkupAccumulator::traverseNodesForSerialization(Node* startNode, No
                     // or b) ancestors that we never encountered during a pre-order traversal starting at startNode:
                     ASSERT(startNode->isDescendantOf(parent));
                     if (shouldEmit)
-                        wrapWithNode(parent);
+                        wrapWithNode(*parent);
                     lastClosed = parent;
                 }
             }
@@ -539,7 +540,7 @@ static Node* highestAncestorToWrapMarkup(const Range* range, EAnnotateForInterch
 
 // FIXME: Shouldn't we omit style info when annotate == DoNotAnnotateForInterchange?
 // FIXME: At least, annotation and style info should probably not be included in range.markupString()
-static String createMarkupInternal(Document& document, const Range* range, const Range* updatedRange, Vector<Node*>* nodes,
+static String createMarkupInternal(Document& document, const Range* range, const Range* updatedRange, Vector<const Node*>* nodes,
     EAnnotateForInterchange shouldAnnotate, bool convertBlocksToInlines, EAbsoluteURLs shouldResolveURLs, Node* constrainingAncestor)
 {
     ASSERT(range);
@@ -600,12 +601,12 @@ static String createMarkupInternal(Document& document, const Range* range, const
                         fullySelectedRootStyle->style()->setProperty(CSSPropertyTextDecoration, CSSValueNone);
                     if (!propertyMissingOrEqualToNone(fullySelectedRootStyle->style(), CSSPropertyWebkitTextDecorationsInEffect))
                         fullySelectedRootStyle->style()->setProperty(CSSPropertyWebkitTextDecorationsInEffect, CSSValueNone);
-                    accumulator.wrapWithStyleNode(fullySelectedRootStyle->style(), &document, true);
+                    accumulator.wrapWithStyleNode(fullySelectedRootStyle->style(), document, true);
                 }
             } else {
                 // Since this node and all the other ancestors are not in the selection we want to set RangeFullySelectsNode to DoesNotFullySelectNode
                 // so that styles that affect the exterior of the node are not included.
-                accumulator.wrapWithNode(ancestor, convertBlocksToInlines, StyledMarkupAccumulator::DoesNotFullySelectNode);
+                accumulator.wrapWithNode(*ancestor, convertBlocksToInlines, StyledMarkupAccumulator::DoesNotFullySelectNode);
             }
             if (nodes)
                 nodes->append(ancestor);
@@ -622,7 +623,7 @@ static String createMarkupInternal(Document& document, const Range* range, const
     return accumulator.takeResults();
 }
 
-String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterchange shouldAnnotate, bool convertBlocksToInlines, EAbsoluteURLs shouldResolveURLs, Node* constrainingAncestor)
+String createMarkup(const Range* range, Vector<const Node*>* nodes, EAnnotateForInterchange shouldAnnotate, bool convertBlocksToInlines, EAbsoluteURLs shouldResolveURLs, Node* constrainingAncestor)
 {
     if (!range)
         return emptyString();
@@ -732,13 +733,13 @@ PassRefPtr<DocumentFragment> createFragmentFromMarkupWithContext(Document& docum
     return fragment;
 }
 
-String createMarkup(const Node* node, EChildrenOnly childrenOnly, Vector<Node*>* nodes, EAbsoluteURLs shouldResolveURLs, Vector<QualifiedName>* tagNamesToSkip)
+String createMarkup(const Node* node, EChildrenOnly childrenOnly, Vector<const Node*>* nodes, EAbsoluteURLs shouldResolveURLs, Vector<QualifiedName>* tagNamesToSkip)
 {
     if (!node)
         return "";
 
     MarkupAccumulator accumulator(nodes, shouldResolveURLs);
-    return accumulator.serializeNodes(const_cast<Node*>(node), childrenOnly, tagNamesToSkip);
+    return accumulator.serializeNodes(*node, childrenOnly, tagNamesToSkip);
 }
 
 static void fillContainerFromString(ContainerNode* paragraph, const String& string)