Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(118)

Issue 2826273005: Don't include descendants into LocalVisualRect if a box has both mask and overflow clip (Closed)

Created:
3 years, 8 months ago by Xianzhu
Modified:
3 years, 8 months ago
Reviewers:
chrishtr
CC:
blink-reviews, blink-reviews-layout_chromium.org, chromium-reviews, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, szager+layoutwatch_chromium.org, zoltan1
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Don't include descendants into LocalVisualRect if a box has both mask and overflow clip This avoids unnecessary paint invalidation when the box's descedants change visual overflow which doesn't affect the box's local visual rect because of the overflow clip. About the bug: the page contains the following HTML: <div style="overflow: hidden; -webkit-mask-image: url(#a)> <div class="animating rotation transform"></div> </div> The animating child changes the container's layout overflow. https://codereview.chromium.org/2791933003/ fixed an under-invalidation issue when layout overflow changes, but unnecessarily invalidated the container because previously LayoutBox::LocalVisualRect() also included descendant visual overflow if there is a mask regardless of overflow clip. BUG=709054 Review-Url: https://codereview.chromium.org/2826273005 Cr-Commit-Position: refs/heads/master@{#466243} Committed: https://chromium.googlesource.com/chromium/src/+/1def26285e4b62c398bb800dadeeecc566a41900

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+34 lines, -1 line) Patch
M third_party/WebKit/Source/core/layout/LayoutBox.cpp View 1 chunk +3 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutBoxTest.cpp View 1 chunk +31 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (8 generated)
Xianzhu
3 years, 8 months ago (2017-04-21 01:06:04 UTC) #3
chrishtr
lgtm
3 years, 8 months ago (2017-04-21 01:32:23 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2826273005/1
3 years, 8 months ago (2017-04-21 03:08:10 UTC) #9
commit-bot: I haz the power
3 years, 8 months ago (2017-04-21 03:12:03 UTC) #12
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/1def26285e4b62c398bb800dadee...

Powered by Google App Engine
This is Rietveld 408576698