Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2885103004:
Rewrite UpdateMarkerRenderedRect() to use EphemeralRange. (Closed)
|
Created:
3 years, 7 months ago by Hwanseung Lee Modified:
3 years, 7 months ago Reviewers:
yosin_UTC9 CC:
blink-reviews, chromium-reviews Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionRewrite UpdateMarkerRenderedRect() to use EphemeralRange.
this patch is last step to replace Range with EphemeralRange in
updateMarkerRenderedRect().
we are progressing three steps.
[1/3] Make RenderedRectsForMarkers() to ignore disconnected nodes.
(https://codereview.chromium.org/2776103002)
[2/3] In-place change of Range::textRects() and boundingBox()
to EphemeralRange version in "Range.cpp"
(https://codereview.chromium.org/2810313003)
[3/3] Rewrite UpdateMarkerRenderedRect() to use EphemeralRange
instead of Range (this patch)
BUG=691202
Review-Url: https://codereview.chromium.org/2885103004
Cr-Commit-Position: refs/heads/master@{#473534}
Committed: https://chromium.googlesource.com/chromium/src/+/479179ca87f415cc18ae18682a3938a4486ea281
Patch Set 1 : 1 #
Total comments: 6
Patch Set 2 : fix the some nit. #Messages
Total messages: 44 (39 generated)
The CQ bit was checked by hs1217.lee@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_android on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...) cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by hs1217.lee@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by hs1217.lee@samsung.com to run a CQ dry run
Patchset #2 (id:20001) has been deleted
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_android on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
Patchset #2 (id:40001) has been deleted
Patchset #2 (id:60001) has been deleted
The CQ bit was checked by hs1217.lee@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by hs1217.lee@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== 1 BUG= ========== to ========== Rewrite updateMarkerRenderedRect() to use EphemeralRange. Rewrite updateMarkerRenderedRect() to use EphemeralRange instead of Range. we are progressing three steps. [1/3] Make RenderedRectsForMarkers() to ignore disconnected nodes. (https://codereview.chromium.org/2776103002) [2/3] In-place change of Range::textRects() and boundingBox() to EphemeralRange version in "Range.cpp" (https://codereview.chromium.org/2810313003) [3/3] Rewrite updateMarkerRenderedRect() to use EphemeralRange instead of Range BUG=691202 ==========
Description was changed from ========== Rewrite updateMarkerRenderedRect() to use EphemeralRange. Rewrite updateMarkerRenderedRect() to use EphemeralRange instead of Range. we are progressing three steps. [1/3] Make RenderedRectsForMarkers() to ignore disconnected nodes. (https://codereview.chromium.org/2776103002) [2/3] In-place change of Range::textRects() and boundingBox() to EphemeralRange version in "Range.cpp" (https://codereview.chromium.org/2810313003) [3/3] Rewrite updateMarkerRenderedRect() to use EphemeralRange instead of Range BUG=691202 ========== to ========== Rewrite updateMarkerRenderedRect() to use EphemeralRange. we are progressing three steps. [1/3] Make RenderedRectsForMarkers() to ignore disconnected nodes. (https://codereview.chromium.org/2776103002) [2/3] In-place change of Range::textRects() and boundingBox() to EphemeralRange version in "Range.cpp" (https://codereview.chromium.org/2810313003) [3/3] Rewrite updateMarkerRenderedRect() to use EphemeralRange instead of Range BUG=691202 ==========
Description was changed from ========== Rewrite updateMarkerRenderedRect() to use EphemeralRange. we are progressing three steps. [1/3] Make RenderedRectsForMarkers() to ignore disconnected nodes. (https://codereview.chromium.org/2776103002) [2/3] In-place change of Range::textRects() and boundingBox() to EphemeralRange version in "Range.cpp" (https://codereview.chromium.org/2810313003) [3/3] Rewrite updateMarkerRenderedRect() to use EphemeralRange instead of Range BUG=691202 ========== to ========== Rewrite updateMarkerRenderedRect() to use EphemeralRange. this patch is last step to replace Range with EphemeralRange in updateMarkerRenderedRect(). we are progressing three steps. [1/3] Make RenderedRectsForMarkers() to ignore disconnected nodes. (https://codereview.chromium.org/2776103002) [2/3] In-place change of Range::textRects() and boundingBox() to EphemeralRange version in "Range.cpp" (https://codereview.chromium.org/2810313003) [3/3] Rewrite updateMarkerRenderedRect() to use EphemeralRange instead of Range BUG=691202 ==========
Patchset #1 (id:1) has been deleted
Patchset #1 (id:80001) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
hs1217.lee@samsung.com changed reviewers: + yosin@chromium.org
PTAL, thank you.
lgtm w/ small nits Thanks for working this! https://codereview.chromium.org/2885103004/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp (right): https://codereview.chromium.org/2885103004/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp:205: Position startPosition(&const_cast<Node&>(node), marker.StartOffset()); nit: s/Position/const Position/ https://codereview.chromium.org/2885103004/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp:206: Position endPostion(&const_cast<Node&>(node), marker.EndOffset()); nit: s/Position/const Position/ https://codereview.chromium.org/2885103004/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp:208: nit: No need to have an extra blank line.
Description was changed from ========== Rewrite updateMarkerRenderedRect() to use EphemeralRange. this patch is last step to replace Range with EphemeralRange in updateMarkerRenderedRect(). we are progressing three steps. [1/3] Make RenderedRectsForMarkers() to ignore disconnected nodes. (https://codereview.chromium.org/2776103002) [2/3] In-place change of Range::textRects() and boundingBox() to EphemeralRange version in "Range.cpp" (https://codereview.chromium.org/2810313003) [3/3] Rewrite updateMarkerRenderedRect() to use EphemeralRange instead of Range BUG=691202 ========== to ========== Rewrite UpdateMarkerRenderedRect() to use EphemeralRange. this patch is last step to replace Range with EphemeralRange in updateMarkerRenderedRect(). we are progressing three steps. [1/3] Make RenderedRectsForMarkers() to ignore disconnected nodes. (https://codereview.chromium.org/2776103002) [2/3] In-place change of Range::textRects() and boundingBox() to EphemeralRange version in "Range.cpp" (https://codereview.chromium.org/2810313003) [3/3] Rewrite UpdateMarkerRenderedRect() to use EphemeralRange instead of Range BUG=691202 ==========
Description was changed from ========== Rewrite UpdateMarkerRenderedRect() to use EphemeralRange. this patch is last step to replace Range with EphemeralRange in updateMarkerRenderedRect(). we are progressing three steps. [1/3] Make RenderedRectsForMarkers() to ignore disconnected nodes. (https://codereview.chromium.org/2776103002) [2/3] In-place change of Range::textRects() and boundingBox() to EphemeralRange version in "Range.cpp" (https://codereview.chromium.org/2810313003) [3/3] Rewrite UpdateMarkerRenderedRect() to use EphemeralRange instead of Range BUG=691202 ========== to ========== Rewrite UpdateMarkerRenderedRect() to use EphemeralRange. this patch is last step to replace Range with EphemeralRange in updateMarkerRenderedRect(). we are progressing three steps. [1/3] Make RenderedRectsForMarkers() to ignore disconnected nodes. (https://codereview.chromium.org/2776103002) [2/3] In-place change of Range::textRects() and boundingBox() to EphemeralRange version in "Range.cpp" (https://codereview.chromium.org/2810313003) [3/3] Rewrite UpdateMarkerRenderedRect() to use EphemeralRange instead of Range (this patch) BUG=691202 ==========
The CQ bit was checked by hs1217.lee@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
thank you. https://codereview.chromium.org/2885103004/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp (right): https://codereview.chromium.org/2885103004/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp:205: Position startPosition(&const_cast<Node&>(node), marker.StartOffset()); On 2017/05/22 04:53:25, yosin_UTC9 wrote: > nit: s/Position/const Position/ Done. https://codereview.chromium.org/2885103004/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp:206: Position endPostion(&const_cast<Node&>(node), marker.EndOffset()); On 2017/05/22 04:53:25, yosin_UTC9 wrote: > nit: s/Position/const Position/ Done. https://codereview.chromium.org/2885103004/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp:208: On 2017/05/22 04:53:25, yosin_UTC9 wrote: > nit: No need to have an extra blank line. Done.
The CQ bit was checked by hs1217.lee@samsung.com
The patchset sent to the CQ was uploaded after l-g-t-m from yosin@chromium.org Link to the patchset: https://codereview.chromium.org/2885103004/#ps120001 (title: "fix the some nit.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 120001, "attempt_start_ts": 1495445947213930,
"parent_rev": "926c794b0b05b04df5a9e0f675b26a2a6d23d2ad", "commit_rev":
"479179ca87f415cc18ae18682a3938a4486ea281"}
Message was sent while issue was closed.
Description was changed from ========== Rewrite UpdateMarkerRenderedRect() to use EphemeralRange. this patch is last step to replace Range with EphemeralRange in updateMarkerRenderedRect(). we are progressing three steps. [1/3] Make RenderedRectsForMarkers() to ignore disconnected nodes. (https://codereview.chromium.org/2776103002) [2/3] In-place change of Range::textRects() and boundingBox() to EphemeralRange version in "Range.cpp" (https://codereview.chromium.org/2810313003) [3/3] Rewrite UpdateMarkerRenderedRect() to use EphemeralRange instead of Range (this patch) BUG=691202 ========== to ========== Rewrite UpdateMarkerRenderedRect() to use EphemeralRange. this patch is last step to replace Range with EphemeralRange in updateMarkerRenderedRect(). we are progressing three steps. [1/3] Make RenderedRectsForMarkers() to ignore disconnected nodes. (https://codereview.chromium.org/2776103002) [2/3] In-place change of Range::textRects() and boundingBox() to EphemeralRange version in "Range.cpp" (https://codereview.chromium.org/2810313003) [3/3] Rewrite UpdateMarkerRenderedRect() to use EphemeralRange instead of Range (this patch) BUG=691202 Review-Url: https://codereview.chromium.org/2885103004 Cr-Commit-Position: refs/heads/master@{#473534} Committed: https://chromium.googlesource.com/chromium/src/+/479179ca87f415cc18ae18682a39... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:120001) as https://chromium.googlesource.com/chromium/src/+/479179ca87f415cc18ae18682a39... |
