Bug 158297 – Sporadic crash in HashTableAddResult following CSSValuePool::createFontFamilyValue

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

Created attachment 280318 [details]
Stack Trace of crash

This also happens in Safari Technology Preview on Mac OSX, and WKWebView in iOS versions since iOS 9.0.
Happens when integrating Bing Maps in a website.

Stack Trace is attached.
Could be some race condition as this is sporadic.
Thanks!

[Benjamin Poulain]

Do you have a test page by any chance?
Any way I can attempt to reproduce?

[Chris Dumez]

Likely means we are calling CSSValuePool::createFontFamilyValue() with a null String?

[Chris Dumez]

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x7fff9553497a WTF::HashTableAddResult<WTF::HashTableIterator<std::__1::pair<WTF::String, bool>, WTF::KeyValuePair<std::__1::pair<WTF::String, bool>, WTF::RefPtr<WebCore::CSSPrimitiveValue> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<std::__1::pair<WTF::String, bool>, WTF::RefPtr<WebCore::CSSPrimitiveValue> > >, WTF::PairHash<WTF::String, bool>, WTF::HashMap<std::__1::pair<WTF::String, bool>, WTF::RefPtr<WebCore::CSSPrimitiveValue>, WTF::PairHash<WTF::String, bool>, WTF::HashTraits<std::__1::pair<WTF::String, bool> >, WTF::HashTraits<WTF::RefPtr<WebCore::CSSPrimitiveValue> > >::KeyValuePairTraits, WTF::HashTraits<std::__1::pair<WTF::String, bool> > > > WTF::HashMap<std::__1::pair<WTF::String, bool>, WTF::RefPtr<WebCore::CSSPrimitiveValue>, WTF::PairHash<WTF::String, bool>, WTF::HashTraits<std::__1::pair<WTF::String, bool> >, WTF::HashTraits<WTF::RefPtr<WebCore::CSSPrimitiveValue> > >::add<std::nullptr_t>(std::__1::pair<WTF::String, bool>&&, std::nullptr_t&&) + 90 (/BuildRoot/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.Internal.sdk/usr/local/include/wtf/text/StringImpl.h:544)
1   com.apple.WebCore             	0x7fff955344a2 WebCore::CSSValuePool::createFontFamilyValue(WTF::String const&, WebCore::FromSystemFontID) + 258 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/WebCore/WebCore-7601.6.17/css/CSSValuePool.cpp:132)
2   com.apple.WebCore             	0x7fff952c563b WebCore::fontFamilyFromStyle(WebCore::RenderStyle*) + 267 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/WebCore/WebCore-7601.6.17/css/CSSComputedStyleDeclaration.cpp:1594)
3   com.apple.WebCore             	0x7fff951cf7bb WebCore::ComputedStyleExtractor::propertyValue(WebCore::CSSPropertyID, WebCore::EUpdateLayout) const + 8411 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/WebCore/WebCore-7601.6.17/css/CSSComputedStyleDeclaration.cpp:2676)
4   com.apple.WebCore             	0x7fff951cd69e WebCore::CSSComputedStyleDeclaration::getPropertyCSSValueInternal(WebCore::CSSPropertyID) + 62 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/WebCore/WebCore-7601.6.17/css/CSSComputedStyleDeclaration.cpp:2147)
5   com.apple.WebCore             	0x7fff951c57b4 WebCore::JSCSSStyleDeclaration::getOwnPropertySlotDelegate(JSC::ExecState*, JSC::PropertyName, JSC::PropertySlot&) + 68 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/WebCore/WebCore-7601.6.17/bindings/js/JSCSSStyleDeclarationCustom.cpp:303)
6   com.apple.WebCore             	0x7fff9589eb5c WebCore::JSCSSStyleDeclaration::getOwnPropertySlot(JSC::JSObject*, JSC::ExecState*, JSC::PropertyName, JSC::PropertySlot&) + 460 (/Library/Caches/com.apple.xbs/Binaries/WebCore/WebCore-7601.6.17~1/Symbols/BuiltProducts/DerivedSources/WebCore/JSCSSStyleDeclaration.cpp:207)
7   com.apple.JavaScriptCore      	0x7fff8be84193 llint_slow_path_get_by_id + 1091 (/Library/Caches/com.apple.xbs/Sources/JavaScriptCore/JavaScriptCore-7601.6.13/runtime/JSObject.h:1154)
8   com.apple.JavaScriptCore      	0x00007fff8c2185f0 llint_entry + 10503

[Chris Dumez]

I thought Myles had fixed this in <http://trac.webkit.org/changeset/186971> already. The crash trace was exactly the same. Since it still reproduces in technology preview, I guess the fix did not suffice or the bug is slightly different.

Unfortunately I don't have a test page, as this is an internal page within the PowerBI.com website with tons of JavaScript that fetches data from a backend. But I'll see if I can create a static, smaller page which reproduces.

I managed to get this crash while debugging the MiniBrowser with XCode built on debug. I believe you are right, and we try to call createFontFamilyValue() with a null string. The JSC::PropertyName was "fontFamily".

Anything else that I can check with the XCode debugger to help?

Also, I suspect this typescript function is causing trouble (might be another case):

        export function getSvgMeasurementProperties(svgElement: SVGTextElement): TextProperties {
            debug.assertValue(svgElement, 'svgElement');

            let style = window.getComputedStyle(svgElement, null);
            return {
                text: svgElement.textContent,
                fontFamily: style.fontFamily,
                fontSize: style.fontSize,
                fontWeight: style.fontWeight,
                fontStyle: style.fontStyle,
                fontVariant: style.fontVariant,
                whiteSpace: style.whiteSpace
            };
        }

[Myles C. Maxfield]

The family name in the FontCascadeDescription is never getting set. Therefore, it is staying with its default construction, which is nullptr.

This means that none of FontCascadeDescription::setOneFamily(), FontCascadeDescription::setFamilies(Vector), nor FontCascadeDescription::setFamilies(RefCountedArray) is being run on the relevant object.

[Myles C. Maxfield]

In order for ComputedStyleExtractor::propertyValue() to progress beyond its (!style) null check, the element must have a style. This should have been done earlier in the function by calling updateStyleIfNeededForNode().

[Myles C. Maxfield]

This is because of the following code in TreeResolver::styleForElement():

    if (!m_document.haveStylesheetsLoaded() && !element.renderer()) {
        m_document.setHasNodesWithPlaceholderStyle();
        return RenderStyle::clonePtr(*placeholderStyle);
    }

This placeholder style does not have its font family set.

[Myles C. Maxfield]

(In reply to comment #9)
> This is because of the following code in TreeResolver::styleForElement():
> 
>     if (!m_document.haveStylesheetsLoaded() && !element.renderer()) {
>         m_document.setHasNodesWithPlaceholderStyle();
>         return RenderStyle::clonePtr(*placeholderStyle);
>     }
> 
> This placeholder style does not have its font family set.

This can be resolved by the following code in ensurePlaceholderStyle()

    FontCascadeDescription fontDescription;
    fontDescription.setOneFamily(standardFamily);
    fontDescription.setKeywordSizeFromIdentifier(CSSValueMedium);
    float size = Style::fontSizeForKeyword(CSSValueMedium, false, document);
    fontDescription.setSpecifiedSize(size);
    fontDescription.setComputedSize(size);
    placeholderStyle->setFontDescription(fontDescription);

[Myles C. Maxfield]

Created attachment 281449 [details]
Reproduction

[Myles C. Maxfield]

It looks like the reproduction isn't working because we wait for CSS to finish downloading before running scripts. However, my script needs to run while CSS is downloading.

[Myles C. Maxfield]

Created attachment 281450 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 281450 [details]
Patch

Clearing flags on attachment: 281450

Committed r202127: <http://trac.webkit.org/changeset/202127>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.