Make pseudo element update work with LocalStyleChange.

Make pseudo element update work with LocalStyleChange.

The missing pseudo element update on LocalStyleChange did not cause any bugs
at the moment since LocalStyleChange is not used when pseudo element selector
matching changes, but necessary to fix in preparation for optimizing :hover,
:active, and :focus updates using LocalStyleChange. If pseudo elements are
updated for LocalStyleChange, style updates for mentioned pseudo classes can
use LocalStyleChange in many cases where SubtreeStyleChange is currently used.

LocalStyleChange recalc style for the given element and descendants which
inherit any of the changes from that element. In addition, there is code
to make sure pseudo element children were being updated when the styles for
the element itself did not change, but matching of its pseudo elements did.

That code (shouldRecalcStyle() check on the actual element before calling
updatePseudoElement() in recalcChildStyle) was introduced in [1], but changed
behavior in [2] because needsRecalcStyle was cleared before recalcChildStyle.

UpdatePseudoElements is added as a new StyleRecalcChange which causes an
update of the pseudo element children, but not other children in the case
where the styles for the actual element were recalculated, but resulted in
NoChange or NoInherit, and any of the pseudo element bits were set on
RenderStyle.

[1] https://src.chromium.org/viewvc/blink?revision=147696&view=revision
[2] https://src.chromium.org/viewvc/blink?revision=161568&view=revision

BUG=337983
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=165981

[esprehn, 2014-01-24 22:53:48]

Can we just put change type checks in recalcChildStyle? I'm not a fan of the
code duplication in ::recalcStyle updating pseudo since we have the same logic
inside recalcChildStyle.

https://codereview.chromium.org/144963002/diff/170001/Source/core/dom/Element...
File Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/144963002/diff/170001/Source/core/dom/Element...
Source/core/dom/Element.cpp:1599: updatePseudoElement(BACKDROP, change);
Can you just merge this into recalcChildStyle? If a pseudo element requests a
recalc then childNeedsStyleRecalc() will be true and we won't even get here. So
the updatePseudoElement() logic is code duplication.

I'd rather you added checks inside recalcChildStyle so that it only recalced the
pseudos.

https://codereview.chromium.org/144963002/diff/170001/Source/core/rendering/s...
File Source/core/rendering/style/RenderStyle.h (right):

https://codereview.chromium.org/144963002/diff/170001/Source/core/rendering/s...
Source/core/rendering/style/RenderStyle.h:1896: return
noninherited_flags._pseudoBits & CONTENT_PSEUDOID_MASK;
CONTENT is confusing since we have ::content which is something else. I might
suggest just using PSEUDO_ELEMENT_MASK.

[rune, 2014-01-25 00:00:38]

https://codereview.chromium.org/144963002/diff/170001/Source/core/dom/Element...
File Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/144963002/diff/170001/Source/core/dom/Element...
Source/core/dom/Element.cpp:1599: updatePseudoElement(BACKDROP, change);
On 2014/01/24 22:53:49, esprehn wrote:
> Can you just merge this into recalcChildStyle? If a pseudo element requests a
> recalc then childNeedsStyleRecalc() will be true and we won't even get here.
So
> the updatePseudoElement() logic is code duplication.
> 
> I'd rather you added checks inside recalcChildStyle so that it only recalced
the
> pseudos.

I've tried in patch set 6. I've changed it so that you'll not get
UpdatePseudoElements if childNeedsStyleRecalc() is true to avoid more
complicated checks inside recalcChildStyle. Still there is more indentation
which might make it less readable. See what you think.

https://codereview.chromium.org/144963002/diff/170001/Source/core/rendering/s...
File Source/core/rendering/style/RenderStyle.h (right):

https://codereview.chromium.org/144963002/diff/170001/Source/core/rendering/s...
Source/core/rendering/style/RenderStyle.h:1896: return
noninherited_flags._pseudoBits & CONTENT_PSEUDOID_MASK;
On 2014/01/24 22:53:49, esprehn wrote:
> CONTENT is confusing since we have ::content which is something else. I might
> suggest just using PSEUDO_ELEMENT_MASK.

Done.

[esprehn, 2014-01-28 18:08:04]

please remove the bool and add braces before landing. You can leave the method
if you think it's more clear.

lgtm

https://codereview.chromium.org/144963002/diff/340001/Source/core/dom/Element...
File Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/144963002/diff/340001/Source/core/dom/Element...
Source/core/dom/Element.cpp:1665: if (!updatePseudoElements)
Get rid of the bool, you're also missing braces.

if (change > UpdatePseudoElements || childNeedsStyleRecalc()) {

https://codereview.chromium.org/144963002/diff/340001/Source/core/dom/Element...
Source/core/dom/Element.cpp:1679: if (!updatePseudoElements) {
Same,

if (change > UpdatePseudoElements || childNeedsStyleRecalc()) {

https://codereview.chromium.org/144963002/diff/340001/Source/core/dom/Node.h
File Source/core/dom/Node.h (right):

https://codereview.chromium.org/144963002/diff/340001/Source/core/dom/Node.h#...
Source/core/dom/Node.h:874: inline bool
Node::shouldUpdatePseudoElements(StyleRecalcChange change) const
You only have this check in two places, I'd just inline it there.

[commit-bot: I haz the power, 2014-01-28 21:27:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rune@opera.com/144963002/400001

[commit-bot: I haz the power, 2014-01-29 00:22:34]

Change committed as 165981

[commit-bot: I haz the power, 2014-01-29 00:24:16]

CQ bit was unchecked on CL. Ignoring.