Make redo command not to set invalid selection

third_party/WebKit/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 792 matching lines @@
 }
 
 void Editor::reappliedEditing(EditCommandComposition* cmd)
 {
     EventQueueScope scope;
     frame().document()->updateStyleAndLayout();
 
     dispatchEditableContentChangedEvents(cmd->startingRootEditableElement(), cmd->endingRootEditableElement());
     dispatchInputEventEditableContentChanged(cmd->startingRootEditableElement(), cmd->endingRootEditableElement(), InputEvent::InputType::Redo, emptyString(), InputEvent::EventIsComposing::NotComposing);
 
+    // TODO(yosin): Since |dispatchEditableContentChangedEvents()| and
+    // |dispatchInputEventEditableContentChanged()|, we would like to know
+    // such case. Once we have a case, this |DCHECK()| should be replaced
+    // with if-statement.
+    DCHECK(frame().document());
     VisibleSelection newSelection(cmd->endingSelection());
-    changeSelectionAfterCommand(newSelection, FrameSelection::CloseTyping | FrameSelection::ClearTypingStyle);
+    if (newSelection.isValidFor(*frame().document()))

[Xiaocheng, 2016/07/04 08:01:37]

The fix looks fine to me, but for curiosity:

It seems that there is no guarantee of having a valid |cmd->endingSelection()|
at this point, for example, the previous event dispatching may just make things
go crazy. So, is it just optional to set selection after redo?

[yosin_UTC9, 2016/07/04 09:57:07]

I may not understand you question fully...

Anything in |cmd| may be invalid, e.g. non-execCommand DOM tree mutation and
event handlers, you mentioned. This reason why we check validity of
|cmd->endSelection()| before calling |changeSelectionAfterCommand()|.

It seems  dispatchEditableContentChangedEvents() and
dispatchInputEventEditableContentChanged() don't validate element parameters, CF
will generate cases for them.

+        changeSelectionAfterCommand(newSelection, FrameSelection::CloseTyping | FrameSelection::ClearTypingStyle);
 
     m_lastEditCommand = nullptr;
     if (UndoStack* undoStack = this->undoStack())
         undoStack->registerUndoStep(cmd);
     respondToChangedContents(newSelection);
 }
 
 Editor* Editor::create(LocalFrame& frame)
 {
     return new Editor(frame);
@@ skipping 606 matching lines @@
 }
 
 DEFINE_TRACE(Editor)
 {
     visitor->trace(m_frame);
     visitor->trace(m_lastEditCommand);
     visitor->trace(m_mark);
 }
 
 } // namespace blink