Make redo command not to set invalid selection

Make redo command not to set invalid selection

This patch makes "redo" command not to set invalid selection to |FrameSeleciton|
via |Editor::changeSelectionAfterCommand()| to avoid |DCHECK()| in
|FrameSelection::setSelection()|.

BUG=623241
TEST=LayoutTests/editing/undo/redo-selection-modify-crash.html

Committed: https://crrev.com/39b2a95285753ccf12ee6d0bff85509d1161e029
Cr-Commit-Position: refs/heads/master@{#403657}

[yosin_UTC9, 2016-07-04 07:20:43]

Description was changed from

==========
2016-07-04T14:02:47

BUG=
==========

to

==========
Make redo command not to set invalid selection

This patch makes "redo" command not to set invalid selection to |FrameSeleciton|
via |Editor::changeSelectionAfterCommand()| to avoid |DCHECK()| in
|FrameSelection::setSelection()|.

BUG=623241
TEST=LayoutTests/editing/undo/redo-selection-modify-crash.html
==========

[yosin_UTC9, 2016-07-04 07:20:58]

yosin@chromium.org changed reviewers:
+ xiaochengh@chromium.org, yoichio@chromium.org

[yosin_UTC9, 2016-07-04 07:20:59]

PTAL

[yoichio, 2016-07-04 07:42:09]

lgtm

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

https://codereview.chromium.org/2120223002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/Editor.cpp (right):

https://codereview.chromium.org/2120223002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/Editor.cpp:819: if
(newSelection.isValidFor(*frame().document()))
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:08]

https://codereview.chromium.org/2120223002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/Editor.cpp (right):

https://codereview.chromium.org/2120223002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/Editor.cpp:819: if
(newSelection.isValidFor(*frame().document()))
On 2016/07/04 at 08:01:37, Xiaocheng wrote:
> 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?

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.

[yosin_UTC9, 2016-07-04 09:57:11]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2016-07-04 09:57:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-04 10:00:37]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-04 10:00:43]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-04 10:02:27]

Description was changed from

==========
Make redo command not to set invalid selection

This patch makes "redo" command not to set invalid selection to |FrameSeleciton|
via |Editor::changeSelectionAfterCommand()| to avoid |DCHECK()| in
|FrameSelection::setSelection()|.

BUG=623241
TEST=LayoutTests/editing/undo/redo-selection-modify-crash.html
==========

to

==========
Make redo command not to set invalid selection

This patch makes "redo" command not to set invalid selection to |FrameSeleciton|
via |Editor::changeSelectionAfterCommand()| to avoid |DCHECK()| in
|FrameSelection::setSelection()|.

BUG=623241
TEST=LayoutTests/editing/undo/redo-selection-modify-crash.html

Committed: https://crrev.com/39b2a95285753ccf12ee6d0bff85509d1161e029
Cr-Commit-Position: refs/heads/master@{#403657}
==========

[commit-bot: I haz the power, 2016-07-04 10:02:28]

Patchset 2 (id:??) landed as
https://crrev.com/39b2a95285753ccf12ee6d0bff85509d1161e029
Cr-Commit-Position: refs/heads/master@{#403657}