Manage DOM and string offsets separately in TextIteratorTextState

Manage DOM and string offsets separately in TextIteratorTextState

TextIteratorTextState stores only one offset range as both DOM offset and
string offset, which is incorrect (e.g., when with text-transform).

This patch changes TextIteratorTextState to store DOM and string offsets
separately, so that the above issue no longer exist in the class (it still
widely exists in many other components, though).

This patch also makes it possible to stop using LayoutText in the class, so
that TextIteratorTextState can also work with Layout NG.

Note: The class originally has a member |text_start_offet_| as a hack when
there is :first-letter, which is removed by the patch. A new member for
string offset is added back, which has the same name by pure coincidence.

BUG=721957
TEST=n/a; no behavioral change

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

[Xiaocheng, 2017-06-02 01:54:20]

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

[commit-bot: I haz the power, 2017-06-02 01:54:51]

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, 2017-06-02 03:51:14]

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

[commit-bot: I haz the power, 2017-06-02 03:51:15]

Dry run: This issue passed the CQ dry run.

[Xiaocheng, 2017-06-02 05:21:29]

Description was changed from

==========
Manage DOM and string offsets separately in TextIteratorTextState

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

to

==========
Manage DOM and string offsets separately in TextIteratorTextState

TextIteratorTextState stores only one offset range as both DOM offset and
string offset, which is incorrect (e.g., when with text-transform).

This patch changes TextIteratorTextState to store DOM and string offsets
separately, so that the above issue no longer exist in the class (it still
widely exists in many other components, though).

This patch also makes it possible to stop using LayoutText in the class, so
that TextIteratorTextState can also work with Layout NG.

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

[Xiaocheng, 2017-06-02 05:21:50]

Description was changed from

==========
Manage DOM and string offsets separately in TextIteratorTextState

TextIteratorTextState stores only one offset range as both DOM offset and
string offset, which is incorrect (e.g., when with text-transform).

This patch changes TextIteratorTextState to store DOM and string offsets
separately, so that the above issue no longer exist in the class (it still
widely exists in many other components, though).

This patch also makes it possible to stop using LayoutText in the class, so
that TextIteratorTextState can also work with Layout NG.

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

to

==========
Manage DOM and string offsets separately in TextIteratorTextState

TextIteratorTextState stores only one offset range as both DOM offset and
string offset, which is incorrect (e.g., when with text-transform).

This patch changes TextIteratorTextState to store DOM and string offsets
separately, so that the above issue no longer exist in the class (it still
widely exists in many other components, though).

This patch also makes it possible to stop using LayoutText in the class, so
that TextIteratorTextState can also work with Layout NG.

BUG=721957
TEST=n/a; no behavioral change
==========

[Xiaocheng, 2017-06-02 05:23:38]

Description was changed from

==========
Manage DOM and string offsets separately in TextIteratorTextState

TextIteratorTextState stores only one offset range as both DOM offset and
string offset, which is incorrect (e.g., when with text-transform).

This patch changes TextIteratorTextState to store DOM and string offsets
separately, so that the above issue no longer exist in the class (it still
widely exists in many other components, though).

This patch also makes it possible to stop using LayoutText in the class, so
that TextIteratorTextState can also work with Layout NG.

BUG=721957
TEST=n/a; no behavioral change
==========

to

==========
Manage DOM and string offsets separately in TextIteratorTextState

TextIteratorTextState stores only one offset range as both DOM offset and
string offset, which is incorrect (e.g., when with text-transform).

This patch changes TextIteratorTextState to store DOM and string offsets
separately, so that the above issue no longer exist in the class (it still
widely exists in many other components, though).

This patch also makes it possible to stop using LayoutText in the class, so
that TextIteratorTextState can also work with Layout NG.

Note: The class originally has a member |text_start_offet_| for the DOM offset
of the remaining text when there is :first-letter, which is removed by the
patch.
The patch adds a new member for string offset, which has the same name
incidentally.

BUG=721957
TEST=n/a; no behavioral change
==========

[Xiaocheng, 2017-06-02 05:24:40]

Description was changed from

==========
Manage DOM and string offsets separately in TextIteratorTextState

TextIteratorTextState stores only one offset range as both DOM offset and
string offset, which is incorrect (e.g., when with text-transform).

This patch changes TextIteratorTextState to store DOM and string offsets
separately, so that the above issue no longer exist in the class (it still
widely exists in many other components, though).

This patch also makes it possible to stop using LayoutText in the class, so
that TextIteratorTextState can also work with Layout NG.

Note: The class originally has a member |text_start_offet_| for the DOM offset
of the remaining text when there is :first-letter, which is removed by the
patch.
The patch adds a new member for string offset, which has the same name
incidentally.

BUG=721957
TEST=n/a; no behavioral change
==========

to

==========
Manage DOM and string offsets separately in TextIteratorTextState

TextIteratorTextState stores only one offset range as both DOM offset and
string offset, which is incorrect (e.g., when with text-transform).

This patch changes TextIteratorTextState to store DOM and string offsets
separately, so that the above issue no longer exist in the class (it still
widely exists in many other components, though).

This patch also makes it possible to stop using LayoutText in the class, so
that TextIteratorTextState can also work with Layout NG.

Note: The class originally has a member |text_start_offet_| as a hack when
there is :first-letter, which is removed by the patch. A new member for string
offset is added back, which has the same name by pure incident.

BUG=721957
TEST=n/a; no behavioral change
==========

[Xiaocheng, 2017-06-02 05:24:47]

Description was changed from

==========
Manage DOM and string offsets separately in TextIteratorTextState

TextIteratorTextState stores only one offset range as both DOM offset and
string offset, which is incorrect (e.g., when with text-transform).

This patch changes TextIteratorTextState to store DOM and string offsets
separately, so that the above issue no longer exist in the class (it still
widely exists in many other components, though).

This patch also makes it possible to stop using LayoutText in the class, so
that TextIteratorTextState can also work with Layout NG.

Note: The class originally has a member |text_start_offet_| as a hack when
there is :first-letter, which is removed by the patch. A new member for string
offset is added back, which has the same name by pure incident.

BUG=721957
TEST=n/a; no behavioral change
==========

to

==========
Manage DOM and string offsets separately in TextIteratorTextState

TextIteratorTextState stores only one offset range as both DOM offset and
string offset, which is incorrect (e.g., when with text-transform).

This patch changes TextIteratorTextState to store DOM and string offsets
separately, so that the above issue no longer exist in the class (it still
widely exists in many other components, though).

This patch also makes it possible to stop using LayoutText in the class, so
that TextIteratorTextState can also work with Layout NG.

Note: The class originally has a member |text_start_offet_| as a hack when
there is :first-letter, which is removed by the patch. A new member for
string offset is added back, which has the same name by pure incident.

BUG=721957
TEST=n/a; no behavioral change
==========

[Xiaocheng, 2017-06-02 05:24:54]

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

[commit-bot: I haz the power, 2017-06-02 05:25:02]

Dry run: CQ is trying da patch.

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

[Xiaocheng, 2017-06-02 05:25:02]

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

[Xiaocheng, 2017-06-02 05:25:03]

PTAL.

[commit-bot: I haz the power, 2017-06-02 07:03:57]

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

[commit-bot: I haz the power, 2017-06-02 07:03:58]

Dry run: This issue passed the CQ dry run.

[yosin_UTC9, 2017-06-02 08:38:57]

lgtm w/ nits

https://codereview.chromium.org/2914313002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.h
(right):

https://codereview.chromium.org/2914313002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.h:96: int
text_start_offset_;
nit: It is better to have |= 0;|, or |= -1; # to detect access before
initialize|.

[Xiaocheng, 2017-06-02 17:35:29]

The CQ bit was checked by xiaochengh@chromium.org

[Xiaocheng, 2017-06-02 17:35:30]

The patchset sent to the CQ was uploaded after l-g-t-m from yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/2914313002/#ps40001
(title: "Fri Jun  2 10:34:55 PDT 2017")

[Xiaocheng, 2017-06-02 17:35:38]

Thanks for reviewing!

https://codereview.chromium.org/2914313002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.h
(right):

https://codereview.chromium.org/2914313002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.h:96: int
text_start_offset_;
On 2017/06/02 at 08:38:56, yosin_UTC9 wrote:
> nit: It is better to have |= 0;|, or |= -1; # to detect access before
initialize|.

Whoops, forgot to initialize...

Done.

[commit-bot: I haz the power, 2017-06-02 17:35:42]

CQ is trying da patch.

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

[Xiaocheng, 2017-06-02 17:36:36]

Description was changed from

==========
Manage DOM and string offsets separately in TextIteratorTextState

TextIteratorTextState stores only one offset range as both DOM offset and
string offset, which is incorrect (e.g., when with text-transform).

This patch changes TextIteratorTextState to store DOM and string offsets
separately, so that the above issue no longer exist in the class (it still
widely exists in many other components, though).

This patch also makes it possible to stop using LayoutText in the class, so
that TextIteratorTextState can also work with Layout NG.

Note: The class originally has a member |text_start_offet_| as a hack when
there is :first-letter, which is removed by the patch. A new member for
string offset is added back, which has the same name by pure incident.

BUG=721957
TEST=n/a; no behavioral change
==========

to

==========
Manage DOM and string offsets separately in TextIteratorTextState

TextIteratorTextState stores only one offset range as both DOM offset and
string offset, which is incorrect (e.g., when with text-transform).

This patch changes TextIteratorTextState to store DOM and string offsets
separately, so that the above issue no longer exist in the class (it still
widely exists in many other components, though).

This patch also makes it possible to stop using LayoutText in the class, so
that TextIteratorTextState can also work with Layout NG.

Note: The class originally has a member |text_start_offet_| as a hack when
there is :first-letter, which is removed by the patch. A new member for
string offset is added back, which has the same name by pure coincidence.

BUG=721957
TEST=n/a; no behavioral change
==========

[commit-bot: I haz the power, 2017-06-02 19:45:55]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496424929979680,
"parent_rev": "5ff3e973ae76550542d100b0714515bb72be3f91", "commit_rev":
"b31e3ce881bd72c860c563cbcf74ff16e3bfbe84"}

[commit-bot: I haz the power, 2017-06-02 19:46:07]

Description was changed from

==========
Manage DOM and string offsets separately in TextIteratorTextState

TextIteratorTextState stores only one offset range as both DOM offset and
string offset, which is incorrect (e.g., when with text-transform).

This patch changes TextIteratorTextState to store DOM and string offsets
separately, so that the above issue no longer exist in the class (it still
widely exists in many other components, though).

This patch also makes it possible to stop using LayoutText in the class, so
that TextIteratorTextState can also work with Layout NG.

Note: The class originally has a member |text_start_offet_| as a hack when
there is :first-letter, which is removed by the patch. A new member for
string offset is added back, which has the same name by pure coincidence.

BUG=721957
TEST=n/a; no behavioral change
==========

to

==========
Manage DOM and string offsets separately in TextIteratorTextState

TextIteratorTextState stores only one offset range as both DOM offset and
string offset, which is incorrect (e.g., when with text-transform).

This patch changes TextIteratorTextState to store DOM and string offsets
separately, so that the above issue no longer exist in the class (it still
widely exists in many other components, though).

This patch also makes it possible to stop using LayoutText in the class, so
that TextIteratorTextState can also work with Layout NG.

Note: The class originally has a member |text_start_offet_| as a hack when
there is :first-letter, which is removed by the patch. A new member for
string offset is added back, which has the same name by pure coincidence.

BUG=721957
TEST=n/a; no behavioral change

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

[commit-bot: I haz the power, 2017-06-02 19:46:08]

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