Get rid of createVisibleSelection() taking PositionWithAffinity

Get rid of createVisibleSelection() to take PositionWithAffinity

This patch gets rid of |createVisibleSelection()| taking |PositionWithAffinity| by
replacing with |SelectionInDOMTree| version to reduce number of overloads for
improving code health.

BUG=657237
TEST=n/a; no behavior changes

Committed: https://crrev.com/9af5344ba29c36f6ec50fd75791c6a606a3309fc
Cr-Commit-Position: refs/heads/master@{#426684}

[yosin_UTC9, 2016-10-19 08:01:41]

Description was changed from

==========
2016-10-19T16:59:42

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

to

==========
Get rid of createVisibleSelection() taking PositionWithAffinity

BUG=657237
TEST=n/a
==========

[yosin_UTC9, 2016-10-19 08:01:46]

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

[commit-bot: I haz the power, 2016-10-19 09:31:07]

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-10-19 12:20:30]

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

[commit-bot: I haz the power, 2016-10-19 12:20:31]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[yosin_UTC9, 2016-10-20 05:01:06]

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

[commit-bot: I haz the power, 2016-10-20 05:01:33]

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-10-20 06:49:41]

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

[commit-bot: I haz the power, 2016-10-20 06:49:42]

Dry run: This issue passed the CQ dry run.

[yosin_UTC9, 2016-10-20 07:16:19]

Description was changed from

==========
Get rid of createVisibleSelection() taking PositionWithAffinity

BUG=657237
TEST=n/a
==========

to

==========
Get rid of createVisibleSelection() to take PositionWithAffinity

This patch gets rid of |createVisibleSelection()| taking |PositionWithAffinity|
by
replacing with |SelectionInDOMTree| version to reduce number of overloads for
improving code health.

BUG=657237
TEST=n/a; no behavior changes
==========

[yosin_UTC9, 2016-10-20 07:17:10]

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

[yosin_UTC9, 2016-10-20 07:17:10]

PTAL

Not only core/editing but also
 - core/input/EventHandlerTest.cpp
 - core/page/DragController.cpp

[Xiaocheng, 2016-10-20 07:35:38]

lgtm

[tkent, 2016-10-20 08:05:19]

https://codereview.chromium.org/2428363004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/DragController.cpp (right):

https://codereview.chromium.org/2428363004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/DragController.cpp:474: if
(!position.isConnected())
In the old code, what happened if !frame->positionForPoint(point).isConnected()?
Is the behavior of the old code and the new code identical?

[yosin_UTC9, 2016-10-20 08:34:00]

https://codereview.chromium.org/2428363004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/DragController.cpp (right):

https://codereview.chromium.org/2428363004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/DragController.cpp:474: if
(!position.isConnected())
On 2016/10/20 at 08:05:19, tkent wrote:
> In the old code, what happened if
!frame->positionForPoint(point).isConnected()?
> Is the behavior of the old code and the new code identical?

It makes empty selection since visible position canonicalization converts it to
null position.
On debug build, it hits DCHECK(position.isConnected()) in VisiblePosition
constructor.

[yosin_UTC9, 2016-10-20 09:43:43]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2016-10-20 09:43:49]

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

[yosin_UTC9, 2016-10-20 09:43:49]

The CQ bit was unchecked by yosin@chromium.org

[tkent, 2016-10-21 01:06:27]

https://codereview.chromium.org/2428363004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/DragController.cpp (right):

https://codereview.chromium.org/2428363004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/DragController.cpp:474: if
(!position.isConnected())
On 2016/10/20 at 08:34:00, Yosi_UTC9 wrote:
> On 2016/10/20 at 08:05:19, tkent wrote:
> > In the old code, what happened if
!frame->positionForPoint(point).isConnected()?
> > Is the behavior of the old code and the new code identical?
> 
> It makes empty selection since visible position canonicalization converts it
to null position.
> On debug build, it hits DCHECK(position.isConnected()) in VisiblePosition
constructor.

Because this code is in |if (frame-selection().isNone())| clause, skipping
setSelection() won't change user-visible behavior, right?

[yosin_UTC9, 2016-10-21 01:23:40]

https://codereview.chromium.org/2428363004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/DragController.cpp (right):

https://codereview.chromium.org/2428363004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/DragController.cpp:474: if
(!position.isConnected())
On 2016/10/21 at 01:06:27, tkent wrote:
> On 2016/10/20 at 08:34:00, Yosi_UTC9 wrote:
> > On 2016/10/20 at 08:05:19, tkent wrote:
> > > In the old code, what happened if
!frame->positionForPoint(point).isConnected()?
> > > Is the behavior of the old code and the new code identical?
> > 
> > It makes empty selection since visible position canonicalization converts it
to null position.
> > On debug build, it hits DCHECK(position.isConnected()) in VisiblePosition
constructor.
> 
> Because this code is in |if (frame-selection().isNone())| clause, skipping
setSelection() won't change user-visible behavior, right?

Yes, since we attempt to set selection to none on none selection.

[tkent, 2016-10-21 01:24:39]

lgtm

[yosin_UTC9, 2016-10-21 01:37:31]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2016-10-21 01:38:13]

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-10-21 01:51:53]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-21 13:25:55]

Description was changed from

==========
Get rid of createVisibleSelection() to take PositionWithAffinity

This patch gets rid of |createVisibleSelection()| taking |PositionWithAffinity|
by
replacing with |SelectionInDOMTree| version to reduce number of overloads for
improving code health.

BUG=657237
TEST=n/a; no behavior changes
==========

to

==========
Get rid of createVisibleSelection() to take PositionWithAffinity

This patch gets rid of |createVisibleSelection()| taking |PositionWithAffinity|
by
replacing with |SelectionInDOMTree| version to reduce number of overloads for
improving code health.

BUG=657237
TEST=n/a; no behavior changes

Committed: https://crrev.com/9af5344ba29c36f6ec50fd75791c6a606a3309fc
Cr-Commit-Position: refs/heads/master@{#426684}
==========

[commit-bot: I haz the power, 2016-10-21 13:25:56]

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