1699614 - Work around use after unlink issue with gProton preference callback

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Andrew McCreight [:mccr8]]

For reasons I don't entirely understand (bug 1543537), you can end up in a situation where a weakly held preference callback is alive while the DOM objects it refers to are thought to be dead by the cycle collector, and so those objects can get various fields nulled out, which causes issues. As seen in bug 1652531, one of these places is the gProton lazy preference getter in browser.js. kmag figured out that this is probably the cause of that assertion.

He also did some work about a year ago to drop weak references to DOM objects when we unlink them. I think we can take advantage of this to avoid the weird callback unlink issue by making the callback closure hold a weak reference to the document instead of a strong reference.

I did some retriggers on the tests that were failing, and it seems to fix the issue:

https://treeherder.mozilla.org/jobs?repo=try&revision=661511aa6bc04699f26653fef6d46056eff5ffb7
https://treeherder.mozilla.org/jobs?repo=try&revision=d203db3686f4ae90c486616df62eda9c65b394de

I think this is only an issue if the browser.proton.enabled pref gets flipped, so I don't know if it needs to be backported or whatever.

Comment 1

[Andrew McCreight [:mccr8]]

My patch has an issue I need to fix but hopefully the idea is still sound.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Bug 1699614 - Work around document use-after-unlink in Proton pref callback.

Due to some kind of weirdness, you can end up with a weakly held preference
callback being run even after the DOM objects it holds references to are unlinked
by the cycle collector, which can cause crashes. This patch works around
that by taking advantage of the fact that we now drop weak references to DOM
objects when they are unlinked to change the preference callback closure to
instead hold a weak reference.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d90bad06766a
Work around document use-after-unlink in Proton pref callback. r=Gijs

Comment 4

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/d90bad06766a