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

Issue 264713015: iPhone 5c marketing site repaints incorrectly with layer squashing (Closed)

Created:
6 years, 7 months ago by abarth-chromium
Modified:
6 years, 7 months ago
CC:
blink-reviews
Visibility:
Public.

Description

iPhone 5c marketing site repaints incorrectly with layer squashing In the PaintsIntoGroupedBacking branch of RenderLayerRepainter::setBackingNeedsRepaintInRect, we can mistakenly use LayoutState if the transformedAncestor of the RenderLayer is actually the RenderView. Unforunately, LayoutState is not yet smart enough to handle repaints into compositing containers, which means we'll compute the wrong repaint rect for the squashed layer. This CL adds a LayoutStateDisabler to this code path so that we'll explicitly not use LayoutState. In the future, if we make LayoutState smart enough to handle composited repaints, we'll need to remove this disabler. BUG=369478 R=leviw@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=173333

Patch Set 1 #

Patch Set 2 : Repaint test with bogus results #

Patch Set 3 : Impoved test, results still for failing case #

Patch Set 4 : Now with fix #

Total comments: 4

Patch Set 5 : Moar const #

Unified diffs Side-by-side diffs Delta from patch set Stats (+73 lines, -11 lines) Patch
A LayoutTests/compositing/squashing/repaint-via-layout-offset.html View 1 2 1 chunk +54 lines, -0 lines 0 comments Download
A + LayoutTests/compositing/squashing/repaint-via-layout-offset-expected.txt View 1 2 3 1 chunk +5 lines, -5 lines 0 comments Download
M Source/core/rendering/RenderLayerRepainter.cpp View 1 2 3 1 chunk +9 lines, -1 line 0 comments Download
M Source/core/rendering/RenderView.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M Source/platform/geometry/IntRect.h View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M Source/platform/geometry/IntRect.cpp View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M Source/platform/geometry/LayoutRect.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M Source/platform/geometry/LayoutRect.cpp View 1 2 3 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 9 (0 generated)
abarth-chromium
6 years, 7 months ago (2014-05-05 18:03:09 UTC) #1
abarth-chromium
https://codereview.chromium.org/264713015/diff/50001/Source/platform/geometry/LayoutRect.h File Source/platform/geometry/LayoutRect.h (right): https://codereview.chromium.org/264713015/diff/50001/Source/platform/geometry/LayoutRect.h#newcode174 Source/platform/geometry/LayoutRect.h:174: void show(bool showRawValue = false) const; I can put ...
6 years, 7 months ago (2014-05-05 18:04:03 UTC) #2
chrishtr
I don't know much about LayoutState, will wait to see what Levi says. https://codereview.chromium.org/264713015/diff/50001/Source/platform/geometry/LayoutRect.cpp File ...
6 years, 7 months ago (2014-05-05 18:06:20 UTC) #3
leviw_travelin_and_unemployed
LGTM. Using the disabler here is totally reasonable. https://codereview.chromium.org/264713015/diff/50001/Source/core/rendering/RenderView.h File Source/core/rendering/RenderView.h (right): https://codereview.chromium.org/264713015/diff/50001/Source/core/rendering/RenderView.h#newcode366 Source/core/rendering/RenderView.h:366: LayoutStateDisabler(const ...
6 years, 7 months ago (2014-05-05 18:16:22 UTC) #4
abarth-chromium
https://codereview.chromium.org/264713015/diff/50001/Source/core/rendering/RenderView.h File Source/core/rendering/RenderView.h (right): https://codereview.chromium.org/264713015/diff/50001/Source/core/rendering/RenderView.h#newcode366 Source/core/rendering/RenderView.h:366: LayoutStateDisabler(const RenderObject& root) On 2014/05/05 18:16:23, leviw wrote: > ...
6 years, 7 months ago (2014-05-05 18:17:55 UTC) #5
abarth-chromium
The CQ bit was checked by abarth@chromium.org
6 years, 7 months ago (2014-05-05 18:18:50 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/abarth@chromium.org/264713015/70001
6 years, 7 months ago (2014-05-05 18:19:08 UTC) #7
leviw_travelin_and_unemployed
On 2014/05/05 18:19:08, I haz the power (commit-bot) wrote: > CQ is trying da patch. ...
6 years, 7 months ago (2014-05-05 18:35:41 UTC) #8
commit-bot: I haz the power
6 years, 7 months ago (2014-05-05 19:25:32 UTC) #9
Message was sent while issue was closed.
Change committed as 173333

Powered by Google App Engine
This is Rietveld 408576698