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

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

Created:
8 months ago by smcgruer
Modified:
8 months 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

Messages

Total messages: 24 (16 generated)
smcgruer
Found this when poking around PaintLayerCompositor. I'm open to removing the DCHECK if desired.
8 months 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 ...
8 months ago (2017-03-17 17:45:57 UTC) #8
smcgruer
+schenney for OWNERS
8 months 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 ...
8 months 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 ...
8 months 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 ...
8 months 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
8 months ago (2017-03-17 21:21:22 UTC) #21
commit-bot: I haz the power
8 months 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 efc10ee0f