Enable compositing of scrolling content on low-DPI when LCD text will be maintained

Enable compositing of scrolling content on low-DPI when LCD text will be maintained

The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.

R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051
Committed: https://crrev.com/ef06887f125941e96ab19fba834d9027d36d4c88
Cr-Commit-Position: refs/heads/master@{#420955}

[Stephen Chennney, 2016-09-21 19:38:37]

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

[Stephen Chennney, 2016-09-21 19:38:57]

Description was changed from

==========
Enable compositing of scrolling content on low-DPI when LCD text will be
maintained

The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.

I've been running layout tests ocally with the change and no failures. Trybots
will check browser tests.

R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051
==========

to

==========
Enable compositing of scrolling content on low-DPI when LCD text will be
maintained

The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.

I've been running layout tests locally with the change and no failures. Trybots
will check browser tests.

R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051
==========

[commit-bot: I haz the power, 2016-09-21 19:39:09]

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

[Stephen Chennney, 2016-09-21 19:40:08]

Based off a similar change in https://codereview.chromium.org/1988423002/. The
goal is to turn on this feature in official builds but have an opt-out mechanism
that we can access for finch experiments.

[commit-bot: I haz the power, 2016-09-21 19:48:29]

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

[commit-bot: I haz the power, 2016-09-21 19:48:30]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Stephen Chennney, 2016-09-21 20:04:25]

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

[commit-bot: I haz the power, 2016-09-21 20:05:02]

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, 2016-09-21 21:06:38]

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

[commit-bot: I haz the power, 2016-09-21 21:06:39]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Stephen Chennney, 2016-09-22 14:59:14]

On 2016/09/21 19:40:08, Stephen Chennney wrote:
> Based off a similar change in https://codereview.chromium.org/1988423002/. The
> goal is to turn on this feature in official builds but have an opt-out
mechanism
> that we can access for finch experiments.

Ouch. Apparently tests have changed. Investigating.

[Stephen Chennney, 2016-09-23 14:49:21]

On 2016/09/22 14:59:14, Stephen Chennney wrote:
> On 2016/09/21 19:40:08, Stephen Chennney wrote:
> > Based off a similar change in https://codereview.chromium.org/1988423002/.
The
> > goal is to turn on this feature in official builds but have an opt-out
> mechanism
> > that we can access for finch experiments.
> 
> Ouch. Apparently tests have changed. Investigating.

Fixed the asserts so there should not be crashers. Some tests have changed so
getting new baselines. There are 3 classes of test result changes:
1) Minor pixel differences on scrollbar painting due to now compositing the
scroller
2) Changes to layer trees now that we composite the scroller. Have manually
verified the test result is otherwise correct.
3) Some invalidations have grown larger, specifically the scrollbar
invalidation. The corresponding content invalidation is the same, but more
scrollbar is invalidated. I will seek an explanation for this.

[chrishtr, 2016-09-23 15:05:24]

lgtm

I see crashes still in the latest CQ dry run:

ASSERTION FAILED: !m_zOrderListsDirty

Is this fixed yet?

[chrishtr, 2016-09-23 15:10:05]

I did not mean to LGTM just yet.

[Stephen Chennney, 2016-09-23 15:22:13]

On 2016/09/23 15:05:24, chrishtr wrote:
> lgtm
> 
> I see crashes still in the latest CQ dry run:
> 
> ASSERTION FAILED: !m_zOrderListsDirty
> 
> Is this fixed yet?

I understand the inadvertent lgtm.

The dirty z order lists assert was fixed by
https://codereview.chromium.org/2365673002. It's confusing, but only the *_rel
trybots are with that patch, and there are no crashes so far.

I'll put up new baselines once I manage to get try runs on all platforms. A
couple of people need to sign off on the changes to test results.

[Stephen Chennney, 2016-09-23 15:44:29]

Description was changed from

==========
Enable compositing of scrolling content on low-DPI when LCD text will be
maintained

The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.

I've been running layout tests locally with the change and no failures. Trybots
will check browser tests.

R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051
==========

to

==========
Enable compositing of scrolling content on low-DPI when LCD text will be
maintained

The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.

R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051
==========

[Stephen Chennney, 2016-09-23 21:21:05]

New baselines.

For paint/invalidation, wangxianxhu could you look at those?

For layers, flackr could you comment?

The cases where we get float overlay wrong is a known issue that we decided to
live with.

Other tests are minor differences due to compositing.

[Xianzhu, 2016-09-23 21:32:26]

On 2016/09/23 21:21:05, Stephen Chennney wrote:
> New baselines.
> 
> For paint/invalidation, wangxianxhu could you look at those?
> 
> For layers, flackr could you comment?
> 
> The cases where we get float overlay wrong is a known issue that we decided to
> live with.
> 
> Other tests are minor differences due to compositing.

Given the layer tree structure changes are all as expected, the new paint
invalidations look legit. Only one minor thing: in
paint/invalidation/flexbox/scrollbars-changed.html, there seem too many
invalidations of the vertical scrollbar layers.

[flackr, 2016-09-23 22:01:54]

On 2016/09/23 at 21:32:26, wangxianzhu wrote:
> On 2016/09/23 21:21:05, Stephen Chennney wrote:
> > New baselines.
> > 
> > For paint/invalidation, wangxianxhu could you look at those?
> > 
> > For layers, flackr could you comment?
> > 
> > The cases where we get float overlay wrong is a known issue that we decided
to
> > live with.
> > 
> > Other tests are minor differences due to compositing.
> 
> Given the layer tree structure changes are all as expected, the new paint
invalidations look legit. Only one minor thing: in
paint/invalidation/flexbox/scrollbars-changed.html, there seem too many
invalidations of the vertical scrollbar layers.

The layer tree structures look correct.

Looks like there is a problem with
third_party/WebKit/LayoutTests/platform/linux/fast/overflow/overflow-stacking-expected.png

[Stephen Chennney, 2016-09-24 00:01:34]

On 2016/09/23 22:01:54, flackr wrote:
> On 2016/09/23 at 21:32:26, wangxianzhu wrote:
> > On 2016/09/23 21:21:05, Stephen Chennney wrote:
> > > New baselines.
> > > 
> > > For paint/invalidation, wangxianxhu could you look at those?
> > > 
> > > For layers, flackr could you comment?
> > > 
> > > The cases where we get float overlay wrong is a known issue that we
decided
> to
> > > live with.
> > > 
> > > Other tests are minor differences due to compositing.
> > 
> > Given the layer tree structure changes are all as expected, the new paint
> invalidations look legit. Only one minor thing: in
> paint/invalidation/flexbox/scrollbars-changed.html, there seem too many
> invalidations of the vertical scrollbar layers.

I propose we file a bug on this one. I agree it's odd but my guess is that it
reveals an existing problem.
 
> The layer tree structures look correct.
> 
> Looks like there is a problem with
>
third_party/WebKit/LayoutTests/platform/linux/fast/overflow/overflow-stacking-expected.png

This is a known issue. Vollick, chrishtr and myself discussed back when we
originally landed the behind-a-flag version and we decided to accept the failure
because it's the fundamental compositing bug and
--enable-prefer-compositing-to-lcd-text users (Mac, ChromeOS) have always seen
it. fast/overflow/overflow-float-stacking-expected.png also demonstrates the
problem.

[kinuko, 2016-09-25 14:49:11]

lgtm for content/ and platform changes once all breakages are fixed & other
reviewers are happy

[kinuko, 2016-09-25 14:49:21]

https://codereview.chromium.org/2358203002/diff/40001/content/public/common/c...
File content/public/common/content_features.cc (right):

https://codereview.chromium.org/2358203002/diff/40001/content/public/common/c...
content/public/common/content_features.cc:25: base::FEATURE_ENABLED_BY_DEFAULT};
The indent looks off

[Stephen Chennney, 2016-09-26 16:59:38]

New patch with fixed Win7 baseline and indent fixed.

https://codereview.chromium.org/2358203002/diff/40001/content/public/common/c...
File content/public/common/content_features.cc (right):

https://codereview.chromium.org/2358203002/diff/40001/content/public/common/c...
content/public/common/content_features.cc:25: base::FEATURE_ENABLED_BY_DEFAULT};
On 2016/09/25 14:49:21, kinuko (slow) wrote:
> The indent looks off

Done.

[Stephen Chennney, 2016-09-26 17:02:51]

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

[commit-bot: I haz the power, 2016-09-26 17:03:31]

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

[chrishtr, 2016-09-26 18:58:35]

lgtm

[Stephen Chennney, 2016-09-26 19:04:50]

The CQ bit was unchecked by schenney@chromium.org

[Stephen Chennney, 2016-09-26 19:04:50]

The CQ bit was checked by schenney@chromium.org

[Stephen Chennney, 2016-09-26 19:04:51]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org
Link to the patchset: https://codereview.chromium.org/2358203002/#ps60001
(title: "Fix indent and a bad rebaseline.")

[commit-bot: I haz the power, 2016-09-26 19:05:34]

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, 2016-09-26 19:16:26]

Description was changed from

==========
Enable compositing of scrolling content on low-DPI when LCD text will be
maintained

The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.

R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051
==========

to

==========
Enable compositing of scrolling content on low-DPI when LCD text will be
maintained

The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.

R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051
==========

[commit-bot: I haz the power, 2016-09-26 19:16:33]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-26 19:21:41]

Description was changed from

==========
Enable compositing of scrolling content on low-DPI when LCD text will be
maintained

The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.

R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051
==========

to

==========
Enable compositing of scrolling content on low-DPI when LCD text will be
maintained

The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.

R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051

Committed: https://crrev.com/ef06887f125941e96ab19fba834d9027d36d4c88
Cr-Commit-Position: refs/heads/master@{#420955}
==========

[commit-bot: I haz the power, 2016-09-26 19:21:43]

Patchset 4 (id:??) landed as
https://crrev.com/ef06887f125941e96ab19fba834d9027d36d4c88
Cr-Commit-Position: refs/heads/master@{#420955}