Improve TextFinder |
|
Issue descriptionCurrently TextFinder's structure is a bit confusing. We want to refactor/rewrite TextFinder. See https://docs.google.com/document/d/12Hqv8KCwqq1_aWh0Djx-rvIAgbwdWVS80HkSnYS2RCs/edit?usp=sharing
,
Aug 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d9060444eb79076dfa07cb6bdee84084c4369ed7 commit d9060444eb79076dfa07cb6bdee84084c4369ed7 Author: Rakina Zata Amni <rakina@chromium.org> Date: Wed Aug 29 19:25:20 2018 Add UMA for time over deadline in Find-in-Page Scoping Bug: 868227 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: I4dc042dbcf7e38b71a64d1caa2982110e2796fef Reviewed-on: https://chromium-review.googlesource.com/1193602 Reviewed-by: Steven Holte <holte@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Cr-Commit-Position: refs/heads/master@{#587239} [modify] https://crrev.com/d9060444eb79076dfa07cb6bdee84084c4369ed7/third_party/blink/renderer/core/editing/finder/text_finder.cc [modify] https://crrev.com/d9060444eb79076dfa07cb6bdee84084c4369ed7/tools/metrics/histograms/histograms.xml
,
Oct 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/153bb13190554713cd65d1c48eee5ae69b2a609f commit 153bb13190554713cd65d1c48eee5ae69b2a609f Author: Rakina Zata Amni <rakina@chromium.org> Date: Mon Oct 15 10:06:04 2018 Move Find-in-page scoping to FindTaskController This CL moves find-in-page scoping methods and idle task to a new class, FindTaskController and use the term "finding" instead of "scoping" there. FindTaskController is responsible for managing IdleFindTask, and notifying TextFinder of matches etc, so that TextFinder can manage markers, tickmark, notifying frame etc. Bug: 868227 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: I20701ec0542acb40ace471891a490612e73d700b Reviewed-on: https://chromium-review.googlesource.com/c/1278074 Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Hayato Ito <hayato@chromium.org> Cr-Commit-Position: refs/heads/master@{#599605} [modify] https://crrev.com/153bb13190554713cd65d1c48eee5ae69b2a609f/third_party/blink/renderer/core/DEPS [modify] https://crrev.com/153bb13190554713cd65d1c48eee5ae69b2a609f/third_party/blink/renderer/core/editing/BUILD.gn [add] https://crrev.com/153bb13190554713cd65d1c48eee5ae69b2a609f/third_party/blink/renderer/core/editing/finder/find_task_controller.cc [add] https://crrev.com/153bb13190554713cd65d1c48eee5ae69b2a609f/third_party/blink/renderer/core/editing/finder/find_task_controller.h [modify] https://crrev.com/153bb13190554713cd65d1c48eee5ae69b2a609f/third_party/blink/renderer/core/editing/finder/text_finder.cc [modify] https://crrev.com/153bb13190554713cd65d1c48eee5ae69b2a609f/third_party/blink/renderer/core/editing/finder/text_finder.h
,
Dec 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/71e551ac476ec9b247be8f19c6eb696632d90cf4 commit 71e551ac476ec9b247be8f19c6eb696632d90cf4 Author: Rakina Zata Amni <rakina@chromium.org> Date: Fri Dec 14 16:15:49 2018 Find-in-page per block using FindBuffer This CL introduces FindBuffer, which Find-in-page will use to find matches instead of using FindPlainText that uses TextIterator. Since find-in-page matches can't span multiple blocks, we are using that fact to know where we can separate the text in the document. FindBuffer collects text one block at a time, and then finds the match in that block. We trigger FindBuffer text collection & finding many times until it goes through everything. FindInPage uses TextSearcherICU to find text, and uses flat-tree traversal, ComputedStyle, and LayoutObject + NGOffsetMapping, whereas the previous way was to use TextIterator. This CL also changes the way we handle newlines - newlines will be saved as object replacement character/whitespaces for Find-in-page purposes. Some tests that rely on newlines are deleted as a result. Note: This change is not user-visible because it is not possible to do a Find-in-page from the UI with newlines. See doc for details: https://docs.google.com/document/d/1D5q7ZMrLPfilXnIGI3SOL0mwdiQC7SQEtGWk-9ysbMg/edit?usp=sharing Bug: 868227 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: Id5dd7e6c428f59442d02ec88ea0cec6eb4e64c22 Reviewed-on: https://chromium-review.googlesource.com/c/1282425 Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/master@{#616701} [modify] https://crrev.com/71e551ac476ec9b247be8f19c6eb696632d90cf4/chrome/browser/ui/find_bar/find_bar_host_browsertest.cc [modify] https://crrev.com/71e551ac476ec9b247be8f19c6eb696632d90cf4/chrome/test/data/find_in_page/LongFind.txt [modify] https://crrev.com/71e551ac476ec9b247be8f19c6eb696632d90cf4/third_party/blink/renderer/core/editing/BUILD.gn [add] https://crrev.com/71e551ac476ec9b247be8f19c6eb696632d90cf4/third_party/blink/renderer/core/editing/finder/find_buffer.cc [add] https://crrev.com/71e551ac476ec9b247be8f19c6eb696632d90cf4/third_party/blink/renderer/core/editing/finder/find_buffer.h [add] https://crrev.com/71e551ac476ec9b247be8f19c6eb696632d90cf4/third_party/blink/renderer/core/editing/finder/find_buffer_test.cc [modify] https://crrev.com/71e551ac476ec9b247be8f19c6eb696632d90cf4/third_party/blink/renderer/core/editing/finder/find_task_controller.cc
,
Dec 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/602a22a3a589f4f67bb63669f1ef778530ba5596 commit 602a22a3a589f4f67bb63669f1ef778530ba5596 Author: Rakina Zata Amni <rakina@chromium.org> Date: Fri Dec 14 17:18:50 2018 Remove unnecessary DCHECK for FIP start position The DCHECK is actually faulty, because the start position can be the last position instead in some cases like <hr> TBR=yosin@chromium.org Bug: 868227 Change-Id: I9ee87d4cfd710e07742b9c8896583be060816c56 Reviewed-on: https://chromium-review.googlesource.com/c/1378153 Reviewed-by: Rakina Zata Amni <rakina@chromium.org> Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Cr-Commit-Position: refs/heads/master@{#616719} [modify] https://crrev.com/602a22a3a589f4f67bb63669f1ef778530ba5596/third_party/blink/renderer/core/editing/finder/find_buffer.cc
,
Dec 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9c484a22d01e9482be2997dea811367505556a86 commit 9c484a22d01e9482be2997dea811367505556a86 Author: Elly Fong-Jones <ellyjones@chromium.org> Date: Fri Dec 14 18:34:33 2018 Revert "Find-in-page per block using FindBuffer" This reverts commit 71e551ac476ec9b247be8f19c6eb696632d90cf4. Reason for revert: broke MHTMLGenerationTest.ViewedMHTMLContainsNoStoreContentIfNoCacheControlPolicy on win7 tests dbg: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20Tests%20%28dbg%29%281%29/73554 [7424:304:1214/084019.035:FATAL:find_buffer.cc(27)] Check failed: start_position == PositionInFlatTree::FirstPositionInNode( *start_position.ComputeContainerNode()). Original change's description: > Find-in-page per block using FindBuffer > > This CL introduces FindBuffer, which Find-in-page will use to find > matches instead of using FindPlainText that uses TextIterator. > > Since find-in-page matches can't span multiple blocks, we are using > that fact to know where we can separate the text in the document. > FindBuffer collects text one block at a time, and then finds the match > in that block. We trigger FindBuffer text collection & finding many > times until it goes through everything. FindInPage uses TextSearcherICU > to find text, and uses flat-tree traversal, ComputedStyle, and > LayoutObject + NGOffsetMapping, whereas the previous way was to use > TextIterator. > > This CL also changes the way we handle newlines - newlines will be > saved as object replacement character/whitespaces for Find-in-page > purposes. Some tests that rely on newlines are deleted as a result. > Note: > This change is not user-visible because it is not possible to do a > Find-in-page from the UI with newlines. > > See doc for details: > https://docs.google.com/document/d/1D5q7ZMrLPfilXnIGI3SOL0mwdiQC7SQEtGWk-9ysbMg/edit?usp=sharing > > Bug: 868227 > Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng > Change-Id: Id5dd7e6c428f59442d02ec88ea0cec6eb4e64c22 > Reviewed-on: https://chromium-review.googlesource.com/c/1282425 > Commit-Queue: Rakina Zata Amni <rakina@chromium.org> > Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> > Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> > Cr-Commit-Position: refs/heads/master@{#616701} TBR=ellyjones@chromium.org,yosin@chromium.org,yoichio@chromium.org,kojii@chromium.org,xiaochengh@chromium.org,rakina@chromium.org Change-Id: Ic06a33d1c630bb4423fc0963bc55b6af5e8ef865 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 868227 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Reviewed-on: https://chromium-review.googlesource.com/c/1378407 Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Reviewed-by: Hongchan Choi <hongchan@chromium.org> Commit-Queue: Hongchan Choi <hongchan@chromium.org> Cr-Commit-Position: refs/heads/master@{#616757} [modify] https://crrev.com/9c484a22d01e9482be2997dea811367505556a86/chrome/browser/ui/find_bar/find_bar_host_browsertest.cc [modify] https://crrev.com/9c484a22d01e9482be2997dea811367505556a86/chrome/test/data/find_in_page/LongFind.txt [modify] https://crrev.com/9c484a22d01e9482be2997dea811367505556a86/third_party/blink/renderer/core/editing/BUILD.gn [delete] https://crrev.com/dacd59c9179d75f89a27be539dde8892984612bc/third_party/blink/renderer/core/editing/finder/find_buffer.cc [delete] https://crrev.com/dacd59c9179d75f89a27be539dde8892984612bc/third_party/blink/renderer/core/editing/finder/find_buffer.h [delete] https://crrev.com/dacd59c9179d75f89a27be539dde8892984612bc/third_party/blink/renderer/core/editing/finder/find_buffer_test.cc [modify] https://crrev.com/9c484a22d01e9482be2997dea811367505556a86/third_party/blink/renderer/core/editing/finder/find_task_controller.cc
,
Dec 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0be1119f808d95d053e7368aa3352b7de29ecc87 commit 0be1119f808d95d053e7368aa3352b7de29ecc87 Author: Rakina Zata Amni <rakina@chromium.org> Date: Sat Dec 15 00:37:08 2018 Reland "Find-in-page per block using FindBuffer" This reverts commit 9c484a22d01e9482be2997dea811367505556a86. Reason for revert: <INSERT REASONING HERE> Original change's description: > Revert "Find-in-page per block using FindBuffer" > > This reverts commit 71e551ac476ec9b247be8f19c6eb696632d90cf4. > > Reason for revert: broke MHTMLGenerationTest.ViewedMHTMLContainsNoStoreContentIfNoCacheControlPolicy on win7 tests dbg: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20Tests%20%28dbg%29%281%29/73554 > > [7424:304:1214/084019.035:FATAL:find_buffer.cc(27)] Check failed: start_position == PositionInFlatTree::FirstPositionInNode( *start_position.ComputeContainerNode()). > > Original change's description: > > Find-in-page per block using FindBuffer > > > > This CL introduces FindBuffer, which Find-in-page will use to find > > matches instead of using FindPlainText that uses TextIterator. > > > > Since find-in-page matches can't span multiple blocks, we are using > > that fact to know where we can separate the text in the document. > > FindBuffer collects text one block at a time, and then finds the match > > in that block. We trigger FindBuffer text collection & finding many > > times until it goes through everything. FindInPage uses TextSearcherICU > > to find text, and uses flat-tree traversal, ComputedStyle, and > > LayoutObject + NGOffsetMapping, whereas the previous way was to use > > TextIterator. > > > > This CL also changes the way we handle newlines - newlines will be > > saved as object replacement character/whitespaces for Find-in-page > > purposes. Some tests that rely on newlines are deleted as a result. > > Note: > > This change is not user-visible because it is not possible to do a > > Find-in-page from the UI with newlines. > > > > See doc for details: > > https://docs.google.com/document/d/1D5q7ZMrLPfilXnIGI3SOL0mwdiQC7SQEtGWk-9ysbMg/edit?usp=sharing > > > > Bug: 868227 > > Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng > > Change-Id: Id5dd7e6c428f59442d02ec88ea0cec6eb4e64c22 > > Reviewed-on: https://chromium-review.googlesource.com/c/1282425 > > Commit-Queue: Rakina Zata Amni <rakina@chromium.org> > > Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> > > Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#616701} > > TBR=ellyjones@chromium.org,yosin@chromium.org,yoichio@chromium.org,kojii@chromium.org,xiaochengh@chromium.org,rakina@chromium.org > > Change-Id: Ic06a33d1c630bb4423fc0963bc55b6af5e8ef865 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: 868227 > Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng > Reviewed-on: https://chromium-review.googlesource.com/c/1378407 > Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> > Reviewed-by: Hongchan Choi <hongchan@chromium.org> > Commit-Queue: Hongchan Choi <hongchan@chromium.org> > Cr-Commit-Position: refs/heads/master@{#616757} TBR=ellyjones@chromium.org,yosin@chromium.org,yoichio@chromium.org,kojii@chromium.org,hongchan@chromium.org,xiaochengh@chromium.org,rakina@chromium.org Change-Id: Ib48618ecfb9a0daa55cd0bd86bda4744a324d5cf No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 868227 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Reviewed-on: https://chromium-review.googlesource.com/c/1379086 Reviewed-by: Rakina Zata Amni <rakina@chromium.org> Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Cr-Commit-Position: refs/heads/master@{#616898} [modify] https://crrev.com/0be1119f808d95d053e7368aa3352b7de29ecc87/chrome/browser/ui/find_bar/find_bar_host_browsertest.cc [modify] https://crrev.com/0be1119f808d95d053e7368aa3352b7de29ecc87/chrome/test/data/find_in_page/LongFind.txt [modify] https://crrev.com/0be1119f808d95d053e7368aa3352b7de29ecc87/third_party/blink/renderer/core/editing/BUILD.gn [add] https://crrev.com/0be1119f808d95d053e7368aa3352b7de29ecc87/third_party/blink/renderer/core/editing/finder/find_buffer.cc [add] https://crrev.com/0be1119f808d95d053e7368aa3352b7de29ecc87/third_party/blink/renderer/core/editing/finder/find_buffer.h [add] https://crrev.com/0be1119f808d95d053e7368aa3352b7de29ecc87/third_party/blink/renderer/core/editing/finder/find_buffer_test.cc [modify] https://crrev.com/0be1119f808d95d053e7368aa3352b7de29ecc87/third_party/blink/renderer/core/editing/finder/find_task_controller.cc |
|
►
Sign in to add a comment |
|
Comment 1 by bugdroid1@chromium.org
, Aug 17