Replace SPv2 check with DCHECK when it is always false

Replace SPv2 check with DCHECK when it is always false

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

Review-Url: https://codereview.chromium.org/2749023004
Cr-Commit-Position: refs/heads/master@{#457675}
Committed: https://chromium.googlesource.com/chromium/src/+/28fbcba6865441b3c83be87bbadc71c726a3d190

[xing.xu, 2017-03-15 03:07:09]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[xing.xu, 2017-03-15 03:07:22]

Description was changed from

==========
Delete SPV2 check when it is always false

BUG=None
==========

to

==========
Delete SPV2 check when it is always false

BUG=None
==========

[xing.xu, 2017-03-15 03:07:22]

xing.xu@intel.com changed reviewers:
+ dcheng@chromium.org

[commit-bot: I haz the power, 2017-03-15 03:07:34]

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-15 04:40:04]

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

[commit-bot: I haz the power, 2017-03-15 04:40:04]

Dry run: This issue passed the CQ dry run.

[xing.xu, 2017-03-15 05:41:49]

PTAL

[dcheng, 2017-03-15 09:18:41]

dcheng@chromium.org changed reviewers:
+ pdr@chromium.org

[dcheng, 2017-03-15 09:18:42]

Is this because slimming paint v2 is always enabled now so we're cleaning up
these checks? The CL description could use some clarification.

In any case, someone on the paint team is better qualified for reviewing this.
+pdr

[xing.xu, 2017-03-15 13:42:39]

On 2017/03/15 09:18:42, dcheng wrote:
> Is this because slimming paint v2 is always enabled now so we're cleaning up
> these checks? The CL description could use some clarification.
> 
> In any case, someone on the paint team is better qualified for reviewing this.
> +pdr
I am not sure if SPv2 is always enabled.
I just found that FrameView::paintGraphicsLayerRecursively is called when
!RuntimeEnabledFeatures::slimmingPaintV2Enabled()
at
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/Fra....
So I think no need to check RuntimeEnabledFeatures::slimmingPaintV2Enabled()
inside FrameView::paintGraphicsLayerRecursively.

[dcheng, 2017-03-15 20:31:30]

On 2017/03/15 13:42:39, xing.xu wrote:
> On 2017/03/15 09:18:42, dcheng wrote:
> > Is this because slimming paint v2 is always enabled now so we're cleaning up
> > these checks? The CL description could use some clarification.
> > 
> > In any case, someone on the paint team is better qualified for reviewing
this.
> > +pdr
> I am not sure if SPv2 is always enabled.
> I just found that FrameView::paintGraphicsLayerRecursively is called when
> !RuntimeEnabledFeatures::slimmingPaintV2Enabled()
> at
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/Fra....
> So I think no need to check RuntimeEnabledFeatures::slimmingPaintV2Enabled()
> inside FrameView::paintGraphicsLayerRecursively.

OK, I would suggest changing this to a DCHECK at the top of the function then.

[pdr., 2017-03-15 22:00:04]

On 2017/03/15 at 20:31:30, dcheng wrote:
> On 2017/03/15 13:42:39, xing.xu wrote:
> > On 2017/03/15 09:18:42, dcheng wrote:
> > > Is this because slimming paint v2 is always enabled now so we're cleaning
up
> > > these checks? The CL description could use some clarification.
> > > 
> > > In any case, someone on the paint team is better qualified for reviewing
this.
> > > +pdr
> > I am not sure if SPv2 is always enabled.
> > I just found that FrameView::paintGraphicsLayerRecursively is called when
> > !RuntimeEnabledFeatures::slimmingPaintV2Enabled()
> > at
> >
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/Fra....
> > So I think no need to check RuntimeEnabledFeatures::slimmingPaintV2Enabled()
> > inside FrameView::paintGraphicsLayerRecursively.
> 
> OK, I would suggest changing this to a DCHECK at the top of the function then.

+1 to dcheng's suggestion

[xing.xu, 2017-03-16 00:31:59]

On 2017/03/15 22:00:04, pdr. wrote:
> On 2017/03/15 at 20:31:30, dcheng wrote:
> > On 2017/03/15 13:42:39, xing.xu wrote:
> > > On 2017/03/15 09:18:42, dcheng wrote:
> > > > Is this because slimming paint v2 is always enabled now so we're
cleaning
> up
> > > > these checks? The CL description could use some clarification.
> > > > 
> > > > In any case, someone on the paint team is better qualified for reviewing
> this.
> > > > +pdr
> > > I am not sure if SPv2 is always enabled.
> > > I just found that FrameView::paintGraphicsLayerRecursively is called when
> > > !RuntimeEnabledFeatures::slimmingPaintV2Enabled()
> > > at
> > >
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/Fra....
> > > So I think no need to check
RuntimeEnabledFeatures::slimmingPaintV2Enabled()
> > > inside FrameView::paintGraphicsLayerRecursively.
> > 
> > OK, I would suggest changing this to a DCHECK at the top of the function
then.
> 
> +1 to dcheng's suggestion

Done, PTAL, thanks!

[xing.xu, 2017-03-16 00:32:10]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-16 00:33:24]

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

[xing.xu, 2017-03-16 00:35:37]

Description was changed from

==========
Delete SPV2 check when it is always false

BUG=None
==========

to

==========
Replace SPv2 check with DCHECK when it is always false

BUG=None
==========

[commit-bot: I haz the power, 2017-03-16 02:01:48]

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

[commit-bot: I haz the power, 2017-03-16 02:01:50]

Dry run: This issue passed the CQ dry run.

[pdr., 2017-03-16 20:36:55]

Description was changed from

==========
Replace SPv2 check with DCHECK when it is always false

BUG=None
==========

to

==========
Replace SPv2 check with DCHECK when it is always false

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

[pdr., 2017-03-16 20:37:27]

LGTM

[xing.xu, 2017-03-17 00:25:25]

The CQ bit was checked by xing.xu@intel.com

[commit-bot: I haz the power, 2017-03-17 00:26:12]

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 03:36:42]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489710325412680,
"parent_rev": "41b5fdf37917746450bd1b89127f98b80a8f66cf", "commit_rev":
"28fbcba6865441b3c83be87bbadc71c726a3d190"}

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

Description was changed from

==========
Replace SPv2 check with DCHECK when it is always false

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

to

==========
Replace SPv2 check with DCHECK when it is always false

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

Review-Url: https://codereview.chromium.org/2749023004
Cr-Commit-Position: refs/heads/master@{#457675}
Committed:
https://chromium.googlesource.com/chromium/src/+/28fbcba6865441b3c83be87bbadc...
==========

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

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