Ensure valid VisiblePosition input for character{Before,After}

Ensure valid VisiblePosition input for character{Before,After}

This patch also drive-by eliminates a |createVisiblePositionDeprecated|
in |CompositeEditCommand::prepareWhitespaceAtPositionForSplit|.

BUG=648949
Committed: https://crrev.com/7b0d88beaef6c050b97d2aacd3310be4ef6fd4c0
Cr-Commit-Position: refs/heads/master@{#421164}

[Xiaocheng, 2016-09-26 06:11:41]

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

[commit-bot: I haz the power, 2016-09-26 06:12:03]

Dry run: 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-09-26 06:14:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 06:14:28]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Xiaocheng, 2016-09-26 06:14:29]

Description was changed from

==========
Mark usages of CharacterBefore/After Deprecated in editing commands

BUG=648949
==========

to

==========
Mark usages of CharacterBefore/After Deprecated in editing commands

This patch marks the following functions in VisibleUnits.cpp deprecated
because they accept invalid VisiblePositions as input:
- characterBefore
- characterAfter

This patch also creates the non-deprecated versions of these functions
that requires valid VisiblePositions as input.

Calls of these functions in editing/commands/ are switched to the deprecated
functions. Calls in other places have ensured valid VisiblePositions as input,
and hence, are calling the non-deprecated functions after this patch.

BUG=648949
==========

[Xiaocheng, 2016-09-27 01:58:45]

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

[commit-bot: I haz the power, 2016-09-27 01:59:13]

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

[Xiaocheng, 2016-09-27 01:59:40]

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

[commit-bot: I haz the power, 2016-09-27 02:00:02]

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

[Xiaocheng, 2016-09-27 02:01:39]

Description was changed from

==========
Mark usages of CharacterBefore/After Deprecated in editing commands

This patch marks the following functions in VisibleUnits.cpp deprecated
because they accept invalid VisiblePositions as input:
- characterBefore
- characterAfter

This patch also creates the non-deprecated versions of these functions
that requires valid VisiblePositions as input.

Calls of these functions in editing/commands/ are switched to the deprecated
functions. Calls in other places have ensured valid VisiblePositions as input,
and hence, are calling the non-deprecated functions after this patch.

BUG=648949
==========

to

==========
Ensure valid VisiblePositionInput for character{Before,After}

This patch also drive-by eliminates a |createVisiblePositionDeprecated|
in |CompositeEditCommand::prepareWhitespaceAtPositionForSplit|.

BUG=648949
==========

[Xiaocheng, 2016-09-27 02:32:12]

Description was changed from

==========
Ensure valid VisiblePositionInput for character{Before,After}

This patch also drive-by eliminates a |createVisiblePositionDeprecated|
in |CompositeEditCommand::prepareWhitespaceAtPositionForSplit|.

BUG=648949
==========

to

==========
Ensure valid VisiblePosition input for character{Before,After}

This patch also drive-by eliminates a |createVisiblePositionDeprecated|
in |CompositeEditCommand::prepareWhitespaceAtPositionForSplit|.

BUG=648949
==========

[commit-bot: I haz the power, 2016-09-27 03:26:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-27 03:26:17]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Xiaocheng, 2016-09-27 05:54:28]

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

[Xiaocheng, 2016-09-27 05:54:29]

PTAL.

[Xiaocheng, 2016-09-27 07:11:56]

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

[commit-bot: I haz the power, 2016-09-27 07:12:10]

Dry run: 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-09-27 07:41:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-27 07:41:52]

Dry run: This issue passed the CQ dry run.

[yosin_UTC9, 2016-09-27 08:30:53]

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

https://codereview.chromium.org/2368903002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/VisibleUnits.cpp:3017: static UChar32
characterBeforeAlgorithm(const VisiblePositionTemplate<Strategy>&
visiblePosition)
Not sure how about making |characterAfter()| to take |Position| instead of
|VisiblePosition|?
|characterAfter()| uses deep equivalent position rather than VP itself.

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

https://codereview.chromium.org/2368903002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp:1499:
VisiblePosition startOfInsertedContent = positionAtStartOfInsertedContent();
This moving may change behavior due by DOM tree modification between L1473 to
L1496

[Xiaocheng, 2016-09-27 08:46:03]

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

[commit-bot: I haz the power, 2016-09-27 08:46:30]

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

[Xiaocheng, 2016-09-27 08:48:51]

Updated.

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

https://codereview.chromium.org/2368903002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/VisibleUnits.cpp:3017: static UChar32
characterBeforeAlgorithm(const VisiblePositionTemplate<Strategy>&
visiblePosition)
On 2016/09/27 at 08:30:53, Yosi_UTC9 wrote:
> Not sure how about making |characterAfter()| to take |Position| instead of
|VisiblePosition|?
> |characterAfter()| uses deep equivalent position rather than VP itself.

Talked offline. A lot of functions in VisibleUnits unnecessarily take VPs as
parameters. It's good if they get simplified.

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

https://codereview.chromium.org/2368903002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp:1499:
VisiblePosition startOfInsertedContent = positionAtStartOfInsertedContent();
On 2016/09/27 at 08:30:53, Yosi_UTC9 wrote:
> This moving may change behavior due by DOM tree modification between L1473 to
L1496

DCHECK has been added to make sure that |m_startOfInsertedContent| remains the
same, and hence, moving the creation of |startOfInsertedContent| does not change
behvior.

[yosin_UTC9, 2016-09-27 08:57:35]

lgtm

[commit-bot: I haz the power, 2016-09-27 09:51:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-27 09:51:03]

Dry run: This issue passed the CQ dry run.

[Xiaocheng, 2016-09-27 10:39:59]

The CQ bit was checked by xiaochengh@chromium.org

[commit-bot: I haz the power, 2016-09-27 10:40:19]

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-09-27 10:45:27]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-27 10:46:55]

Description was changed from

==========
Ensure valid VisiblePosition input for character{Before,After}

This patch also drive-by eliminates a |createVisiblePositionDeprecated|
in |CompositeEditCommand::prepareWhitespaceAtPositionForSplit|.

BUG=648949
==========

to

==========
Ensure valid VisiblePosition input for character{Before,After}

This patch also drive-by eliminates a |createVisiblePositionDeprecated|
in |CompositeEditCommand::prepareWhitespaceAtPositionForSplit|.

BUG=648949

Committed: https://crrev.com/7b0d88beaef6c050b97d2aacd3310be4ef6fd4c0
Cr-Commit-Position: refs/heads/master@{#421164}
==========

[commit-bot: I haz the power, 2016-09-27 10:46:56]

Patchset 3 (id:??) landed as
https://crrev.com/7b0d88beaef6c050b97d2aacd3310be4ef6fd4c0
Cr-Commit-Position: refs/heads/master@{#421164}