Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(109)

Issue 2756903002: Code cleanup: remove unnecessary conditional guard in PaintLayerCompositor (Closed)

Created:
10 months, 1 week ago by smcgruer
Modified:
10 months, 1 week ago
CC:
chromium-reviews, pdr+renderingwatchlist_chromium.org, zoltan1, blink-reviews-layout_chromium.org, szager+layoutwatch_chromium.org, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, blink-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Code cleanup: remove unnecessary conditional guard in PaintLayerCompositor We early-exit for CompositingUpdateNone on line 375, and the only change to updateType after that is line 410 where it is at most increased. Therefore, with the current code the updateType can't be none at line 417. BUG=702616 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2756903002 Cr-Commit-Position: refs/heads/master@{#457913} Committed: https://chromium.googlesource.com/chromium/src/+/255379a40c015ed5a5003ff914c0cce5d0893001

Patch Set 1 #

Patch Set 2 : Remove DCHECK #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -21 lines) Patch
M third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp View 1 1 chunk +20 lines, -21 lines 3 comments Download

Messages

Total messages: 24 (16 generated)
smcgruer
Found this when poking around PaintLayerCompositor. I'm open to removing the DCHECK if desired.
10 months, 1 week ago (2017-03-17 17:37:39 UTC) #7
flackr
LGTM, I think the DCHECK is probably unnecessary given the rest of the function presumably ...
10 months, 1 week ago (2017-03-17 17:45:57 UTC) #8
smcgruer
+schenney for OWNERS
10 months, 1 week ago (2017-03-17 18:03:30 UTC) #10
Stephen Chennney
Question: https://codereview.chromium.org/2756903002/diff/20001/third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp File third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp (right): https://codereview.chromium.org/2756903002/diff/20001/third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp#newcode429 third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:429: m_scrollLayer->setCompositorMutableProperties(mutableProperties); This doesn't require a scrollingElement like it ...
10 months, 1 week ago (2017-03-17 18:31:36 UTC) #14
smcgruer
https://codereview.chromium.org/2756903002/diff/20001/third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp File third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp (right): https://codereview.chromium.org/2756903002/diff/20001/third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp#newcode429 third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:429: m_scrollLayer->setCompositorMutableProperties(mutableProperties); On 2017/03/17 18:31:36, Stephen Chennney wrote: > This ...
10 months, 1 week ago (2017-03-17 18:38:56 UTC) #15
Stephen Chennney
lgtm https://codereview.chromium.org/2756903002/diff/20001/third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp File third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp (right): https://codereview.chromium.org/2756903002/diff/20001/third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp#newcode429 third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:429: m_scrollLayer->setCompositorMutableProperties(mutableProperties); On 2017/03/17 18:38:56, smcgruer wrote: > On ...
10 months, 1 week ago (2017-03-17 18:47:58 UTC) #16
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/2756903002/20001
10 months, 1 week ago (2017-03-17 21:21:22 UTC) #21
commit-bot: I haz the power
10 months, 1 week ago (2017-03-18 01:04:50 UTC) #24
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/255379a40c015ed5a5003ff914c0...

Powered by Google App Engine
This is Rietveld 408576698