Bug 202476

Created attachment 380148 [details] Patch

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

(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.

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.

Comment on attachment 380148 [details] Patch Clearing flags on attachment: 380148 Committed r250693: <https://trac.webkit.org/changeset/250693>

All reviewed patches have been landed. Closing bug.

<rdar://problem/55968375>

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)

(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.

(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>.