Have ReplaceSelectionCommand::InsertedNodes API take references in argument

Source/core/editing/ReplaceSelectionCommand.cpp

 /*
  * Copyright (C) 2005, 2006, 2008 Apple Inc. All rights reserved.
  * Copyright (C) 2009, 2010, 2011 Google Inc. 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
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 304 matching lines @@
     while (node) {
         RefPtr<Node> next = NodeTraversal::next(node);
         if (isInterchangeConvertedSpaceSpan(node)) {
             next = NodeTraversal::nextSkippingChildren(node);
             removeNodePreservingChildren(node);
         }
         node = next.get();
     }
 }
 
-inline void ReplaceSelectionCommand::InsertedNodes::respondToNodeInsertion(Node* node)
+inline void ReplaceSelectionCommand::InsertedNodes::respondToNodeInsertion(Node& node)
 {
-    if (!node)
-        return;
+    if (!m_firstNodeInserted)
+        m_firstNodeInserted = &node;
 
-    if (!m_firstNodeInserted)
-        m_firstNodeInserted = node;
-
-    m_lastNodeInserted = node;
+    m_lastNodeInserted = &node;
 }
 
-inline void ReplaceSelectionCommand::InsertedNodes::willRemoveNodePreservingChildren(Node* node)
+inline void ReplaceSelectionCommand::InsertedNodes::willRemoveNodePreservingChildren(Node& node)
 {
     if (m_firstNodeInserted == node)
-        m_firstNodeInserted = NodeTraversal::next(node);
+        m_firstNodeInserted = NodeTraversal::next(&node);
     if (m_lastNodeInserted == node)
-        m_lastNodeInserted = node->lastChild() ? node->lastChild() : NodeTraversal::nextSkippingChildren(node);
+        m_lastNodeInserted = node.lastChild() ? node.lastChild() : NodeTraversal::nextSkippingChildren(&node);
 }
 
-inline void ReplaceSelectionCommand::InsertedNodes::willRemoveNode(Node* node)
+inline void ReplaceSelectionCommand::InsertedNodes::willRemoveNode(Node& node)
 {
     if (m_firstNodeInserted == node && m_lastNodeInserted == node) {
         m_firstNodeInserted = 0;
         m_lastNodeInserted = 0;
     } else if (m_firstNodeInserted == node)
         m_firstNodeInserted = NodeTraversal::nextSkippingChildren(m_firstNodeInserted.get());
     else if (m_lastNodeInserted == node)
         m_lastNodeInserted = NodeTraversal::previousSkippingChildren(m_lastNodeInserted.get());
 }
 
-inline void ReplaceSelectionCommand::InsertedNodes::didReplaceNode(Node* node, Node* newNode)
+inline void ReplaceSelectionCommand::InsertedNodes::didReplaceNode(Node& node, Node& newNode)
 {
     if (m_firstNodeInserted == node)
-        m_firstNodeInserted = newNode;
+        m_firstNodeInserted = &newNode;
     if (m_lastNodeInserted == node)
-        m_lastNodeInserted = newNode;
+        m_lastNodeInserted = &newNode;
 }
 
 ReplaceSelectionCommand::ReplaceSelectionCommand(Document& document, PassRefPtr<DocumentFragment> fragment, CommandOptions options, EditAction editAction)
     : CompositeEditCommand(document)
     , m_selectReplacement(options & SelectReplacement)
     , m_smartReplace(options & SmartReplace)
     , m_matchStyle(options & MatchStyle)
     , m_documentFragment(fragment)
     , m_preventNesting(options & PreventNesting)
     , m_movingParagraph(options & MovingParagraph)
@@ skipping 104 matching lines @@
         if (!node->isStyledElement())
             continue;
 
         Element* element = toElement(node);
 
         const StylePropertySet* inlineStyle = element->inlineStyle();
         RefPtr<EditingStyle> newInlineStyle = EditingStyle::create(inlineStyle);
         if (inlineStyle) {
             if (element->isHTMLElement()) {
                 Vector<QualifiedName> attributes;
-                HTMLElement* htmlElement = toHTMLElement(element);
+                HTMLElement& htmlElement = toHTMLElement(*element);

[adamk, 2013/10/31 20:10:41]

This doesn't look like much of a win to me, especially considering...

[Inactive, 2013/10/31 20:33:26]

Done.

 
-                if (newInlineStyle->conflictsWithImplicitStyleOfElement(htmlElement)) {
+                if (newInlineStyle->conflictsWithImplicitStyleOfElement(&htmlElement)) {
                     // e.g. <b style="font-weight: normal;"> is converted to <span style="font-weight: normal;">
-                    node = replaceElementWithSpanPreservingChildrenAndAttributes(htmlElement);
+                    node = replaceElementWithSpanPreservingChildrenAndAttributes(PassRefPtr<HTMLElement>(htmlElement));

[adamk, 2013/10/31 20:10:41]

...this line

                     element = toElement(node);
-                    insertedNodes.didReplaceNode(htmlElement, node.get());
-                } else if (newInlineStyle->extractConflictingImplicitStyleOfAttributes(htmlElement, EditingStyle::PreserveWritingDirection, 0, attributes,
+                    insertedNodes.didReplaceNode(htmlElement, *node);

[Inactive, 2013/10/31 20:16:24]

Ok, then I guess I will have to dereference here instead.

+                } else if (newInlineStyle->extractConflictingImplicitStyleOfAttributes(&htmlElement, EditingStyle::PreserveWritingDirection, 0, attributes,
                     EditingStyle::DoNotExtractMatchingStyle)) {
                     // e.g. <font size="3" style="font-size: 20px;"> is converted to <font style="font-size: 20px;">
                     for (size_t i = 0; i < attributes.size(); i++)
                         removeNodeAttribute(element, attributes[i]);
                 }
             }
 
             ContainerNode* context = element->parentNode();
 
             // If Mail wraps the fragment with a Paste as Quotation blockquote, or if you're pasting into a quoted region,
             // styles from blockquoteNode are allowed to override those from the source document, see <rdar://problem/4930986> and <rdar://problem/5089327>.
             Node* blockquoteNode = !context || isMailPasteAsQuotationNode(context) ? context : enclosingNodeOfType(firstPositionInNode(context), isMailBlockquote, CanCrossEditingBoundary);
             if (blockquoteNode)
                 newInlineStyle->removeStyleFromRulesAndContext(element, document().documentElement());
 
             newInlineStyle->removeStyleFromRulesAndContext(element, context);
         }
 
         if (!inlineStyle || newInlineStyle->isEmpty()) {
             if (isStyleSpanOrSpanWithOnlyStyleAttribute(element) || isEmptyFontTag(element, AllowNonEmptyStyleAttribute)) {
-                insertedNodes.willRemoveNodePreservingChildren(element);
+                insertedNodes.willRemoveNodePreservingChildren(*element);
                 removeNodePreservingChildren(element);
                 continue;
             }
             removeNodeAttribute(element, styleAttr);
         } else if (newInlineStyle->style()->propertyCount() != inlineStyle->propertyCount())
             setNodeAttribute(element, styleAttr, newInlineStyle->style()->asText());
 
         // FIXME: Tolerate differences in id, class, and style attributes.
         if (isNonTableCellHTMLBlockElement(element) && areIdenticalElements(element, element->parentNode())
             && VisiblePosition(firstPositionInNode(element->parentNode())) == VisiblePosition(firstPositionInNode(element))
             && VisiblePosition(lastPositionInNode(element->parentNode())) == VisiblePosition(lastPositionInNode(element))) {
-            insertedNodes.willRemoveNodePreservingChildren(element);
+            insertedNodes.willRemoveNodePreservingChildren(*element);
             removeNodePreservingChildren(element);
             continue;
         }
 
         if (element->parentNode() && element->parentNode()->rendererIsRichlyEditable())
             removeNodeAttribute(element, contenteditableAttr);
 
         // WebKit used to not add display: inline and float: none on copy.
         // Keep this code around for backward compatibility
         if (isLegacyAppleStyleSpan(element)) {
             if (!element->firstChild()) {
-                insertedNodes.willRemoveNodePreservingChildren(element);
+                insertedNodes.willRemoveNodePreservingChildren(*element);
                 removeNodePreservingChildren(element);
                 continue;
             }
             // There are other styles that style rules can give to style spans,
             // but these are the two important ones because they'll prevent
             // inserted content from appearing in the right paragraph.
             // FIXME: Hyatt is concerned that selectively using display:inline will give inconsistent
             // results. We already know one issue because td elements ignore their display property
             // in quirks mode (which Mail.app is always in). We should look for an alternative.
 
@@ skipping 116 matching lines @@
 }
 
 void ReplaceSelectionCommand::removeUnrenderedTextNodesAtEnds(InsertedNodes& insertedNodes)
 {
     document().updateLayoutIgnorePendingStylesheets();
 
     Node* lastLeafInserted = insertedNodes.lastLeafInserted();
     if (lastLeafInserted && lastLeafInserted->isTextNode() && !nodeHasVisibleRenderText(toText(lastLeafInserted))
         && !enclosingNodeWithTag(firstPositionInOrBeforeNode(lastLeafInserted), selectTag)
         && !enclosingNodeWithTag(firstPositionInOrBeforeNode(lastLeafInserted), scriptTag)) {
-        insertedNodes.willRemoveNode(lastLeafInserted);
+        insertedNodes.willRemoveNode(*lastLeafInserted);
         removeNode(lastLeafInserted);
     }
 
     // We don't have to make sure that firstNodeInserted isn't inside a select or script element, because
     // it is a top level node in the fragment and the user can't insert into those elements.
     Node* firstNodeInserted = insertedNodes.firstNodeInserted();
     if (firstNodeInserted && firstNodeInserted->isTextNode() && !nodeHasVisibleRenderText(toText(firstNodeInserted))) {
-        insertedNodes.willRemoveNode(firstNodeInserted);
+        insertedNodes.willRemoveNode(*firstNodeInserted);
         removeNode(firstNodeInserted);
     }
 }
 
 VisiblePosition ReplaceSelectionCommand::positionAtEndOfInsertedContent() const
 {
     // FIXME: Why is this hack here?  What's special about <select> tags?
     Node* enclosingSelect = enclosingNodeWithTag(m_endOfInsertedContent, selectTag);
     return enclosingSelect ? lastPositionInOrAfterNode(enclosingSelect) : m_endOfInsertedContent;
 }
@@ skipping 87 matching lines @@
     style->prepareToApplyAt(firstPositionInNode(context));
 
     // Remove block properties in the span's style. This prevents properties that probably have no effect
     // currently from affecting blocks later if the style is cloned for a new block element during a future
     // editing operation.
     // FIXME: They *can* have an effect currently if blocks beneath the style span aren't individually marked
     // with block styles by the editing engine used to style them.  WebKit doesn't do this, but others might.
     style->removeBlockProperties();
 
     if (style->isEmpty() || !wrappingStyleSpan->firstChild()) {
-        insertedNodes.willRemoveNodePreservingChildren(wrappingStyleSpan);
+        insertedNodes.willRemoveNodePreservingChildren(*wrappingStyleSpan);
         removeNodePreservingChildren(wrappingStyleSpan);
     } else
         setNodeAttribute(wrappingStyleSpan, styleAttr, style->style()->asText());
 }
 
 void ReplaceSelectionCommand::mergeEndIfNeeded()
 {
     if (!m_shouldMergeEnd)
         return;
 
@@ skipping 218 matching lines @@
     // position forward without changing the visible position so we're still at the same visible location, but
     // outside of preceding tags.
     insertionPos = positionAvoidingPrecedingNodes(insertionPos);
 
     // Paste into run of tabs splits the tab span.
     insertionPos = positionOutsideTabSpan(insertionPos);
 
     bool handledStyleSpans = handleStyleSpansBeforeInsertion(fragment, insertionPos);
 
     // We're finished if there is nothing to add.
     if (fragment.isEmpty() || !fragment.firstChild())

[Inactive, 2013/10/31 20:16:24]

If fragment.firstChild() was null, we would have returned early, here.

         return;
 
     // If we are not trying to match the destination style we prefer a position
     // that is outside inline elements that provide style.
     // This way we can produce a less verbose markup.
     // We can skip this optimization for fragments not wrapped in one of
     // our style spans and for positions inside list items
     // since insertAsListItems already does the right thing.
     if (!m_matchStyle && !enclosingList(insertionPos.containerNode())) {
         if (insertionPos.containerNode()->isTextNode() && insertionPos.offsetInContainerNode() && !insertionPos.atLastEditingPositionForNode()) {
@@ skipping 20 matching lines @@
     // 3) Merge the start of the added content with the content before the position being pasted into.
     // 4) Do one of the following: a) expand the last br if the fragment ends with one and it collapsed,
     // b) merge the last paragraph of the incoming fragment with the paragraph that contained the
     // end of the selection that was pasted into, or c) handle an interchange newline at the end of the
     // incoming fragment.
     // 5) Add spaces for smart replace.
     // 6) Select the replacement if requested, and match style if requested.
 
     InsertedNodes insertedNodes;
     RefPtr<Node> refNode = fragment.firstChild();
+    ASSERT(refNode);

[adamk, 2013/10/31 20:10:41]

Huh, I wonder why we know this here. The editing code is such a mess.

     RefPtr<Node> node = refNode->nextSibling();
 
     fragment.removeNode(refNode);
 
     Node* blockStart = enclosingBlock(insertionPos.deprecatedNode());
     if ((isListElement(refNode.get()) || (isLegacyAppleStyleSpan(refNode.get()) && isListElement(refNode->firstChild())))
         && blockStart && blockStart->renderer()->isListItem())
         refNode = insertAsListItems(toHTMLElement(refNode), blockStart, insertionPos, insertedNodes);
     else {
         insertNodeAt(refNode, insertionPos);
-        insertedNodes.respondToNodeInsertion(refNode.get());
+        insertedNodes.respondToNodeInsertion(*refNode);
     }
 
     // Mutation events (bug 22634) may have already removed the inserted content
     if (!refNode->inDocument())
         return;
 
     bool plainTextFragment = isPlainTextMarkup(refNode.get());
 
     while (node) {
         RefPtr<Node> next = node->nextSibling();
         fragment.removeNode(node.get());
-        insertNodeAfter(node, refNode.get());
-        insertedNodes.respondToNodeInsertion(node.get());
+        insertNodeAfter(node, refNode);
+        insertedNodes.respondToNodeInsertion(*node);
 
         // Mutation events (bug 22634) may have already removed the inserted content
         if (!node->inDocument())
             return;
 
         refNode = node;
         if (node && plainTextFragment)
             plainTextFragment = isPlainTextMarkup(node.get());
         node = next;
     }
 
     removeUnrenderedTextNodesAtEnds(insertedNodes);
 
     if (!handledStyleSpans)
         handleStyleSpans(insertedNodes);
 
     // Mutation events (bug 20161) may have already removed the inserted content
     if (!insertedNodes.firstNodeInserted() || !insertedNodes.firstNodeInserted()->inDocument())
         return;
 
     VisiblePosition startOfInsertedContent = firstPositionInOrBeforeNode(insertedNodes.firstNodeInserted());
 
     // We inserted before the startBlock to prevent nesting, and the content before the startBlock wasn't in its own block and
     // didn't have a br after it, so the inserted content ended up in the same paragraph.
     if (startBlock && insertionPos.deprecatedNode() == startBlock->parentNode() && (unsigned)insertionPos.deprecatedEditingOffset() < startBlock->nodeIndex() && !isStartOfParagraph(startOfInsertedContent))
         insertNodeAt(createBreakElement(document()).get(), startOfInsertedContent.deepEquivalent());
 
     if (endBR && (plainTextFragment || shouldRemoveEndBR(endBR, originalVisPosBeforeEndBR))) {
         RefPtr<Node> parent = endBR->parentNode();
-        insertedNodes.willRemoveNode(endBR);
+        insertedNodes.willRemoveNode(*endBR);
         removeNode(endBR);
         if (Node* nodeToRemove = highestNodeToRemoveInPruning(parent.get())) {
-            insertedNodes.willRemoveNode(nodeToRemove);
+            insertedNodes.willRemoveNode(*nodeToRemove);
             removeNode(nodeToRemove);
         }
     }
 
     makeInsertedContentRoundTrippableWithHTMLTreeBuilder(insertedNodes);
 
     removeRedundantStylesAndKeepStyleSpanInline(insertedNodes);
 
     if (m_sanitizeFragment)
         applyCommandToComposite(SimplifyMarkupCommand::create(document(), insertedNodes.firstNodeInserted(), insertedNodes.pastLastLeaf()));
@@ skipping 288 matching lines @@
         int textNodeOffset = insertPos.offsetInContainerNode();
         if (insertPos.deprecatedNode()->isTextNode() && textNodeOffset > 0)
             splitTextNode(toText(insertPos.deprecatedNode()), textNodeOffset);
         splitTreeToNode(insertPos.deprecatedNode(), lastNode, true);
     }
 
     while (RefPtr<Node> listItem = listElement->firstChild()) {
         listElement->removeChild(listItem.get(), ASSERT_NO_EXCEPTION);
         if (isStart || isMiddle) {
             insertNodeBefore(listItem, lastNode);
-            insertedNodes.respondToNodeInsertion(listItem.get());
+            insertedNodes.respondToNodeInsertion(*listItem);
         } else if (isEnd) {
             insertNodeAfter(listItem, lastNode);
-            insertedNodes.respondToNodeInsertion(listItem.get());
+            insertedNodes.respondToNodeInsertion(*listItem);
             lastNode = listItem.get();
         } else
             ASSERT_NOT_REACHED();
     }
     if (isStart || isMiddle)
         lastNode = lastNode->previousSibling();
     return lastNode;
 }
 
 void ReplaceSelectionCommand::updateNodesInserted(Node *node)
@@ skipping 38 matching lines @@
         removeNodeAndPruneAncestors(nodeAfterInsertionPos.get());
 
     VisibleSelection selectionAfterReplace(m_selectReplacement ? start : end, end);
 
     setEndingSelection(selectionAfterReplace);
 
     return true;
 }
 
 } // namespace WebCore