Use our own grapheme boundary handling.

Use our own grapheme boundary handling

This CL replaces the grapheme boundary detection
from ICU's implementation to our own.

By replacing with own implementation, we can support
latest Unicode spec without waiting ICU update.
For example, grapheme boundary for the recent complex
emoji sequence is now supported.

There is another benefit of new implementation. The
interface of new implementation does not require the
full continuous text buffer. This is good if the target
text crosses the Nodes.

BUG=594923
Committed: https://crrev.com/3d284a415f093a8e0ddbdd955f9f3ee5e11c2a57
Cr-Commit-Position: refs/heads/master@{#385720}

[Seigo Nonaka, 2016-04-03 19:28:35]

Description was changed from

==========
Replace Grapheme with our own

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

to

==========
Replace grapheme boundary movement with our own implementation

BUG=594923
==========

[Seigo Nonaka, 2016-04-03 19:28:35]

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

[Seigo Nonaka, 2016-04-06 06:18:56]

Yosi-san, this is the last CL for grapheme boundary.
Could you kindly take a look?

Thank you.

[yosin_UTC9, 2016-04-06 07:33:43]

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

https://codereview.chromium.org/1853033002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:574: int
uncheckedPreviousOffset(const Node* node, int current)
It seems make |uncheckedPreviousOffset()| and |uncheckNextOffset()| to work on
grapheme is right approach.

We need to introduce |PositionMoveType::GraphemeCluster| and make
|{previous,next}PositionOf()| to call |unchecked{Previous,Next}Of()| to use them
for |PositionMoveType::GraphemeCluster|, and, change
|PositionMoveType::CodePoint| of |{previous,next}PositionOf()| to work on code
point instead of calling 
|PositionMoveType::GraphemeCluster|.

[yosin_UTC9, 2016-04-06 07:44:00]

To reduce confusion, I recommend rename 
- uncheckedNextOffset() to nextGraphmeBoundaryOf()
- uncheckedPreviousOffset() to previousGraphmeBoundaryOf()
first in another patch.

[Seigo Nonaka, 2016-04-06 10:05:15]

Patchset #3 (id:40001) has been deleted

[Seigo Nonaka, 2016-04-06 10:05:21]

Patchset #3 (id:60001) has been deleted

[Seigo Nonaka, 2016-04-06 10:11:17]

Description was changed from

==========
Replace grapheme boundary movement with our own implementation

BUG=594923
==========

to

==========
Use our own grapheme boundary handling

This CL replaces the grapheme boundary detection 
from ICU's implementation to our own.

BUG=594923
==========

[Seigo Nonaka, 2016-04-06 10:13:04]

Thank you for your review.
I rebased for renaming CL.

Could you kindly take an another look?

[yosin_UTC9, 2016-04-07 01:06:54]

Could you add why we switch to use "our own grapheme boundary handling" in
change description?

https://codereview.chromium.org/1853033002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/EditingUtilities.cpp (right):

https://codereview.chromium.org/1853033002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:574: int
previousGraphemeBoundaryOf(const Node* node, int current)
Could you add TODO comment that we need to support grapheme crossing |Node|
boundary.

https://codereview.chromium.org/1853033002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:581: text, current);
nit: Could you make this call in one line to follow other parts?

https://codereview.chromium.org/1853033002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:599: if
(!node->isTextNode())
Could you add TODO comment that we need to support grapheme crossing |Node|
boundary.

https://codereview.chromium.org/1853033002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:604: if (current >=
(length - 1))
nit: We don't need to have parenthesis for |(length - 1)|.

https://codereview.chromium.org/1853033002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:607: text, current);
nit: Could you make this call in one line to follow other parts?

[Seigo Nonaka, 2016-04-07 03:44:42]

Description was changed from

==========
Use our own grapheme boundary handling

This CL replaces the grapheme boundary detection 
from ICU's implementation to our own.

BUG=594923
==========

to

==========
Use our own grapheme boundary handling

This CL replaces the grapheme boundary detection 
from ICU's implementation to our own.

By replacing with own implementation, we can support
latest Unicode spec without waiting ICU update.
For example, grapheme boundary for the recent complex
emoji sequence is now supported.

There is another benefit of new implementation. The
interface of new implementation does not require the
full continuous text buffer. This is good if the target
text crosses the Nodes.

BUG=594923
==========

[Seigo Nonaka, 2016-04-07 03:44:59]

Thank you for your review.
Please take an another look.

https://codereview.chromium.org/1853033002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/EditingUtilities.cpp (right):

https://codereview.chromium.org/1853033002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:574: int
previousGraphemeBoundaryOf(const Node* node, int current)
On 2016/04/07 01:06:53, Yosi_UTC9 wrote:
> Could you add TODO comment that we need to support grapheme crossing |Node|
> boundary.

Done.

https://codereview.chromium.org/1853033002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:581: text, current);
On 2016/04/07 01:06:53, Yosi_UTC9 wrote:
> nit: Could you make this call in one line to follow other parts?

Done.

https://codereview.chromium.org/1853033002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:599: if
(!node->isTextNode())
On 2016/04/07 01:06:53, Yosi_UTC9 wrote:
> Could you add TODO comment that we need to support grapheme crossing |Node|
> boundary.

Done.

https://codereview.chromium.org/1853033002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:604: if (current >=
(length - 1))
On 2016/04/07 01:06:53, Yosi_UTC9 wrote:
> nit: We don't need to have parenthesis for |(length - 1)|.

Done.

https://codereview.chromium.org/1853033002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:607: text, current);
On 2016/04/07 01:06:53, Yosi_UTC9 wrote:
> nit: Could you make this call in one line to follow other parts?

Done.

[yosin_UTC9, 2016-04-07 04:25:39]

The CQ bit was checked by yosin@chromium.org

[yosin_UTC9, 2016-04-07 04:25:40]

lgtm

[commit-bot: I haz the power, 2016-04-07 04:25:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1853033002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1853033002/100001

[yosin_UTC9, 2016-04-07 04:25:59]

The CQ bit was unchecked by yosin@chromium.org

[Seigo Nonaka, 2016-04-07 04:26:57]

Thank you for your review, but I'm sorry this cl is not ready for submit. This
depends on https://codereview.chromium.org/1857633002/

I'm going to ping him.

Thank you.

[Seigo Nonaka, 2016-04-07 06:28:17]

The CQ bit was checked by nona@chromium.org

[commit-bot: I haz the power, 2016-04-07 06:28:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1853033002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1853033002/100001

[commit-bot: I haz the power, 2016-04-07 07:37:47]

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

[commit-bot: I haz the power, 2016-04-07 07:37:48]

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

[Seigo Nonaka, 2016-04-07 07:39:37]

Ugh, I'm sorry the failure looks real.
I'm going to address it.

[Seigo Nonaka, 2016-04-07 08:55:32]

Patchset #6 (id:140001) has been deleted

[Seigo Nonaka, 2016-04-07 09:02:55]

Looks like https://codereview.chromium.org/1867793003/ is necessary for passing
paste-multiline-text-input.html

[Seigo Nonaka, 2016-04-07 11:01:45]

The CQ bit was checked by nona@chromium.org

[Seigo Nonaka, 2016-04-07 11:01:46]

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/1853033002/#ps160001
(title: "left TODO")

[commit-bot: I haz the power, 2016-04-07 11:02:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1853033002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1853033002/160001

[commit-bot: I haz the power, 2016-04-07 12:16:57]

Description was changed from

==========
Use our own grapheme boundary handling

This CL replaces the grapheme boundary detection 
from ICU's implementation to our own.

By replacing with own implementation, we can support
latest Unicode spec without waiting ICU update.
For example, grapheme boundary for the recent complex
emoji sequence is now supported.

There is another benefit of new implementation. The
interface of new implementation does not require the
full continuous text buffer. This is good if the target
text crosses the Nodes.

BUG=594923
==========

to

==========
Use our own grapheme boundary handling

This CL replaces the grapheme boundary detection 
from ICU's implementation to our own.

By replacing with own implementation, we can support
latest Unicode spec without waiting ICU update.
For example, grapheme boundary for the recent complex
emoji sequence is now supported.

There is another benefit of new implementation. The
interface of new implementation does not require the
full continuous text buffer. This is good if the target
text crosses the Nodes.

BUG=594923
==========

[commit-bot: I haz the power, 2016-04-07 12:16:58]

Committed patchset #6 (id:160001)

[commit-bot: I haz the power, 2016-04-07 12:17:55]

Description was changed from

==========
Use our own grapheme boundary handling

This CL replaces the grapheme boundary detection 
from ICU's implementation to our own.

By replacing with own implementation, we can support
latest Unicode spec without waiting ICU update.
For example, grapheme boundary for the recent complex
emoji sequence is now supported.

There is another benefit of new implementation. The
interface of new implementation does not require the
full continuous text buffer. This is good if the target
text crosses the Nodes.

BUG=594923
==========

to

==========
Use our own grapheme boundary handling

This CL replaces the grapheme boundary detection
from ICU's implementation to our own.

By replacing with own implementation, we can support
latest Unicode spec without waiting ICU update.
For example, grapheme boundary for the recent complex
emoji sequence is now supported.

There is another benefit of new implementation. The
interface of new implementation does not require the
full continuous text buffer. This is good if the target
text crosses the Nodes.

BUG=594923

Committed: https://crrev.com/3d284a415f093a8e0ddbdd955f9f3ee5e11c2a57
Cr-Commit-Position: refs/heads/master@{#385720}
==========

[commit-bot: I haz the power, 2016-04-07 12:17:56]

Patchset 6 (id:??) landed as
https://crrev.com/3d284a415f093a8e0ddbdd955f9f3ee5e11c2a57
Cr-Commit-Position: refs/heads/master@{#385720}