Make {Character,Directional}GranularityStrategy::updateExtent() to handle null position

Make {Character,Directional}GranularityStrategy::updateExtent() to handle null position

From patch[1], |GranularityStrategy::updateExtent()| returns invalid
|SelectionInDOMTree|, which base is non-null and extent is null, when
|updateExtent()| is called with |IntPoint| points an element which can not
contain caret, e.g. VIDEO, SELECT, etc. This invalid selection can cause
|nullptr| reference in |SelectionInDOMtree::computeStartPositon()| in
|Position::operator<()|.

Before patch[1], |GranularityStrategy::updateExtent()| returns empty
|VisibleSelection| for invalid |SelectionInDOMTree|, base is non-null and
extent is null, by |VisibleSelection::validate()|.

This patch changes |{Character,Directional}GranularityStrategy::updateExtent()|
to handle above case by checking result of
|visiblePositionForContentsPoint()|.

[1] http://crrev.com/2735143002: Make GranularityStrategy::updateExtent() to
return SelectionInDOM instead of VisibleSelection

BUG=704529
TEST=run_webkit_unit_tests --gtest_filter=UpdateExtentWithNullPositionForCharacter
TEST=run_webkit_unit_tests --gtest_filter=UpdateExtentWithNullPositionForDirectional

Review-Url: https://codereview.chromium.org/2772163002
Cr-Commit-Position: refs/heads/master@{#459725}
Committed: https://chromium.googlesource.com/chromium/src/+/a4e860c32d99883c2178a332708e69049307209a

[yosin_UTC9, 2017-03-25 14:17:05]

Patchset #1 (id:1) has been deleted

[yosin_UTC9, 2017-03-25 14:18:48]

Description was changed from

==========
2017-03-25T23:08:44

BUG=704529
==========

to

==========
2017-03-25T23:08:44

[1] http://crrev.com/2735143002: Make GranularityStrategy::updateExtent() to
return SelectionInDOM instead of VisibleSelection

BUG=704529
TEST=run_webkit_unit_tests
--gtest_filter=UpdateExtentWithNullPositionForCharacter
TEST=run_webkit_unit_tests
--gtest_filter=UpdateExtentWithNullPositionForDirectional
==========

[yosin_UTC9, 2017-03-25 14:18:54]

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

[commit-bot: I haz the power, 2017-03-25 14:19:07]

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

[yosin_UTC9, 2017-03-25 14:38:19]

Description was changed from

==========
2017-03-25T23:08:44

[1] http://crrev.com/2735143002: Make GranularityStrategy::updateExtent() to
return SelectionInDOM instead of VisibleSelection

BUG=704529
TEST=run_webkit_unit_tests
--gtest_filter=UpdateExtentWithNullPositionForCharacter
TEST=run_webkit_unit_tests
--gtest_filter=UpdateExtentWithNullPositionForDirectional
==========

to

==========
Make {Character,Directional}GranularityStrategy::updateExtent() to handle null
position

From patch[1], |GranularityStrategy::updateExtent()| returns invalid
|SelectionInDOMTree|, which base is non-null and extent is null, when
|updateExtent()| is called with |IntPoint| points an element which can not
contain caret, e.g. VIDEO, SELECT, etc.

This patch changes |{Character,Directional}GranularityStrategy::updateExtent()|
to handle above case by checking result of
|visiblePositionForContentsPoint()|.

[1] http://crrev.com/2735143002: Make GranularityStrategy::updateExtent() to
return SelectionInDOM instead of VisibleSelection

BUG=704529
TEST=run_webkit_unit_tests
--gtest_filter=UpdateExtentWithNullPositionForCharacter
TEST=run_webkit_unit_tests
--gtest_filter=UpdateExtentWithNullPositionForDirectional
==========

[yosin_UTC9, 2017-03-25 14:43:37]

Description was changed from

==========
Make {Character,Directional}GranularityStrategy::updateExtent() to handle null
position

From patch[1], |GranularityStrategy::updateExtent()| returns invalid
|SelectionInDOMTree|, which base is non-null and extent is null, when
|updateExtent()| is called with |IntPoint| points an element which can not
contain caret, e.g. VIDEO, SELECT, etc.

This patch changes |{Character,Directional}GranularityStrategy::updateExtent()|
to handle above case by checking result of
|visiblePositionForContentsPoint()|.

[1] http://crrev.com/2735143002: Make GranularityStrategy::updateExtent() to
return SelectionInDOM instead of VisibleSelection

BUG=704529
TEST=run_webkit_unit_tests
--gtest_filter=UpdateExtentWithNullPositionForCharacter
TEST=run_webkit_unit_tests
--gtest_filter=UpdateExtentWithNullPositionForDirectional
==========

to

==========
Make {Character,Directional}GranularityStrategy::updateExtent() to handle null
position

From patch[1], |GranularityStrategy::updateExtent()| returns invalid
|SelectionInDOMTree|, which base is non-null and extent is null, when
|updateExtent()| is called with |IntPoint| points an element which can not
contain caret, e.g. VIDEO, SELECT, etc. This invalid selection can cause
|nullptr| reference in |SelectionInDOMtree::computeStartPositon()| in
|Position::operator<()|.

Before patch[1], |GranularityStrategy::updateExtent()| returns empty
|VisibleSelection| for invalid |SelectionInDOMTree|, base is non-null and
extent is null, by |VisibleSelection::validate()|.

This patch changes |{Character,Directional}GranularityStrategy::updateExtent()|
to handle above case by checking result of
|visiblePositionForContentsPoint()|.

[1] http://crrev.com/2735143002: Make GranularityStrategy::updateExtent() to
return SelectionInDOM instead of VisibleSelection

BUG=704529
TEST=run_webkit_unit_tests
--gtest_filter=UpdateExtentWithNullPositionForCharacter
TEST=run_webkit_unit_tests
--gtest_filter=UpdateExtentWithNullPositionForDirectional
==========

[commit-bot: I haz the power, 2017-03-25 16:13:54]

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

[commit-bot: I haz the power, 2017-03-25 16:13:54]

Dry run: This issue passed the CQ dry run.

[yosin_UTC9, 2017-03-25 16:15:32]

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

[yosin_UTC9, 2017-03-25 16:15:32]

PTAL

[yoichio, 2017-03-27 02:05:35]

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

https://codereview.chromium.org/2772163002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/GranularityStrategyTest.cpp:732:
selection().moveRangeSelectionExtent(IntPoint(0, 0));
Could you confirm |visiblePositionForContentsPoint(IntPoint(0, 0))| returns null
VisiblePosition?

[yosin_UTC9, 2017-03-27 04:42:52]

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

[commit-bot: I haz the power, 2017-03-27 04:43:02]

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

[yosin_UTC9, 2017-03-27 05:40:03]

PTAL

Add ASSERT_EQ for visiblePositionForContentsPoint(IntPoint(0, 0))

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

https://codereview.chromium.org/2772163002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/GranularityStrategyTest.cpp:732:
selection().moveRangeSelectionExtent(IntPoint(0, 0));
On 2017/03/27 at 02:05:35, yoichio wrote:
> Could you confirm |visiblePositionForContentsPoint(IntPoint(0, 0))| returns
null VisiblePosition?

Done

[commit-bot: I haz the power, 2017-03-27 06:02:19]

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

[commit-bot: I haz the power, 2017-03-27 06:02:20]

Dry run: This issue passed the CQ dry run.

[yoichio, 2017-03-27 08:09:02]

lgtm

[yosin_UTC9, 2017-03-27 08:44:49]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2017-03-27 08:45:04]

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, 2017-03-27 08:49:48]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490604289023500,
"parent_rev": "ae2b29ed27f84f0209be6f0482f5b85c0578ee75", "commit_rev":
"a4e860c32d99883c2178a332708e69049307209a"}

[commit-bot: I haz the power, 2017-03-27 08:50:36]

Description was changed from

==========
Make {Character,Directional}GranularityStrategy::updateExtent() to handle null
position

From patch[1], |GranularityStrategy::updateExtent()| returns invalid
|SelectionInDOMTree|, which base is non-null and extent is null, when
|updateExtent()| is called with |IntPoint| points an element which can not
contain caret, e.g. VIDEO, SELECT, etc. This invalid selection can cause
|nullptr| reference in |SelectionInDOMtree::computeStartPositon()| in
|Position::operator<()|.

Before patch[1], |GranularityStrategy::updateExtent()| returns empty
|VisibleSelection| for invalid |SelectionInDOMTree|, base is non-null and
extent is null, by |VisibleSelection::validate()|.

This patch changes |{Character,Directional}GranularityStrategy::updateExtent()|
to handle above case by checking result of
|visiblePositionForContentsPoint()|.

[1] http://crrev.com/2735143002: Make GranularityStrategy::updateExtent() to
return SelectionInDOM instead of VisibleSelection

BUG=704529
TEST=run_webkit_unit_tests
--gtest_filter=UpdateExtentWithNullPositionForCharacter
TEST=run_webkit_unit_tests
--gtest_filter=UpdateExtentWithNullPositionForDirectional
==========

to

==========
Make {Character,Directional}GranularityStrategy::updateExtent() to handle null
position

From patch[1], |GranularityStrategy::updateExtent()| returns invalid
|SelectionInDOMTree|, which base is non-null and extent is null, when
|updateExtent()| is called with |IntPoint| points an element which can not
contain caret, e.g. VIDEO, SELECT, etc. This invalid selection can cause
|nullptr| reference in |SelectionInDOMtree::computeStartPositon()| in
|Position::operator<()|.

Before patch[1], |GranularityStrategy::updateExtent()| returns empty
|VisibleSelection| for invalid |SelectionInDOMTree|, base is non-null and
extent is null, by |VisibleSelection::validate()|.

This patch changes |{Character,Directional}GranularityStrategy::updateExtent()|
to handle above case by checking result of
|visiblePositionForContentsPoint()|.

[1] http://crrev.com/2735143002: Make GranularityStrategy::updateExtent() to
return SelectionInDOM instead of VisibleSelection

BUG=704529
TEST=run_webkit_unit_tests
--gtest_filter=UpdateExtentWithNullPositionForCharacter
TEST=run_webkit_unit_tests
--gtest_filter=UpdateExtentWithNullPositionForDirectional

Review-Url: https://codereview.chromium.org/2772163002
Cr-Commit-Position: refs/heads/master@{#459725}
Committed:
https://chromium.googlesource.com/chromium/src/+/a4e860c32d99883c2178a332708e...
==========

[commit-bot: I haz the power, 2017-03-27 08:50:37]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/a4e860c32d99883c2178a332708e...