Bug 202476 – Allow pages using FontFaceSet to enter the back/forward cache

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Chris Dumez]

Allow pages using FontFaceSet to enter the back/forward cache. This causes failures on sites with Youtube embeds for example:

[Chris Dumez]

Created attachment 380148 [details]
Patch

[Myles C. Maxfield]

I'm also doing a little bit of additional investigation around this bug.

[Chris Dumez]

(In reply to Myles C. Maxfield from comment #2)
> I'm also doing a little bit of additional investigation around this bug.

There are 2 issues:
1. Sometimes, the FontFace think it is loading when it actually is not (which I did not address and Myles is investigating).
2. We fail to enter page cache because the FontFace think it is still loading (which I address).

I agree that it'd be good to investigate 1 but fixing 1 would not be sufficient, because we want to enter PageCache no matter what, even if the font really is loading.
This is why I think implementing queueing in FontFaceSet like my patch does is the right thing to do.

[Myles C. Maxfield]

Comment on attachment 380148 [details]
Patch

I've gotten a bug about this kind of behavior: https://github.com/doanythingfordethklok/safari-cache-bug but if we are really willing to use the policy of cancelling all in-flight requests during navigation, and never reloading them upon 'back', then this patch is a good one.

[WebKit Commit Bot]

Comment on attachment 380148 [details]
Patch

Clearing flags on attachment: 380148

Committed r250693: <https://trac.webkit.org/changeset/250693>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/55968375>

[Truitt Savell]

It looks like the changes in https://trac.webkit.org/changeset/250693/webkit

caused 4 new crashes on Debug:
fast/text/document-fonts-while-loading-crash.html 
fast/text/font-face-set-destroy-document.html 
fast/text/font-face-set-javascript.html 
http/tests/navigation/page-cache-fragment-referrer.html

History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Ftext%2Fdocument-fonts-while-loading-crash.html%20fast%2Ftext%2Ffont-face-set-destroy-document.html%20fast%2Ftext%2Ffont-face-set-javascript.html%20http%2Ftests%2Fnavigation%2Fpage-cache-fragment-referrer.html

Assertion crash:
ASSERTION FAILED: !m_valueOrException
/Volumes/Data/slave/mojave-debug/build/Source/WebCore/bindings/js/DOMPromiseProxy.h(294) : void WebCore::DOMPromiseProxyWithResolveCallback<WebCore::IDLInterface<WebCore::FontFaceSet> >::resolve(typename IDLType::ParameterType) [IDLType = WebCore::IDLInterface<WebCore::FontFaceSet>]

ASSERTION FAILED: !is<Document>(m_scriptExecutionContext) || &downcast<Document>(m_scriptExecutionContext)->contextDocument() == downcast<Document>(m_scriptExecutionContext)

[Chris Dumez]

(In reply to Truitt Savell from comment #8)
> It looks like the changes in https://trac.webkit.org/changeset/250693/webkit
> 
> caused 4 new crashes on Debug:
> fast/text/document-fonts-while-loading-crash.html 
> fast/text/font-face-set-destroy-document.html 
> fast/text/font-face-set-javascript.html 
> http/tests/navigation/page-cache-fragment-referrer.html
> 
> History:
> https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=fast%2Ftext%2Fdocument-fonts-while-loading-crash.
> html%20fast%2Ftext%2Ffont-face-set-destroy-document.
> html%20fast%2Ftext%2Ffont-face-set-javascript.
> html%20http%2Ftests%2Fnavigation%2Fpage-cache-fragment-referrer.html
> 
> Assertion crash:
> ASSERTION FAILED: !m_valueOrException
> /Volumes/Data/slave/mojave-debug/build/Source/WebCore/bindings/js/
> DOMPromiseProxy.h(294) : void
> WebCore::DOMPromiseProxyWithResolveCallback<WebCore::IDLInterface<WebCore::
> FontFaceSet> >::resolve(typename IDLType::ParameterType) [IDLType =
> WebCore::IDLInterface<WebCore::FontFaceSet>]
> 
> ASSERTION FAILED: !is<Document>(m_scriptExecutionContext) ||
> &downcast<Document>(m_scriptExecutionContext)->contextDocument() ==
> downcast<Document>(m_scriptExecutionContext)

Looking now.

[Chris Dumez]

(In reply to Chris Dumez from comment #9)
> (In reply to Truitt Savell from comment #8)
> > It looks like the changes in https://trac.webkit.org/changeset/250693/webkit
> > 
> > caused 4 new crashes on Debug:
> > fast/text/document-fonts-while-loading-crash.html 
> > fast/text/font-face-set-destroy-document.html 
> > fast/text/font-face-set-javascript.html 
> > http/tests/navigation/page-cache-fragment-referrer.html
> > 
> > History:
> > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> > html#showAllRuns=true&tests=fast%2Ftext%2Fdocument-fonts-while-loading-crash.
> > html%20fast%2Ftext%2Ffont-face-set-destroy-document.
> > html%20fast%2Ftext%2Ffont-face-set-javascript.
> > html%20http%2Ftests%2Fnavigation%2Fpage-cache-fragment-referrer.html
> > 
> > Assertion crash:
> > ASSERTION FAILED: !m_valueOrException
> > /Volumes/Data/slave/mojave-debug/build/Source/WebCore/bindings/js/
> > DOMPromiseProxy.h(294) : void
> > WebCore::DOMPromiseProxyWithResolveCallback<WebCore::IDLInterface<WebCore::
> > FontFaceSet> >::resolve(typename IDLType::ParameterType) [IDLType =
> > WebCore::IDLInterface<WebCore::FontFaceSet>]
> > 
> > ASSERTION FAILED: !is<Document>(m_scriptExecutionContext) ||
> > &downcast<Document>(m_scriptExecutionContext)->contextDocument() ==
> > downcast<Document>(m_scriptExecutionContext)
> 
> Looking now.

Fixed in <https://trac.webkit.org/changeset/250731>.