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

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

Created:
1 year, 1 month ago by smcgruer
Modified:
1 year, 1 month 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.
1 year, 1 month 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 ...
1 year, 1 month ago (2017-03-17 17:45:57 UTC) #8
smcgruer
+schenney for OWNERS
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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
1 year, 1 month ago (2017-03-17 21:21:22 UTC) #21
commit-bot: I haz the power
1 year, 1 month 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