[webkit-changes] [WebKit/WebKit] 0b63ff: Ensure that CachedFontLoadRequest::fontLoaded long...

[Vitor Roriz]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 0b63ffe87813e5068683d0b5b86bfccb364bffad
      https://github.com/WebKit/WebKit/commit/0b63ffe87813e5068683d0b5b86bfccb364bffad
  Author: Vitor Roriz <vitor.roriz at apple.com>
  Date:   2023-10-20 (Fri, 20 Oct 2023)

  Changed paths:
    M Source/WebCore/loader/cache/CachedFontLoadRequest.h

  Log Message:
  -----------
  Ensure that CachedFontLoadRequest::fontLoaded longs Font before font is released
https://bugs.webkit.org/show_bug.cgi?id=263400
rdar://117077759

Reviewed by Chris Dumez.

The problem happens because CachedFontLoadRequest::fontLoaded calls
m_fontLoadRequestClient->fontLoaded(*this) which will end up destroying CachedFontLoadRequest itself.

We use a local Ref object to try to guarantee the ownership of the object's CSSFontFace& m_face.

We end up calling the destructor of  CSSFontFace once we leave the function's scope. This means that this Ref was the only one to the FontFace, but there was no way for us to know that, since we have a l-value ref to it here.

The CSSFontFace will destroy its unique_ptr to CSSFontFaceSource which will destroy its FontLoadRequest (actually a CachedFontLoadRequest) that will finally destroy its CachedResourceHandle<CachedFont> m_font;

The caller of CSSFontFaceSource::fontLoaded is CachedFontLoadRequest::fontLoaded.  CachedFontLoadRequest has a CachedResourceHandle<CachedFont> m_font that is accessed in fontLoaded, causing the bug.

Proposed solution:
CachedFontLoadRequest::fontLoaded is the function that does the actual use after free by using the object's m_font member after this pointer is deleted. We can simply do the logging before m_fontLoadRequestClient->fontLoaded starts the deallocation cycle.

This will work to avoid this specific use, but we should strengthen, but
we should consider using a WeakPtr to CSSFontFace at CSSFontFaceSource
instead of a l-value reference so we can check if the FontFace that owns this CSSFontFaceSource is no longer existent.

* Source/WebCore/loader/cache/CachedFontLoadRequest.h:

Canonical link: https://commits.webkit.org/269562@main