Make CalcSelection return EphemeralRange.

Make CalcSelection return EphemeralRange.

This is preparation to split LayoutSelection::start and end:
https://codereview.chromium.org/2962143002/

BUG=739062
TEST=No change in behavior

Review-Url: https://codereview.chromium.org/2970653003
Cr-Commit-Position: refs/heads/master@{#484086}
Committed: https://chromium.googlesource.com/chromium/src/+/4ff2d41025006a482ea0d5fb40fb4aa3a585e3d3

[yoichio, 2017-07-04 05:28:43]

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

[commit-bot: I haz the power, 2017-07-04 05:28:50]

Dry run: CQ is trying da patch.

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

[yoichio, 2017-07-04 05:29:15]

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

[yosin_UTC9, 2017-07-04 05:56:41]

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

https://codereview.chromium.org/2970653003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/LayoutSelection.cpp:123: static
std::pair<PositionInFlatTree, PositionInFlatTree> CalcSelection(
Let's use |EphemeralRangeInFlatTree()| rather than
|std::pair<PositionInFlatTree, PositionInFlatTree>|
to utilize existing class and this pair holds first <= second and this pair
should be first.IsNull() when second.IsNull().

[yoichio, 2017-07-04 06:31:30]

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

https://codereview.chromium.org/2970653003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/LayoutSelection.cpp:123: static
std::pair<PositionInFlatTree, PositionInFlatTree> CalcSelection(
On 2017/07/04 05:56:41, yosin_UTC9 wrote:
> Let's use |EphemeralRangeInFlatTree()| rather than
> |std::pair<PositionInFlatTree, PositionInFlatTree>|
> to utilize existing class and this pair holds first <= second and this pair
> should be first.IsNull() when second.IsNull().
I will introduce a class kind of pair<LayoutObject, int>. It may hold
 LayoutObject w/o Node like  list-style-image.

[commit-bot: I haz the power, 2017-07-04 07:23:15]

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

[commit-bot: I haz the power, 2017-07-04 07:23:16]

Dry run: This issue passed the CQ dry run.

[yosin_UTC9, 2017-07-04 07:28:43]

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

https://codereview.chromium.org/2970653003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/LayoutSelection.cpp:123: static
std::pair<PositionInFlatTree, PositionInFlatTree> CalcSelection(
On 2017/07/04 at 06:31:30, yoichio wrote:
> On 2017/07/04 05:56:41, yosin_UTC9 wrote:
> > Let's use |EphemeralRangeInFlatTree()| rather than
> > |std::pair<PositionInFlatTree, PositionInFlatTree>|
> > to utilize existing class and this pair holds first <= second and this pair
> > should be first.IsNull() when second.IsNull().
> I will introduce a class kind of pair<LayoutObject, int>. It may hold
>  LayoutObject w/o Node like  list-style-image.

We may want to introduce SelectionRange instead of std::pair<SelectionPosition,
Position>,
although |SelectionPositon| and |SelectionRange| aren't good names.

std:pair<a, b> is a kind of anonymous struct, we want to access members with
meaningful name
e.g. start/end instead of first/second.

Since we want to reduce usage of VisibleSelection, crbug.com/657237, it is good
idea to
return |EphemeralRange| instead of |VisibleSeleciton| instead of a preparation
of
crrev.com/2962143002/

[yoichio, 2017-07-04 08:02:41]

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

[yoichio, 2017-07-04 08:02:45]

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

https://codereview.chromium.org/2970653003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/LayoutSelection.cpp:123: static
std::pair<PositionInFlatTree, PositionInFlatTree> CalcSelection(
On 2017/07/04 07:28:43, yosin_UTC9 wrote:
> On 2017/07/04 at 06:31:30, yoichio wrote:
> > On 2017/07/04 05:56:41, yosin_UTC9 wrote:
> > > Let's use |EphemeralRangeInFlatTree()| rather than
> > > |std::pair<PositionInFlatTree, PositionInFlatTree>|
> > > to utilize existing class and this pair holds first <= second and this
pair
> > > should be first.IsNull() when second.IsNull().
> > I will introduce a class kind of pair<LayoutObject, int>. It may hold
> >  LayoutObject w/o Node like  list-style-image.
> 
> We may want to introduce SelectionRange instead of
std::pair<SelectionPosition,
> Position>,
> although |SelectionPositon| and |SelectionRange| aren't good names.
> 
> std:pair<a, b> is a kind of anonymous struct, we want to access members with
> meaningful name
> e.g. start/end instead of first/second.
> 
> Since we want to reduce usage of VisibleSelection, crbug.com/657237, it is
good
> idea to
> return |EphemeralRange| instead of |VisibleSeleciton| instead of a preparation
> of
> crrev.com/2962143002/
Intorducd |using SelectionRange = std::pair<PositionInFlatTree,
PositionInFlatTree>|.

[commit-bot: I haz the power, 2017-07-04 08:02:53]

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-07-04 08:23:41]

On 2017/07/04 at 08:02:45, yoichio wrote:
>
https://codereview.chromium.org/2970653003/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/editing/LayoutSelection.cpp (right):
> 
>
https://codereview.chromium.org/2970653003/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/editing/LayoutSelection.cpp:123: static
std::pair<PositionInFlatTree, PositionInFlatTree> CalcSelection(
> On 2017/07/04 07:28:43, yosin_UTC9 wrote:
> > On 2017/07/04 at 06:31:30, yoichio wrote:
> > > On 2017/07/04 05:56:41, yosin_UTC9 wrote:
> > > > Let's use |EphemeralRangeInFlatTree()| rather than
> > > > |std::pair<PositionInFlatTree, PositionInFlatTree>|
> > > > to utilize existing class and this pair holds first <= second and this
pair
> > > > should be first.IsNull() when second.IsNull().
> > > I will introduce a class kind of pair<LayoutObject, int>. It may hold
> > >  LayoutObject w/o Node like  list-style-image.
> > 
> > We may want to introduce SelectionRange instead of
std::pair<SelectionPosition,
> > Position>,
> > although |SelectionPositon| and |SelectionRange| aren't good names.
> > 
> > std:pair<a, b> is a kind of anonymous struct, we want to access members with
> > meaningful name
> > e.g. start/end instead of first/second.
> > 
> > Since we want to reduce usage of VisibleSelection, crbug.com/657237, it is
good
> > idea to
> > return |EphemeralRange| instead of |VisibleSeleciton| instead of a
preparation
> > of
> > crrev.com/2962143002/
> Intorducd |using SelectionRange = std::pair<PositionInFlatTree,
PositionInFlatTree>|.

I'm not a fan of using std::pair<> here, since we have EphemeralRange.
|.first| and |.second| aren't good for readability.

Name |SelectionRange| is hard to understand.
I think there are no good reasons to use std::pair<PositionInFlatTree,
PositionInFlatTree> here.

[yoichio, 2017-07-04 08:41:05]

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

[commit-bot: I haz the power, 2017-07-04 08:41:14]

Dry run: CQ is trying da patch.

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

[yoichio, 2017-07-04 08:43:14]

> I'm not a fan of using std::pair<> here, since we have EphemeralRange.
> |.first| and |.second| aren't good for readability.
> 
> Name |SelectionRange| is hard to understand.
> I think there are no good reasons to use std::pair<PositionInFlatTree,
> PositionInFlatTree> here.

O.K. Until we introduce "LayoutPosition", EphemeralRange works so far.

[yosin_UTC9, 2017-07-04 08:56:25]

lgtm

[yoichio, 2017-07-04 08:59:18]

Description was changed from

==========
Make CalcSelection return std::pair<PositionInFlatTree, PositionInFlatTree>.

This is preparation to split LayoutSelection::start and end:
https://codereview.chromium.org/2962143002/

BUG=739062
TEST=No change in behavior
==========

to

==========
Make CalcSelection return EphemeralRange.

This is preparation to split LayoutSelection::start and end:
https://codereview.chromium.org/2962143002/

BUG=739062
TEST=No change in behavior
==========

[yoichio, 2017-07-04 08:59:18]

Description was changed from

==========
Make CalcSelection return std::pair<PositionInFlatTree, PositionInFlatTree>.

This is preparation to split LayoutSelection::start and end:
https://codereview.chromium.org/2962143002/

BUG=739062
TEST=No change in behavior
==========

to

==========
Make CalcSelection return EphemeralRange.

This is preparation to split LayoutSelection::start and end:
https://codereview.chromium.org/2962143002/

BUG=739062
TEST=No change in behavior
==========

[yoichio, 2017-07-04 10:43:01]

The CQ bit was unchecked by yoichio@chromium.org

[yoichio, 2017-07-04 10:43:07]

The CQ bit was checked by yoichio@chromium.org

[commit-bot: I haz the power, 2017-07-04 10:43:17]

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-07-04 11:07:49]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1499164987066680,
"parent_rev": "75f581893dabbe3c79450016da08ba063511133e", "commit_rev":
"4ff2d41025006a482ea0d5fb40fb4aa3a585e3d3"}

[commit-bot: I haz the power, 2017-07-04 11:08:06]

Description was changed from

==========
Make CalcSelection return EphemeralRange.

This is preparation to split LayoutSelection::start and end:
https://codereview.chromium.org/2962143002/

BUG=739062
TEST=No change in behavior
==========

to

==========
Make CalcSelection return EphemeralRange.

This is preparation to split LayoutSelection::start and end:
https://codereview.chromium.org/2962143002/

BUG=739062
TEST=No change in behavior

Review-Url: https://codereview.chromium.org/2970653003
Cr-Commit-Position: refs/heads/master@{#484086}
Committed:
https://chromium.googlesource.com/chromium/src/+/4ff2d41025006a482ea0d5fb40fb...
==========

[commit-bot: I haz the power, 2017-07-04 11:08:08]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/4ff2d41025006a482ea0d5fb40fb...