Chromium Code Reviews
Help | Chromium Project | Sign in
(7)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 6 days ago by smcgruer
Modified:
1 week, 5 days 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.
1 week, 5 days 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 week, 5 days ago (2017-03-17 17:45:57 UTC) #8
smcgruer
+schenney for OWNERS
1 week, 5 days 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 week, 5 days 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 week, 5 days 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 week, 5 days 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 week, 5 days ago (2017-03-17 21:21:22 UTC) #21
commit-bot: I haz the power
1 week, 5 days 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 cc6ac46