Introduce StyledMarkupTraverser to StyledMarkupSerializer

Source/core/editing/StyledMarkupSerializer.cpp

 /*
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2008, 2009, 2010, 2011 Google Inc. All rights reserved.
  * Copyright (C) 2011 Igalia S.L.
  * Copyright (C) 2011 Motorola Mobility. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
@@ skipping 41 matching lines @@
 {
     if (position.isNull())
         return TextOffset();
 
     if (!position.containerNode()->isTextNode())
         return TextOffset();
 
     return TextOffset(toText(position.containerNode()), position.offsetInContainerNode());
 }
 
+template<typename Strategy>
+class Traverser {
+    WTF_MAKE_NONCOPYABLE(Traverser);
+    STACK_ALLOCATED();
+public:
+    Traverser()
+        : Traverser(nullptr, nullptr)
+    {
+    }
+    Traverser(StyledMarkupAccumulator* accumulator, Node* lastClosed)

[yosin_UTC9, 2015/06/17 13:07:57]

How about moving this ctor before L244, I hope code review tool make small diff
with old L203-L204 and new L76.

[hajimehoshi, 2015/06/18 02:04:56]

Done.

+        : m_accumulator(accumulator)

[yosin_UTC9, 2015/06/17 13:07:57]

ASSERT(m_lastClosed || !m_accumlator)?

I think when m_accumulator is nullptr, m_lastClosed is nullptr too.

[hajimehoshi, 2015/06/18 02:04:56]

Done.

+        , m_lastClosed(lastClosed)

[yosin_UTC9, 2015/06/17 13:07:58]

It is better to initialize |m_wrappingStyle| to nullptr.

[hajimehoshi, 2015/06/18 02:04:56]

Done.

+    {
+        if (m_accumulator && m_lastClosed && Strategy::parent(*m_lastClosed))

[yosin_UTC9, 2015/06/17 13:07:58]

Let's use early return pattern.

[hajimehoshi, 2015/06/18 02:04:56]

Done.

+            m_wrappingStyle = EditingStyle::wrappingStyleForSerialization(Strategy::parent(*m_lastClosed), m_accumulator->shouldAnnotate());

[yosin_UTC9, 2015/06/17 13:31:02]

Since, we use |m_accumulator->shouldAnnotate()| four times, let's make
|Traverser| has a member function |shouldAnnoate()|. Also, diff size becomes
smaller. ;-)

[hajimehoshi, 2015/06/18 02:04:56]

Done.

+    }
+    Node* traverseNodesForSerialization(Node*, Node*);
+    void wrapWithNode(ContainerNode&, PassRefPtrWillBeRawPtr<EditingStyle>);
+    RefPtrWillBeRawPtr<EditingStyle> createInlineStyleIfNeeded(Node&);
+
+private:
+    void appendStartMarkup(Node&);
+    void appendEndMarkup(Node&);
+    RefPtrWillBeRawPtr<EditingStyle> createInlineStyle(Element&);
+    bool needsInlineStyle(const Element&);
+    bool shouldApplyWrappingStyle(const Node&) const;
+
+    StyledMarkupAccumulator* m_accumulator;
+    Node* m_lastClosed;
+    RefPtrWillBeMember<EditingStyle> m_wrappingStyle;
+};
+
 } // namespace
 
 using namespace HTMLNames;
 
 static Position toPositionInDOMTree(const Position& position)
 {
     return position;
 }
 
 template<typename Strategy>
@@ skipping 39 matching lines @@
     RefPtrWillBeRawPtr<EditingStyle> style = EditingStyle::create(element->inlineStyle());
     // FIXME: Having to const_cast here is ugly, but it is quite a bit of work to untangle
     // the non-const-ness of styleFromMatchedRulesForElement.
     style->mergeStyleFromRules(const_cast<HTMLElement*>(element));
     return style.release();
 }
 
 template<typename Strategy>
 String StyledMarkupSerializer<Strategy>::createMarkup()
 {
-    StyledMarkupAccumulator markupAccumulator(m_shouldResolveURLs, toTextOffset(m_start.parentAnchoredEquivalent()), toTextOffset(m_end.parentAnchoredEquivalent()), m_start.document(), m_shouldAnnotate);
+    StyledMarkupAccumulator markupAccumulator(m_shouldResolveURLs, toTextOffset(m_start.parentAnchoredEquivalent()), toTextOffset(m_end.parentAnchoredEquivalent()), m_start.document(), m_shouldAnnotate, m_convertBlocksToInlines);
 
     Node* pastEnd = m_end.nodeAsRangePastLastNode();
 
     Node* firstNode = m_start.nodeAsRangeFirstNode();
     VisiblePosition visibleStart(toPositionInDOMTree(m_start), VP_DEFAULT_AFFINITY);
     VisiblePosition visibleEnd(toPositionInDOMTree(m_end), VP_DEFAULT_AFFINITY);
     if (shouldAnnotate() && needInterchangeNewlineAfter(visibleStart)) {
         markupAccumulator.appendInterchangeNewline();
         if (visibleStart == visibleEnd.previous())
             return markupAccumulator.takeResults();
 
         firstNode = visibleStart.next().deepEquivalent().deprecatedNode();
 
         if (pastEnd && Strategy::PositionType::beforeNode(firstNode).compareTo(Strategy::PositionType::beforeNode(pastEnd)) >= 0) {
             // This condition hits in editing/pasteboard/copy-display-none.html.
             return markupAccumulator.takeResults();
         }
     }
 
-    Node* lastClosed = serializeNodes(firstNode, pastEnd, &markupAccumulator);
+    if (!m_lastClosed)
+        m_lastClosed = Traverser<Strategy>().traverseNodesForSerialization(firstNode, pastEnd);
+    Traverser<Strategy> traverser(&markupAccumulator, m_lastClosed);
+    Node* lastClosed = traverser.traverseNodesForSerialization(firstNode, pastEnd);
 
     if (m_highestNodeToBeSerialized && lastClosed) {
         // TODO(hajimehoshi): This is calculated at createMarkupInternal too.
         Node* commonAncestor = Strategy::commonAncestor(*m_start.containerNode(), *m_end.containerNode());
         ASSERT(commonAncestor);
         HTMLBodyElement* body = toHTMLBodyElement(enclosingElementWithTag(firstPositionInNode(commonAncestor), bodyTag));
         HTMLBodyElement* fullySelectedRoot = nullptr;
         // FIXME: Do this for all fully selected blocks, not just the body.
         if (body && areSameRanges(body, m_start, m_end))
             fullySelectedRoot = body;
 
         // Also include all of the ancestors of lastClosed up to this special ancestor.
         // FIXME: What is ancestor?
         for (ContainerNode* ancestor = Strategy::parent(*lastClosed); ancestor; ancestor = Strategy::parent(*ancestor)) {
-            if (ancestor == fullySelectedRoot && !convertBlocksToInlines()) {
+            if (ancestor == fullySelectedRoot && !markupAccumulator.convertBlocksToInlines()) {
                 RefPtrWillBeRawPtr<EditingStyle> fullySelectedRootStyle = styleFromMatchedRulesAndInlineDecl(fullySelectedRoot);
 
                 // Bring the background attribute over, but not as an attribute because a background attribute on a div
                 // appears to have no effect.
                 if ((!fullySelectedRootStyle || !fullySelectedRootStyle->style() || !fullySelectedRootStyle->style()->getPropertyCSSValue(CSSPropertyBackgroundImage))
                     && fullySelectedRoot->hasAttribute(backgroundAttr))
                     fullySelectedRootStyle->style()->setProperty(CSSPropertyBackgroundImage, "url('" + fullySelectedRoot->getAttribute(backgroundAttr) + "')");
 
                 if (fullySelectedRootStyle->style()) {
                     // Reset the CSS properties to avoid an assertion error in addStyleMarkup().
                     // This assertion is caused at least when we select all text of a <body> element whose
                     // 'text-decoration' property is "inherit", and copy it.
                     if (!propertyMissingOrEqualToNone(fullySelectedRootStyle->style(), CSSPropertyTextDecoration))
                         fullySelectedRootStyle->style()->setProperty(CSSPropertyTextDecoration, CSSValueNone);
                     if (!propertyMissingOrEqualToNone(fullySelectedRootStyle->style(), CSSPropertyWebkitTextDecorationsInEffect))
                         fullySelectedRootStyle->style()->setProperty(CSSPropertyWebkitTextDecorationsInEffect, CSSValueNone);
                     markupAccumulator.wrapWithStyleNode(fullySelectedRootStyle->style());
                 }
             } else {
-                RefPtrWillBeRawPtr<EditingStyle> style = createInlineStyleIfNeeded(*ancestor);
+                RefPtrWillBeRawPtr<EditingStyle> style = traverser.createInlineStyleIfNeeded(*ancestor);
                 // Since this node and all the other ancestors are not in the selection we want
                 // styles that affect the exterior of the node not to be not included.
                 // If the node is not fully selected by the range, then we don't want to keep styles that affect its relationship to the nodes around it
                 // only the ones that affect it and the nodes within it.
                 if (style && style->style())
                     style->style()->removeProperty(CSSPropertyFloat);
-                wrapWithNode(markupAccumulator, *ancestor, style);
+                traverser.wrapWithNode(*ancestor, style);
             }
 
             if (ancestor == m_highestNodeToBeSerialized)
                 break;
         }
     }
 
     // FIXME: The interchange newline should be placed in the block that it's in, not after all of the content, unconditionally.
     if (shouldAnnotate() && needInterchangeNewlineAt(visibleEnd))
         markupAccumulator.appendInterchangeNewline();
 
     return markupAccumulator.takeResults();
 }
 
 template<typename Strategy>
-Node* StyledMarkupSerializer<Strategy>::serializeNodes(Node* startNode, Node* pastEnd, StyledMarkupAccumulator* markupAccumulator)
-{
-    if (!m_lastClosed)
-        m_lastClosed = traverseNodesForSerialization(startNode, pastEnd, nullptr);
-    if (m_lastClosed && Strategy::parent(*m_lastClosed))
-        m_wrappingStyle = EditingStyle::wrappingStyleForSerialization(Strategy::parent(*m_lastClosed), shouldAnnotate());
-    return traverseNodesForSerialization(startNode, pastEnd, markupAccumulator);
-}
-
-template<typename Strategy>
-Node* StyledMarkupSerializer<Strategy>::traverseNodesForSerialization(Node* startNode, Node* pastEnd, StyledMarkupAccumulator* markupAccumulator)
+Node* Traverser<Strategy>::traverseNodesForSerialization(Node* startNode, Node* pastEnd)
 {
     WillBeHeapVector<RawPtrWillBeMember<ContainerNode>> ancestorsToClose;
     Node* next;
     Node* lastClosed = nullptr;
     for (Node* n = startNode; n != pastEnd; n = next) {
         // According to <rdar://problem/5730668>, it is possible for n to blow
         // past pastEnd and become null here. This shouldn't be possible.
         // This null check will prevent crashes (but create too much markup)
         // and the ASSERT will hopefully lead us to understanding the problem.
         ASSERT(n);
         if (!n)
             break;
 
         next = Strategy::next(*n);
 
         if (isBlock(n) && canHaveChildrenForEditing(n) && next == pastEnd) {
             // Don't write out empty block containers that aren't fully selected.
             continue;
         }
 
         if (!n->layoutObject() && !enclosingElementWithTag(firstPositionInOrBeforeNode(n), selectTag)) {
             next = Strategy::nextSkippingChildren(*n);
             // Don't skip over pastEnd.
             if (pastEnd && Strategy::isDescendantOf(*pastEnd, *n))
                 next = pastEnd;
         } else {
             // Add the node to the markup if we're not skipping the descendants
-            if (markupAccumulator)
-                appendStartMarkup(*markupAccumulator, *n);
+            appendStartMarkup(*n);
 
             // If node has no children, close the tag now.
             if (Strategy::hasChildren(*n)) {
                 ancestorsToClose.append(toContainerNode(n));
                 continue;
             }
-            if (markupAccumulator && n->isElementNode())
-                markupAccumulator->appendEndTag(toElement(*n));
+            appendEndMarkup(*n);
             lastClosed = n;
         }
 
         // If we didn't insert open tag and there's no more siblings or we're at the end of the traversal, take care of ancestors.
         // FIXME: What happens if we just inserted open tag and reached the end?
         if (Strategy::nextSibling(*n) && next != pastEnd)
             continue;
 
         // Close up the ancestors.
         while (!ancestorsToClose.isEmpty()) {
             ContainerNode* ancestor = ancestorsToClose.last();
             ASSERT(ancestor);
             if (next != pastEnd && Strategy::isDescendantOf(*next, *ancestor))
                 break;
             // Not at the end of the range, close ancestors up to sibling of next node.
-            if (markupAccumulator && ancestor->isElementNode())
-                markupAccumulator->appendEndTag(toElement(*ancestor));
+            appendEndMarkup(*ancestor);
             lastClosed = ancestor;
             ancestorsToClose.removeLast();
         }
 
         // Surround the currently accumulated markup with markup for ancestors we never opened as we leave the subtree(s) rooted at those ancestors.
         ContainerNode* nextParent = next ? Strategy::parent(*next) : nullptr;
         if (next == pastEnd || n == nextParent)
             continue;
 
         ASSERT(n);
         Node* lastAncestorClosedOrSelf = Strategy::isDescendantOf(*n, *lastClosed) ? lastClosed : n;
         for (ContainerNode* parent = Strategy::parent(*lastAncestorClosedOrSelf); parent && parent != nextParent; parent = Strategy::parent(*parent)) {
             // All ancestors that aren't in the ancestorsToClose list should either be a) unrendered:
             if (!parent->layoutObject())
                 continue;
             // or b) ancestors that we never encountered during a pre-order traversal starting at startNode:
             ASSERT(startNode);
             ASSERT(Strategy::isDescendantOf(*startNode, *parent));
-            if (markupAccumulator) {
-                RefPtrWillBeRawPtr<EditingStyle> style = createInlineStyleIfNeeded(*parent);
-                wrapWithNode(*markupAccumulator, *parent, style);
-            }
+            RefPtrWillBeRawPtr<EditingStyle> style = createInlineStyleIfNeeded(*parent);
+            wrapWithNode(*parent, style);
             lastClosed = parent;
         }
     }
 
     return lastClosed;
 }
 
 template<typename Strategy>
-bool StyledMarkupSerializer<Strategy>::needsInlineStyle(const Element& element)
+bool Traverser<Strategy>::needsInlineStyle(const Element& element)
 {
     if (!element.isHTMLElement())
         return false;
-    if (shouldAnnotate())
+    if (m_accumulator->shouldAnnotate())
         return true;
-    return convertBlocksToInlines() && isBlock(&element);
+    return m_accumulator->convertBlocksToInlines() && isBlock(&element);
 }
 
 template<typename Strategy>
-void StyledMarkupSerializer<Strategy>::wrapWithNode(StyledMarkupAccumulator& accumulator, ContainerNode& node, PassRefPtrWillBeRawPtr<EditingStyle> style)
+void Traverser<Strategy>::wrapWithNode(ContainerNode& node, PassRefPtrWillBeRawPtr<EditingStyle> style)
 {
+    if (!m_accumulator)
+        return;
     StringBuilder markup;
     if (node.isDocumentNode()) {
         MarkupFormatter::appendXMLDeclaration(markup, toDocument(node));
-        accumulator.pushMarkup(markup.toString());
+        m_accumulator->pushMarkup(markup.toString());
         return;
     }
     if (!node.isElementNode())
         return;
     Element& element = toElement(node);
     if (shouldApplyWrappingStyle(element) || needsInlineStyle(element))

[yosin_UTC9, 2015/06/17 14:33:06]

Note: in ALL-IN-ONE, we check |shouldApplyWrappingStyle(element)| and
|needInlineStyle(element)| before creating style to avoid useless style creation
for reducing memory and save execution time. So, we can exclude this
optimization.

[hajimehoshi, 2015/06/18 02:04:56]

As we talked, let's do such fix later.

-        accumulator.appendElementWithInlineStyle(markup, element, style);
+        m_accumulator->appendElementWithInlineStyle(markup, element, style);
     else
-        accumulator.appendElement(markup, element);
-    accumulator.pushMarkup(markup.toString());
-    accumulator.appendEndTag(toElement(node));
+        m_accumulator->appendElement(markup, element);
+    m_accumulator->pushMarkup(markup.toString());
+    m_accumulator->appendEndTag(toElement(node));
 }
 
 template<typename Strategy>
-RefPtrWillBeRawPtr<EditingStyle> StyledMarkupSerializer<Strategy>::createInlineStyleIfNeeded(Node& node)
+RefPtrWillBeRawPtr<EditingStyle> Traverser<Strategy>::createInlineStyleIfNeeded(Node& node)
 {
+    if (!m_accumulator)
+        return nullptr;
     if (!node.isElementNode())
         return nullptr;
     RefPtrWillBeRawPtr<EditingStyle> inlineStyle = createInlineStyle(toElement(node));
-    if (convertBlocksToInlines() && isBlock(&node))
+    if (m_accumulator->convertBlocksToInlines() && isBlock(&node))

[yosin_UTC9, 2015/06/17 14:06:28]

We have two |m_accumulator->convertBlocksToInlines()|, so it may be better to
have |Tarverser::convertBlocksToInlines()| for reducing diff count.

[hajimehoshi, 2015/06/18 02:04:56]

Done.

         inlineStyle->forceInline();
     return inlineStyle;
 }
 
 template<typename Strategy>
-void StyledMarkupSerializer<Strategy>::appendStartMarkup(StyledMarkupAccumulator& accumulator, Node& node)
+void Traverser<Strategy>::appendStartMarkup(Node& node)
 {
+    if (!m_accumulator)
+        return;
     switch (node.nodeType()) {
     case Node::TEXT_NODE: {
         Text& text = toText(node);
         if (text.parentElement() && text.parentElement()->tagQName() == textareaTag) {

[yosin_UTC9, 2015/06/17 13:31:01]

isHTMLTextAreaElement(text.parentElement())

[hajimehoshi, 2015/06/18 02:04:56]

Done.

-            accumulator.appendText(text);
+            m_accumulator->appendText(text);
             break;
         }
         RefPtrWillBeRawPtr<EditingStyle> inlineStyle = nullptr;
         if (shouldApplyWrappingStyle(text)) {
             inlineStyle = m_wrappingStyle->copy();
             // FIXME: <rdar://problem/5371536> Style rules that match pasted content can change it's appearance
             // Make sure spans are inline style in paste side e.g. span { display: block }.
             inlineStyle->forceInline();
             // FIXME: Should this be included in forceInline?
             inlineStyle->style()->setProperty(CSSPropertyFloat, CSSValueNone);
         }
-        accumulator.appendTextWithInlineStyle(text, inlineStyle);
+        m_accumulator->appendTextWithInlineStyle(text, inlineStyle);

[yosin_UTC9, 2015/06/17 13:31:02]

Note: For supporting composed tree, we should have entry mask for parent here.
ALL-IN-ONE does so.

When |!shouldAnnotate()| in |appendTextWithInlineStyle|, we need to calculate
entry mask from Strategy::parent from text node, we should calculate entry mask
here and pass to |appendText()|.

[hajimehoshi, 2015/06/18 02:04:56]

As we talked, do such fix later.

         break;
     }
     case Node::ELEMENT_NODE: {
         Element& element = toElement(node);
-        if ((element.isHTMLElement() && shouldAnnotate()) || shouldApplyWrappingStyle(element)) {
+        if ((element.isHTMLElement() && m_accumulator->shouldAnnotate()) || shouldApplyWrappingStyle(element)) {
             RefPtrWillBeRawPtr<EditingStyle> inlineStyle = createInlineStyle(element);
-            accumulator.appendElementWithInlineStyle(element, inlineStyle);
+            m_accumulator->appendElementWithInlineStyle(element, inlineStyle);
             break;
         }
-        accumulator.appendElement(element);
+        m_accumulator->appendElement(element);
         break;
     }
     default:
-        accumulator.appendStartMarkup(node);
+        m_accumulator->appendStartMarkup(node);
         break;
     }
 }
 
 template<typename Strategy>
-bool StyledMarkupSerializer<Strategy>::shouldApplyWrappingStyle(const Node& node) const
+void Traverser<Strategy>::appendEndMarkup(Node& node)
+{
+    if (m_accumulator && node.isElementNode())

[yosin_UTC9, 2015/06/17 13:31:02]

Let's use early return pattern.

[hajimehoshi, 2015/06/18 02:04:56]

Done.

+        m_accumulator->appendEndTag(toElement(node));

[yosin_UTC9, 2015/06/17 14:06:28]

We need to check whether we use empty tag or not, it depends on document ,
|Strategy::hasChildren()|, and |elemtnCannotHaveEndTag()|. See ALL-IN-ONE.

[hajimehoshi, 2015/06/18 02:04:56]

As we talked, do such fix later.

+}
+
+template<typename Strategy>
+bool Traverser<Strategy>::shouldApplyWrappingStyle(const Node& node) const
 {
     return m_lastClosed && Strategy::parent(*m_lastClosed) == Strategy::parent(node)
         && m_wrappingStyle && m_wrappingStyle->style();
 }
 
 template<typename Strategy>
-RefPtrWillBeRawPtr<EditingStyle> StyledMarkupSerializer<Strategy>::createInlineStyle(Element& element)
+RefPtrWillBeRawPtr<EditingStyle> Traverser<Strategy>::createInlineStyle(Element& element)
 {
     RefPtrWillBeRawPtr<EditingStyle> inlineStyle = nullptr;
 
     if (shouldApplyWrappingStyle(element)) {
         inlineStyle = m_wrappingStyle->copy();
         inlineStyle->removePropertiesInElementDefaultStyle(&element);
         inlineStyle->removeStyleConflictingWithStyleOfElement(&element);
     } else {
         inlineStyle = EditingStyle::create();
     }
 
     if (element.isStyledElement() && element.inlineStyle())
         inlineStyle->overrideWithStyle(element.inlineStyle());
 
-    if (element.isHTMLElement() && shouldAnnotate())
+    if (element.isHTMLElement() && m_accumulator->shouldAnnotate())
         inlineStyle->mergeStyleFromRulesForSerialization(&toHTMLElement(element));
 
     return inlineStyle;
 }
 
 template class StyledMarkupSerializer<EditingStrategy>;
 
 } // namespace blink