Closed Bug 1426649 Opened 2 years ago Closed 2 years ago

Remove nsCSSFrameConstructor updates

Categories

(Core :: Layout, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

Details

Attachments

(2 files)

They're pretty useless now.
Comment on attachment 8938458 [details]
Bug 1426649: Stop tracking DOM changes from painting.

https://reviewboard.mozilla.org/r/209162/#review214976

I.. don't know anything about this code. It looks like it was added by roc and reviewed by dbaron, so I'm going to redirect the review to dbaron.
Attachment #8938458 - Flags: review?(bugmail) → review?(dbaron)
Comment on attachment 8938459 [details]
Bug 1426649: Remove nsCSSFrameConstructor updates.

https://reviewboard.mozilla.org/r/209164/#review214992

These code changes looks fine to me, assuming that part 1 is OK.
Attachment #8938459 - Flags: review?(mats) → review+
Comment on attachment 8938458 [details]
Bug 1426649: Stop tracking DOM changes from painting.

This is because Flash could modify the dom during painting, see here
https://bugzilla.mozilla.org/show_bug.cgi?id=594774#c6

So this should still be relevant until we completely disable flash, no?
The crashtest that bug 594774 added (modules/plugin/test/crashtests/522512-1.html)
was removed with the comment that the crash can't happen anymore:
https://hg.mozilla.org/mozilla-central/rev/671bd739dbff
(presumably as a result of bug 596451)
See Also: → 984690
It seems like it would be a little bit safer to *ship* the code as a MOZ_RELEASE_ASSERT for a bit, and check crash-stats?

(I'm also not sure who would know what our current levels of flash usage are like on nightly vs. release... or whether out-of-process plugins would have fixed this fully.)
Flags: needinfo?(emilio)
Comment on attachment 8938458 [details]
Bug 1426649: Stop tracking DOM changes from painting.

https://reviewboard.mozilla.org/r/209162/#review219484

Then again, given comment 8, I guess I'm fine with just doing this.
Attachment #8938458 - Flags: review?(dbaron) → review+
Yeah, per comment 8 I think this should be fine. Thanks for looking at the history of that test mats!
Flags: needinfo?(emilio)
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/302d28afd44f
Stop tracking DOM changes from painting. r=dbaron
https://hg.mozilla.org/integration/autoland/rev/682a00d90865
Remove nsCSSFrameConstructor updates. r=mats
https://hg.mozilla.org/mozilla-central/rev/302d28afd44f
https://hg.mozilla.org/mozilla-central/rev/682a00d90865
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.