[All-in-one patch] Implement own grapheme boundary breaker for editing.

Implement own grapheme boundary breaker for editing.

Followings are differences from the previous grapheme breaker:
- Follows latest(including proposed) grapheme breaker spec:
  Unicode Standard Annex #29 http://www.unicode.org/reports/tr29

- Removed Tamil exception rule from Indic syllable composition.
  Introduced by https://bugs.webkit.org/show_bug.cgi?id=15790
  Neither bug nor change description doesn't mention about Tamil
  exception but latest Unicode standards doesn't have exception.

- Revive Rule GB9b rule: don't break after Prepend property.
  Introduced by https://bugs.webkit.org/show_bug.cgi?id=24342
  It is safe to revive this rule since Thai characters no longer have
  Prepend property since Unicode 6.1.
  Note that there are no character having Prepend property until
  Unicode 9.0 after Unicode 6.1

BUG=594923
R=yosin

[Seigo Nonaka, 2016-03-28 07:26:21]

Patchset #1 (id:1) has been deleted

[Seigo Nonaka, 2016-03-28 08:00:08]

nona@chromium.org changed reviewers:
+ yosin@chromium.org
- yosin@google.com

[Seigo Nonaka, 2016-03-28 08:00:09]

Hi Yosi-san,

Could you kindly take a look?

Thank you.

[yosin_UTC9, 2016-03-28 09:34:49]

https://codereview.chromium.org/1833413002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/DEPS (right):

https://codereview.chromium.org/1833413002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/DEPS:12:
"+third_party/icu/source/common/unicode",
MEMO: This change is also in http://crrev.com/1839613002, Move
LayoutObject::previousBackspaceOffset()

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

https://codereview.chromium.org/1833413002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:577: const int
length =
Please use |WTF_ARRAY_LENGTH(array)| in "wtf/StdLibExtras.h"

https://codereview.chromium.org/1833413002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:586: // The
following breaking rules come from Unicode Standard Annex #29 on Unicode Text
Segmaentation.
Could you add following |DCHECK()|?

DCHECK_GE(length, 0);
DCHECK_GE(offset, 0);
DCHECK_LE(offset, length);

https://codereview.chromium.org/1833413002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:593: if (offset <= 0
|| offset >= length) {
nit: No need to have |{}| for one line then-clause.

https://codereview.chromium.org/1833413002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:692: if
(!node->isTextNode())
Could you add |DCHECK_GE(current, 0)|?

https://codereview.chromium.org/1833413002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:706: while
(!isGraphemeBreak(buf, length, --current)) {}
MEMO: Google style allowed one line loop:
https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements

https://codereview.chromium.org/1833413002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:717: if
(!node->isTextNode())
Could you add |DCHECK_GE(current, 0)|?

[Seigo Nonaka, 2016-03-28 13:42:19]

Thank you for your quick review.
I addressed your comments. please take an another look.

Thank you.

https://codereview.chromium.org/1833413002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/DEPS (right):

https://codereview.chromium.org/1833413002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/DEPS:12:
"+third_party/icu/source/common/unicode",
On 2016/03/28 09:34:49, Yosi_UTC9 wrote:
> MEMO: This change is also in http://crrev.com/1839613002, Move
> LayoutObject::previousBackspaceOffset()

Acknowledged. I'm going to remove this line once I got lgtm on other CLs.
This is for shutting up the PRESUBMIT script.

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

https://codereview.chromium.org/1833413002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:577: const int
length =
On 2016/03/28 09:34:49, Yosi_UTC9 wrote:
> Please use |WTF_ARRAY_LENGTH(array)| in "wtf/StdLibExtras.h"

Done.

https://codereview.chromium.org/1833413002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:586: // The
following breaking rules come from Unicode Standard Annex #29 on Unicode Text
Segmaentation.
On 2016/03/28 09:34:49, Yosi_UTC9 wrote:
> Could you add following |DCHECK()|?
> 
> DCHECK_GE(length, 0);
> DCHECK_GE(offset, 0);
> DCHECK_LE(offset, length);

Done.

https://codereview.chromium.org/1833413002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:593: if (offset <= 0
|| offset >= length) {
On 2016/03/28 09:34:49, Yosi_UTC9 wrote:
> nit: No need to have |{}| for one line then-clause.

Done.

https://codereview.chromium.org/1833413002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:692: if
(!node->isTextNode())
On 2016/03/28 09:34:49, Yosi_UTC9 wrote:
> Could you add |DCHECK_GE(current, 0)|?

Done.

https://codereview.chromium.org/1833413002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:706: while
(!isGraphemeBreak(buf, length, --current)) {}
On 2016/03/28 09:34:49, Yosi_UTC9 wrote:
> MEMO: Google style allowed one line loop:
> https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements

Sure, but let me keep current style since neither one line continue nor empty
body passes the PRESUBMIT check.

Thank you for letting me know.

https://codereview.chromium.org/1833413002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:717: if
(!node->isTextNode())
On 2016/03/28 09:34:49, Yosi_UTC9 wrote:
> Could you add |DCHECK_GE(current, 0)|?

Done.

[yosin_UTC9, 2016-03-29 01:27:30]

Sorry for changing my mind... m(_ _)m

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

https://codereview.chromium.org/1833413002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:583: bool
isGraphemeBreak(const uint16_t* buf, int length, int offset)
Can we make this in state machine as BackspaceStateMachine to make
uncheckedPreviousOffset() and uncheckedPreviousBackspaceOffset() to work with
multiple Text nodes?

It is OK to keep uncheckedPreviousOffset() and
uncheckedPreviousBackspaceOffset() to take a node and offset now. We'll rewrite
them later.

Since, Text node can hold any code unit sequence and simple implementation can
split one word into multiple Text node.
For example,
We have a "<b>abc</b>", there is one Text node "abc". A script attempts insert
"XYZ" between "a" and "b".
A script does
 - text_node = document.querySelector('b').firstChild;
 - remainder_node = text_node.split(1); // split "abc" into "a" and "bc"
 - remainder_node.parentNode.insertBefore(xyz_node, remainder_node)
Then we have "<b>aXYZbc</b>" in outerHTML, "b" element has three Text node "a",
"XYZ", "bc".

nextPositionOf(), previousPositionOf(), previousBackspacePositionOf() should
work above with graphme cluster.

Pseudo implementation previousPositionOf() could be:

Position previousPositionOf(const Position& position) {
 ... other staff
 if (position.isOffsetInAnchor() && position.anchorNode()->isTextNode()) {
    int offset = position.offsetInAnchor();
    if (offset == 0)
       return previousPositionOf(Position::beforeNode(position.anchorNode());
    // We assume |offset| at graphme boundary
    PreviousGraphmeMachine machine;
    const Text* textNode = toText(position.anchorNode())
  continue_feed:
    for (int runner = offset; runner >= 0; --runner) {
      auto const next = machine.transit(textNode->characterAt(runner));
      if (next == Done)
        return Position(position.anchorNode(), runner);
    }
    // We are at middle of graphme, we need to feed more character to machine.
    if (textNode->previousSibling() &&
textNode->previousSibling()->isTextNode()) {
      // Start over at end of previous sibling Text node
      textNode = toText(textNode->previousSibling());
      offset = textNode->length();
      goto continue_feed;
    }
    // Broken graphme cluster, just go previous node.
    return previousPositionOf(Position::beforeNode(position.anchorNode());
 }
}

[Seigo Nonaka, 2016-04-03 18:59:18]

Description was changed from

==========
Implement own grapheme boundary breaker for editing.

Followings are differences from the previous grapheme breaker:
- Follows latest(including proposed) grapheme breaker spec:
  Unicode Standard Annex #29 http://www.unicode.org/reports/tr29

- Removed Tamil exception rule from Indic syllable composition.
  Introduced by https://bugs.webkit.org/show_bug.cgi?id=15790
  Neither bug nor change description doesn't mention about Tamil
  exception but latest Unicode standards doesn't have exception.

- Revive Rule GB9b rule: don't break after Prepend property.
  Introduced by https://bugs.webkit.org/show_bug.cgi?id=24342
  It is safe to revive this rule since Thai characters no longer have
  Prepend property since Unicode 6.1.
  Note that there are no character having Prepend property until
  Unicode 9.0 after Unicode 6.1

BUG=594923
R=yosin
==========

to

==========
Implement own grapheme boundary breaker for editing.

Followings are differences from the previous grapheme breaker:
- Follows latest(including proposed) grapheme breaker spec:
  Unicode Standard Annex #29 http://www.unicode.org/reports/tr29

- Removed Tamil exception rule from Indic syllable composition.
  Introduced by https://bugs.webkit.org/show_bug.cgi?id=15790
  Neither bug nor change description doesn't mention about Tamil
  exception but latest Unicode standards doesn't have exception.

- Revive Rule GB9b rule: don't break after Prepend property.
  Introduced by https://bugs.webkit.org/show_bug.cgi?id=24342
  It is safe to revive this rule since Thai characters no longer have
  Prepend property since Unicode 6.1.
  Note that there are no character having Prepend property until
  Unicode 9.0 after Unicode 6.1

BUG=594923
R=yosin
==========

[Seigo Nonaka, 2016-04-03 19:31:33]

I'm sorry for super slow reply.

I'd like to split this CL into four.

Introduce core common logic:
https://codereview.chromium.org/1856603002/

Introduce backward movement logic:
https://codereview.chromium.org/1853033002/

Introduce forward movement logic:
https://codereview.chromium.org/1857633002/

Replace implementation:
https://codereview.chromium.org/1855813002/

As the all-in-one patch, I uploaded latest patch set to this.

Thank you for all your support.