New issue
Advanced search Search tips

Issue 771398 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task

Blocked on: View detail
issue 812535
issue 830956
issue 793098

Blocking:
issue 707656



Sign in to add a comment

Remove editing's dependency on legacy inline structure

Project Member Reported by xiaoche...@chromium.org, Oct 3 2017

Issue description

To make editing work with LayoutNG, we should remove it's dependency on the legacy layout tree.

As the first step, we should remove its dependency on the legacy inline structure (InlineBox, InlineTextBox, RootLineBox). Proper high-level APIs should be used instead, for example, the offset mapping API.

A (possibly incomplete) dashboard: https://docs.google.com/spreadsheets/d/1aAYd9VA3Kjyo-TEkGETYZJlg1U3cAfl3qDCKDbzd-OU/edit?usp=sharing
 
Labels: -Type-Bug Type-Task
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2a8d71964a5947c149343db49b20605db49abcae

commit 2a8d71964a5947c149343db49b20605db49abcae
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Wed Oct 04 03:54:09 2017

Make LayoutText::IsRenderedCharacter absorbed by EditCommand::IsRenderedCharacter

LayoutText::IsRenderedCharacter is only used by editing commands. Hence,
this patch makes it absorbed by its only call site in EditCommand, as
a preparation step to stop editing from depending on the legacy line
layout structure.

This patch also removes some TODOs related to this function. These TODOs
originates from an ancient CL [1], but do not make a lot of sense.

[1] crrev.com/289fa98c43250e4e9fc44aad39670122e4046bf6

Bug: 771398,  731540 
Change-Id: I38b85f5a4c8a8adfba0568632337f1e42cfeded5
Reviewed-on: https://chromium-review.googlesource.com/699011
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506297}
[modify] https://crrev.com/2a8d71964a5947c149343db49b20605db49abcae/third_party/WebKit/Source/core/editing/commands/DeleteSelectionCommand.cpp
[modify] https://crrev.com/2a8d71964a5947c149343db49b20605db49abcae/third_party/WebKit/Source/core/editing/commands/EditCommand.cpp
[modify] https://crrev.com/2a8d71964a5947c149343db49b20605db49abcae/third_party/WebKit/Source/core/editing/commands/EditCommand.h
[modify] https://crrev.com/2a8d71964a5947c149343db49b20605db49abcae/third_party/WebKit/Source/core/editing/commands/InsertLineBreakCommand.cpp
[modify] https://crrev.com/2a8d71964a5947c149343db49b20605db49abcae/third_party/WebKit/Source/core/editing/commands/InsertParagraphSeparatorCommand.cpp
[modify] https://crrev.com/2a8d71964a5947c149343db49b20605db49abcae/third_party/WebKit/Source/core/layout/LayoutText.cpp
[modify] https://crrev.com/2a8d71964a5947c149343db49b20605db49abcae/third_party/WebKit/Source/core/layout/LayoutText.h

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5bb682ed35527e9d047fd92021b620f620f993b0

commit 5bb682ed35527e9d047fd92021b620f620f993b0
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Thu Oct 05 02:17:09 2017

Move InlineBoxPosition related code to InlineBoxPosition.h/cpp

Bug: 771398
Change-Id: Ic35d94a75a5bb9d4ef00fc9abdb839c0e26c2c78
Reviewed-on: https://chromium-review.googlesource.com/701414
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506626}
[modify] https://crrev.com/5bb682ed35527e9d047fd92021b620f620f993b0/third_party/WebKit/Source/core/editing/BUILD.gn
[add] https://crrev.com/5bb682ed35527e9d047fd92021b620f620f993b0/third_party/WebKit/Source/core/editing/InlineBoxPosition.cpp
[add] https://crrev.com/5bb682ed35527e9d047fd92021b620f620f993b0/third_party/WebKit/Source/core/editing/InlineBoxPosition.h
[add] https://crrev.com/5bb682ed35527e9d047fd92021b620f620f993b0/third_party/WebKit/Source/core/editing/InlineBoxPositionTest.cpp
[modify] https://crrev.com/5bb682ed35527e9d047fd92021b620f620f993b0/third_party/WebKit/Source/core/editing/RenderedPosition.cpp
[modify] https://crrev.com/5bb682ed35527e9d047fd92021b620f620f993b0/third_party/WebKit/Source/core/editing/SelectionModifier.cpp
[modify] https://crrev.com/5bb682ed35527e9d047fd92021b620f620f993b0/third_party/WebKit/Source/core/editing/SelectionModifierCharacter.cpp
[modify] https://crrev.com/5bb682ed35527e9d047fd92021b620f620f993b0/third_party/WebKit/Source/core/editing/SelectionModifierWord.cpp
[modify] https://crrev.com/5bb682ed35527e9d047fd92021b620f620f993b0/third_party/WebKit/Source/core/editing/VisibleUnits.cpp
[modify] https://crrev.com/5bb682ed35527e9d047fd92021b620f620f993b0/third_party/WebKit/Source/core/editing/VisibleUnits.h
[modify] https://crrev.com/5bb682ed35527e9d047fd92021b620f620f993b0/third_party/WebKit/Source/core/editing/VisibleUnitsLine.cpp
[modify] https://crrev.com/5bb682ed35527e9d047fd92021b620f620f993b0/third_party/WebKit/Source/core/editing/VisibleUnitsTest.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c8e75319a45c01871247c613a51cf9a7314b2505

commit c8e75319a45c01871247c613a51cf9a7314b2505
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Thu Oct 05 05:17:15 2017

Add better documentation to canonicalization-related functions

This patch adds better documentation/comments to these functions:
- IsVisuallyEquivalentCandidateAlgorithm
- CanBeBackwardCaretPosition
- CanBeForwardCaretPosition

So that we have better understanding of them, and can implement
InlineBox-free canonicalization with LayoutNG easier.

NOTRY=TRUE

Bug: 771398
Change-Id: I6b117656658b3db9059156426efbf44ad62979bc
Reviewed-on: https://chromium-review.googlesource.com/701836
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506657}
[modify] https://crrev.com/c8e75319a45c01871247c613a51cf9a7314b2505/third_party/WebKit/Source/core/editing/VisibleUnits.cpp

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/82f006f6844b7f98c3fcbddd9971669994bd2d7f

commit 82f006f6844b7f98c3fcbddd9971669994bd2d7f
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Thu Oct 05 21:15:43 2017

Eliminate RenderedPosition::UncachedInlineBox with Optional

RenderedPosition caches two |InlineBox*|, and uses 1 as the magic value
indicating the uncached value, which is a hacky approach. This patch
changes them to Optional<InlineBox*> to make the caching not hacky.

Bug: 771398
Change-Id: If8bef20755fcc75b04093de6c2211138d336600e
Reviewed-on: https://chromium-review.googlesource.com/702924
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506861}
[modify] https://crrev.com/82f006f6844b7f98c3fcbddd9971669994bd2d7f/third_party/WebKit/Source/core/editing/RenderedPosition.cpp
[modify] https://crrev.com/82f006f6844b7f98c3fcbddd9971669994bd2d7f/third_party/WebKit/Source/core/editing/RenderedPosition.h

Owner: xiaoche...@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5ecb24c91d9efe01aa0317f57a92d36dce0d0aff

commit 5ecb24c91d9efe01aa0317f57a92d36dce0d0aff
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Wed Oct 11 00:30:50 2017

[LayoutNG] Add offset mapping API to skip collapsed characters

This CL adds two functions to the offset mapping APIs to inspect if a given
position is next to collapsed whitespace, and if so, skip (in either direction)
the collapsed whitespaces and return the offset of the last/next non-collapsed
character.

The functions will be widely useful in editing code, for example:
- EditCommand::IsRenderedCharacter
- LayoutText::CaretMin/MaxOffset
...

Bug: 771398
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: Ib804372fd5c145d1969107d3180135253c3bd33b
Reviewed-on: https://chromium-review.googlesource.com/707729
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507839}
[modify] https://crrev.com/5ecb24c91d9efe01aa0317f57a92d36dce0d0aff/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node_offset_mapping_test.cc
[modify] https://crrev.com/5ecb24c91d9efe01aa0317f57a92d36dce0d0aff/third_party/WebKit/Source/core/layout/ng/inline/ng_offset_mapping_result.cc
[modify] https://crrev.com/5ecb24c91d9efe01aa0317f57a92d36dce0d0aff/third_party/WebKit/Source/core/layout/ng/inline/ng_offset_mapping_result.h

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a3bdef6d7bb0650f672e70045d7e39e3a54bc0db

commit a3bdef6d7bb0650f672e70045d7e39e3a54bc0db
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Thu Oct 12 00:21:17 2017

Add LayoutNG variants of three LayoutText methods

This patch adds LayoutNG variants of the following methods on top of the
LayoutNG offset mapping API:
- LayoutText::CaretMinOffset()
- LayoutText::CaretMaxOffset()
- LayoutText::ResolvedTextLength()

Bug: 771398
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: Ic50b52b627838686d9bb6827166077e1485439ba
Reviewed-on: https://chromium-review.googlesource.com/710754
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508190}
[modify] https://crrev.com/a3bdef6d7bb0650f672e70045d7e39e3a54bc0db/third_party/WebKit/Source/core/layout/LayoutText.cpp
[modify] https://crrev.com/a3bdef6d7bb0650f672e70045d7e39e3a54bc0db/third_party/WebKit/Source/core/layout/LayoutText.h
[modify] https://crrev.com/a3bdef6d7bb0650f672e70045d7e39e3a54bc0db/third_party/WebKit/Source/core/layout/LayoutTextTest.cpp

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a977d3a43beb047987340774dbd82d69a5502069

commit a977d3a43beb047987340774dbd82d69a5502069
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Thu Oct 12 05:09:12 2017

[LayoutNG] Add IsNonCollapsedCharacter to offset mapping API

This patch adds a new API to LayoutNG offset mapping so that we can
easily check whether a DOM character is collapsed or not.

Bug: 771398
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I1eb576661eca74034cd388f037784b0fe2bd3543
Reviewed-on: https://chromium-review.googlesource.com/714445
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508259}
[modify] https://crrev.com/a977d3a43beb047987340774dbd82d69a5502069/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node_offset_mapping_test.cc
[modify] https://crrev.com/a977d3a43beb047987340774dbd82d69a5502069/third_party/WebKit/Source/core/layout/ng/inline/ng_offset_mapping_result.cc
[modify] https://crrev.com/a977d3a43beb047987340774dbd82d69a5502069/third_party/WebKit/Source/core/layout/ng/inline/ng_offset_mapping_result.h

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/db9141e8c3155ea841dbeadf1d3fe27c5f19ffbc

commit db9141e8c3155ea841dbeadf1d3fe27c5f19ffbc
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Thu Oct 12 19:57:01 2017

Change some callers of LayoutText::HasTextBoxes() to use LayoutNG alternative

Some callers of LayoutText::HasTextBoxes() do not actually care about text
boxes, but instead, whether the text node is fully collapsed or not.

Hence, this patch introduces LayoutText::HasNonCollapsedText() that
- Checks text boxes when LayoutNG is not enabled
- Checks LayoutNG offset mapping when LayoutNG is enabled
and change those call sites to use HasNonCollapsedText() instead.

Bug: 771398
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: Iada3978209bffe828a70e0e81d3b189589c8a94d
Reviewed-on: https://chromium-review.googlesource.com/710860
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508403}
[modify] https://crrev.com/db9141e8c3155ea841dbeadf1d3fe27c5f19ffbc/third_party/WebKit/Source/core/editing/VisibleUnits.cpp
[modify] https://crrev.com/db9141e8c3155ea841dbeadf1d3fe27c5f19ffbc/third_party/WebKit/Source/core/layout/LayoutText.cpp
[modify] https://crrev.com/db9141e8c3155ea841dbeadf1d3fe27c5f19ffbc/third_party/WebKit/Source/core/layout/LayoutText.h

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e51c0061ce2164cba9eb02c39084d9036739192e

commit e51c0061ce2164cba9eb02c39084d9036739192e
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Thu Oct 12 21:00:12 2017

[LayoutNG] Add NG alternative of EditCommand::IsRenderedCharacter

Bug: 771398
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I987ad066c763a53f3de8a86a7ab4d2b87d8e7bb6
Reviewed-on: https://chromium-review.googlesource.com/715157
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508460}
[modify] https://crrev.com/e51c0061ce2164cba9eb02c39084d9036739192e/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/e51c0061ce2164cba9eb02c39084d9036739192e/third_party/WebKit/Source/core/editing/commands/EditCommand.cpp

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5d729ebb2e2e018acb3165889d5765036743b466

commit 5d729ebb2e2e018acb3165889d5765036743b466
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Fri Oct 13 18:29:26 2017

Use scoped RuntimeEnabledFeature setters in LayoutTextTest

Since RuntimeEnabledFeature settings are not automatically reset
between tests, tests should use scoped setters so that they do not
introduce flakes.

Bug:  769541 , 771398
Change-Id: I98ceb13014f5a1a39e49227d1ecb1b209b673487
Reviewed-on: https://chromium-review.googlesource.com/716241
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508762}
[modify] https://crrev.com/5d729ebb2e2e018acb3165889d5765036743b466/third_party/WebKit/Source/core/layout/LayoutTextTest.cpp
[modify] https://crrev.com/5d729ebb2e2e018acb3165889d5765036743b466/third_party/WebKit/Source/platform/testing/RuntimeEnabledFeaturesTestHelpers.h

Project Member

Comment 13 by bugdroid1@chromium.org, Oct 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/55c4c4db4d339c9a6ce09dd57e77884016cbe723

commit 55c4c4db4d339c9a6ce09dd57e77884016cbe723
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Fri Oct 13 19:46:39 2017

Simplify MostBackwardCaretPosition by using the correct LayoutObject

In CanBeBackwardCaretPosition, which is a subroutine of
MostBackwardCaretPosition, there is a special handling that when the
node has first-letter style, positions at the beginning of the remaining
text layout object is considered as a caret position without further
checking.

This special handling is in fact due to that incorrect layout object
is fetched in MostBackwardCaretPosition: when the position is at the
boundary between first-letter and remaining text, we should inspect the
first-letter layout object instead of remaining text.

This patch removes the special handling by fetching the first-letter
layout object when the position is at boundary, by adding an additional
parameter to AssociatedLayoutObjectOf().

Bug: 771398
Change-Id: I2174053ad0bf8dbb80ec8ac5381e91e688a8a6da
Reviewed-on: https://chromium-review.googlesource.com/716670
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508784}
[modify] https://crrev.com/55c4c4db4d339c9a6ce09dd57e77884016cbe723/third_party/WebKit/Source/core/editing/VisibleUnits.cpp
[modify] https://crrev.com/55c4c4db4d339c9a6ce09dd57e77884016cbe723/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/55c4c4db4d339c9a6ce09dd57e77884016cbe723/third_party/WebKit/Source/core/layout/LayoutObject.h

Project Member

Comment 14 by bugdroid1@chromium.org, Oct 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0289e4ac1388e042be0f2258332ffbb6a01ffa2a

commit 0289e4ac1388e042be0f2258332ffbb6a01ffa2a
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Mon Oct 16 05:33:39 2017

Remove IsVisuallyEmpty() in place of better-defined alternative

HasRenderedNonAnonymousDescendantsWithHeight() and IsVisuallyEmpty()
are pretty much doing the same job, and serve the same purpose of
identifying the visible position in an empty block.

Since HasRenderedNonAnonymousDescendantsWithHeight() has better
semantics, this patch replaces the call of IsVisuallyEmpty() by
HasRenderedNonAnonymousDescendantsWithHeight() for better code health.

Bug: 771398
Change-Id: Iecd3e0cf7ba2ef956a30e61ca7b0d2180ee529b7
Reviewed-on: https://chromium-review.googlesource.com/719355
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508988}
[modify] https://crrev.com/0289e4ac1388e042be0f2258332ffbb6a01ffa2a/third_party/WebKit/Source/core/editing/VisibleUnits.cpp

Project Member

Comment 15 by bugdroid1@chromium.org, Oct 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ae4a9500c53a27205ae32f6c392eb188c6af21f2

commit ae4a9500c53a27205ae32f6c392eb188c6af21f2
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Tue Oct 17 06:20:07 2017

[LayoutNG] Handle ::first-letter in three LayoutText methods

This patch adds ::first-letter handling to the following functions by adding
LayoutTextFragment overrides:
- LayoutText::CaretMinOffset
- LayoutText::CaretMaxOffset
- LayoutText::ResolvedTextLength

Bug: 771398
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I5309e706a5161498b9e909b9d2098a232e328020
Reviewed-on: https://chromium-review.googlesource.com/719410
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509299}
[modify] https://crrev.com/ae4a9500c53a27205ae32f6c392eb188c6af21f2/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/ae4a9500c53a27205ae32f6c392eb188c6af21f2/third_party/WebKit/Source/core/layout/LayoutText.cpp
[modify] https://crrev.com/ae4a9500c53a27205ae32f6c392eb188c6af21f2/third_party/WebKit/Source/core/layout/LayoutText.h
[modify] https://crrev.com/ae4a9500c53a27205ae32f6c392eb188c6af21f2/third_party/WebKit/Source/core/layout/LayoutTextFragment.cpp
[modify] https://crrev.com/ae4a9500c53a27205ae32f6c392eb188c6af21f2/third_party/WebKit/Source/core/layout/LayoutTextFragment.h
[add] https://crrev.com/ae4a9500c53a27205ae32f6c392eb188c6af21f2/third_party/WebKit/Source/core/layout/LayoutTextFragmentTest.cpp

Project Member

Comment 16 by bugdroid1@chromium.org, Oct 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0ec9ade3c6bfe89cbee20aa7d1032cc390edb0df

commit 0ec9ade3c6bfe89cbee20aa7d1032cc390edb0df
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Tue Oct 17 10:15:15 2017

Refactor InRenderedText() with LayoutText::ContainsCaretOffset()

This patch refactors InRenderedText() by moving the work on InlineTextBox to
LayoutText::ContainsCaretOffset(), so that InRenderedText() no longer works
on InlineTextBox directly.

A follow-up patch will introduce a LayoutNG version of
LayoutText::ContainsCaretOffset().

Bug: 771398
Change-Id: If786ff57cf881869632e72e2347741433f6a4a17
Reviewed-on: https://chromium-review.googlesource.com/721629
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509342}
[modify] https://crrev.com/0ec9ade3c6bfe89cbee20aa7d1032cc390edb0df/third_party/WebKit/Source/core/editing/VisibleUnits.cpp
[modify] https://crrev.com/0ec9ade3c6bfe89cbee20aa7d1032cc390edb0df/third_party/WebKit/Source/core/layout/LayoutText.cpp
[modify] https://crrev.com/0ec9ade3c6bfe89cbee20aa7d1032cc390edb0df/third_party/WebKit/Source/core/layout/LayoutText.h
[modify] https://crrev.com/0ec9ade3c6bfe89cbee20aa7d1032cc390edb0df/third_party/WebKit/Source/core/layout/LayoutTextTest.cpp

Project Member

Comment 17 by bugdroid1@chromium.org, Oct 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8a7b38d24b19b97b51f7a3ba69e85d8c74d17cf5

commit 8a7b38d24b19b97b51f7a3ba69e85d8c74d17cf5
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Wed Oct 18 23:08:43 2017

[LayoutNG] Add IsAfterNonCollapsedCharacter(node, offset) API

This patch adds a new API to LayoutNG offset mapping, so that we can
easily check if a given DOM offset is right after a non-collapsed
character.

This patch is a preparation for adding an NG version of
LayoutText::ContainsCaretOffset().

Bug: 771398
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I93e8a89089661c6f17da6a7a847f75914cb36f75
Reviewed-on: https://chromium-review.googlesource.com/724230
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509915}
[modify] https://crrev.com/8a7b38d24b19b97b51f7a3ba69e85d8c74d17cf5/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node_offset_mapping_test.cc
[modify] https://crrev.com/8a7b38d24b19b97b51f7a3ba69e85d8c74d17cf5/third_party/WebKit/Source/core/layout/ng/inline/ng_offset_mapping_result.cc
[modify] https://crrev.com/8a7b38d24b19b97b51f7a3ba69e85d8c74d17cf5/third_party/WebKit/Source/core/layout/ng/inline/ng_offset_mapping_result.h

Comment 18 by e...@chromium.org, Oct 18 2017

Labels: -Pri-1 Pri-2
Project Member

Comment 19 by bugdroid1@chromium.org, Oct 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1469545083768159caf6d51e5601cc3992eeedf6

commit 1469545083768159caf6d51e5601cc3992eeedf6
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Wed Oct 18 23:38:51 2017

[LayoutNG] Reference the canonical text in offset mapping

This patch adds a reference to the canonical text in offset mapping, so
that we don't need to make a detour to NGInlineNode when working on both
offset mapping and canonical text.

This patch is a preparation for adding NG version of
LayoutText::ContainsCaretOffset().

Bug: 771398
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I8cf035c44ab90452819d92b91e7853b848781a41
Reviewed-on: https://chromium-review.googlesource.com/724286
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509930}
[modify] https://crrev.com/1469545083768159caf6d51e5601cc3992eeedf6/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node.cc
[modify] https://crrev.com/1469545083768159caf6d51e5601cc3992eeedf6/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node_offset_mapping_test.cc
[modify] https://crrev.com/1469545083768159caf6d51e5601cc3992eeedf6/third_party/WebKit/Source/core/layout/ng/inline/ng_offset_mapping_builder.cc
[modify] https://crrev.com/1469545083768159caf6d51e5601cc3992eeedf6/third_party/WebKit/Source/core/layout/ng/inline/ng_offset_mapping_builder.h
[modify] https://crrev.com/1469545083768159caf6d51e5601cc3992eeedf6/third_party/WebKit/Source/core/layout/ng/inline/ng_offset_mapping_result.cc
[modify] https://crrev.com/1469545083768159caf6d51e5601cc3992eeedf6/third_party/WebKit/Source/core/layout/ng/inline/ng_offset_mapping_result.h

Project Member

Comment 20 by bugdroid1@chromium.org, Oct 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/63d12e3af49b452a4ed2e0e01196ba6a249af9b2

commit 63d12e3af49b452a4ed2e0e01196ba6a249af9b2
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Thu Oct 19 17:54:56 2017

[LayoutNG] Add NG alternatives of LayoutText::ContainsCaretOffset

This patch adds LayoutNG alternatives of LayoutText::ContainsCaretOffset for
both "common" LayoutText and ::first-letter LayoutTextFragments, with an
extensive set of unit tests.

Bug: 771398
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I59b17046c271bf8c5c1fb4fdf7421b37a3183073
Reviewed-on: https://chromium-review.googlesource.com/724262
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510130}
[modify] https://crrev.com/63d12e3af49b452a4ed2e0e01196ba6a249af9b2/third_party/WebKit/Source/core/layout/LayoutText.cpp
[modify] https://crrev.com/63d12e3af49b452a4ed2e0e01196ba6a249af9b2/third_party/WebKit/Source/core/layout/LayoutText.h
[modify] https://crrev.com/63d12e3af49b452a4ed2e0e01196ba6a249af9b2/third_party/WebKit/Source/core/layout/LayoutTextFragment.cpp
[modify] https://crrev.com/63d12e3af49b452a4ed2e0e01196ba6a249af9b2/third_party/WebKit/Source/core/layout/LayoutTextFragment.h
[modify] https://crrev.com/63d12e3af49b452a4ed2e0e01196ba6a249af9b2/third_party/WebKit/Source/core/layout/LayoutTextFragmentTest.cpp
[modify] https://crrev.com/63d12e3af49b452a4ed2e0e01196ba6a249af9b2/third_party/WebKit/Source/core/layout/LayoutTextTest.cpp
[modify] https://crrev.com/63d12e3af49b452a4ed2e0e01196ba6a249af9b2/third_party/WebKit/Source/core/layout/ng/inline/ng_offset_mapping_result.cc
[modify] https://crrev.com/63d12e3af49b452a4ed2e0e01196ba6a249af9b2/third_party/WebKit/Source/core/layout/ng/inline/ng_offset_mapping_result.h

Project Member

Comment 21 by bugdroid1@chromium.org, Oct 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/83f1a4eda0e845f41da434d3d4516db61dca6e98

commit 83f1a4eda0e845f41da434d3d4516db61dca6e98
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Fri Oct 20 09:30:52 2017

Stop checking InlineTextBox in HasRenderedNonAnonymousDescendantsWithHeight

HasRenderedNonAnonymousDescendantsWithHeight is a subroutine called
during position canonicalization to detect caret positions in empty
blocks. Its current dependency on legacy inline structure blocks the
NG conversion of canonicalization.

This patch changes the above mentioned function to decouple it from
legacy InlineTextBox, to make editing work better with LayoutNG.

Although the function still relies on InlineBox after the patch
(via LayoutInline::LinesBoundingBox), at least we get an NG version of
canonicalization that works in most cases, i.e., when there are text
nodes. In other words, this patch allows us to run canonicalization in
the NG world in most cases.

Note: there is no need to enable layout_ng bot in this patch because
the bot doesn't have LayoutNGPaintFragments enabled, and hence, the patch
has no effect there.

Bug: 771398
Change-Id: Idbd595b0a2776dd843840a1ab903f5de7d9d5d59
Reviewed-on: https://chromium-review.googlesource.com/729040
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510387}
[modify] https://crrev.com/83f1a4eda0e845f41da434d3d4516db61dca6e98/third_party/WebKit/Source/core/editing/VisibleUnits.cpp

Project Member

Comment 22 by bugdroid1@chromium.org, Oct 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7ba6e953274608d945b9efc904b5ee2a64fb7c13

commit 7ba6e953274608d945b9efc904b5ee2a64fb7c13
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Fri Oct 20 16:45:52 2017

[LayoutNG] Add NG alternatives of CanBe{Forward,Backward}CaretPosition

This patch adds InlineBox-free versions of the above two functions by:

1. Moving them as LayoutText::IsBefore/AfterNonCollapsedCharacter for
more explanatory names and layering (since they are working on layout)

2. Adding NG versions of LayoutText::IsBefore/AfterNonCollapsedCharacter

An extensive set of unit tests is also added, which includes some cases
that legacy fails to handle but NG handles well.

Bug: 771398
Change-Id: Ibe0b404054a339568f2e588adda08cf9a1e37723
Reviewed-on: https://chromium-review.googlesource.com/729329
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510465}
[modify] https://crrev.com/7ba6e953274608d945b9efc904b5ee2a64fb7c13/third_party/WebKit/Source/core/editing/VisibleUnits.cpp
[modify] https://crrev.com/7ba6e953274608d945b9efc904b5ee2a64fb7c13/third_party/WebKit/Source/core/layout/LayoutText.cpp
[modify] https://crrev.com/7ba6e953274608d945b9efc904b5ee2a64fb7c13/third_party/WebKit/Source/core/layout/LayoutText.h
[modify] https://crrev.com/7ba6e953274608d945b9efc904b5ee2a64fb7c13/third_party/WebKit/Source/core/layout/LayoutTextFragment.cpp
[modify] https://crrev.com/7ba6e953274608d945b9efc904b5ee2a64fb7c13/third_party/WebKit/Source/core/layout/LayoutTextFragment.h
[modify] https://crrev.com/7ba6e953274608d945b9efc904b5ee2a64fb7c13/third_party/WebKit/Source/core/layout/LayoutTextFragmentTest.cpp
[modify] https://crrev.com/7ba6e953274608d945b9efc904b5ee2a64fb7c13/third_party/WebKit/Source/core/layout/LayoutTextTest.cpp

Project Member

Comment 23 by bugdroid1@chromium.org, Nov 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0989d3043f2b020a7b1d4126d0915d7771a1613c

commit 0989d3043f2b020a7b1d4126d0915d7771a1613c
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Tue Nov 14 22:26:58 2017

[LayoutNG] Add DCHECK of CanUseInlineBox() in TextControlElement.cpp

TextControlElement::ValueWithHardLineBreaks() uses legacy inline
structures, which will be removed when LayoutNG is enabled. This patch
adds a DCHECK to ease the tracking of the conversion of existing usage of
legacy inline structures.

Bug: 590199, 771398
Change-Id: I1b4a7b90492ed1bb308f4d14cc0d578953c7f88f
Reviewed-on: https://chromium-review.googlesource.com/769627
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516461}
[modify] https://crrev.com/0989d3043f2b020a7b1d4126d0915d7771a1613c/third_party/WebKit/Source/core/html/forms/TextControlElement.cpp

Project Member

Comment 24 by bugdroid1@chromium.org, Nov 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/923f4e28e1f4a53524cc34dbfa659b2b9d24d159

commit 923f4e28e1f4a53524cc34dbfa659b2b9d24d159
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Fri Nov 17 15:56:46 2017

Slight reorganization of ComputeInlineBoxPosition

This patch slightly reorganizes the above mentioned function to make the
logic clearer that, the function has three major branches dependig on
the type of |layout_object|: text, block flow with children, or atomic
inline.

Detail: Adding |IsAtomicInlineLevel()| check to the last part of the
function doesn't introduce any behavior change. This is because that
|LayoutBox::InlineBoxWrapper()| returns non-null result only when the
layout object is an atomic inline.

Bug: 771398
Change-Id: Id61830a9ad11de40c5ae7456e3e13b6a4094b1a9
Reviewed-on: https://chromium-review.googlesource.com/775659
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517410}
[modify] https://crrev.com/923f4e28e1f4a53524cc34dbfa659b2b9d24d159/third_party/WebKit/Source/core/editing/InlineBoxPosition.cpp

Project Member

Comment 25 by bugdroid1@chromium.org, Nov 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f8fa76d69af93f39a58b0698fc761bf163fc3bdb

commit f8fa76d69af93f39a58b0698fc761bf163fc3bdb
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Fri Nov 17 20:11:04 2017

Minor revision to ContainerNode::GetUpperLeftCorner

Bug: 771398
Change-Id: Ibe4053be6d8b35e351e71b2e7be28e47ef41d472
Reviewed-on: https://chromium-review.googlesource.com/776406
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517529}
[modify] https://crrev.com/f8fa76d69af93f39a58b0698fc761bf163fc3bdb/third_party/WebKit/Source/core/dom/ContainerNode.cpp

Project Member

Comment 26 by bugdroid1@chromium.org, Nov 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/965bbf35c6645b47427b236cf49d696faf7d306a

commit 965bbf35c6645b47427b236cf49d696faf7d306a
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Mon Nov 20 06:42:06 2017

Further reorganize code flow in ComputeInlineBoxPosition

This patch moves the atomic inline handling code before the block flow
handling code, so that there is no longer fall through from block flow
to atomic inline.

Bug: 771398
Change-Id: Idb4013e5616a2e81a13f4c6829c3afb2b86faa68
Reviewed-on: https://chromium-review.googlesource.com/775550
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517766}
[modify] https://crrev.com/965bbf35c6645b47427b236cf49d696faf7d306a/third_party/WebKit/Source/core/editing/InlineBoxPosition.cpp

Project Member

Comment 27 by bugdroid1@chromium.org, Nov 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/53c5cda53e2e275d4f3718361bcc0a8fa1b0d653

commit 53c5cda53e2e275d4f3718361bcc0a8fa1b0d653
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Mon Nov 20 19:59:56 2017

Introduce ComputeInlineBoxPositionForAtomicInline

This patch wraps code in ComputeInlineBoxPosition into a new
function, so that the code flow becomes clearer.

Bug: 771398
Change-Id: I6b702f736fe7e21b454aa207180aebadeba63699
Reviewed-on: https://chromium-review.googlesource.com/777702
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517895}
[modify] https://crrev.com/53c5cda53e2e275d4f3718361bcc0a8fa1b0d653/third_party/WebKit/Source/core/editing/InlineBoxPosition.cpp

Project Member

Comment 28 by bugdroid1@chromium.org, Nov 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/71a2a05c393c3f92c24f419ca95fd40eb1d95bd5

commit 71a2a05c393c3f92c24f419ca95fd40eb1d95bd5
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Mon Nov 20 22:48:39 2017

Introduce ComputeInlineBoxPositionForBlockFlow

This patch wraps code in ComputeInlineBoxPosition into a new
function, so that the code flow becomes clearer.

Bug: 771398
Change-Id: Ib61108cf0603a60a97583d620bc3e1127860b122
Reviewed-on: https://chromium-review.googlesource.com/778194
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517962}
[modify] https://crrev.com/71a2a05c393c3f92c24f419ca95fd40eb1d95bd5/third_party/WebKit/Source/core/editing/InlineBoxPosition.cpp

Project Member

Comment 29 by bugdroid1@chromium.org, Nov 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/12aa0c6679bd58808cd87106e2bd41cba9912858

commit 12aa0c6679bd58808cd87106e2bd41cba9912858
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Tue Nov 21 04:57:08 2017

Final preparation for adding DCHECK(CanUseInlineBox) in ComputeInlineBoxPosition()

This patch further refactors ComputeInlineBoxPositionForAtomicInline(),
so that it is now straightforward to add DCHECK(CanUseInlineBox) into
the code to trach usage of legacy inline boxes.

Bug: 771398
Change-Id: I3a3e839ebbc489cd2fa251259fe94921bf25de15
Reviewed-on: https://chromium-review.googlesource.com/780365
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518123}
[modify] https://crrev.com/12aa0c6679bd58808cd87106e2bd41cba9912858/third_party/WebKit/Source/core/editing/InlineBoxPosition.cpp

Project Member

Comment 30 by bugdroid1@chromium.org, Nov 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1f1cab54da2c7eca322543b7f61710fc5f1e3b15

commit 1f1cab54da2c7eca322543b7f61710fc5f1e3b15
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Wed Nov 22 21:08:09 2017

Revert "Final preparation for adding DCHECK(CanUseInlineBox) in ComputeInlineBoxPosition()"

This reverts commit 12aa0c6679bd58808cd87106e2bd41cba9912858.

Reason for revert: causing  crbug.com/787764 

Original change's description:
> Final preparation for adding DCHECK(CanUseInlineBox) in ComputeInlineBoxPosition()
> 
> This patch further refactors ComputeInlineBoxPositionForAtomicInline(),
> so that it is now straightforward to add DCHECK(CanUseInlineBox) into
> the code to trach usage of legacy inline boxes.
> 
> Bug: 771398
> Change-Id: I3a3e839ebbc489cd2fa251259fe94921bf25de15
> Reviewed-on: https://chromium-review.googlesource.com/780365
> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#518123}

TBR=yosin@chromium.org,yoichio@chromium.org,kojii@chromium.org,xiaochengh@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 771398,  787764 
Change-Id: I86b044132e4ed5472641e9e3674bdae71a9455e5
Reviewed-on: https://chromium-review.googlesource.com/786451
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518744}
[modify] https://crrev.com/1f1cab54da2c7eca322543b7f61710fc5f1e3b15/third_party/WebKit/Source/core/editing/InlineBoxPosition.cpp

Project Member

Comment 31 by bugdroid1@chromium.org, Nov 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3ba0b497029615dfb029519428244f82b7ad8914

commit 3ba0b497029615dfb029519428244f82b7ad8914
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Wed Nov 22 23:17:00 2017

Reland "Final preparation for adding DCHECK(CanUseInlineBox) in ComputeInlineBoxPosition()"

This is a reland of 12aa0c6679bd58808cd87106e2bd41cba9912858
Original change's description:
> Final preparation for adding DCHECK(CanUseInlineBox) in ComputeInlineBoxPosition()
>
> This patch further refactors ComputeInlineBoxPositionForAtomicInline(),
> so that it is now straightforward to add DCHECK(CanUseInlineBox) into
> the code to trach usage of legacy inline boxes.
>
> Bug: 771398
> Change-Id: I3a3e839ebbc489cd2fa251259fe94921bf25de15
> Reviewed-on: https://chromium-review.googlesource.com/780365
> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#518123}

Tbr: yosin@chromium.org, yoichio@chromium.org, kojii@chromium.org
Bug: 771398
Change-Id: I997d6718d4eb0d88f02e667215fc4299448040b3
Reviewed-on: https://chromium-review.googlesource.com/785971
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518793}
[modify] https://crrev.com/3ba0b497029615dfb029519428244f82b7ad8914/third_party/WebKit/Source/core/editing/InlineBoxPosition.cpp

Project Member

Comment 32 by bugdroid1@chromium.org, Nov 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/190f5fd55c81111ec0dcb18298e54c5ac02e422d

commit 190f5fd55c81111ec0dcb18298e54c5ac02e422d
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Wed Nov 29 07:20:18 2017

Simplify argument list of ComputeInlineBoxPosition

All overloads of ComputeInlineBoxPosition take position and affinity.
This patch changes them to take PositionWithAffinity instead so that
the argument list is simpler, and follow-up refactoring is easier.

Bug: 771398
Change-Id: I3eca1a2a4a6d86b801c1805182bcebe5098ba8d6
Reviewed-on: https://chromium-review.googlesource.com/795363
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520034}
[modify] https://crrev.com/190f5fd55c81111ec0dcb18298e54c5ac02e422d/third_party/WebKit/Source/core/editing/InlineBoxPosition.cpp
[modify] https://crrev.com/190f5fd55c81111ec0dcb18298e54c5ac02e422d/third_party/WebKit/Source/core/editing/InlineBoxPosition.h
[modify] https://crrev.com/190f5fd55c81111ec0dcb18298e54c5ac02e422d/third_party/WebKit/Source/core/editing/InlineBoxPositionTest.cpp
[modify] https://crrev.com/190f5fd55c81111ec0dcb18298e54c5ac02e422d/third_party/WebKit/Source/core/editing/RenderedPosition.cpp
[modify] https://crrev.com/190f5fd55c81111ec0dcb18298e54c5ac02e422d/third_party/WebKit/Source/core/editing/SelectionModifierCharacter.cpp
[modify] https://crrev.com/190f5fd55c81111ec0dcb18298e54c5ac02e422d/third_party/WebKit/Source/core/editing/SelectionModifierWord.cpp
[modify] https://crrev.com/190f5fd55c81111ec0dcb18298e54c5ac02e422d/third_party/WebKit/Source/core/editing/VisibleUnits.cpp

Project Member

Comment 33 by bugdroid1@chromium.org, Nov 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c82891caf839fa0d24c006af432ac5801dd12f79

commit c82891caf839fa0d24c006af432ac5801dd12f79
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Wed Nov 29 16:48:12 2017

Break ComputeInlineBoxPosition() for LayoutNG conversion

This patch refactors ComputeInlineBoxPosition() by breaking it into:
- ComputeInlineAdjustedPosition for adjusting a position into text or
  atomic inline anchored
- ComputeInlineBoxPositionForInlineAdjustedPosition, which takes a
  position returned by the previous function and computes an
  InlineBoxPosition from it

This is the first patch to convert callers of ComputeInlineBoxPosition
into NG versions. The full plan is at: https://goo.gl/6WtiUm

Bug: 771398
Change-Id: Ifb324939e940afb3f2afa490af4b9c5ae5bd1731
Reviewed-on: https://chromium-review.googlesource.com/786784
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520142}
[modify] https://crrev.com/c82891caf839fa0d24c006af432ac5801dd12f79/third_party/WebKit/Source/core/editing/InlineBoxPosition.cpp

Project Member

Comment 34 by bugdroid1@chromium.org, Dec 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7f1f6e6a7ce0b080c0ef113d4966ef50f609f3a5

commit 7f1f6e6a7ce0b080c0ef113d4966ef50f609f3a5
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Mon Dec 04 05:40:56 2017

Expose subroutines of ComputeInlinePosition

This patch exposes two subroutines of ComputeInlineBoxPosition:
- ComputeInlineAdjustedPosition
- ComputeInlineBoxPositionForInlineAdjustedPosition

After this patch, we can start converting callers of
ComputeInlineBoxPosition with the following pattern:

DoSomeBusiness(position) {
  adjusted = ComputeInlineAdjustedPosition(position);
  if (/* adjusted is laid out with LayoutNG */)
    return NGAlternativeImplementation(adjusted);
  legacy_box_position =
      ComputeInlineBoxPositionForInlineAdjustedPosition(adjusted);
  DoLegacyBusiness(legacy_box_position);
}

Bug: 771398
Change-Id: Iea01925e7ef986c5b00dd5b6e7172f7065c8df9e
Reviewed-on: https://chromium-review.googlesource.com/792453
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521274}
[modify] https://crrev.com/7f1f6e6a7ce0b080c0ef113d4966ef50f609f3a5/third_party/WebKit/Source/core/editing/InlineBoxPosition.cpp
[modify] https://crrev.com/7f1f6e6a7ce0b080c0ef113d4966ef50f609f3a5/third_party/WebKit/Source/core/editing/InlineBoxPosition.h

Blockedon: 793098
Project Member

Comment 36 by bugdroid1@chromium.org, Dec 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4e3f9a3296f0e0cae45f62ee84678b27473797ce

commit 4e3f9a3296f0e0cae45f62ee84678b27473797ce
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Fri Dec 08 05:41:29 2017

Break calls of ComputeInlineBoxPosition in VisibleUnits.cpp

This patch breaks the two calls of ComputeInlineBoxPosition() in
VisibleUnits.cpp into ComputeInlineAdjustedPosition() +
ComputeInlineBoxPositionForInlineAdjustedPosition().

This is a preparation patch for plugging in NG-alternative
implementation between the two functions.

Bug: 771398
Change-Id: I662067dbfc26fb5c7443d712928aef59a6b0e29b
Reviewed-on: https://chromium-review.googlesource.com/812144
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522719}
[modify] https://crrev.com/4e3f9a3296f0e0cae45f62ee84678b27473797ce/third_party/WebKit/Source/core/editing/VisibleUnits.cpp

Project Member

Comment 37 by bugdroid1@chromium.org, Jan 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8decaf9f1be8805e4be6526208c139e33a2940a7

commit 8decaf9f1be8805e4be6526208c139e33a2940a7
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Tue Jan 09 01:30:44 2018

[LayoutNG] Introduce NGPhysicalTextFragment::IsAnonymousText

This patch introduce the function to tell whether an NGPhysicalTextFragment
is due to generated text (from, e.g., a pseudo element) or DOM text node.

This patch is split off from crrev.com/c/803015

Bug: 771398
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I69d053cf50493b3d1dd6e605d7bb68b90056fe98
Reviewed-on: https://chromium-review.googlesource.com/854728
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527855}
[modify] https://crrev.com/8decaf9f1be8805e4be6526208c139e33a2940a7/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/8decaf9f1be8805e4be6526208c139e33a2940a7/third_party/WebKit/Source/core/layout/ng/inline/ng_physical_text_fragment.cc
[modify] https://crrev.com/8decaf9f1be8805e4be6526208c139e33a2940a7/third_party/WebKit/Source/core/layout/ng/inline/ng_physical_text_fragment.h
[add] https://crrev.com/8decaf9f1be8805e4be6526208c139e33a2940a7/third_party/WebKit/Source/core/layout/ng/inline/ng_physical_text_fragment_test.cc

Project Member

Comment 38 by bugdroid1@chromium.org, Jan 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cb8da9b440961324ec7832129705048218262e51

commit cb8da9b440961324ec7832129705048218262e51
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Tue Jan 09 16:02:31 2018

Record current behavior of LocalCaretRectOfPosition() with unit tests

This patch records the current behavior with unit tests, to help
the implementation of the LayoutNG version of LocalCaretRect.

Bug: 771398
Change-Id: I48a95e185a1379712dfa3248ba8abfd47f9cf1d7
Reviewed-on: https://chromium-review.googlesource.com/855316
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528007}
[modify] https://crrev.com/cb8da9b440961324ec7832129705048218262e51/third_party/WebKit/Source/core/editing/VisibleUnitsTest.cpp

Project Member

Comment 39 by bugdroid1@chromium.org, Jan 10 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e65d6289cd28260dcd05078ffd49ebdd8e8dbf77

commit e65d6289cd28260dcd05078ffd49ebdd8e8dbf77
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Wed Jan 10 22:04:44 2018

Move LocalCaretRect-related code to LocalCaretRect.{h,cpp}

This patch follows from the review discussion in crrev.com/c/803015 to
ease the introduction of NG versions of LocalCaretRect computation.

Bug: 771398
Change-Id: I33b7227479d2521cb9b02059490031d4413d08aa
Reviewed-on: https://chromium-review.googlesource.com/857900
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528440}
[modify] https://crrev.com/e65d6289cd28260dcd05078ffd49ebdd8e8dbf77/third_party/WebKit/Source/core/editing/BUILD.gn
[modify] https://crrev.com/e65d6289cd28260dcd05078ffd49ebdd8e8dbf77/third_party/WebKit/Source/core/editing/CaretDisplayItemClient.cpp
[add] https://crrev.com/e65d6289cd28260dcd05078ffd49ebdd8e8dbf77/third_party/WebKit/Source/core/editing/LocalCaretRect.cpp
[add] https://crrev.com/e65d6289cd28260dcd05078ffd49ebdd8e8dbf77/third_party/WebKit/Source/core/editing/LocalCaretRect.h
[add] https://crrev.com/e65d6289cd28260dcd05078ffd49ebdd8e8dbf77/third_party/WebKit/Source/core/editing/LocalCaretRectTest.cpp
[modify] https://crrev.com/e65d6289cd28260dcd05078ffd49ebdd8e8dbf77/third_party/WebKit/Source/core/editing/SelectionModifier.cpp
[modify] https://crrev.com/e65d6289cd28260dcd05078ffd49ebdd8e8dbf77/third_party/WebKit/Source/core/editing/VisibleUnits.cpp
[modify] https://crrev.com/e65d6289cd28260dcd05078ffd49ebdd8e8dbf77/third_party/WebKit/Source/core/editing/VisibleUnits.h
[modify] https://crrev.com/e65d6289cd28260dcd05078ffd49ebdd8e8dbf77/third_party/WebKit/Source/core/editing/VisibleUnitsTest.cpp

Project Member

Comment 40 by bugdroid1@chromium.org, Jan 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eac28c5dbcd529536aae75575e33204fb6935cf4

commit eac28c5dbcd529536aae75575e33204fb6935cf4
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Tue Jan 23 07:57:41 2018

[LayoutNG] Introduce NGCaretPosition and its computation

This patch introduces a new struct, NGCaretPosition, to specify a
position of possible caret placement relative to an NGPhysicalFragment.

The patch also introduces computation of NGCaretPosition in an
inline formatting context, given a text content offset./

A follow-up patch (crrev.com/c/843148) will utilize NGCaretPosition
to compute LocalCaretRect from DOM positions.

Bug: 771398
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: If437191d2def9eee33c0da79a74c27e796d98234
Reviewed-on: https://chromium-review.googlesource.com/803015
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531177}
[modify] https://crrev.com/eac28c5dbcd529536aae75575e33204fb6935cf4/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/eac28c5dbcd529536aae75575e33204fb6935cf4/third_party/WebKit/Source/core/layout/BUILD.gn
[add] https://crrev.com/eac28c5dbcd529536aae75575e33204fb6935cf4/third_party/WebKit/Source/core/layout/ng/inline/ng_caret_rect.cc
[add] https://crrev.com/eac28c5dbcd529536aae75575e33204fb6935cf4/third_party/WebKit/Source/core/layout/ng/inline/ng_caret_rect.h
[add] https://crrev.com/eac28c5dbcd529536aae75575e33204fb6935cf4/third_party/WebKit/Source/core/layout/ng/inline/ng_caret_rect_test.cc

Project Member

Comment 41 by bugdroid1@chromium.org, Jan 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/06cc5abb97d8333ac2f94b890b483818af9205aa

commit 06cc5abb97d8333ac2f94b890b483818af9205aa
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Wed Jan 24 22:08:49 2018

[LayoutNG] Introduce NGPhysicalLineBoxFragment::First/LastLogicalLeaf()

This patch introduces the two helper functions as a preparation for
crrev.com/c/882292, where we need to check if an inline fragment is the
logically first/last leaf in its line to resolve caret position around
line wrap.

Bug: 771398
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I905ddc8923417da7178f8962db9c01e6fdf24754
Reviewed-on: https://chromium-review.googlesource.com/884381
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531703}
[modify] https://crrev.com/06cc5abb97d8333ac2f94b890b483818af9205aa/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/06cc5abb97d8333ac2f94b890b483818af9205aa/third_party/WebKit/Source/core/layout/ng/inline/ng_physical_line_box_fragment.cc
[modify] https://crrev.com/06cc5abb97d8333ac2f94b890b483818af9205aa/third_party/WebKit/Source/core/layout/ng/inline/ng_physical_line_box_fragment.h
[add] https://crrev.com/06cc5abb97d8333ac2f94b890b483818af9205aa/third_party/WebKit/Source/core/layout/ng/inline/ng_physical_line_box_fragment_test.cc

Project Member

Comment 42 by bugdroid1@chromium.org, Jan 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/200a0427134504fb30581430bcf16c40f56489bc

commit 200a0427134504fb30581430bcf16c40f56489bc
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Wed Jan 24 22:27:42 2018

[LayoutNG] Fix caret position resolution for rtl and deep descendants

The existing implementation assumes logical fragment ordering in line
boxes, which is incorrect for rtl text direction. It also assumes "flat"
structure in line boxes, which is incorrect when we have other inline
container fragments, e.g., SPAN with border.

This patch fixe the issues by using
NGPhysicalLineBox::First/LastLogicalLeaf instead of hacking into the
child list of the line box.

Bug: 771398
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: Iaa3c5f2d3812929c58d80c15e067fe6e1f60f314
Reviewed-on: https://chromium-review.googlesource.com/882292
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531717}
[modify] https://crrev.com/200a0427134504fb30581430bcf16c40f56489bc/third_party/WebKit/Source/core/layout/ng/inline/ng_caret_rect.cc
[modify] https://crrev.com/200a0427134504fb30581430bcf16c40f56489bc/third_party/WebKit/Source/core/layout/ng/inline/ng_caret_rect_test.cc

Project Member

Comment 43 by bugdroid1@chromium.org, Jan 26 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7b055e632b4b11cdb7a19dec6bffc3b2dbb62fbc

commit 7b055e632b4b11cdb7a19dec6bffc3b2dbb62fbc
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Fri Jan 26 22:08:54 2018

Add more tests cases to LocalCaretRectTest

This patch adds test cases about vertical text and overflowing or
underflowing text to LocalCaretRectTest, to help the implementation
of the NG version of LocalCaretRectOfPosition().

Bug: 771398
Change-Id: I21a6a07cf1246b9dd867e7b5a4fe44aa101a2aff
Reviewed-on: https://chromium-review.googlesource.com/887918
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532072}
[modify] https://crrev.com/7b055e632b4b11cdb7a19dec6bffc3b2dbb62fbc/third_party/WebKit/Source/core/editing/LocalCaretRectTest.cpp

Project Member

Comment 44 by bugdroid1@chromium.org, Jan 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d36a2daa8159e19246738d4c2cb0587316dce05

commit 2d36a2daa8159e19246738d4c2cb0587316dce05
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Mon Jan 29 21:58:24 2018

Refactor RenderedPosition::AbsoluteRect with LocalCaretRectOfPosition()

This patch refactors the function so that it no longer requires a
RenderedPosition instance, by making it call LocalCaretRectOfPosition()
instead of calling LayoutObject::LocalCaretRect() directly.

This also helps converting RenderedPosition::AbsoluteRect to the NG
code path.

Bug: 771398
Change-Id: I9eb61a68cad265769e35e206fd5d230414f83680
Reviewed-on: https://chromium-review.googlesource.com/889643
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532611}
[modify] https://crrev.com/2d36a2daa8159e19246738d4c2cb0587316dce05/third_party/WebKit/Source/core/editing/Editor.cpp
[modify] https://crrev.com/2d36a2daa8159e19246738d4c2cb0587316dce05/third_party/WebKit/Source/core/editing/LocalCaretRect.cpp
[modify] https://crrev.com/2d36a2daa8159e19246738d4c2cb0587316dce05/third_party/WebKit/Source/core/editing/LocalCaretRect.h
[modify] https://crrev.com/2d36a2daa8159e19246738d4c2cb0587316dce05/third_party/WebKit/Source/core/editing/RenderedPosition.cpp
[modify] https://crrev.com/2d36a2daa8159e19246738d4c2cb0587316dce05/third_party/WebKit/Source/core/editing/RenderedPosition.h

Project Member

Comment 45 by bugdroid1@chromium.org, Feb 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/63dbc4bee3a544a27a61e6682e1f1f449d4f657a

commit 63dbc4bee3a544a27a61e6682e1f1f449d4f657a
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Thu Feb 01 23:54:24 2018

[LayoutNG] Implement LocalCaretRectOfPosition() in LayoutNG

This patch finishes the NG implementation of LocalCaretRectOfPosition()
by filling the gap between NGCaretPosition and LocalCaretRect.

The implementation is a brand new one, not a conversion of the existing
implementations in LayoutText/LayoutBox::LocalCaretRect(), unfortunately,
for two reasons:

1. The existing implementations are too highly coupled to the legacy
InlineBox, such that a conversion with, e.g., templates, results in the
shattered pieces of code, making the code base even more hacky and harder
to understand.

2. The existing implementation contains some computation that seems
unnecessarily complicated (e.g., handling of TextAlign in LayoutText), which
seems replaceable by cleaner logic in the new implementation. However, it
might be risky to remove them from the legacy implementation.

Bug: 771398
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_layout_ng;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ica7ce378e028dffb47407fc490e431d769b66c6a
Reviewed-on: https://chromium-review.googlesource.com/843148
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533855}
[modify] https://crrev.com/63dbc4bee3a544a27a61e6682e1f1f449d4f657a/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/63dbc4bee3a544a27a61e6682e1f1f449d4f657a/third_party/WebKit/Source/core/editing/LocalCaretRect.cpp
[modify] https://crrev.com/63dbc4bee3a544a27a61e6682e1f1f449d4f657a/third_party/WebKit/Source/core/editing/LocalCaretRectTest.cpp
[modify] https://crrev.com/63dbc4bee3a544a27a61e6682e1f1f449d4f657a/third_party/WebKit/Source/core/editing/NGFlatTreeShorthands.cpp
[modify] https://crrev.com/63dbc4bee3a544a27a61e6682e1f1f449d4f657a/third_party/WebKit/Source/core/editing/NGFlatTreeShorthands.h
[modify] https://crrev.com/63dbc4bee3a544a27a61e6682e1f1f449d4f657a/third_party/WebKit/Source/core/layout/ng/inline/ng_caret_rect.cc
[modify] https://crrev.com/63dbc4bee3a544a27a61e6682e1f1f449d4f657a/third_party/WebKit/Source/core/layout/ng/inline/ng_caret_rect.h

Project Member

Comment 46 by bugdroid1@chromium.org, Feb 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6bcc68e61635f931e513b193156a33d4476ba2dd

commit 6bcc68e61635f931e513b193156a33d4476ba2dd
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Fri Feb 09 04:12:49 2018

[LayoutNG] Add dummy implementation of LocalSelectionRectOfPosition

The only difference between LocalSelectionRectOfPosition() and
LocalCaretRectOfPosition() is that, the format uses selection height
that includes more things (e.g., text emphasis marks), while the latter
doesn't include them.

As we currently don't have a distinction between caret height and
selection height in LayoutNG, this patch adds a dummy implementation
of LocalSelectionRectOfPosition() that's the same as
LocalCaretRectOfPosition(), and leaves a TODO to handle the difference
as future work.

This patch also eliminates the last caller of LayoutObject::LocalCaretRect
with non-null InlineBox.

Bug: 771398
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: If00ef5f841ffaf826c9e31c98ed5c3bd82c41bed
Reviewed-on: https://chromium-review.googlesource.com/910125
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535633}
[modify] https://crrev.com/6bcc68e61635f931e513b193156a33d4476ba2dd/third_party/WebKit/Source/core/editing/LocalCaretRect.cpp

Blockedon: 812535
Project Member

Comment 48 by bugdroid1@chromium.org, Apr 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/07fb5bbe507a3af4d947d19b846bff746383a132

commit 07fb5bbe507a3af4d947d19b846bff746383a132
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Tue Apr 03 18:01:35 2018

Remove an unnecessary if statement from InlineBoxPosition.cpp

The |if| statement's condition is always false, since |level| is set to
|inline_box->BidiLevel()| in L165.

Note: The code was originally introduced in crrev.com/edfe20, which
already contains this redundant branching.

Bug: 771398
Change-Id: If83f178b8ab91f15fcc7176e429541e69a681b15
Reviewed-on: https://chromium-review.googlesource.com/991604
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547762}
[modify] https://crrev.com/07fb5bbe507a3af4d947d19b846bff746383a132/third_party/WebKit/Source/core/editing/InlineBoxPosition.cpp

Project Member

Comment 49 by bugdroid1@chromium.org, Apr 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6a93bb17fb2477de9bddce5c0275f422b1cb1cce

commit 6a93bb17fb2477de9bddce5c0275f422b1cb1cce
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Wed Apr 04 05:23:15 2018

Fix 'unicode-bidi: plaintext' handling in ComputeInlineBoxPosition

Existing code performs bidi adjustment for 'unicode-bidi: plaintext' only
for the right edge of InlineBoxes. This patch makes it handle both edges
to allow caret to appear at the left edge.

This patch also makes the bidi adjustment code symmetric for left and
right edge handling, so that we can de-duplicate in future refactoring.

Bug:  828657 , 771398
Change-Id: I3fd22280327c7186fcfddfb0ba1bb13c4a5dfeff
Reviewed-on: https://chromium-review.googlesource.com/994389
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547987}
[modify] https://crrev.com/6a93bb17fb2477de9bddce5c0275f422b1cb1cce/third_party/WebKit/Source/core/editing/InlineBoxPosition.cpp
[modify] https://crrev.com/6a93bb17fb2477de9bddce5c0275f422b1cb1cce/third_party/WebKit/Source/core/editing/LocalCaretRectTest.cpp

Blockedon: 830956
Project Member

Comment 51 by bugdroid1@chromium.org, Apr 25 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f87db449278842c23a2c56ac1a1584520e73fe54

commit f87db449278842c23a2c56ac1a1584520e73fe54
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Wed Apr 25 02:21:31 2018

Improve readability of SelectionModifierCharacter

This patch improves the readability of the file by moving variable
|line_layout_item| into the iteration body and making it const.

The refactoring is correct because:
- Every time |box| is modified, |line_layout_item| is modified accordingly
- Every time |box| is modified, the iteration either breaks or continues

Bug: 771398
Change-Id: I2a4eecc27870f33a0f16e50762742fff9811cda1
Reviewed-on: https://chromium-review.googlesource.com/1026801
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553426}
[modify] https://crrev.com/f87db449278842c23a2c56ac1a1584520e73fe54/third_party/blink/renderer/core/editing/selection_modifier_character.cc

Sign in to add a comment