Closed Bug 1378280 Opened 7 years ago Closed 7 years ago

Assert against style refreshes in GeckoRestyleManager::{ContentStateChanged,ContentAttributeWillChange}.

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

Details

Attachments

(1 file)

I added those assertions to Servo a while ago. I think Gecko's restyle manager doesn't account for it either, and today bug 1377344 made those assertions fire in Stylo. Of course the problem went unnoticed for a while.

Let's prevent this from happening again, and assert also in GeckoRM.
Err, I meant bug 1372433 above.
See Also: 13773441372433
Comment on attachment 8883459 [details]
Bug 1378280: Assert we're not in a style refresh when attributes or state changes.

https://reviewboard.mozilla.org/r/154364/#review159486

I'm not sure what things might go wrong if we were in the middle of a style refresh, but I guess it indeed would be unexpected.
Attachment #8883459 - Flags: review?(cam) → review+
I think we were asserting against content state changes in ContentStateChangedInternal, as you noticed, and also Wes did.

I think RestyleTracker protects against reentrancy swapping and popping from the array before-hand. However it still seems to me like it may miss a restyle if the state of the element changes while it's being restyled... In any case, it sounds somewhat fishy, and I'm glad the assertion caught that particular case.
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/fb1e5ee5c0e4
Assert we're not in a style refresh when attributes or state changes. r=heycam
https://hg.mozilla.org/mozilla-central/rev/fb1e5ee5c0e4
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Assignee: nobody → emilio
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: