New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 619633 link

Starred by 1 user
Status: Fixed
Owner:
wangxianzhu@chromium.org
Closed: Jun 2016
Cc:
█ ranjitkan@chromium.org
vmp...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug

Blocking:
issue 619103



Sign in to add a comment

Paint invalidation issues with find-in-page

Project Member Reported by jbroman@chromium.org, Jun 13 2016 
Repros at both stable (M51) and ToT. Orange (currently-focused) and yellow (other found) highlights are not correctly invalidated when using the find-in-page (Ctrl+F) feature.

Occurs on other pages, but here is one set of steps that currently works:

1. Navigate to http://www.macrumors.com/roundup/wwdc/.
2. Ctrl+F, "Apple". Press Ctrl+G repeatedly until the issue is observed.
findinpage.png
85.2 KB View Download

Comment 1 by jbroman@chromium.org, Jun 13 2016

Components: UI>Browser>FindInPage

Comment 2 by wangxianzhu@chromium.org, Jun 13 2016

Cc: -wangxianzhu@chromium.org
Labels: -Pri-3 Pri-1
Owner: wangxianzhu@chromium.org
Status: Assigned (was: Untriaged)
Bisected to:

https://chromium.googlesource.com/chromium/src/+log/5a49cbd778aef637c586a8ebae8bb2f952d30a09..93b3bb34672a3e5f4a319ba422b425eaa484a92b

It might be https://chromium.googlesource.com/chromium/src/+/23db47f341747ea0a4e176e3047fc9f8650c07f5 (partial-raster) that exposed this under-invalidation.

Comment 3 by wangxianzhu@chromium.org, Jun 13 2016 

Reduced test case
find.html
190 bytes View Download

Comment 4 by wangxianzhu@chromium.org, Jun 13 2016

Blocking: 619103
Labels: M-52
LayoutText is inconsistent about whether to include the line spacing above the first line in different functions:
- included in LayoutText::absoluteRects();
- not included in LayoutText::localOverflowRectForPaintInvalidation().

We use the former for document marker rects, but when the marker changes we use the latter to invalidate, causing painting exceeds invalidation.

Comment 5 by wangxianzhu@chromium.org, Jun 13 2016 

#4 is wrong. The problem is actually in LayoutTextBoxPainter::paintTextMatchMarkerBackground() which uses selection bounds to paint document marker which may exceed the paint invalidation rect.
Project Member

Comment 6 by sheriffbot@chromium.org, Jun 14 2016

Labels: -M-52 M-53 MovedFrom-52
Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 16 2016 

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

commit 889581a0249d4ccddcb7d85d3c45e54c2af93a48
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Thu Jun 16 03:20:54 2016

Don't use selection bounds to paint document marker

If there is no selection, selection bounds may exceed the paint
invalidation rect.

The original reason of using selection bounds to paint document
marker "when the selection and this highlight are on the same word
there are no pieces sticking out" no longer applies because we don't
allow selection and document markers at the same time.

BUG= 619633 

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

[modify] https://crrev.com/889581a0249d4ccddcb7d85d3c45e54c2af93a48/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/889581a0249d4ccddcb7d85d3c45e54c2af93a48/third_party/WebKit/LayoutTests/paint/text/text-match-highlights-big-line-height-expected.html
[add] https://crrev.com/889581a0249d4ccddcb7d85d3c45e54c2af93a48/third_party/WebKit/LayoutTests/paint/text/text-match-highlights-big-line-height.html
[modify] https://crrev.com/889581a0249d4ccddcb7d85d3c45e54c2af93a48/third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp

Comment 8 by wangxianzhu@chromium.org, Jun 16 2016

Labels: -M-53 Merge-Request-52 M-52

Comment 9 by tin...@google.com, Jun 17 2016

Labels: -Merge-Request-52 Merge-Approved-52 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M52 (branch: 2743)

Comment 10 by wangxianzhu@chromium.org, Jun 17 2016

Status: Fixed (was: Assigned)
Project Member

Comment 11 by bugdroid1@chromium.org, Jun 17 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/136821a59dcfb5650b3a467fafd8fcfb7ec7f49f

commit 136821a59dcfb5650b3a467fafd8fcfb7ec7f49f
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Fri Jun 17 02:38:22 2016

Don't use selection bounds to paint document marker

If there is no selection, selection bounds may exceed the paint
invalidation rect.

The original reason of using selection bounds to paint document
marker "when the selection and this highlight are on the same word
there are no pieces sticking out" no longer applies because we don't
allow selection and document markers at the same time.

BUG= 619633 

Review URL: https://codereview.chromium.org/2073993002 .

Review-Url: https://codereview.chromium.org/2065723002
Cr-Original-Commit-Position: refs/heads/master@{#400080}
Cr-Commit-Position: refs/branch-heads/2743@{#376}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/136821a59dcfb5650b3a467fafd8fcfb7ec7f49f/third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 17 2016 

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

commit 43df003ba3e7c4660ecd964d87cae61da6181b6e
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Fri Jun 17 05:17:34 2016

NeedsRebaseline for  crbug.com/619633 

BUG= 619633 

Review URL: https://codereview.chromium.org/2071253002 .

Cr-Commit-Position: refs/heads/master@{#400363}

[modify] https://crrev.com/43df003ba3e7c4660ecd964d87cae61da6181b6e/third_party/WebKit/LayoutTests/TestExpectations
Project Member

Comment 13 by bugdroid1@chromium.org, Jun 17 2016 

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

commit 76b3ae31af95c378b33f1f5bd26726332e3abf34
Author: Rebaseline Bot <blink-rebaseline-bot@chromium.org>
Date: Fri Jun 17 06:37:47 2016

Auto-rebaseline for r400363

https://chromium.googlesource.com/chromium/src/+/43df003ba

BUG= 619633 
TBR=wangxianzhu@chromium.org

Review URL: https://codereview.chromium.org/2071033003 .

Cr-Commit-Position: refs/heads/master@{#400382}

[modify] https://crrev.com/76b3ae31af95c378b33f1f5bd26726332e3abf34/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/76b3ae31af95c378b33f1f5bd26726332e3abf34/third_party/WebKit/LayoutTests/platform/android/compositing/overflow/text-match-highlight-expected.png
[add] https://crrev.com/76b3ae31af95c378b33f1f5bd26726332e3abf34/third_party/WebKit/LayoutTests/platform/android/virtual/prefer_compositing_to_lcd_text/compositing/overflow/text-match-highlight-expected.png
[add] https://crrev.com/76b3ae31af95c378b33f1f5bd26726332e3abf34/third_party/WebKit/LayoutTests/platform/linux-precise/svg/custom/text-match-highlight-expected.png
[modify] https://crrev.com/76b3ae31af95c378b33f1f5bd26726332e3abf34/third_party/WebKit/LayoutTests/platform/linux/compositing/overflow/text-match-highlight-expected.png
[modify] https://crrev.com/76b3ae31af95c378b33f1f5bd26726332e3abf34/third_party/WebKit/LayoutTests/platform/linux/svg/custom/text-match-highlight-expected.png
[modify] https://crrev.com/76b3ae31af95c378b33f1f5bd26726332e3abf34/third_party/WebKit/LayoutTests/platform/linux/virtual/prefer_compositing_to_lcd_text/compositing/overflow/text-match-highlight-expected.png
[modify] https://crrev.com/76b3ae31af95c378b33f1f5bd26726332e3abf34/third_party/WebKit/LayoutTests/platform/win/compositing/overflow/text-match-highlight-expected.png
[modify] https://crrev.com/76b3ae31af95c378b33f1f5bd26726332e3abf34/third_party/WebKit/LayoutTests/platform/win/svg/custom/text-match-highlight-expected.png
[modify] https://crrev.com/76b3ae31af95c378b33f1f5bd26726332e3abf34/third_party/WebKit/LayoutTests/platform/win/virtual/prefer_compositing_to_lcd_text/compositing/overflow/text-match-highlight-expected.png
[modify] https://crrev.com/76b3ae31af95c378b33f1f5bd26726332e3abf34/third_party/WebKit/LayoutTests/platform/win7/compositing/overflow/text-match-highlight-expected.png
[modify] https://crrev.com/76b3ae31af95c378b33f1f5bd26726332e3abf34/third_party/WebKit/LayoutTests/platform/win7/svg/custom/text-match-highlight-expected.png
[modify] https://crrev.com/76b3ae31af95c378b33f1f5bd26726332e3abf34/third_party/WebKit/LayoutTests/platform/win7/virtual/prefer_compositing_to_lcd_text/compositing/overflow/text-match-highlight-expected.png

Comment 14 by ranjitkan@chromium.org, Jun 17 2016

Cc: ranjitkan@chromium.org
Labels: TE-Verified-M53 TE-Verified-53.0.2770.0
Rechecked this on Chrome canary version 53.0.2770.0 on Windows 7, MAC 10.11.5 and dev version# 53.0.2770.0 on Ubuntu 14.04. Fix is working as intended. Adding TE-Verified labels

Comment 15 by msrchandra@chromium.org, Jun 22 2016

Labels: TE-Verified-M52 TE-Verified-52.0.2743.49
Verified the issue on Chrome Beta# 52.0.2743.49 on Windows, Mac and Linux and is working on intended. Hence adding TE-Verified Labels.

Attaching a screenshot for reference.
Thank You.
619633.jpg
119 KB View Download
►

Sign in to add a comment