New issue
Advanced search Search tips

Issue 868229 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Sanitize DocumentMarkerController for text node

Project Member Reported by yoichio@chromium.org, Jul 27

Issue description

DocumentMarkerController functions should only receive text node rather than Node
and DocumentMarker->endoffset should not go over it's length.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 31

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

commit 202eb7f8e7c0ef6046ecda013edd73671ca9cd12
Author: Yoichi Osato <yoichio@chromium.org>
Date: Tue Jul 31 03:41:05 2018

DCHECK marker end offset in DocumentMarkerController.

This patch introduces a DCHECK checking if DocumentMarker.endoffset is
not over text length when adding each marker.

Bug: 868229
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I4f2681460d1de1aacf78b55d7bba454dfc557cf2
Reviewed-on: https://chromium-review.googlesource.com/1149763
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579307}
[modify] https://crrev.com/202eb7f8e7c0ef6046ecda013edd73671ca9cd12/third_party/blink/renderer/core/editing/markers/document_marker_controller.cc

Owner: yoichio@chromium.org
Status: Started (was: Untriaged)
yoicho@ is working now. http://crrev.com/c/1159548
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 13

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

commit 47d687e9a760b8e5182ab94569e11ba8e468c06e
Author: Yoichi Osato <yoichio@chromium.org>
Date: Mon Aug 13 07:06:02 2018

Refactoring: DocumentMarkerVector::MarkersFor should receive ref than ptr.

This patch makes the function to only receive |const Text&|
rather than |Text*| as other MarkersXXX functions.

Bug: 868229
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I547e35fe6f95d959759b7042cebf81e27aad9d96
Reviewed-on: https://chromium-review.googlesource.com/1168948
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582515}
[modify] https://crrev.com/47d687e9a760b8e5182ab94569e11ba8e468c06e/third_party/blink/renderer/core/editing/commands/split_text_node_command_test.cc
[modify] https://crrev.com/47d687e9a760b8e5182ab94569e11ba8e468c06e/third_party/blink/renderer/core/editing/ime/input_method_controller_test.cc
[modify] https://crrev.com/47d687e9a760b8e5182ab94569e11ba8e468c06e/third_party/blink/renderer/core/editing/markers/document_marker_controller.cc
[modify] https://crrev.com/47d687e9a760b8e5182ab94569e11ba8e468c06e/third_party/blink/renderer/core/editing/markers/document_marker_controller.h
[modify] https://crrev.com/47d687e9a760b8e5182ab94569e11ba8e468c06e/third_party/blink/renderer/core/editing/spellcheck/spell_checker.cc
[modify] https://crrev.com/47d687e9a760b8e5182ab94569e11ba8e468c06e/third_party/blink/renderer/core/editing/suggestion/text_suggestion_controller_test.cc
[modify] https://crrev.com/47d687e9a760b8e5182ab94569e11ba8e468c06e/third_party/blink/renderer/core/exported/web_frame_test.cc
[modify] https://crrev.com/47d687e9a760b8e5182ab94569e11ba8e468c06e/third_party/blink/renderer/core/testing/internals.cc
[modify] https://crrev.com/47d687e9a760b8e5182ab94569e11ba8e468c06e/third_party/blink/renderer/modules/accessibility/ax_node_object.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 16

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

commit 09363365d37cff41e0528bb4a5253cc1868277d5
Author: Yoichi Osato <yoichio@chromium.org>
Date: Thu Aug 16 05:13:09 2018

Refactoring: let DocumentMarkerController::MoveMarkers only receive |const Text&|.

This patch makes the function to only receive |const Text&|
rather than |const Node*| as other DMC functions.

Bug: 868229
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I97f82ff8fcc60bcede866599db154c98e32cc0d4
Reviewed-on: https://chromium-review.googlesource.com/1176893
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583544}
[modify] https://crrev.com/09363365d37cff41e0528bb4a5253cc1868277d5/third_party/blink/renderer/core/editing/commands/split_text_node_command.cc
[modify] https://crrev.com/09363365d37cff41e0528bb4a5253cc1868277d5/third_party/blink/renderer/core/editing/markers/document_marker_controller.cc
[modify] https://crrev.com/09363365d37cff41e0528bb4a5253cc1868277d5/third_party/blink/renderer/core/editing/markers/document_marker_controller.h

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 27

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

commit 4f9fb41c074ae995c01b7d403b0ca59a8766ecf9
Author: Yoichi Osato <yoichio@chromium.org>
Date: Thu Sep 27 15:48:45 2018

Remove unused DocumentMarkerController::HasMarkers()

HasMarkers and its caller InlineTextBoxPainter::PaintsMarkerHighlights is
not called anywhere. Remove them.

Bug: 868229
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I2ad088d8637d461178e7e3af3017c923b6c1f4c3
Reviewed-on: https://chromium-review.googlesource.com/1248282
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594736}
[modify] https://crrev.com/4f9fb41c074ae995c01b7d403b0ca59a8766ecf9/third_party/blink/renderer/core/editing/markers/document_marker_controller.h
[modify] https://crrev.com/4f9fb41c074ae995c01b7d403b0ca59a8766ecf9/third_party/blink/renderer/core/paint/inline_text_box_painter.cc
[modify] https://crrev.com/4f9fb41c074ae995c01b7d403b0ca59a8766ecf9/third_party/blink/renderer/core/paint/inline_text_box_painter.h

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 28

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

commit 17cfcec5ef6f6cce61d2be19cfc18f26459fb5b6
Author: Yoichi Osato <yoichio@chromium.org>
Date: Fri Sep 28 10:02:55 2018

Change DocumentMarkerController::MarkerMap key type to Text

This patch is refactoring.
Since we have only inserted Text node rather than Node(see L263),
we should type the internal hash map key to Text rather than Node.

Bug: 868229
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I582b3b0b5f3090ac94d89dfb99a32f1fd43527d4
Reviewed-on: https://chromium-review.googlesource.com/1248362
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595038}
[modify] https://crrev.com/17cfcec5ef6f6cce61d2be19cfc18f26459fb5b6/third_party/blink/renderer/core/editing/markers/document_marker_controller.cc
[modify] https://crrev.com/17cfcec5ef6f6cce61d2be19cfc18f26459fb5b6/third_party/blink/renderer/core/editing/markers/document_marker_controller.h

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 1

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

commit 19455fec6447b136cb5ba89f18cc2702962d6125
Author: Yoichi Osato <yoichio@chromium.org>
Date: Mon Oct 01 09:55:57 2018

Refactoring: DocumentMarkerVector::RemoveMarkers should receive only Text node.

Since DocumentMarkerController::MarkerMap only contains Text nodes as key,
the removing map entry functions should receive only Text node rather than Node.

Bug: 868229
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Id9b4daa1d116660f13d3c3961d5ed97758ceb01b
Reviewed-on: https://chromium-review.googlesource.com/1244698
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595415}
[modify] https://crrev.com/19455fec6447b136cb5ba89f18cc2702962d6125/third_party/blink/renderer/core/dom/node.cc
[modify] https://crrev.com/19455fec6447b136cb5ba89f18cc2702962d6125/third_party/blink/renderer/core/editing/markers/document_marker_controller.cc
[modify] https://crrev.com/19455fec6447b136cb5ba89f18cc2702962d6125/third_party/blink/renderer/core/editing/markers/document_marker_controller.h
[modify] https://crrev.com/19455fec6447b136cb5ba89f18cc2702962d6125/third_party/blink/renderer/core/editing/markers/document_marker_controller_test.cc
[modify] https://crrev.com/19455fec6447b136cb5ba89f18cc2702962d6125/third_party/blink/renderer/core/editing/spellcheck/spell_checker.cc
[modify] https://crrev.com/19455fec6447b136cb5ba89f18cc2702962d6125/third_party/blink/renderer/core/editing/suggestion/text_suggestion_controller.cc

Sign in to add a comment