Before turning objects into spanners, check that they are not already spanners.

Before turning objects into spanners, check that they are not already spanners.

During style recalculation, we may end up in a situation where we think that we
go from a state where an object couldn't contain spanners, to being able to
contain them, while in reality, the object was able to contain spanners all
along.

This happens when changing the writing mode on the multicol container and all
objects in the parent chain between the spanner and the multicol container (and
there is nothing that prevents the descendant from being a spanner). The
problem is that when determining whether an object is a writing mode root, we
compare the object's writing mode to that of its parent. If they are different,
we decide that it's a writing mode root. However, if we're in styleWillChange()
for said object, and its writing mode is actually about to change to the same
value as that of the parent, there'll be no writing mode root in the end.
Still, we're going to think that we used to be a writing mode root (i.e. not be
able to contain spanners).

It would be possible to fix it for writing mode roots, to provide a reliable
implementation of isWritingModeRoot(), by using a bit in LayoutObject to
specify whether it's a writing mode root, rather than using current computed
style to determine that. Using computed style during style recalculation is
risky. That said, it's probably better to be fault-tolerant for such situations
in toggleSpannersInSubtree() instead, especially since may be other (unknown,
at the time being) scenarios where this situation may occur.

BUG=662754
Committed: https://crrev.com/3a83cc34b92519d437c479e2e88f69589ebc5427
Cr-Commit-Position: refs/heads/master@{#430887}

[mstensho (USE GERRIT), 2016-11-08 12:36:24]

The CQ bit was checked by mstensho@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-08 12:36:33]

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-11-08 13:56:06]

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

[commit-bot: I haz the power, 2016-11-08 13:56:07]

Dry run: This issue passed the CQ dry run.

[mstensho (USE GERRIT), 2016-11-08 13:58:39]

mstensho@opera.com changed reviewers:
+ eae@chromium.org

[mstensho (USE GERRIT), 2016-11-08 13:58:40]

One-liner, with lots of documentation. :(

[eae, 2016-11-09 00:10:22]

LGTM

[mstensho (USE GERRIT), 2016-11-09 07:58:37]

The CQ bit was checked by mstensho@opera.com

[commit-bot: I haz the power, 2016-11-09 07:58: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-11-09 08:04:04]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-09 08:06:03]

Description was changed from

==========
Before turning objects into spanners, check that they are not already spanners.

During style recalculation, we may end up in a situation where we think that we
go from a state where an object couldn't contain spanners, to being able to
contain them, while in reality, the object was able to contain spanners all
along.

This happens when changing the writing mode on the multicol container and all
objects in the parent chain between the spanner and the multicol container (and
there is nothing that prevents the descendant from being a spanner). The
problem is that when determining whether an object is a writing mode root, we
compare the object's writing mode to that of its parent. If they are different,
we decide that it's a writing mode root. However, if we're in styleWillChange()
for said object, and its writing mode is actually about to change to the same
value as that of the parent, there'll be no writing mode root in the end.
Still, we're going to think that we used to be a writing mode root (i.e. not be
able to contain spanners).

It would be possible to fix it for writing mode roots, to provide a reliable
implementation of isWritingModeRoot(), by using a bit in LayoutObject to
specify whether it's a writing mode root, rather than using current computed
style to determine that. Using computed style during style recalculation is
risky. That said, it's probably better to be fault-tolerant for such situations
in toggleSpannersInSubtree() instead, especially since may be other (unknown,
at the time being) scenarios where this situation may occur.

BUG=662754
==========

to

==========
Before turning objects into spanners, check that they are not already spanners.

During style recalculation, we may end up in a situation where we think that we
go from a state where an object couldn't contain spanners, to being able to
contain them, while in reality, the object was able to contain spanners all
along.

This happens when changing the writing mode on the multicol container and all
objects in the parent chain between the spanner and the multicol container (and
there is nothing that prevents the descendant from being a spanner). The
problem is that when determining whether an object is a writing mode root, we
compare the object's writing mode to that of its parent. If they are different,
we decide that it's a writing mode root. However, if we're in styleWillChange()
for said object, and its writing mode is actually about to change to the same
value as that of the parent, there'll be no writing mode root in the end.
Still, we're going to think that we used to be a writing mode root (i.e. not be
able to contain spanners).

It would be possible to fix it for writing mode roots, to provide a reliable
implementation of isWritingModeRoot(), by using a bit in LayoutObject to
specify whether it's a writing mode root, rather than using current computed
style to determine that. Using computed style during style recalculation is
risky. That said, it's probably better to be fault-tolerant for such situations
in toggleSpannersInSubtree() instead, especially since may be other (unknown,
at the time being) scenarios where this situation may occur.

BUG=662754

Committed: https://crrev.com/3a83cc34b92519d437c479e2e88f69589ebc5427
Cr-Commit-Position: refs/heads/master@{#430887}
==========

[commit-bot: I haz the power, 2016-11-09 08:06:04]

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