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

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 109 matching lines @@
     // ends "inside" that paragraph, so it would be confusing if InsertUn{Ordered}List
     // operated on that paragraph.
     // FIXME: We paint the gap before some paragraphs that are indented with left
     // margin/padding, but not others.  We should make the gap painting more consistent and
     // then use a left margin/padding rule here.
     if (visibleEnd != visibleStart && isStartOfParagraph(visibleEnd, CanSkipOverEditingBoundary))
         setEndingSelection(VisibleSelection(visibleStart, visibleEnd.previous(CannotCrossEditingBoundary), endingSelection().isDirectional()));
 
     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();
+
+        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);
 
         if (startOfParagraph(startOfSelection, CanSkipOverEditingBoundary) != startOfLastParagraph) {
-            RefPtr<ContainerNode> scope;
-            int indexForEndOfSelection = indexForVisiblePosition(endOfSelection, scope);
-            bool forceCreateList = !selectionHasListOfType(selection, listTag);
-
-            RefPtr<Range> currentSelection = endingSelection().firstRange();
+            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
                 // if it's in the same list item as startOfCurrentParagraph.  Return early to avoid an
                 // infinite loop and because there is no more work to be done.
                 // FIXME(<rdar://problem/5983974>): The endingSelection() may be incorrect here.  Compute
                 // 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());
                 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());
                     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());
-            // 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());
 }
 
 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().
@@ skipping 187 matching lines @@
             start = startOfParagraph(originalStart, CanSkipOverEditingBoundary);
             end = endOfParagraph(start, CanSkipOverEditingBoundary);
         }
     }
 
     moveParagraph(start, end, positionBeforeNode(placeholder.get()), true);
 
     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;
 }
 
 }