Get rid of flat tree version of createVisibleSelection() taking two VisiblePositionInFlatTree

Get rid of flat tree version of createVisibleSelection() taking two VisiblePositionInFlatTree

This patch gets rid of flat tree version of |createVisibleSelection()| taking
to |VisiblePositionInFlatTree|by replacing with |SelectionInFlatTree| version to
reduce number of overloads for improving code health.

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

Committed: https://crrev.com/dfb8322ccb6687fa5fd64029e9746049c12e06e1
Cr-Commit-Position: refs/heads/master@{#426419}

[yosin_UTC9, 2016-10-17 08:17:19]

Description was changed from

==========
2016-10-17T17:15:40

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

to

==========
Get rid of flat tree version of createVisibleSelection() taking two
VisiblePositionInFlatTree

This patch gets rid of flat tree version of |createVisibleSelection()| taking
to |VisiblePositionInFlatTree|by replacing with |SelectionInFlatTree| version to
reduce number of overloads for improving code health.

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

[yosin_UTC9, 2016-10-17 08:17:24]

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

[commit-bot: I haz the power, 2016-10-17 08:17:47]

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-17 08:58:10]

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

[commit-bot: I haz the power, 2016-10-17 08:58:11]

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

[yosin_UTC9, 2016-10-18 08:19:57]

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

[commit-bot: I haz the power, 2016-10-18 08:20:08]

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

[yosin_UTC9, 2016-10-18 08:32:56]

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

[commit-bot: I haz the power, 2016-10-18 08:33:20]

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-18 10:18:26]

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

[commit-bot: I haz the power, 2016-10-18 10:18:27]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[yosin_UTC9, 2016-10-19 01:05:49]

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

[yosin_UTC9, 2016-10-19 01:05:50]

PTAL

[Xiaocheng, 2016-10-19 02:33:47]

https://codereview.chromium.org/2425623002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/PendingSelection.cpp (right):

https://codereview.chromium.org/2425623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/PendingSelection.cpp:85: if
(visibleStart.isNull())
Does it change the behavior? |createVisiblePosition(pos, pos)| currently uses
SelectionTemplate::Builder::setBaseAndExtentDeprecated, which does not
necessarily returns null when |visibleStart| is null.

https://codereview.chromium.org/2425623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/PendingSelection.cpp:90: if
(visibleExtent.isNull())
Same question.

https://codereview.chromium.org/2425623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/PendingSelection.cpp:101: if
(visibleEnd.isNull())
Same question.

[yosin_UTC9, 2016-10-19 03:59:58]

https://codereview.chromium.org/2425623002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/PendingSelection.cpp (right):

https://codereview.chromium.org/2425623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/PendingSelection.cpp:85: if
(visibleStart.isNull())
On 2016/10/19 at 02:33:47, Xiaocheng wrote:
> Does it change the behavior? |createVisiblePosition(pos, pos)| currently uses
SelectionTemplate::Builder::setBaseAndExtentDeprecated, which does not
necessarily returns null when |visibleStart| is null.

In the call site (only one), we do:

  const VisibleSelectionInFlatTree& selection =
      calcVisibleSelection(originalSelection);

  if (!selection.isRange()) {
    layoutView.clearSelection();
    return;
  }

So, caret and none are handled as same.

I think this is yet another example, it is better to handle null explicitly. ;-)
If original code does, so we don't need to discuss now.

https://codereview.chromium.org/2425623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/PendingSelection.cpp:90: if
(visibleExtent.isNull())
On 2016/10/19 at 02:33:47, Xiaocheng wrote:
> Same question.

ditto.

https://codereview.chromium.org/2425623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/PendingSelection.cpp:101: if
(visibleEnd.isNull())
On 2016/10/19 at 02:33:47, Xiaocheng wrote:
> Same question.

ditto.

[Xiaocheng, 2016-10-19 04:04:21]

I see. lgtm then.

[yosin_UTC9, 2016-10-19 04:12:25]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2016-10-19 04:14:08]

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 09:53:58]

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

[commit-bot: I haz the power, 2016-10-19 09:53:59]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[yosin_UTC9, 2016-10-19 09:54:25]

The CQ bit was checked by yosin@chromium.org

[yosin_UTC9, 2016-10-19 09:55:05]

Description was changed from

==========
Get rid of flat tree version of createVisibleSelection() taking two
VisiblePositionInFlatTree

This patch gets rid of flat tree version of |createVisibleSelection()| taking
to |VisiblePositionInFlatTree|by replacing with |SelectionInFlatTree| version to
reduce number of overloads for improving code health.

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

to

==========
Get rid of flat tree version of createVisibleSelection() taking two
VisiblePositionInFlatTree

This patch gets rid of flat tree version of |createVisibleSelection()| taking
to |VisiblePositionInFlatTree|by replacing with |SelectionInFlatTree| version to
reduce number of overloads for improving code health.

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

[commit-bot: I haz the power, 2016-10-19 09:55:39]

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 09:59:09]

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

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

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

[yosin_UTC9, 2016-10-20 04:20:14]

The CQ bit was checked by yosin@chromium.org

[yosin_UTC9, 2016-10-20 04:20:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
xiaochengh@chromium.org
Link to the patchset: https://codereview.chromium.org/2425623002/#ps80001
(title: "2016-10-20T13:19:29")

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

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 05:47:38]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-10-21 13:16:12]

Description was changed from

==========
Get rid of flat tree version of createVisibleSelection() taking two
VisiblePositionInFlatTree

This patch gets rid of flat tree version of |createVisibleSelection()| taking
to |VisiblePositionInFlatTree|by replacing with |SelectionInFlatTree| version to
reduce number of overloads for improving code health.

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

to

==========
Get rid of flat tree version of createVisibleSelection() taking two
VisiblePositionInFlatTree

This patch gets rid of flat tree version of |createVisibleSelection()| taking
to |VisiblePositionInFlatTree|by replacing with |SelectionInFlatTree| version to
reduce number of overloads for improving code health.

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

Committed: https://crrev.com/dfb8322ccb6687fa5fd64029e9746049c12e06e1
Cr-Commit-Position: refs/heads/master@{#426419}
==========

[commit-bot: I haz the power, 2016-10-21 13:16:15]

Patchset 5 (id:??) landed as
https://crrev.com/dfb8322ccb6687fa5fd64029e9746049c12e06e1
Cr-Commit-Position: refs/heads/master@{#426419}