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

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Cc:
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

Issue description

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
Components: UI>Browser>FindInPage
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.
Reduced test case
find.html
190 bytes View Download
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.
#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

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)
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

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

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
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