New issue
Advanced search Search tips

Issue 721957 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Task

Blocking:
issue 707656



Sign in to add a comment

InlineBox Oblivious TextIterator

Project Member Reported by xiaoche...@chromium.org, May 12 2017

Issue description

The ultimate goal is to make InlineBoxes oblivious to TextIterator, so that TextIterator can emit everything in a TextNode at once, instead of iterating over the text boxes and emitting the text box by box.

Rough plan:

1. Clean up the maintenance of certain variables in TextIterator, so that they are maintained and used by text-node-handling functions (or chunks of code) that do not call TextIterator functions.

A tentative list of variables:
- offset_
- text_box_
- remaining_text_box_
- first_letter_text_
- last_text_node_
- last_text_node_ended_with_collapsed_space_
- sorted_text_boxes_
- sorted_text_boxes_position_
- handled_first_letter
- first_letter_start_offset_
- remaining_text_start_offset_

2. Create a wrapper class for text node handling. It should contain at least the following functions:
- HandleTextNode
- HandleTextBox
- HandleTextNodeFirstLetter
- PrepareForFirstLetterInitialization
- HasNotAdvancedToStartPosition
- EmitText

SpliceBuffer is currently kind-of shared by both text and non-text node handling. Still need to figure out the correct place to put this function.

3. Make the wrapper class concatenate strings of text boxes, and add positions mapping between DOM and plain text offsets.
 
Blocking: 707656
Project Member

Comment 2 by bugdroid1@chromium.org, May 16 2017

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

commit 957d630f9df1f52475fa03759ea9451864b1a33b
Author: xiaochengh <xiaochengh@chromium.org>
Date: Tue May 16 06:30:37 2017

Refactor some first-letter handling code in TextIterator

This patch reorganizes code in TextIterator as a preparation to reduce
the level of hackiness in the class. The patch consists of:

1. Introduction of helper functions to reduce code duplication
2. Delay definition of |run_start| in HandleTextBox()
3. Changing some variable types of |const unsigned|

A follow-up patch will reduce the hackiness: crrev.com/2883163002

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

Review-Url: https://codereview.chromium.org/2888463002
Cr-Commit-Position: refs/heads/master@{#472031}

[modify] https://crrev.com/957d630f9df1f52475fa03759ea9451864b1a33b/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
[modify] https://crrev.com/957d630f9df1f52475fa03759ea9451864b1a33b/third_party/WebKit/Source/core/editing/iterators/TextIterator.h

Project Member

Comment 3 by bugdroid1@chromium.org, May 17 2017

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

commit d91d54ced7ed0c79f8140e232f4b6b80efb5921d
Author: xiaochengh <xiaochengh@chromium.org>
Date: Wed May 17 07:24:52 2017

Un-insanify first-letter handling in TextIterator (to some degree)

Background: (Once upon a time) TextIterator used to work on :first-letter
in some cases, with several flawed designs magically cancelling each other's
flaw. Recently, [1] fixed some flaw in TextIteratorTextState. However, it
also built another level of hack over all the existing flawed designed in
TextIterator, so that the class works in more cases.

This patch removes the hack added by [1] and makes TextIterator work with
:first-letter in an understandable way:

1. Fix the logic when TextIterator decides:
- Whether text in :first-letter should be emitted; Existing implementation emits
:first-letter only when |offset_ == 0|, which is wrong if the iteration starts in
:first-letter..
- Whether the iteration should end in :first-letter; Existing implementation has
no handling of this at all.

2. When advancing from a text node's :first-letter to remaining text,
|offset_| should be set to the starting offset of the remaining text, instead
of 0.

3. Fix the logic when dealing with DOM and layout offsets in a text node
with :first-letter. When a text node has :first-letter, offsets in DOM and
layout object differ by |TextStartOffset()|. HandleTextNode() and HandleTextBox()
should be aware of and handle the difference at all time; Existing implementation
has no handling of the difference at all.

New unit tests have also been added for the correct behavior.

This patch is also a preparation for abstracting the text box handling logic
out of TextIterator.

[1] crrev.com/5185769b9ba7e9239d68c7dc672a27dd19b1685f

BUG= 721957 
TEST=TextIteratorTest.*

Review-Url: https://codereview.chromium.org/2883163002
Cr-Commit-Position: refs/heads/master@{#472368}

[modify] https://crrev.com/d91d54ced7ed0c79f8140e232f4b6b80efb5921d/third_party/WebKit/LayoutTests/accessibility/first-letter-text-transform-causes-crash-expected.txt
[modify] https://crrev.com/d91d54ced7ed0c79f8140e232f4b6b80efb5921d/third_party/WebKit/LayoutTests/fast/text/editing-text-crash-expected.txt
[modify] https://crrev.com/d91d54ced7ed0c79f8140e232f4b6b80efb5921d/third_party/WebKit/LayoutTests/fast/text/text-iterator-crash-expected.txt
[modify] https://crrev.com/d91d54ced7ed0c79f8140e232f4b6b80efb5921d/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
[modify] https://crrev.com/d91d54ced7ed0c79f8140e232f4b6b80efb5921d/third_party/WebKit/Source/core/editing/iterators/TextIterator.h
[modify] https://crrev.com/d91d54ced7ed0c79f8140e232f4b6b80efb5921d/third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, May 19 2017

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

commit 9b781c354432f97658390c848359d2d70ceb1cb5
Author: xiaochengh <xiaochengh@chromium.org>
Date: Fri May 19 03:56:12 2017

No extra space emission in TextIterator::HandleTextNode

Due to historical reasons, TextIterator emits an extra space before a text node
with "white-space: pre" style if the last text node contains only collapsed
whitespaces. This might be a workaround for some other buggy behavior, which
no longer exists today.

Hence, this patch removes the extra space emission.

BUG= 721957 

Review-Url: https://codereview.chromium.org/2894543002
Cr-Commit-Position: refs/heads/master@{#473072}

[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/accessibility/draw-focus-if-needed-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/accessibility/image-link-inline-cont-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/compositing/draws-content/canvas-background-layer-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/compositing/draws-content/webgl-background-layer-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/compositing/geometry/composited-in-columns-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/compositing/layer-creation/rotate3d-overlap-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/compositing/webgl/webgl-copy-image-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/css3/flexbox/line-wrapping-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/block/float/block-with-negative-margin-clears-float-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/block/float/floats-do-not-overhang-from-block-formatting-context-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/block/positioning/positioned-child-inside-relative-positioned-anonymous-block-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/css-grid-layout/flex-and-minmax-content-resolution-columns-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-line-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/css-intrinsic-dimensions/css-tables-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/css-intrinsic-dimensions/fit-content-container-with-replaced-child-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/css-intrinsic-dimensions/intrinsic-sized-blocks-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/css-intrinsic-dimensions/tables-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/css/display-inline-block-scrollbar-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/css/positioned-overflow-scroll-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/deprecated-flexbox/intrinsic-min-width-applies-with-fixed-width-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/dom/Element/scroll-width-hidden-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/dom/Element/scroll-width-visible-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/dom/elementsFromPoint/elementsFromPoint-svg-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/dom/insertedIntoDocument-no-crash-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/dom/shadow/event-path-for-user-agent-shadow-tree-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/encoding/bracket-in-tag-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/events/content-changed-during-drop-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/events/keypress-removed-node-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/inline/inline-offsetLeft-continuation-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/inline/inline-with-empty-inline-children-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/multicol/break-before-first-line-in-first-child-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/multicol/nested-one-line-in-inner-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-div-in-anchor-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-fully-aligned-horizontally-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-fully-aligned-vertically-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-zero-margin-content-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/table/convert-inline-anonoymous-wrapper-to-block-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/table/fixed-table-layout/table-with-percent-width-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/table/row-in-inline-block-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/writing-mode/auto-sizing-orthogonal-flows-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/xpath/preceding-axis-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/fast/xpath/union-context-node-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/hittesting/inner-border-radius-hittest-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-sync-no-progress-events-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getBoxModel-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/inspector-protocol/heap-profiler/heap-snapshot-with-active-dom-object-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/inspector-protocol/heap-profiler/heap-snapshot-with-detached-dom-tree-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/inspector-protocol/heap-profiler/heap-snapshot-with-event-listener-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/inspector/console/console-format-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/inspector/elements/shadow/inspect-deep-shadow-element-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/inspector/elements/styles-2/add-import-rule-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/inspector/elements/styles-3/style-rule-from-imported-stylesheet-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/inspector/elements/styles-4/svg-style-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/inspector/extensions/extensions-audits-content-script-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/inspector/extensions/extensions-audits-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/inspector/extensions/extensions-timeline-api-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/platform/linux/compositing/geometry/composited-in-columns-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/platform/linux/css3/flexbox/line-wrapping-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/platform/mac/images/color-profile-munsell-adobe-to-srgb-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/platform/mac/images/color-profile-munsell-srgb-to-srgb-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu-rasterization/images/color-profile-munsell-adobe-to-srgb-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu-rasterization/images/color-profile-munsell-srgb-to-srgb-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/platform/win/compositing/geometry/composited-in-columns-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/platform/win/css3/flexbox/line-wrapping-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/platform/win/images/color-profile-munsell-adobe-to-srgb-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/platform/win/images/color-profile-munsell-srgb-to-srgb-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/platform/win/virtual/disable-spinvalidation/compositing/geometry/composited-in-columns-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/platform/win/virtual/gpu-rasterization/images/color-profile-munsell-adobe-to-srgb-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/printing/webgl-repeated-printing-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/printing/webgl-repeated-printing-preservedrawingbuffer-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/svg/custom/tabindex-order-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/svg/dom/svg-root-lengths-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/traversal/tree-walker-001-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/traversal/tree-walker-002-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/traversal/tree-walker-003-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/traversal/tree-walker-004-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/traversal/tree-walker-005-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/LayoutTests/virtual/without-smil/svg/animations/exposed/effect-expected.txt
[modify] https://crrev.com/9b781c354432f97658390c848359d2d70ceb1cb5/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp

Currently blocked by whitespace handling.

HandleTextNode/Box maintains a flag |last_text_node_ended_with_collapsed_space_|, which is in turn used in HandleReplacedElement. This blocks abstracting text box handling out from TextIterator.

The flag is used to emit an extra space in HandleTextNode and HandleReplaceElement. While it fixes some bugs in some cases, it creates bugs in some other cases:

https://jsfiddle.net/t49qkwh2/

The space emission is required in div3 and div4, but results in an extra space in div1 and div2. We should move the space emission to HandleTextNode/Box.
I'm not going forward on fixing the whitespace handling in TextIterator. It's impossible to handle it in a clean way with the legacy layout tree, especially when it's combined with line breaking.

We'll have another try when DOM-Layout offset mapping is ready in LayoutNG, where we can access the text nodes before line breaking.
Project Member

Comment 7 by bugdroid1@chromium.org, May 23 2017

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

commit ea8d0167edd856fc587d630b9b2d635cefab5798
Author: xiaochengh <xiaochengh@chromium.org>
Date: Tue May 23 02:12:18 2017

Revert of No extra space emission in TextIterator::HandleTextNode (patchset #6 id:100001 of https://codereview.chromium.org/2894543002/ )

Reason for revert:
Breaking this case:

<div class=container id=div4 style="width:30px">
foooooooooooooooo

<span style="white-space: pre">bar</span>
</div>

Original issue's description:
> No extra space emission in TextIterator::HandleTextNode
>
> Due to historical reasons, TextIterator emits an extra space before a text node
> with "white-space: pre" style if the last text node contains only collapsed
> whitespaces. This might be a workaround for some other buggy behavior, which
> no longer exists today.
>
> Hence, this patch removes the extra space emission.
>
> BUG= 721957 
>
> Review-Url: https://codereview.chromium.org/2894543002
> Cr-Commit-Position: refs/heads/master@{#473072}
> Committed: https://chromium.googlesource.com/chromium/src/+/9b781c354432f97658390c848359d2d70ceb1cb5

TBR=yosin@chromium.org,xiaochengh@google.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 721957 

Review-Url: https://codereview.chromium.org/2896033003
Cr-Commit-Position: refs/heads/master@{#473785}

[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/accessibility/draw-focus-if-needed-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/accessibility/image-link-inline-cont-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/compositing/draws-content/canvas-background-layer-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/compositing/draws-content/webgl-background-layer-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/compositing/geometry/composited-in-columns-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/compositing/layer-creation/rotate3d-overlap-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/compositing/webgl/webgl-copy-image-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/css3/flexbox/line-wrapping-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/block/float/block-with-negative-margin-clears-float-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/block/float/floats-do-not-overhang-from-block-formatting-context-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/block/positioning/positioned-child-inside-relative-positioned-anonymous-block-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/css-grid-layout/flex-and-minmax-content-resolution-columns-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-line-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/css-intrinsic-dimensions/css-tables-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/css-intrinsic-dimensions/fit-content-container-with-replaced-child-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/css-intrinsic-dimensions/intrinsic-sized-blocks-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/css-intrinsic-dimensions/tables-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/css/display-inline-block-scrollbar-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/css/positioned-overflow-scroll-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/deprecated-flexbox/intrinsic-min-width-applies-with-fixed-width-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/dom/Element/scroll-width-hidden-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/dom/Element/scroll-width-visible-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/dom/elementsFromPoint/elementsFromPoint-svg-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/dom/insertedIntoDocument-no-crash-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/dom/shadow/event-path-for-user-agent-shadow-tree-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/encoding/bracket-in-tag-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/events/content-changed-during-drop-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/events/keypress-removed-node-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/inline/inline-offsetLeft-continuation-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/inline/inline-with-empty-inline-children-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/multicol/break-before-first-line-in-first-child-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/multicol/nested-one-line-in-inner-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-div-in-anchor-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-fully-aligned-horizontally-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-fully-aligned-vertically-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-zero-margin-content-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/table/convert-inline-anonoymous-wrapper-to-block-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/table/fixed-table-layout/table-with-percent-width-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/table/row-in-inline-block-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/writing-mode/auto-sizing-orthogonal-flows-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/xpath/preceding-axis-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/fast/xpath/union-context-node-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/hittesting/inner-border-radius-hittest-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-sync-no-progress-events-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getBoxModel-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/inspector-protocol/heap-profiler/heap-snapshot-with-active-dom-object-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/inspector-protocol/heap-profiler/heap-snapshot-with-detached-dom-tree-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/inspector-protocol/heap-profiler/heap-snapshot-with-event-listener-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/inspector/console/console-format-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/inspector/elements/elements-tab-stops-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/inspector/elements/shadow/inspect-deep-shadow-element-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/inspector/elements/styles-2/add-import-rule-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/inspector/elements/styles-3/style-rule-from-imported-stylesheet-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/inspector/elements/styles-4/svg-style-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/inspector/extensions/extensions-audits-content-script-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/inspector/extensions/extensions-audits-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/inspector/extensions/extensions-timeline-api-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/platform/linux/compositing/geometry/composited-in-columns-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/platform/linux/css3/flexbox/line-wrapping-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/platform/mac/images/color-profile-munsell-adobe-to-srgb-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/platform/mac/images/color-profile-munsell-srgb-to-srgb-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu-rasterization/images/color-profile-munsell-adobe-to-srgb-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu-rasterization/images/color-profile-munsell-srgb-to-srgb-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/platform/win/compositing/geometry/composited-in-columns-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/platform/win/css3/flexbox/line-wrapping-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/platform/win/images/color-profile-munsell-adobe-to-srgb-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/platform/win/images/color-profile-munsell-srgb-to-srgb-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/platform/win/virtual/disable-spinvalidation/compositing/geometry/composited-in-columns-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/platform/win/virtual/gpu-rasterization/images/color-profile-munsell-adobe-to-srgb-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/printing/webgl-repeated-printing-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/printing/webgl-repeated-printing-preservedrawingbuffer-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/svg/custom/tabindex-order-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/svg/dom/svg-root-lengths-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/traversal/tree-walker-001-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/traversal/tree-walker-002-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/traversal/tree-walker-003-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/traversal/tree-walker-004-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/traversal/tree-walker-005-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/LayoutTests/virtual/without-smil/svg/animations/exposed/effect-expected.txt
[modify] https://crrev.com/ea8d0167edd856fc587d630b9b2d635cefab5798/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp

Comment 8 by yosin@chromium.org, May 23 2017

Labels: M-61
Project Member

Comment 9 by bugdroid1@chromium.org, May 24 2017

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

commit c49c1c508631745d9be1bdeea115c63038f8cdf0
Author: xiaochengh <xiaochengh@chromium.org>
Date: Wed May 24 07:58:59 2017

Add TextIterator::HandlePreFormattedTextNode to wrap relevant logic

This patch wraps code in TextIterator for extracting text from pre-formatted
text node into a new function to improve code health and prepare for follow-up
refactoring.

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

Review-Url: https://codereview.chromium.org/2901713002
Cr-Commit-Position: refs/heads/master@{#474205}

[modify] https://crrev.com/c49c1c508631745d9be1bdeea115c63038f8cdf0/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
[modify] https://crrev.com/c49c1c508631745d9be1bdeea115c63038f8cdf0/third_party/WebKit/Source/core/editing/iterators/TextIterator.h

I originally planned to make TextIterator emit everything from a text node all at once.

With the second thought, I don't think this is a good idea. This is introducing extra complexity into the DOM-Layout offset mapping, especially for CharacterIterator:
- The current impl emits one InlineTextBox at once, so the mapping is easy
- If we emit everything from the text node at once, we need to remember and use the collapsed parts of the text node during the mapping, which can be hard and inefficient

So we are still emitting one text box at a time. Since there is no strict equivalence to InlineTextBox in LayoutNG, users of TextIterator should not depend on InlineTextBox directly. In other words, users should only know that content of a text node is emitted progressively as several parts, but the division into those parts can be arbitrary (from the perspective of the users).
Project Member

Comment 11 by bugdroid1@chromium.org, May 26 2017

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

commit ae7cd2d737112818ef88c1167c816e171e2e4623
Author: xiaochengh <xiaochengh@chromium.org>
Date: Fri May 26 23:31:17 2017

Ensure single entry into TextIterator::HandleTextNode for each node

This patch ensures that TextIterator::HandleTextNode is called at most
once on each text node. This allows us to maintain text node related
states in this function instead of spreading out in the entire
TextIterator, which will be done by a followup patch.

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

Review-Url: https://codereview.chromium.org/2902683005
Cr-Commit-Position: refs/heads/master@{#475186}

[modify] https://crrev.com/ae7cd2d737112818ef88c1167c816e171e2e4623/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
[modify] https://crrev.com/ae7cd2d737112818ef88c1167c816e171e2e4623/third_party/WebKit/Source/core/editing/iterators/TextIterator.h

Project Member

Comment 12 by bugdroid1@chromium.org, May 27 2017

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

commit 2d8f80cfe9a0069c01a44d3f209b69e5186da2fc
Author: xiaochengh <xiaochengh@chromium.org>
Date: Sat May 27 03:23:09 2017

Reorganize the maintenance of several text node related members in TextIterator

Some members of TextIterator are used only by text node related functions, but
are initialized/reset at multiple places in the class. This patch reorganizes
them so that they are maintained solely by text node related functions/code.
- offset_
- handled_first_letter_
- first_letter_text_

BUG= 721957 
TEST=n/a; shouldn't have any behavioral change

Review-Url: https://codereview.chromium.org/2896023003
Cr-Commit-Position: refs/heads/master@{#475231}

[modify] https://crrev.com/2d8f80cfe9a0069c01a44d3f209b69e5186da2fc/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp

Project Member

Comment 13 by bugdroid1@chromium.org, May 27 2017

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

commit b109b249a23d3146575b12b1fa4781ade53dac28
Author: xiaochengh <xiaochengh@chromium.org>
Date: Sat May 27 05:04:23 2017

Add member TextIterator::text_node_

This patch adds a new member TextIterator::text_node_ to record
the text node that is being handled in TextIterator. As a result,
text node handling and the remaining part of TextIterator share one
less member, so that the encapsulation of text node handling is
easier.

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

Review-Url: https://codereview.chromium.org/2902173003
Cr-Commit-Position: refs/heads/master@{#475237}

[modify] https://crrev.com/b109b249a23d3146575b12b1fa4781ade53dac28/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
[modify] https://crrev.com/b109b249a23d3146575b12b1fa4781ade53dac28/third_party/WebKit/Source/core/editing/iterators/TextIterator.h

Project Member

Comment 14 by bugdroid1@chromium.org, May 27 2017

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

commit d1b64ea824a10da503d0ee572442cb3119c3d2ed
Author: xiaochengh <xiaochengh@chromium.org>
Date: Sat May 27 06:13:14 2017

Reorganize the maintenance and usage of TextIterator::last_text_node_

This patch reorganizes the maintenance and usage of the data member, so that
it is only used at non-text-node handling places, so that text node handling
code can be moved into a wrapper class.

BUG= 721957 
TEST=n/a; shouldn't be any behavioral change...

Review-Url: https://codereview.chromium.org/2903203002
Cr-Commit-Position: refs/heads/master@{#475242}

[modify] https://crrev.com/d1b64ea824a10da503d0ee572442cb3119c3d2ed/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp

Project Member

Comment 15 by bugdroid1@chromium.org, May 27 2017

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

commit d555fc27510ae3b0c155bfabc089f67edf412bda
Author: xiaochengh <xiaochengh@chromium.org>
Date: Sat May 27 06:48:56 2017

Wrap text node handling code in TextIterator into wrapper functions

This patch introduces wrapper functions to wrap some text node handling
code that is not in dedicated functions yet. A followup patch will remove
these functions and the data members that they use into a wrapper class.

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

Review-Url: https://codereview.chromium.org/2904813003
Cr-Commit-Position: refs/heads/master@{#475243}

[modify] https://crrev.com/d555fc27510ae3b0c155bfabc089f67edf412bda/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
[modify] https://crrev.com/d555fc27510ae3b0c155bfabc089f67edf412bda/third_party/WebKit/Source/core/editing/iterators/TextIterator.h

Project Member

Comment 16 by bugdroid1@chromium.org, May 30 2017

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

commit 8d639e9e2ac1811b38e6bacc13e73049b27750f4
Author: xiaochengh <xiaochengh@chromium.org>
Date: Tue May 30 03:03:29 2017

Introduce a wrapper class to handle text node in TextIterator

This patch adds (in-place) a wrapper class for the text node handling code
in TextIterator. With this patch, TextIterator doesn't need to know the
details of the text layout, so it can switch to Layout NG easier.

A follow-up patch will remove the wrapper class to dedicated files.

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

Review-Url: https://codereview.chromium.org/2903693005
Cr-Commit-Position: refs/heads/master@{#475411}

[modify] https://crrev.com/8d639e9e2ac1811b38e6bacc13e73049b27750f4/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
[modify] https://crrev.com/8d639e9e2ac1811b38e6bacc13e73049b27750f4/third_party/WebKit/Source/core/editing/iterators/TextIterator.h

Project Member

Comment 18 by bugdroid1@chromium.org, May 31 2017

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

commit 4ac11ed7d02420bda817ec1441af56f2b56a0a02
Author: xiaochengh <xiaochengh@chromium.org>
Date: Wed May 31 03:35:16 2017

Wrap code into TextIterator::HandleRememberedProgress

TextIterator::Advance() used to contain some code before its main loop
to handle the remembered progress, if any. This patch wraps the code
into a new function TextIterator::HandleRememberedProgress() for better
code health.

Follow-up patches will utilize this function to make the control flow
in TextIterator cleaner.

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

Review-Url: https://codereview.chromium.org/2911373003
Cr-Commit-Position: refs/heads/master@{#475772}

[modify] https://crrev.com/4ac11ed7d02420bda817ec1441af56f2b56a0a02/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
[modify] https://crrev.com/4ac11ed7d02420bda817ec1441af56f2b56a0a02/third_party/WebKit/Source/core/editing/iterators/TextIterator.h

Project Member

Comment 19 by bugdroid1@chromium.org, May 31 2017

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

commit 529b9affa165a95a6b9369809ab26d7c5d37466b
Author: xiaochengh <xiaochengh@chromium.org>
Date: Wed May 31 19:12:39 2017

Use remembered progress to handle replaced elements in TextIterator

This patch utilizes the "remembered progress" mechanism in TextIterator
to handle a replaced element when it emits multiple text runs. In this
way, the control flow becomes cleaner as we don't need to go into the
mail loop of TextIterator::Advance() again.

Follow-up patches will remove the return value of HandlePlacedElement()
because this patch makes it almost useless.

BUG= 721957 
TEST=n/a; shouldn't have any behavioral change

Review-Url: https://codereview.chromium.org/2912283002
Cr-Commit-Position: refs/heads/master@{#475977}

[modify] https://crrev.com/529b9affa165a95a6b9369809ab26d7c5d37466b/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
[modify] https://crrev.com/529b9affa165a95a6b9369809ab26d7c5d37466b/third_party/WebKit/Source/core/editing/iterators/TextIterator.h

Project Member

Comment 20 by bugdroid1@chromium.org, Jun 1 2017

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

commit d2ec4830b36d30ebf8ab495d18a6282db1020db1
Author: xiaochengh <xiaochengh@chromium.org>
Date: Thu Jun 01 01:54:01 2017

Change return type of TextIterator::HandleXXX to void

TextIterator used to have the following member functions returning |bool|
to indicate whether the handling of a node is complete or not:
- HandleTextNode
- HandleReplacedElement
- HandleNonTextNode

However, the return value has become arbitrary and not that useful after
the class has evolved for so many years. Hence, this patch removes their
return value to make the state change in TextIterator clean.

BUG= 721957 
TEST=n/a; shouldn't have any behavioral change

Review-Url: https://codereview.chromium.org/2914883002
Cr-Commit-Position: refs/heads/master@{#476144}

[modify] https://crrev.com/d2ec4830b36d30ebf8ab495d18a6282db1020db1/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
[modify] https://crrev.com/d2ec4830b36d30ebf8ab495d18a6282db1020db1/third_party/WebKit/Source/core/editing/iterators/TextIterator.h
[modify] https://crrev.com/d2ec4830b36d30ebf8ab495d18a6282db1020db1/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextNodeHandler.cpp
[modify] https://crrev.com/d2ec4830b36d30ebf8ab495d18a6282db1020db1/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextNodeHandler.h

Project Member

Comment 21 by bugdroid1@chromium.org, Jun 1 2017

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

commit 3c26208e98b83e8c698a213e63a0ac3e5f1e3b42
Author: xiaochengh <xiaochengh@chromium.org>
Date: Thu Jun 01 17:19:52 2017

Change how iteration range is passed to TextIteratorTextNodeHandler

TextIteratorTextNodeHandler used to store the full iteration range, and
have comparisons between |text_node_| and |start/end_container_| scattered
in the class to decide the offset range on |text_node_| from which text
should be emitted.

This patch changes it that:
- TextIterator, when calling HandleTextNode(), compares the current text
  node with the start and end containers to decide the offset range on it,
  and pass the offset range to TextNodeHandler
- TextNodeHandler no longer stores the full iteration, but only the offset
  range passed from TextIterator.
- TextNodeHandler no longer decides the offset range by itself, but uses
  the passed-in offset range directly.

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

Review-Url: https://codereview.chromium.org/2913673004
Cr-Commit-Position: refs/heads/master@{#476336}

[modify] https://crrev.com/3c26208e98b83e8c698a213e63a0ac3e5f1e3b42/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
[modify] https://crrev.com/3c26208e98b83e8c698a213e63a0ac3e5f1e3b42/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextNodeHandler.cpp
[modify] https://crrev.com/3c26208e98b83e8c698a213e63a0ac3e5f1e3b42/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextNodeHandler.h

Project Member

Comment 22 by bugdroid1@chromium.org, Jun 1 2017

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

commit c8070f461957f5f2aaaf41147538ebe811b4abce
Author: xiaochengh <xiaochengh@chromium.org>
Date: Thu Jun 01 19:43:46 2017

Reorganize member initialization code of TextIterator

This patch reorganizes the initialization of TextIterator for better code
health.

1. Members originally initialized in the function body of TextIterator::ctor
   are changed to be initialized in the initialization list. Some wrapper
   functions are created for members with non-trivial initialization.
   - start_container_
   - start_offset_
   - end_container_
   - end_offset_
   - end_node_
   - past_end_node_
   - node_
   - iteration_progress_
   - shadow_depth_

2. Members with trivial initialization are initialized in class declaration
   instead of ctor:
   - needs_another_new_line_
   - needs_handle_replaced_element_
   - should_stop_
   - handle_shadow_root_

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

Review-Url: https://codereview.chromium.org/2921483002
Cr-Commit-Position: refs/heads/master@{#476393}

[modify] https://crrev.com/c8070f461957f5f2aaaf41147538ebe811b4abce/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
[modify] https://crrev.com/c8070f461957f5f2aaaf41147538ebe811b4abce/third_party/WebKit/Source/core/editing/iterators/TextIterator.h

Project Member

Comment 23 by bugdroid1@chromium.org, Jun 2 2017

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

commit 6c2afab777022cf01f26bc6772237f11d99238b5
Author: xiaochengh <xiaochengh@chromium.org>
Date: Fri Jun 02 01:20:08 2017

Move TextIteratorTextState's member initiailization to class declaration

This patch moves TextIteratorTextState's member initialization to the class
declaration, since they are all trivially initialized.

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

Review-Url: https://codereview.chromium.org/2922553002
Cr-Commit-Position: refs/heads/master@{#476511}

[modify] https://crrev.com/6c2afab777022cf01f26bc6772237f11d99238b5/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.cpp
[modify] https://crrev.com/6c2afab777022cf01f26bc6772237f11d99238b5/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.h

Project Member

Comment 24 by bugdroid1@chromium.org, Jun 2 2017

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

commit fa70ca74ea7d7a5e7355e2b7a155b8f46a390a0f
Author: xiaochengh <xiaochengh@chromium.org>
Date: Fri Jun 02 01:47:17 2017

Remove TextIteratorTextState::GetString()

The above mentioned function simply returns a string member of the class,
and is only used inside the class. Hence, this patch removes the function
to reduce code complexity and any possible confusion.

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

Review-Url: https://codereview.chromium.org/2915863005
Cr-Commit-Position: refs/heads/master@{#476518}

[modify] https://crrev.com/fa70ca74ea7d7a5e7355e2b7a155b8f46a390a0f/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.cpp
[modify] https://crrev.com/fa70ca74ea7d7a5e7355e2b7a155b8f46a390a0f/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.h

Project Member

Comment 25 by bugdroid1@chromium.org, Jun 2 2017

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

commit c7fee9a6165a732a2cb346c9db63ab65adab5398
Author: xiaochengh <xiaochengh@chromium.org>
Date: Fri Jun 02 08:48:15 2017

Put TextIteratorTextNodeHandler and TextIteratorTextState on heap

This patch puts TextIteratorTextNodeHandle and TextIteratorTextState on heap,
so that we can use forward declaration, and also opens up possibility for
switching them to a Layout NG version.

Note: we can't only put TextIteratorTextNodeHandler on heap because it holds
a reference to TextIteratorTextState, but an on-heap object cannot hold reference
to a stack-allocated object.

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

Review-Url: https://codereview.chromium.org/2915043004
Cr-Commit-Position: refs/heads/master@{#476605}

[modify] https://crrev.com/c7fee9a6165a732a2cb346c9db63ab65adab5398/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
[modify] https://crrev.com/c7fee9a6165a732a2cb346c9db63ab65adab5398/third_party/WebKit/Source/core/editing/iterators/TextIterator.h
[modify] https://crrev.com/c7fee9a6165a732a2cb346c9db63ab65adab5398/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextNodeHandler.cpp
[modify] https://crrev.com/c7fee9a6165a732a2cb346c9db63ab65adab5398/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextNodeHandler.h
[modify] https://crrev.com/c7fee9a6165a732a2cb346c9db63ab65adab5398/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.cpp
[modify] https://crrev.com/c7fee9a6165a732a2cb346c9db63ab65adab5398/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.h

Project Member

Comment 26 by bugdroid1@chromium.org, Jun 2 2017

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

commit b31e3ce881bd72c860c563cbcf74ff16e3bfbe84
Author: xiaochengh <xiaochengh@chromium.org>
Date: Fri Jun 02 19:45:52 2017

Manage DOM and string offsets separately in TextIteratorTextState

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

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

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

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

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

Review-Url: https://codereview.chromium.org/2914313002
Cr-Commit-Position: refs/heads/master@{#476759}

[modify] https://crrev.com/b31e3ce881bd72c860c563cbcf74ff16e3bfbe84/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
[modify] https://crrev.com/b31e3ce881bd72c860c563cbcf74ff16e3bfbe84/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextNodeHandler.cpp
[modify] https://crrev.com/b31e3ce881bd72c860c563cbcf74ff16e3bfbe84/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.cpp
[modify] https://crrev.com/b31e3ce881bd72c860c563cbcf74ff16e3bfbe84/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.h

Project Member

Comment 27 by bugdroid1@chromium.org, Jun 5 2017

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

commit ca0bc1dcb2a5a4d3125e06d0e255b39a0eae431e
Author: xiaochengh <xiaochengh@chromium.org>
Date: Mon Jun 05 05:55:52 2017

Stop TextIteratorTextState from using LayoutText

This patch changes TextIteratorTextState::EmitText into taking a node and a
string separately, instead of taking a LayoutText, so that the class no
longer uses LayoutText. Handling of LayoutText is hoisted to
TextIteratorTextNodeHandler.

With this patch, TextIteratorTextState no longer depends on legacy layout,
and can be used by Layout NG.

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

Review-Url: https://codereview.chromium.org/2915013005
Cr-Commit-Position: refs/heads/master@{#476950}

[modify] https://crrev.com/ca0bc1dcb2a5a4d3125e06d0e255b39a0eae431e/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextNodeHandler.cpp
[modify] https://crrev.com/ca0bc1dcb2a5a4d3125e06d0e255b39a0eae431e/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.cpp
[modify] https://crrev.com/ca0bc1dcb2a5a4d3125e06d0e255b39a0eae431e/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.h

Project Member

Comment 28 by bugdroid1@chromium.org, Jun 5 2017

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

commit d4fd4e36421025fb37bc556408e6f30b8e94a55a
Author: xiaochengh <xiaochengh@chromium.org>
Date: Mon Jun 05 07:59:01 2017

Stop using TextIteratorBehavior in TextIteratorTextState

This patch removes the dependency on TextIteratorBehavior from
TextIteratorTextState. After this patch, TextIteratorTextState becomes a
clean class doing exactly what it names suggests: storing the text state.
More specifically, it manages only:
- The current text
- The DOM position of the current text

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

Review-Url: https://codereview.chromium.org/2919433006
Cr-Commit-Position: refs/heads/master@{#476958}

[modify] https://crrev.com/d4fd4e36421025fb37bc556408e6f30b8e94a55a/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
[modify] https://crrev.com/d4fd4e36421025fb37bc556408e6f30b8e94a55a/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextNodeHandler.cpp
[modify] https://crrev.com/d4fd4e36421025fb37bc556408e6f30b8e94a55a/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.cpp
[modify] https://crrev.com/d4fd4e36421025fb37bc556408e6f30b8e94a55a/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.h

Status: Fixed (was: Assigned)
Project Member

Comment 30 by bugdroid1@chromium.org, Jul 20 2017

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

commit b79cfdc8b3d15f80d2d74877fbae2bd353835194
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Thu Jul 20 02:28:11 2017

Move TextIteratorTextNodeHandler and TextIteratorTextState back to stack

We previously moved the two classes from stack to heap in order to make
TextNodeHandler also work with LayoutNG by polymorphism. However, the
on-going prototype (*) doesn't work in this way. Furthermore, we might
need a TextNodeHandler that works with both legacy and NG in order to
handle mixed layout tree.

Therefore, this patch makes the two classes stack allocated again.

(*) https://codereview.chromium.org/2968803002

Bug:  721957 
Change-Id: I84ed1bff2acd92685fd53555e7b8f8ba62dbd4c3
Reviewed-on: https://chromium-review.googlesource.com/578715
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488102}
[modify] https://crrev.com/b79cfdc8b3d15f80d2d74877fbae2bd353835194/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
[modify] https://crrev.com/b79cfdc8b3d15f80d2d74877fbae2bd353835194/third_party/WebKit/Source/core/editing/iterators/TextIterator.h
[modify] https://crrev.com/b79cfdc8b3d15f80d2d74877fbae2bd353835194/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextNodeHandler.cpp
[modify] https://crrev.com/b79cfdc8b3d15f80d2d74877fbae2bd353835194/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextNodeHandler.h
[modify] https://crrev.com/b79cfdc8b3d15f80d2d74877fbae2bd353835194/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.cpp
[modify] https://crrev.com/b79cfdc8b3d15f80d2d74877fbae2bd353835194/third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.h

Sign in to add a comment