reset VisibleSelection at the Undo command.

Source/core/editing/Editor.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  *
  * 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 692 matching lines @@
         // Only register a new undo command if the command passed in is
         // different from the last command
         m_lastEditCommand = cmd;
         if (UndoStack* undoStack = this->undoStack())
             undoStack->registerUndoStep(m_lastEditCommand->ensureComposition());
     }
 
     respondToChangedContents(newSelection);
 }
 
+static bool isValidPosition(const Position& position)
+{
+    if (!position.inDocument())
+        return false;
+
+    if (position.anchorType() != Position::PositionIsOffsetInAnchor)
+        return true;
+
+    if (position.offsetInContainerNode() < 0)
+        return false;
+
+    const unsigned offset = static_cast<unsigned>(position.offsetInContainerNode());
+    const unsigned nodeLength = Range::lengthOfContentsInNode(position.anchorNode());
+    return offset <= nodeLength;
+}
+
 void Editor::unappliedEditing(PassRefPtrWillBeRawPtr<EditCommandComposition> cmd)
 {
     EventQueueScope scope;
     m_frame.document()->updateLayout();
 
     dispatchEditableContentChangedEvents(cmd->startingRootEditableElement(), cmd->endingRootEditableElement());
 
     VisibleSelection newSelection(cmd->startingSelection());
+    if (!isValidPosition(newSelection.base()) || !isValidPosition(newSelection.extent()) || !isValidPosition(newSelection.start()) || !isValidPosition(newSelection.end()))
+        newSelection = VisibleSelection(newSelection.base(), newSelection.extent(), newSelection.affinity(), newSelection.isDirectional());

[yosin_UTC9, 2014/06/04 01:17:30]

Q: Why do we create |VisibleSelection| from invalid positions?

[yoichio, 2014/06/04 02:01:46]

It's OK to create null VisibleSelection if there are invalid positions.
However, some of them have valid information like anchor Node and the
VisibleSelection constructor creates new valid VisibleSelection even from the
invalid Positions through the canonicalization.
I think it's better to recycle VisibleSelection from the information than to
reset VisibleSelection.

[yosin_UTC9, 2014/06/04 02:20:57]

I think it is better to reset rather than set unexpected value.
yutak@, How do you think?

[Yuta Kitamura, 2014/06/05 08:37:55]

First of all, in my view this line is very hard to understand, since it seems to
create another VisibleSelection with the exact same set of arguments. It's
totally
not obvious that it resets out-of-index positions.

My take is that VisibleSelection should be responsible for making sure it is in
a valid state. In that light, it should be VisibleSelection's copy constructor
to check the validity of Positions, not Editor.

If changing VisibleSelection's copy constructor is not feasible (because it
slows
down the performance or breaks compatibility), then VisibleSelection should have
a member function that corrects the positions, and lines 736-738 should be
replaced with something like:

    VisibleSelection newSelection(
        cmd->startingSelection().ensureValidPositions());

[yoichio, 2014/06/06 04:49:21]

VisibleSelection is a DISALLOW_ALLOCATION class. It means that its instances are
always only on stack and you must copy an object if you need another variable of
that.
Thus to insert ::valid() to the copy constructor may make performance down.
Additionally, VisibleSelection::valid() change a VisibleSelection which has
'valid' Positions because of the  valid() behavior to the notorious base,
extend, start and end relationship.

As conclusion, I add VisibleSelection::validatePositionsIfNeeded to validate
each position if its position is invalid.

     changeSelectionAfterCommand(newSelection, FrameSelection::CloseTyping | FrameSelection::ClearTypingStyle);
 
     m_lastEditCommand = nullptr;
     if (UndoStack* undoStack = this->undoStack())
         undoStack->registerRedoStep(cmd);
     respondToChangedContents(newSelection);
 }
 
 void Editor::reappliedEditing(PassRefPtrWillBeRawPtr<EditCommandComposition> cmd)
 {
@@ skipping 504 matching lines @@
     frame().selection().setShouldShowBlockCursor(m_overwriteModeEnabled);
 }
 
 void Editor::trace(Visitor* visitor)
 {
     visitor->trace(m_lastEditCommand);
     visitor->trace(m_mark);
 }
 
 } // namespace WebCore