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)
Core
CSS Parsing and Computation
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.
Assignee | ||
Comment 1•7 years ago
|
||
Err, I meant bug 1372433 above.
Comment hidden (mozreview-request) |
Comment 3•7 years ago
|
||
mozreview-review |
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+
Assignee | ||
Comment 4•7 years ago
|
||
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
Comment 6•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Updated•6 years ago
|
Assignee: nobody → emilio
You need to log in
before you can comment on or make changes to this bug.
Description
•