Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(451)

Issue 2915013005: Stop TextIteratorTextState from using LayoutText (Closed)

Created:
3 years, 6 months ago by Xiaocheng
Modified:
3 years, 6 months ago
Reviewers:
yosin_UTC9
CC:
blink-reviews, chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

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} Committed: https://chromium.googlesource.com/chromium/src/+/ca0bc1dcb2a5a4d3125e06d0e255b39a0eae431e

Patch Set 1 #

Patch Set 2 : Thu Jun 1 18:55:15 PDT 2017 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -13 lines) Patch
M third_party/WebKit/Source/core/editing/iterators/TextIteratorTextNodeHandler.cpp View 1 chunk +7 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.h View 1 2 chunks +4 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.cpp View 3 chunks +10 lines, -8 lines 2 comments Download

Depends on Patchset:

Dependent Patchsets:

Messages

Total messages: 17 (13 generated)
Xiaocheng
PTAL.
3 years, 6 months ago (2017-06-02 17:46:47 UTC) #9
yosin_UTC9
lgtm This is good patch! I hope to see removal of |behavior_| from |TextIteratorState| in ...
3 years, 6 months ago (2017-06-05 03:42:22 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2915013005/20001
3 years, 6 months ago (2017-06-05 03:42:35 UTC) #14
commit-bot: I haz the power
3 years, 6 months ago (2017-06-05 05:56:11 UTC) #17
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/ca0bc1dcb2a5a4d3125e06d0e255...

Powered by Google App Engine
This is Rietveld 408576698