Bug 1777301 Comment 1 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Note, `Document::UnblockOnload` has a handful of callers, so there are some older crashes with this same signature which seem to be unrelated. This bug here is focused on the recent crash reports where `nsFontFaceLoader::Cancel` is the caller.

For those ones, we're crashing with a null deref on this line:
```
void Document::UnblockOnload(bool aFireSync) {
  if (mDisplayDocument) {
```

...implying that `this` itself is nullptr.

So, going up one level, to nsFontFaceLoader::Cancel(), we have this line:
```
  mFontFaceSet->Document()->UnblockOnload(false);
```
...and it looks like `mFontFaceSet->Document()` must be returning nullptr there.

That Document() API is just a getter for the `FontFaceSetDocumentImpl::mDocument` pointer, which is set to nullptr here:
```
void FontFaceSetDocumentImpl::Destroy() {
[...]
  mDocument = nullptr;
```
https://searchfox.org/mozilla-central/rev/2946e9b450cb35afaf8dad927a8d187975dcd74d/layout/style/FontFaceSetDocumentImpl.cpp#98


So presumably the issue here is that our nsFontFaceLoader's mFontFaceSet has had its `Destroy()` method already called, which invalidates it (or at least makes its `Document()` API surprisingly null-returning), and then we trip over that in `nsFontFaceLoader::Cancel`.

I'm not familiar with this code, so take my thoughts with a grain of salt, but presumably we need to either make sure that mFontFaceSet::Destroy() invalidates the nsFontFaceLoader as well (e.g. disconnecting itself); or maybe we need to make nsFontFaceLoader::Cancel null-check the return value of the Document() function-call here, though that's a little silly given the current infallible naming of that function.
Note, `Document::UnblockOnload` has a handful of callers, so there are some older crashes with this same signature which seem to be unrelated. This bug here is focused on the recent crash reports where `nsFontFaceLoader::Cancel` is the caller.

For those ones, we're crashing with a null deref on this line:
```
void Document::UnblockOnload(bool aFireSync) {
  if (mDisplayDocument) {
```

...implying that `this` itself is nullptr.

So, going up one level, to nsFontFaceLoader::Cancel(), we have this line:
```
  mFontFaceSet->Document()->UnblockOnload(false);
```
...and it looks like `mFontFaceSet->Document()` must be returning nullptr there.

That Document() API is just a getter for the `FontFaceSetDocumentImpl::mDocument` pointer, which is set to nullptr here:
```c++
void FontFaceSetDocumentImpl::Destroy() {
[...]
  mDocument = nullptr;
```
https://searchfox.org/mozilla-central/rev/2946e9b450cb35afaf8dad927a8d187975dcd74d/layout/style/FontFaceSetDocumentImpl.cpp#98


So presumably the issue here is that our nsFontFaceLoader's mFontFaceSet has had its `Destroy()` method already called, which invalidates it (or at least makes its `Document()` API surprisingly null-returning), and then we trip over that in `nsFontFaceLoader::Cancel`.

I'm not familiar with this code, so take my thoughts with a grain of salt, but presumably we need to either make sure that mFontFaceSet::Destroy() invalidates the nsFontFaceLoader as well (e.g. disconnecting itself); or maybe we need to make nsFontFaceLoader::Cancel null-check the return value of the Document() function-call here, though that's a little silly given the current infallible naming of that function.

Back to Bug 1777301 Comment 1