[css-align] Check runtime flags for align-items 'auto' value resolution

[css-align] Check runtime flags for align-items 'auto' value resolution

The StyleAdjuster::adjustStyleForAlignment logic is optimized to avoid
resolving 'auto' values on alignment properties using its parent's
align-items default value. This way we can minimize the copy-on-write
overhead, since 'auto' is the default value of some CSS Alignment
properties.

However, align-items default value is different depending on whether the
CSS Grid Layout feature is enabled in runtime flags or not. We were not
checking out the runtime flags when applying the optimization mentioned
before, hence, when Grid is disabled, we are overwriting all the
align-self properties with 'auto' values (the default) with its parent's
align-items default value (stretch). This was verified as the root cause
of the performance regression reported in bug 642263.

This patch solves the issue by using the proper default value, depending
on the runtime flags, when evaluating whether resolving the align-self's
'auto' values or not.

BUG=642263
Committed: https://crrev.com/bab582c0efeee11acecc6754e91d1af982044bd1
Cr-Commit-Position: refs/heads/master@{#415795}

[jfernandez, 2016-08-31 13:45:36]

Description was changed from

==========
[css-align] Check runtime flags for align-items 'auto' value resolution

The StyleAdjuster::adjustStyleForAlignment logic is optimized to avoid
resolving 'auto' values on alignment properties using its parent's
align-items default value. This way we can minimize the copy-on-write
overhead, since 'auto' is the default value of some CSS Alignment
properties.

However, align-items default value is different depending on whether the
CSS Grid Layout feature is enabled in runtime flags or not. We were not
checking out the runtime flags when applying the optimization mentioned
before, hence, when Grid is disabled, we are overwriting all the
align-self properties with 'auto' values (the default) with its parent's
align-items default value (stretch). This was verified as the root cause
of the performance regression reported in bug 642263.

This patch solves the issue by using the proper default value, depending
on the runtime flags, when evaluating whether resolving the align-self's
'auto' values or not.

BUG=642263
==========

to

==========
[css-align] Check runtime flags for align-items 'auto' value resolution

The StyleAdjuster::adjustStyleForAlignment logic is optimized to avoid
resolving 'auto' values on alignment properties using its parent's
align-items default value. This way we can minimize the copy-on-write
overhead, since 'auto' is the default value of some CSS Alignment
properties.

However, align-items default value is different depending on whether the
CSS Grid Layout feature is enabled in runtime flags or not. We were not
checking out the runtime flags when applying the optimization mentioned
before, hence, when Grid is disabled, we are overwriting all the
align-self properties with 'auto' values (the default) with its parent's
align-items default value (stretch). This was verified as the root cause
of the performance regression reported in bug 642263.

This patch solves the issue by using the proper default value, depending
on the runtime flags, when evaluating whether resolving the align-self's
'auto' values or not.

BUG=642263
==========

[jfernandez, 2016-08-31 13:45:37]

jfernandez@igalia.com changed reviewers:
+ cbiesinger@chromium.org, eae@chromium.org, rune@opera.com, timloh@chromium.org

[jfernandez, 2016-08-31 13:45:49]

Sent for review.

[cbiesinger, 2016-08-31 15:09:19]

lgtm, but the layout test failure on windows may be real. (also note that the
_dbg bots do not run layout tests)

[rune, 2016-08-31 15:33:40]

On 2016/08/31 15:09:19, cbiesinger wrote:
> lgtm, but the layout test failure on windows may be real. (also note that the
> _dbg bots do not run layout tests)

Yes, the failure looks like it's caused by this change.

[jfernandez, 2016-08-31 15:54:37]

On 2016/08/31 15:33:40, rune wrote:
> On 2016/08/31 15:09:19, cbiesinger wrote:
> > lgtm, but the layout test failure on windows may be real. (also note that
the
> > _dbg bots do not run layout tests)
> 
> Yes, the failure looks like it's caused by this change.

I'm not sure about all the tests failures, but the fieldset ones may be related
to
the following issue:

https://crrev.com/2302463002/

[cbiesinger, 2016-08-31 15:56:48]

On 2016/08/31 15:54:37, jfernandez wrote:
> On 2016/08/31 15:33:40, rune wrote:
> > On 2016/08/31 15:09:19, cbiesinger wrote:
> > > lgtm, but the layout test failure on windows may be real. (also note that
> the
> > > _dbg bots do not run layout tests)
> > 
> > Yes, the failure looks like it's caused by this change.
> 
> I'm not sure about all the tests failures, but the fieldset ones may be
related
> to
> the following issue:
> 
> https://crrev.com/2302463002/

I was thinking of
fast/alignment/ensure-flexbox-compatibility-with-initial-values.html -- see the
bottom under "retry summary"

[jfernandez, 2016-08-31 16:07:14]

On 2016/08/31 15:56:48, cbiesinger wrote:
> On 2016/08/31 15:54:37, jfernandez wrote:
> > On 2016/08/31 15:33:40, rune wrote:
> > > On 2016/08/31 15:09:19, cbiesinger wrote:
> > > > lgtm, but the layout test failure on windows may be real. (also note
that
> > the
> > > > _dbg bots do not run layout tests)
> > > 
> > > Yes, the failure looks like it's caused by this change.
> > 
> > I'm not sure about all the tests failures, but the fieldset ones may be
> related
> > to
> > the following issue:
> > 
> > https://crrev.com/2302463002/
> 
> I was thinking of
> fast/alignment/ensure-flexbox-compatibility-with-initial-values.html -- see
the
> bottom under "retry summary"

Umm, yes, that might be related. I'll investigate it.

[eae, 2016-08-31 16:07:49]

LGTM

[jfernandez, 2016-08-31 19:06:50]

Patchset #2 (id:20001) has been deleted

[jfernandez, 2016-08-31 21:43:24]

The CQ bit was checked by jfernandez@igalia.com

[jfernandez, 2016-08-31 21:43:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
cbiesinger@chromium.org, eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2298613004/#ps40001
(title: "Using Default-Alignment initial value instead of a harcode default
value.")

[commit-bot: I haz the power, 2016-08-31 21:43:58]

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-08-31 23:09:34]

Description was changed from

==========
[css-align] Check runtime flags for align-items 'auto' value resolution

The StyleAdjuster::adjustStyleForAlignment logic is optimized to avoid
resolving 'auto' values on alignment properties using its parent's
align-items default value. This way we can minimize the copy-on-write
overhead, since 'auto' is the default value of some CSS Alignment
properties.

However, align-items default value is different depending on whether the
CSS Grid Layout feature is enabled in runtime flags or not. We were not
checking out the runtime flags when applying the optimization mentioned
before, hence, when Grid is disabled, we are overwriting all the
align-self properties with 'auto' values (the default) with its parent's
align-items default value (stretch). This was verified as the root cause
of the performance regression reported in bug 642263.

This patch solves the issue by using the proper default value, depending
on the runtime flags, when evaluating whether resolving the align-self's
'auto' values or not.

BUG=642263
==========

to

==========
[css-align] Check runtime flags for align-items 'auto' value resolution

The StyleAdjuster::adjustStyleForAlignment logic is optimized to avoid
resolving 'auto' values on alignment properties using its parent's
align-items default value. This way we can minimize the copy-on-write
overhead, since 'auto' is the default value of some CSS Alignment
properties.

However, align-items default value is different depending on whether the
CSS Grid Layout feature is enabled in runtime flags or not. We were not
checking out the runtime flags when applying the optimization mentioned
before, hence, when Grid is disabled, we are overwriting all the
align-self properties with 'auto' values (the default) with its parent's
align-items default value (stretch). This was verified as the root cause
of the performance regression reported in bug 642263.

This patch solves the issue by using the proper default value, depending
on the runtime flags, when evaluating whether resolving the align-self's
'auto' values or not.

BUG=642263
==========

[commit-bot: I haz the power, 2016-08-31 23:09:37]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-08-31 23:12:41]

Description was changed from

==========
[css-align] Check runtime flags for align-items 'auto' value resolution

The StyleAdjuster::adjustStyleForAlignment logic is optimized to avoid
resolving 'auto' values on alignment properties using its parent's
align-items default value. This way we can minimize the copy-on-write
overhead, since 'auto' is the default value of some CSS Alignment
properties.

However, align-items default value is different depending on whether the
CSS Grid Layout feature is enabled in runtime flags or not. We were not
checking out the runtime flags when applying the optimization mentioned
before, hence, when Grid is disabled, we are overwriting all the
align-self properties with 'auto' values (the default) with its parent's
align-items default value (stretch). This was verified as the root cause
of the performance regression reported in bug 642263.

This patch solves the issue by using the proper default value, depending
on the runtime flags, when evaluating whether resolving the align-self's
'auto' values or not.

BUG=642263
==========

to

==========
[css-align] Check runtime flags for align-items 'auto' value resolution

The StyleAdjuster::adjustStyleForAlignment logic is optimized to avoid
resolving 'auto' values on alignment properties using its parent's
align-items default value. This way we can minimize the copy-on-write
overhead, since 'auto' is the default value of some CSS Alignment
properties.

However, align-items default value is different depending on whether the
CSS Grid Layout feature is enabled in runtime flags or not. We were not
checking out the runtime flags when applying the optimization mentioned
before, hence, when Grid is disabled, we are overwriting all the
align-self properties with 'auto' values (the default) with its parent's
align-items default value (stretch). This was verified as the root cause
of the performance regression reported in bug 642263.

This patch solves the issue by using the proper default value, depending
on the runtime flags, when evaluating whether resolving the align-self's
'auto' values or not.

BUG=642263

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

[commit-bot: I haz the power, 2016-08-31 23:12:42]

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