Fix issue with selection being lost when InsertListCommand changes the list type.

Source/core/editing/InsertListCommand.cpp

Index: Source/core/editing/InsertListCommand.cpp
diff --git a/Source/core/editing/InsertListCommand.cpp b/Source/core/editing/InsertListCommand.cpp
index aff29397f9c64f84b03cb735cd0a84f0d028195d..d356125875eaea399f83a6ba696293e7871c46ba 100644
--- a/Source/core/editing/InsertListCommand.cpp
+++ b/Source/core/editing/InsertListCommand.cpp
@@ -127,18 +127,24 @@ void InsertListCommand::doApply()
 
     const QualifiedName& listTag = (m_type == OrderedList) ? olTag : ulTag;
     if (endingSelection().isRange()) {
+        bool forceCreateList = false;
         VisibleSelection selection = selectionForParagraphIteration(endingSelection());
         ASSERT(selection.isRange());
         VisiblePosition startOfSelection = selection.visibleStart();
         VisiblePosition endOfSelection = selection.visibleEnd();
         VisiblePosition startOfLastParagraph = startOfParagraph(endOfSelection, CanSkipOverEditingBoundary);
+        RefPtr<Range> currentSelection = endingSelection().firstRange();
 
-        if (startOfParagraph(startOfSelection, CanSkipOverEditingBoundary) != startOfLastParagraph) {
-            RefPtr<ContainerNode> scope;
-            int indexForEndOfSelection = indexForVisiblePosition(endOfSelection, scope);
-            bool forceCreateList = !selectionHasListOfType(selection, listTag);
+        RefPtr<ContainerNode> scopeForStartOfSelection;
+        RefPtr<ContainerNode> scopeForEndOfSelection;
+        // 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.
+        int indexForStartOfSelection = indexForVisiblePosition(startOfSelection, scopeForStartOfSelection);
+        int indexForEndOfSelection = indexForVisiblePosition(endOfSelection, scopeForEndOfSelection);
 
-            RefPtr<Range> currentSelection = endingSelection().firstRange();
+        if (startOfParagraph(startOfSelection, CanSkipOverEditingBoundary) != startOfLastParagraph) {
+            forceCreateList = !selectionHasListOfType(selection, listTag);
             VisiblePosition startOfCurrentParagraph = startOfSelection;
             while (!inSameParagraph(startOfCurrentParagraph, startOfLastParagraph, CanCrossEditingBoundary)) {
                 // doApply() may operate on and remove the last paragraph of the selection from the document
@@ -152,12 +158,9 @@ void InsertListCommand::doApply()
 
                 // 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());
                 if (endOfSelection.isNull() || endOfSelection.isOrphan() || startOfLastParagraph.isNull() || startOfLastParagraph.isOrphan()) {
-                    endOfSelection = visiblePositionForIndex(indexForEndOfSelection, scope.get());
+                    endOfSelection = visiblePositionForIndex(indexForEndOfSelection, scopeForEndOfSelection.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());
@@ -166,26 +169,24 @@ void InsertListCommand::doApply()
                     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());
-            // 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(forceCreateList, listTag, currentSelection.get());
+        // Fetch the end of the selection, for the reason mentioned above.
+        if (endOfSelection.isNull() || endOfSelection.isOrphan()) {
+            endOfSelection = visiblePositionForIndex(indexForEndOfSelection, scopeForEndOfSelection.get());
+            if (endOfSelection.isNull())
+                return;
+        }
+        if (startOfSelection.isNull() || startOfSelection.isOrphan()) {
+            startOfSelection = visiblePositionForIndex(indexForStartOfSelection, scopeForStartOfSelection.get());
+            if (startOfSelection.isNull())
+                return;
+        }
+        setEndingSelection(VisibleSelection(startOfSelection, endOfSelection, endingSelection().isDirectional()));
+        return;
     }
 
     doApplyForSingleParagraph(false, listTag, endingSelection().firstRange().get());
@@ -393,8 +394,10 @@ PassRefPtr<HTMLElement> InsertListCommand::listifyParagraph(const VisiblePositio
     if (listElement)
         return mergeWithNeighboringLists(listElement);
 
-    if (canMergeLists(previousList, nextList))
+    if (canMergeLists(previousList, nextList)) {
         mergeIdenticalElements(previousList, nextList);
+        listItemElement->document().updateLayoutIgnorePendingStylesheets();

[yosin_UTC9, 2013/12/13 01:10:41]

nit: Just |document()|, which is member function of EditCommand.
Why do we need to update layout? We should avoid explicit call of update layout.

+    }
 
     return listElement;
 }