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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 1 week ago by smcgruer
Modified:
2 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

Messages

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

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 650457f06