[css-align] Avoid the style Reattach whenever align-items changes.

[css-align] Avoid the style Reattach whenever align-items changes.

The align-items property defines the default beahvior of children boxes
with 'auto' values on the align-self property.

Since the spec states that the computed style of these properties must
be resolved, using different values depending on the kind of element, we
have implemented this style resolution using the StyleAdjuster logic.

Because of this approach in the implementation, when the value of the
align-items property changes we have to recalculate the style tree,
because some of the 'auto' values have been resolved already to the
previous value. Hence, we need to trigger again the style resolution
logic, via the StyleAdjustment, which requires the style changes to
force a Reattach.

There are several problems with this approach. One of them are the
excessive number of style and paint invalidations when any of this
alignment properties change. Another problem, which is the one
reported in the bug this patch address, is that it prevents animations
and transitions to be executed when they involve changes in the
alignment properties.

BUG=580070
Committed: https://crrev.com/89b4a2addd0e3e82a1ca8213d31323fdc2e6395a
Cr-Commit-Position: refs/heads/master@{#372666}

[jfernandez, 2016-01-28 22:35:29]

jfernandez@igalia.com changed reviewers:
+ cbiesinger@chromium.org, esprehn@chromium.org, svillar@igalia.com

[esprehn, 2016-01-29 04:20:56]

lgtm w/ a TODO to fix the broken-ness in shadow dom, and also switching to an
instance method.

https://codereview.chromium.org/1625993004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
(right):

https://codereview.chromium.org/1625993004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp:1613: const
ComputedStyle* parentStyle = styledNode->parentNode() ?
styledNode->parentNode()->ensureComputedStyle() : nullptr;
You need to use LayoutTreeBuilderTraversal::parent here, this is broken for
shadow dom. See Node::parentComputedStyle.

I guess this code was already broken, please add a TODO here.

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

https://codereview.chromium.org/1625993004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutGrid.cpp:1908: OverflowAlignment
overflow = ComputedStyle::resolvedAlignment(styleRef(), child.styleRef(),
ItemPositionStretch).overflow();
this should be an instance method.

styleRef().resolveAlignment(child.styleRef(), ItemPositionStretch).overflow()

https://codereview.chromium.org/1625993004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):

https://codereview.chromium.org/1625993004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:185: ||
oldStyle->justifyItems() != newStyle->justifyItems())
we need to fix justifyItems too btw (in another patch), it should be very hard
to cause a Reattach

https://codereview.chromium.org/1625993004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:208: const
StyleSelfAlignmentData ComputedStyle::resolvedAlignment(const ComputedStyle&
parentStyle, const ComputedStyle& childStyle, ItemPosition
resolvedAutoPositionForLayoutObject)
This should be an instance method on either the child or the parent,

[cbiesinger, 2016-01-29 21:18:16]

I agree with Elliott's comments here.

[jfernandez, 2016-02-01 14:41:52]

The CQ bit was checked by jfernandez@igalia.com

[jfernandez, 2016-02-01 14:41:53]

The patchset sent to the CQ was uploaded after l-g-t-m from esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/1625993004/#ps40001
(title: "Applied suggested changes.")

[commit-bot: I haz the power, 2016-02-01 14:42:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1625993004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1625993004/40001

[commit-bot: I haz the power, 2016-02-01 15:50:05]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-01 15:51:02]

Description was changed from

==========
[css-align] Avoid the style Reattach whenever align-items changes.

The align-items property defines the default beahvior of children boxes
with 'auto' values on the align-self property.

Since the spec states that the computed style of these properties must
be resolved, using different values depending on the kind of element, we
have implemented this style resolution using the StyleAdjuster logic.

Because of this approach in the implementation, when the value of the
align-items property changes we have to recalculate the style tree,
because some of the 'auto' values have been resolved already to the
previous value. Hence, we need to trigger again the style resolution
logic, via the StyleAdjustment, which requires the style changes to
force a Reattach.

There are several problems with this approach. One of them are the
excessive number of style and paint invalidations when any of this
alignment properties change. Another problem, which is the one
reported in the bug this patch address, is that it prevents animations
and transitions to be executed when they involve changes in the
alignment properties.

BUG=580070
==========

to

==========
[css-align] Avoid the style Reattach whenever align-items changes.

The align-items property defines the default beahvior of children boxes
with 'auto' values on the align-self property.

Since the spec states that the computed style of these properties must
be resolved, using different values depending on the kind of element, we
have implemented this style resolution using the StyleAdjuster logic.

Because of this approach in the implementation, when the value of the
align-items property changes we have to recalculate the style tree,
because some of the 'auto' values have been resolved already to the
previous value. Hence, we need to trigger again the style resolution
logic, via the StyleAdjustment, which requires the style changes to
force a Reattach.

There are several problems with this approach. One of them are the
excessive number of style and paint invalidations when any of this
alignment properties change. Another problem, which is the one
reported in the bug this patch address, is that it prevents animations
and transitions to be executed when they involve changes in the
alignment properties.

BUG=580070

Committed: https://crrev.com/89b4a2addd0e3e82a1ca8213d31323fdc2e6395a
Cr-Commit-Position: refs/heads/master@{#372666}
==========

[commit-bot: I haz the power, 2016-02-01 15:51:03]

Patchset 3 (id:??) landed as
https://crrev.com/89b4a2addd0e3e82a1ca8213d31323fdc2e6395a
Cr-Commit-Position: refs/heads/master@{#372666}