Prune deprecated functions from ApplyStyleCommand

Prune deprecated functions from ApplyStyleCommand

This patch prunes the following deprecated functions from ApplyStyleCommand by
ensuring clean layout and valid input parameters:
- createVisiblePositionDeprecated
- createVisibleSelectionDeprecated
- startOfParagraphDeprecated
- endOfParagraphDeprecated

BUG=647219, 651373
Committed: https://crrev.com/33e97547aa6173ea92f3d7241434f06d73e94e75
Cr-Commit-Position: refs/heads/master@{#423440}

[Xiaocheng, 2016-10-06 03:47:24]

The CQ bit was checked by xiaochengh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-06 03:47:38]

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

[Xiaocheng, 2016-10-06 04:40:45]

Description was changed from

==========
Prune deprecated functions from ApplyStyleCommand

BUG=647219,651373
==========

to

==========
Prune deprecated functions from ApplyStyleCommand

This patch prunes the following deprecated functions from ApplyStyleCommand by
ensuring clean layout and valid input parameters:
- createVisiblePositionDeprecated
- createVisibleSelectionDeprecated
- startOfParagraphDeprecated
- endOfParagraphDeprecated

BUG=647219,651373
==========

[Xiaocheng, 2016-10-06 04:40:45]

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

[Xiaocheng, 2016-10-06 04:40:51]

PTAL.

[yosin_UTC9, 2016-10-06 04:51:43]

The CQ bit was unchecked by yosin@chromium.org

[yosin_UTC9, 2016-10-06 04:51:50]

lgtm

https://codereview.chromium.org/2391193004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp
(right):

https://codereview.chromium.org/2391193004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp:193:
setEndingSelection(createVisibleSelection(newStart, newEnd,
BTW, we would like to have SelectionForUndoStep or SelectionRecord or whatever,
which is very similar to Selection class (http://crrev.com/1958093002).
|Selection| class is sensitive on DOM tree version but SelectionForUndoStep
doesn't. It holds positions then compute visible selection at undo/redo time
with validating whether these positions are in document.

See Editor::unappliedEditing() and Editor::reappliedEditing() for how we restore
selection from undo stack.

Given that, we don't need to hold VisibleSelection for
m_{staring,ending}Selection, since we should compute VisibleSelection at
undo/redo time.

[yosin_UTC9, 2016-10-06 04:51:57]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2016-10-06 04:52:13]

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

[Xiaocheng, 2016-10-06 05:04:46]

https://codereview.chromium.org/2391193004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp
(right):

https://codereview.chromium.org/2391193004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp:193:
setEndingSelection(createVisibleSelection(newStart, newEnd,
On 2016/10/06 at 04:51:50, Yosi_UTC9 wrote:
> BTW, we would like to have SelectionForUndoStep or SelectionRecord or
whatever, which is very similar to Selection class
(http://crrev.com/1958093002).
> |Selection| class is sensitive on DOM tree version but SelectionForUndoStep
doesn't. It holds positions then compute visible selection at undo/redo time
> with validating whether these positions are in document.
> 
> See Editor::unappliedEditing() and Editor::reappliedEditing() for how we
restore selection from undo stack.
> 
> Given that, we don't need to hold VisibleSelection for
m_{staring,ending}Selection, since we should compute VisibleSelection at
undo/redo time.

If I understand correctly, a VisibleSelection should indicate a range of
VisiblePositions, and hence, its stored positions are kind of out-of-date when
mutations take place.

In this sense, I agree with the idea of storing two Positions (with proper
wrapping) instead of a VS. Or perhaps RelocatablePosition.

It should reduce the call sites of layout update calls, since creation of VS
requires clean layout but storing two positions does not.

[commit-bot: I haz the power, 2016-10-06 05:21:39]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-10-06 05:23:30]

Description was changed from

==========
Prune deprecated functions from ApplyStyleCommand

This patch prunes the following deprecated functions from ApplyStyleCommand by
ensuring clean layout and valid input parameters:
- createVisiblePositionDeprecated
- createVisibleSelectionDeprecated
- startOfParagraphDeprecated
- endOfParagraphDeprecated

BUG=647219,651373
==========

to

==========
Prune deprecated functions from ApplyStyleCommand

This patch prunes the following deprecated functions from ApplyStyleCommand by
ensuring clean layout and valid input parameters:
- createVisiblePositionDeprecated
- createVisibleSelectionDeprecated
- startOfParagraphDeprecated
- endOfParagraphDeprecated

BUG=647219,651373

Committed: https://crrev.com/33e97547aa6173ea92f3d7241434f06d73e94e75
Cr-Commit-Position: refs/heads/master@{#423440}
==========

[commit-bot: I haz the power, 2016-10-06 05:23:31]

Patchset 1 (id:??) landed as
https://crrev.com/33e97547aa6173ea92f3d7241434f06d73e94e75
Cr-Commit-Position: refs/heads/master@{#423440}