Code cleanup: remove unnecessary conditional guard in PaintLayerCompositor

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

[smcgruer, 2017-03-17 14:59:02]

Description was changed from

==========
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.

Left a DCHECK in place to make the intention clear.

BUG=702616
==========

to

==========
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.

Left a DCHECK in place to make the intention clear.

BUG=702616
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[smcgruer, 2017-03-17 15:00:26]

The CQ bit was checked by smcgruer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 15:00:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-17 17:35:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-17 17:35:38]

Dry run: This issue passed the CQ dry run.

[smcgruer, 2017-03-17 17:37:38]

smcgruer@chromium.org changed reviewers:
+ ajuma@chromium.org, flackr@chromium.org

[smcgruer, 2017-03-17 17:37:39]

Found this when poking around PaintLayerCompositor. I'm open to removing the
DCHECK if desired.

[flackr, 2017-03-17 17:45:57]

LGTM, I think the DCHECK is probably unnecessary given the rest of the function
presumably also requires that the update type is not None as it early returns
otherwise.

[smcgruer, 2017-03-17 18:03:29]

smcgruer@chromium.org changed reviewers:
+ schenney@chromium.org

[smcgruer, 2017-03-17 18:03:30]

+schenney for OWNERS

[smcgruer, 2017-03-17 18:04:13]

The CQ bit was checked by smcgruer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 18:04:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[smcgruer, 2017-03-17 18:29:46]

Description was changed from

==========
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.

Left a DCHECK in place to make the intention clear.

BUG=702616
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
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
==========

[Stephen Chennney, 2017-03-17 18:31:36]

Question:

https://codereview.chromium.org/2756903002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp
(right):

https://codereview.chromium.org/2756903002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:429:
m_scrollLayer->setCompositorMutableProperties(mutableProperties);
This doesn't require a scrollingElement like it did before?

[smcgruer, 2017-03-17 18:38:56]

https://codereview.chromium.org/2756903002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp
(right):

https://codereview.chromium.org/2756903002/diff/20001/third_party/WebKit/Sour...
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 doesn't require a scrollingElement like it did before?

This is still inside that conditional. You are perhaps being misled because
presubmit required me to add braces to the 'if
(scrollElement->hasCompositorProxy())' conditional block, because its body is
too long.

[Stephen Chennney, 2017-03-17 18:47:58]

lgtm

https://codereview.chromium.org/2756903002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp
(right):

https://codereview.chromium.org/2756903002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:429:
m_scrollLayer->setCompositorMutableProperties(mutableProperties);
On 2017/03/17 18:38:56, smcgruer wrote:
> On 2017/03/17 18:31:36, Stephen Chennney wrote:
> > This doesn't require a scrollingElement like it did before?
> 
> This is still inside that conditional. You are perhaps being misled because
> presubmit required me to add braces to the 'if
> (scrollElement->hasCompositorProxy())' conditional block, because its body is
> too long.

Sorry I missed that. Plus one for the style guide, as then I would not have had
to reason about it.

[commit-bot: I haz the power, 2017-03-17 20:37:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-17 20:37:56]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[smcgruer, 2017-03-17 21:20:39]

The CQ bit was checked by smcgruer@chromium.org

[smcgruer, 2017-03-17 21:20:40]

The patchset sent to the CQ was uploaded after l-g-t-m from flackr@chromium.org
Link to the patchset: https://codereview.chromium.org/2756903002/#ps20001
(title: "Remove DCHECK")

[commit-bot: I haz the power, 2017-03-17 21:21:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-18 01:01:43]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489785639321740,
"parent_rev": "e2827154a7f84462ecad9a9319032f1baa127786", "commit_rev":
"255379a40c015ed5a5003ff914c0cce5d0893001"}

[commit-bot: I haz the power, 2017-03-18 01:04:49]

Description was changed from

==========
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
==========

to

==========
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/+/255379a40c015ed5a5003ff914c0...
==========

[commit-bot: I haz the power, 2017-03-18 01:04:50]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/255379a40c015ed5a5003ff914c0...