Make associatedLayoutObjectOf() to handle ::first-letter pseudo-element correctly

Make associatedLayoutObjectOf() to handle ::first-letter pseudo-element correctly

This patch makes |associatedLayoutObjectOf()| to handle all cases of layout tree
shape for ::first-letter pseudo-element.

Before this patch, |associatedLayoutObjectOf()| handled first-letter only |Text|
node incorrectly.

BUG=560689
TEST=webkit_unit_tests --gtest_filter=VisibleUnitsTest.associatedLayoutObjectOf*
TEST=webkit_unit_tests --gtest_filter=VisibleUnitsTest.mostBackwardCaretPositionFirstLetterSplit

Committed: https://crrev.com/7c9e1437f3de1149a5b1a9437d927d4e0d7802ba
Cr-Commit-Position: refs/heads/master@{#372299}

[yosin_UTC9, 2016-01-27 09:26:56]

Description was changed from

==========
2016-01-27T18:23:21

BUG=560689
==========

to

==========
Make associatedLayoutObjectOf() to handle first-letter character only Text node


BUG=560689
TEST=webkit_unit_tests --gtest_filter=VisibleUnitsTest.associatedLayoutObjectOf*
TEST=webkit_unit_tests
--gtest_filter=VisibleUnitsTest.mostBackwardCaretPositionFirstLetterSplit
==========

[yosin_UTC9, 2016-01-27 09:27:02]

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

[commit-bot: I haz the power, 2016-01-27 09:27:35]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1636373002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1636373002/1

[commit-bot: I haz the power, 2016-01-27 09:41:00]

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

[commit-bot: I haz the power, 2016-01-27 09:41:00]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[yosin_UTC9, 2016-01-28 05:03:56]

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

[commit-bot: I haz the power, 2016-01-28 05:04:57]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1636373002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1636373002/20001

[yosin_UTC9, 2016-01-28 06:27:44]

Description was changed from

==========
Make associatedLayoutObjectOf() to handle first-letter character only Text node


BUG=560689
TEST=webkit_unit_tests --gtest_filter=VisibleUnitsTest.associatedLayoutObjectOf*
TEST=webkit_unit_tests
--gtest_filter=VisibleUnitsTest.mostBackwardCaretPositionFirstLetterSplit
==========

to

==========
Make associatedLayoutObjectOf() to handle first-letter character only Text node

This patch makes |associatedLayoutObjectOf()| to handle first-letter character
only |Text| node.

There are three cases for first-letter:

#1 Text node holds first-letter "a", no remaining text.  <div>abc</div>, "a" and
    "bc" are different text node.
  LayoutBlockFlow {DIV} at (0,0) size 784x55
    LayoutInline {<pseudo:first-letter>} at (0,0) size 22x53
      LayoutTextFragment (anonymous) at (0,1) size 22x53
        text run at (0,1) width 22: "a"
    LayoutTextFragment {#text} at (0,0) size 0x0
    LayoutText {#text} at (21,30) size 16x17
      text run at (21,30) width 16: "bc"

#2 Text node hols first-letter "a" and remaining text "bc", <div>abc</div>
  LayoutBlockFlow {DIV} at (0,0) size 784x55
    LayoutInline {<pseudo:first-letter>} at (0,0) size 22x53
      LayoutTextFragment (anonymous) at (0,1) size 22x53
        text run at (0,1) width 22: "a"
    LayoutTextFragment {#text} at (21,30) size 16x17
      text run at (21,30) width 16: "bc"

#3 Text node with trailing whitespace, e.g. <div>a\n</div>
  layer at (0,0) size 800x600
    LayoutView at (0,0) size 800x600
  layer at (0,0) size 800x600
    LayoutBlockFlow {HTML} at (0,0) size 800x600
      LayoutBlockFlow {BODY} at (8,8) size 784x584
        LayoutBlockFlow {DIV} at (0,0) size 784x55
          LayoutInline {<pseudo:first-letter>} at (0,0) size 22x53
            LayoutTextFragment (anonymous) at (0,1) size 22x53
              text run at (0,1) width 22: "a"
          LayoutTextFragment {#text} at (0,0) size 0x0

BUG=560689
TEST=webkit_unit_tests --gtest_filter=VisibleUnitsTest.associatedLayoutObjectOf*
TEST=webkit_unit_tests
--gtest_filter=VisibleUnitsTest.mostBackwardCaretPositionFirstLetterSplit
==========

[yosin_UTC9, 2016-01-28 06:27:44]

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

[commit-bot: I haz the power, 2016-01-28 06:56:25]

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

[commit-bot: I haz the power, 2016-01-28 06:56:25]

Dry run: This issue passed the CQ dry run.

[yosin_UTC9, 2016-01-28 09:11:11]

PTAL

[tkent, 2016-01-29 00:56:09]

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

https://codereview.chromium.org/1636373002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/VisibleUnits.cpp:2190:
ASSERT(layoutTextFragment->isRemainingTextLayoutObject());
This ASSERT isn't helpful because
layoutTextFragment->isRemainingTextLayoutObject() was checked at line 2181.

https://codereview.chromium.org/1636373002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/VisibleUnits.cpp:2193: LayoutObject*
firstLetterLayoutObject =
layoutTextFragment->firstLetterPseudoElement()->layoutObject();
nullptr-check for firstLetterLayoutObject is removed.  Is it intentional?

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

https://codereview.chromium.org/1636373002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/VisibleUnits.h:78: CORE_EXPORT
LayoutObject* associatedLayoutObjectOf(const Node&, int offsetInNode);
Is this |offset functions on Node|?

[yosin_UTC9, 2016-01-29 03:43:24]

Description was changed from

==========
Make associatedLayoutObjectOf() to handle first-letter character only Text node

This patch makes |associatedLayoutObjectOf()| to handle first-letter character
only |Text| node.

There are three cases for first-letter:

#1 Text node holds first-letter "a", no remaining text.  <div>abc</div>, "a" and
    "bc" are different text node.
  LayoutBlockFlow {DIV} at (0,0) size 784x55
    LayoutInline {<pseudo:first-letter>} at (0,0) size 22x53
      LayoutTextFragment (anonymous) at (0,1) size 22x53
        text run at (0,1) width 22: "a"
    LayoutTextFragment {#text} at (0,0) size 0x0
    LayoutText {#text} at (21,30) size 16x17
      text run at (21,30) width 16: "bc"

#2 Text node hols first-letter "a" and remaining text "bc", <div>abc</div>
  LayoutBlockFlow {DIV} at (0,0) size 784x55
    LayoutInline {<pseudo:first-letter>} at (0,0) size 22x53
      LayoutTextFragment (anonymous) at (0,1) size 22x53
        text run at (0,1) width 22: "a"
    LayoutTextFragment {#text} at (21,30) size 16x17
      text run at (21,30) width 16: "bc"

#3 Text node with trailing whitespace, e.g. <div>a\n</div>
  layer at (0,0) size 800x600
    LayoutView at (0,0) size 800x600
  layer at (0,0) size 800x600
    LayoutBlockFlow {HTML} at (0,0) size 800x600
      LayoutBlockFlow {BODY} at (8,8) size 784x584
        LayoutBlockFlow {DIV} at (0,0) size 784x55
          LayoutInline {<pseudo:first-letter>} at (0,0) size 22x53
            LayoutTextFragment (anonymous) at (0,1) size 22x53
              text run at (0,1) width 22: "a"
          LayoutTextFragment {#text} at (0,0) size 0x0

BUG=560689
TEST=webkit_unit_tests --gtest_filter=VisibleUnitsTest.associatedLayoutObjectOf*
TEST=webkit_unit_tests
--gtest_filter=VisibleUnitsTest.mostBackwardCaretPositionFirstLetterSplit
==========

to

==========
Make associatedLayoutObjectOf() to handle ::first-letter pseudo-element
correctly

This patch makes |associatedLayoutObjectOf()| to handle all cases of layout tree
shape for ::first-letter pseudo-element.

Before this patch, |associatedLayoutObjectOf()| handled first-letter only |Text|
node incorrectly.

BUG=560689
TEST=webkit_unit_tests --gtest_filter=VisibleUnitsTest.associatedLayoutObjectOf*
TEST=webkit_unit_tests
--gtest_filter=VisibleUnitsTest.mostBackwardCaretPositionFirstLetterSplit
==========

[yosin_UTC9, 2016-01-29 03:45:05]

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

[yosin_UTC9, 2016-01-29 03:45:13]

PTAL

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

https://codereview.chromium.org/1636373002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/VisibleUnits.cpp:2190:
ASSERT(layoutTextFragment->isRemainingTextLayoutObject());
On 2016/01/29 at 00:56:09, tkent wrote:
> This ASSERT isn't helpful because
layoutTextFragment->isRemainingTextLayoutObject() was checked at line 2181.

Done.

https://codereview.chromium.org/1636373002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/VisibleUnits.cpp:2193: LayoutObject*
firstLetterLayoutObject =
layoutTextFragment->firstLetterPseudoElement()->layoutObject();
On 2016/01/29 at 00:56:09, tkent wrote:
> nullptr-check for firstLetterLayoutObject is removed.  Is it intentional?

Yes, it is intentional. This case should not be happened. If it is happened, we
would like to know the case.
I don't add nullptr-check ASSERT, since following line uses it.

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

https://codereview.chromium.org/1636373002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/VisibleUnits.h:78: CORE_EXPORT
LayoutObject* associatedLayoutObjectOf(const Node&, int offsetInNode);
On 2016/01/29 at 00:56:09, tkent wrote:
> Is this |offset functions on Node|?

Done.

[commit-bot: I haz the power, 2016-01-29 03:45:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1636373002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1636373002/40001

[tkent, 2016-01-29 03:52:16]

lgtm

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

https://codereview.chromium.org/1636373002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/VisibleUnits.cpp:2182: // patter, e.g.
"B\n", or remaining part "abc".
patter -> part?

I don't understand what you mean by "e.g. "B\n", or remaining part "abc"."

[commit-bot: I haz the power, 2016-01-29 03:58:36]

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

[commit-bot: I haz the power, 2016-01-29 03:58:36]

Dry run: 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_...)

[yosin_UTC9, 2016-01-29 05:27:45]

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

https://codereview.chromium.org/1636373002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/VisibleUnits.cpp:2182: // patter, e.g.
"B\n", or remaining part "abc".
On 2016/01/29 at 03:52:16, tkent wrote:
> patter -> part?
> 
> I don't understand what you mean by "e.g. "B\n", or remaining part "abc"."

Add more explanation about first-letter.

[yosin_UTC9, 2016-01-29 05:28:05]

On 2016/01/29 at 05:27:45, Yosi_UTC9 wrote:
>
https://codereview.chromium.org/1636373002/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/editing/VisibleUnits.cpp (right):
> 
>
https://codereview.chromium.org/1636373002/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/editing/VisibleUnits.cpp:2182: // patter, e.g.
"B\n", or remaining part "abc".
> On 2016/01/29 at 03:52:16, tkent wrote:
> > patter -> part?
> > 
> > I don't understand what you mean by "e.g. "B\n", or remaining part "abc"."
> 
> Add more explanation about first-letter.

Committing...

[yosin_UTC9, 2016-01-29 05:28:14]

The CQ bit was checked by yosin@chromium.org

[yosin_UTC9, 2016-01-29 05:28:14]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/1636373002/#ps80001
(title: "2016-01-29T14:21:36")

[commit-bot: I haz the power, 2016-01-29 05:28:39]

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

[commit-bot: I haz the power, 2016-01-29 05:41:51]

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

[commit-bot: I haz the power, 2016-01-29 05:41:52]

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

[yosin_UTC9, 2016-01-29 06:22:58]

The CQ bit was checked by yosin@chromium.org

[yosin_UTC9, 2016-01-29 06:22:58]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/1636373002/#ps100001
(title: "2016-01-29T15:21:03")

[commit-bot: I haz the power, 2016-01-29 06:23:45]

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

[commit-bot: I haz the power, 2016-01-29 07:33:40]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-01-29 07:34:33]

Description was changed from

==========
Make associatedLayoutObjectOf() to handle ::first-letter pseudo-element
correctly

This patch makes |associatedLayoutObjectOf()| to handle all cases of layout tree
shape for ::first-letter pseudo-element.

Before this patch, |associatedLayoutObjectOf()| handled first-letter only |Text|
node incorrectly.

BUG=560689
TEST=webkit_unit_tests --gtest_filter=VisibleUnitsTest.associatedLayoutObjectOf*
TEST=webkit_unit_tests
--gtest_filter=VisibleUnitsTest.mostBackwardCaretPositionFirstLetterSplit
==========

to

==========
Make associatedLayoutObjectOf() to handle ::first-letter pseudo-element
correctly

This patch makes |associatedLayoutObjectOf()| to handle all cases of layout tree
shape for ::first-letter pseudo-element.

Before this patch, |associatedLayoutObjectOf()| handled first-letter only |Text|
node incorrectly.

BUG=560689
TEST=webkit_unit_tests --gtest_filter=VisibleUnitsTest.associatedLayoutObjectOf*
TEST=webkit_unit_tests
--gtest_filter=VisibleUnitsTest.mostBackwardCaretPositionFirstLetterSplit

Committed: https://crrev.com/7c9e1437f3de1149a5b1a9437d927d4e0d7802ba
Cr-Commit-Position: refs/heads/master@{#372299}
==========

[commit-bot: I haz the power, 2016-01-29 07:34:34]

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