Font loading during layout can cause layout code to be re-entered via resource load delegate Here is an example: #import <Foundation/Foundation.h> #import <WebKit/WebKit.h> @interface ResourceLoadDelegate : NSObject { } @end @implementation ResourceLoadDelegate - (NSURLRequest *)webView:(WebView *)sender resource:(id)identifier willSendRequest:(NSURLRequest *)request redirectResponse:(NSURLResponse *)redirectResponse fromDataSource:(WebDataSource *)dataSource { if ([[[request URL] path] hasSuffix:@".ttf"]) { DOMElement *documentElement = [[[sender mainFrame] DOMDocument] documentElement]; [[documentElement style] setProperty:@"display" value:@"none" priority:@""]; [documentElement offsetTop]; } return request; } @end int main (int argc, const char * argv[]) { @autoreleasepool { WebView *webView = [[WebView alloc] initWithFrame:NSZeroRect frameName:nil groupName:[[NSBundle mainBundle] bundleIdentifier]]; ResourceLoadDelegate *delegate = [[ResourceLoadDelegate alloc] init]; webView.resourceLoadDelegate = delegate; [[webView mainFrame] loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"http://www.alistapart.com/d/cssatten/heid.html"]]]; [[NSRunLoop mainRunLoop] run]; } return 0; }
<rdar://problem/9835028>
Created attachment 101886 [details] Defer font loading using a zero-delay timer so that it does not happen during layout
Comment on attachment 101886 [details] Defer font loading using a zero-delay timer so that it does not happen during layout View in context: https://bugs.webkit.org/attachment.cgi?id=101886&action=review > Source/WebCore/css/CSSFontFaceSource.cpp:205 > + m_fontSelector.clear(); please use m_fontSelector = nullptr; here.
Fixed in <http://trac.webkit.org/r91699>.
I reverted r91699 in <http://trac.webkit.org/r91710> because some tests had relied on the synchronous behavior in the cached, error, and local cases.
Created attachment 101952 [details] Patch
Comment on attachment 101952 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=101952&action=review > Source/WebCore/css/CSSFontFaceSource.cpp:178 > + // the middle of layout, and the loader may invoke aribtrary delegate or event handler code. typo: aribtrary > Source/WebCore/css/CSSFontFaceSource.cpp:181 > + if (!m_startLoadingTimer.isActive()) > + m_startLoadingTimer.startOneShot(0); Sometimes it seems to me that these zero-length timers aren’t really timers. > Source/WebCore/css/CSSFontFaceSource.cpp:203 > + if (CachedResourceLoader* cachedResourceLoader = m_fontSelector->cachedResourceLoader()) > + m_font->beginLoadIfNeeded(cachedResourceLoader); Could just call it loader. > Source/WebCore/css/CSSFontFaceSource.h:81 > + Timer<CSSFontFaceSource> m_startLoadingTimer; That’s a verb phrase, not a noun phrase. Maybe m_loadStartTimer? > LayoutTests/ChangeLog:10 > + Unfortunately, font loading does not fire any DOM events, so in most cases > + a constant timeout had to be introduced. Wah! Well at least the timeout is only for test failures.
Committed <http://trac.webkit.org/r91738>.
Build fix in <http://trac.webkit.org/r91739>.
This patch appears to have made these tests flaky: svg/custom/svg-fonts-with-no-element-reference.html svg/custom/svg-fonts-without-missing-glyph.xhtml
Comment on attachment 101952 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=101952&action=review > LayoutTests/fast/css/custom-font-xheight.html:59 > + }, 100); > } I just noticed Darins comment "Wah! Well at least the timeout is only for test failures.". I think this observation is wrong, the timeout affects these tests in general. According to chromiums flakiness dashboard Recent examples of failure from their dashboard: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=svg%2Fcustom%2Fsvg-fonts-fallback.xhtml%2Csvg%2Fcustom%2Fsvg-fonts-segmented.xhtml%2Csvg%2Ftext%2Ftext-overflow-ellipsis-svgfont.html&showExpectations=true I've spotted this while working with bashi on his testcase in bug 71765. To fix this fully we need to be able to listen for events indicating the custom font has finished loading. I think we could do this through window.internals, what do you think?
Sure would be nice to have a way to do this without a timer.