Have positionInParentBeforeNode() / positionInParentAfterNode() take a reference

Source/core/editing/InsertListCommand.cpp

 /*
  * Copyright (C) 2006, 2010 Apple 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
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 138 matching lines @@
                 // the new location of endOfSelection and use it as the end of the new selection.
                 if (!startOfLastParagraph.deepEquivalent().inDocument())
                     return;
                 setEndingSelection(startOfCurrentParagraph);
 
                 // Save and restore endOfSelection and startOfLastParagraph when necessary
                 // since moveParagraph and movePragraphWithClones can remove nodes.
                 // FIXME: This is an inefficient way to keep selection alive because indexForVisiblePosition walks from
                 // the beginning of the document to the endOfSelection everytime this code is executed.
                 // But not using index is hard because there are so many ways we can lose selection inside doApplyForSingleParagraph.
-                doApplyForSingleParagraph(forceCreateList, listTag, currentSelection.get());
+                doApplyForSingleParagraph(forceCreateList, listTag, *currentSelection);
                 if (endOfSelection.isNull() || endOfSelection.isOrphan() || startOfLastParagraph.isNull() || startOfLastParagraph.isOrphan()) {
                     endOfSelection = visiblePositionForIndex(indexForEndOfSelection, scope.get());
                     // If endOfSelection is null, then some contents have been deleted from the document.
                     // This should never happen and if it did, exit early immediately because we've lost the loop invariant.
                     ASSERT(endOfSelection.isNotNull());
                     if (endOfSelection.isNull())
                         return;
                     startOfLastParagraph = startOfParagraph(endOfSelection, CanSkipOverEditingBoundary);
                 }
 
                 // Fetch the start of the selection after moving the first paragraph,
                 // because moving the paragraph will invalidate the original start.
                 // We'll use the new start to restore the original selection after
                 // we modified all selected paragraphs.
                 if (startOfCurrentParagraph == startOfSelection)
                     startOfSelection = endingSelection().visibleStart();
 
                 startOfCurrentParagraph = startOfNextParagraph(endingSelection().visibleStart());
             }
             setEndingSelection(endOfSelection);
-            doApplyForSingleParagraph(forceCreateList, listTag, currentSelection.get());
+            doApplyForSingleParagraph(forceCreateList, listTag, *currentSelection);
             // Fetch the end of the selection, for the reason mentioned above.
             if (endOfSelection.isNull() || endOfSelection.isOrphan()) {
                 endOfSelection = visiblePositionForIndex(indexForEndOfSelection, scope.get());
                 if (endOfSelection.isNull())
                     return;
             }
             setEndingSelection(VisibleSelection(startOfSelection, endOfSelection, endingSelection().isDirectional()));
             return;
         }
     }
 
-    doApplyForSingleParagraph(false, listTag, endingSelection().firstRange().get());
+    ASSERT(endingSelection().firstRange());
+    doApplyForSingleParagraph(false, listTag, *endingSelection().firstRange());
 }
 
-void InsertListCommand::doApplyForSingleParagraph(bool forceCreateList, const QualifiedName& listTag, Range* currentSelection)
+void InsertListCommand::doApplyForSingleParagraph(bool forceCreateList, const QualifiedName& listTag, Range& currentSelection)
 {
     // FIXME: This will produce unexpected results for a selection that starts just before a
     // table and ends inside the first cell, selectionForParagraphIteration should probably
     // be renamed and deployed inside setEndingSelection().
     Node* selectionNode = endingSelection().start().deprecatedNode();
     Node* listChildNode = enclosingListChild(selectionNode);
     bool switchListType = false;
     if (listChildNode) {
         // Remove the list chlild.
         RefPtr<HTMLElement> listNode = enclosingList(listChildNode);
         if (!listNode) {
             listNode = fixOrphanedListChild(listChildNode);
             listNode = mergeWithNeighboringLists(listNode);
         }
         if (!listNode->hasTagName(listTag))
             // listChildNode will be removed from the list and a list of type m_type will be created.
             switchListType = true;
 
         // If the list is of the desired type, and we are not removing the list, then exit early.
         if (!switchListType && forceCreateList)
             return;
 
         // If the entire list is selected, then convert the whole list.
-        if (switchListType && isNodeVisiblyContainedWithin(listNode.get(), currentSelection)) {
-            bool rangeStartIsInList = visiblePositionBeforeNode(listNode.get()) == VisiblePosition(currentSelection->startPosition());
-            bool rangeEndIsInList = visiblePositionAfterNode(listNode.get()) == VisiblePosition(currentSelection->endPosition());
+        if (switchListType && isNodeVisiblyContainedWithin(*listNode, currentSelection)) {
+            bool rangeStartIsInList = visiblePositionBeforeNode(*listNode) == VisiblePosition(currentSelection.startPosition());
+            bool rangeEndIsInList = visiblePositionAfterNode(*listNode) == VisiblePosition(currentSelection.endPosition());
 
             RefPtr<HTMLElement> newList = createHTMLElement(document(), listTag);
             insertNodeBefore(newList, listNode);
 
             Node* firstChildInList = enclosingListChild(VisiblePosition(firstPositionInNode(listNode.get())).deepEquivalent().deprecatedNode(), listNode.get());
             Node* outerBlock = firstChildInList && firstChildInList->isBlockFlowElement() ? firstChildInList : listNode.get();
 
             moveParagraphWithClones(VisiblePosition(firstPositionInNode(listNode.get())), VisiblePosition(lastPositionInNode(listNode.get())), newList.get(), outerBlock);
 
             // Manually remove listNode because moveParagraphWithClones sometimes leaves it behind in the document.
             // See the bug 33668 and editing/execCommand/insert-list-orphaned-item-with-nested-lists.html.
             // FIXME: This might be a bug in moveParagraphWithClones or deleteSelection.
             if (listNode && listNode->inDocument())
                 removeNode(listNode);
 
             newList = mergeWithNeighboringLists(newList);
 
             // Restore the start and the end of current selection if they started inside listNode
             // because moveParagraphWithClones could have removed them.
             if (rangeStartIsInList && newList)
-                currentSelection->setStart(newList, 0, IGNORE_EXCEPTION);
+                currentSelection.setStart(newList, 0, IGNORE_EXCEPTION);
             if (rangeEndIsInList && newList)
-                currentSelection->setEnd(newList, lastOffsetInNode(newList.get()), IGNORE_EXCEPTION);
+                currentSelection.setEnd(newList, lastOffsetInNode(newList.get()), IGNORE_EXCEPTION);
 
             setEndingSelection(VisiblePosition(firstPositionInNode(newList.get())));
 
             return;
         }
 
         unlistifyParagraph(endingSelection().visibleStart(), listNode.get(), listChildNode);
     }
 
     if (!listChildNode || switchListType || forceCreateList)
@@ skipping 112 matching lines @@
 
         // Insert the list at a position visually equivalent to start of the
         // paragraph that is being moved into the list.
         // Try to avoid inserting it somewhere where it will be surrounded by
         // inline ancestors of start, since it is easier for editing to produce
         // clean markup when inline elements are pushed down as far as possible.
         Position insertionPos(start.deepEquivalent().upstream());
         // Also avoid the containing list item.
         Node* listChild = enclosingListChild(insertionPos.deprecatedNode());
         if (listChild && listChild->hasTagName(liTag))
-            insertionPos = positionInParentBeforeNode(listChild);
+            insertionPos = positionInParentBeforeNode(*listChild);
 
         insertNodeAt(listElement, insertionPos);
 
         // We inserted the list at the start of the content we're about to move
         // Update the start of content, so we don't try to move the list into itself.  bug 19066
         // Layout is necessary since start's node's inline renderers may have been destroyed by the insertion
         // The end of the content may have changed after the insertion and layout so update it as well.
         if (insertionPos == start.deepEquivalent()) {
             listElement->document().updateLayoutIgnorePendingStylesheets();
             start = startOfParagraph(originalStart, CanSkipOverEditingBoundary);
             end = endOfParagraph(start, CanSkipOverEditingBoundary);
         }
     }
 
     moveParagraph(start, end, VisiblePosition(positionBeforeNode(placeholder.get())), true);
 
     if (listElement)
         return mergeWithNeighboringLists(listElement);
 
     if (canMergeLists(previousList, nextList))
         mergeIdenticalElements(previousList, nextList);
 
     return listElement;
 }
 
 }