Bug 194671 – Web Inspector: Occasional crash under WebCore::CSSStyleSheet::item called from Inspector

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

Occasional crash under WebCore::CSSStyleSheet::item called from Inspector

Unsure how to reproduce, but crash logs have:

----
Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BREAKPOINT (SIGTRAP)
Exception Codes:       0x0000000000000002, 0x0000000000000000
Exception Note:        EXC_CORPSE_NOTIFY 

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x00007fff51490a02 WebCore::CSSStyleSheet::item(unsigned int) + 370
1   com.apple.WebCore             	0x00007fff520acf1a void WebCore::InspectorCSSOMWrappers::collect<WebCore::CSSStyleSheet>(WebCore::CSSStyleSheet*) + 74
2   com.apple.WebCore             	0x00007fff520ad34c WebCore::InspectorCSSOMWrappers::collectDocumentWrappers(WebCore::ExtensionStyleSheets&) + 332
3   com.apple.WebCore             	0x00007fff5243b383 WebCore::InspectorCSSAgent::buildObjectForRule(WebCore::StyleRule*, WebCore::StyleResolver&, WebCore::Element&) + 67
4   com.apple.WebCore             	0x00007fff52437efa WebCore::InspectorCSSAgent::buildArrayForMatchedRuleList(WTF::Vector<WTF::RefPtr<WebCore::StyleRule, WTF::DumbPtrTraits<WebCore::StyleRule> >, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::StyleResolver&, WebCore::Element&, WebCore::PseudoId) + 298
5   com.apple.WebCore             	0x00007fff5243755c WebCore::InspectorCSSAgent::getMatchedStylesForNode(WTF::String&, int, bool const*, bool const*, WTF::RefPtr<WTF::JSONImpl::ArrayOf<Inspector::Protocol::CSS::RuleMatch>, WTF::DumbPtrTraits<WTF::JSONImpl::ArrayOf<Inspector::Protocol::CSS::RuleMatch> > >&, WTF::RefPtr<WTF::JSONImpl::ArrayOf<Inspector::Protocol::CSS::PseudoIdMatches>, WTF::DumbPtrTraits<WTF::JSONImpl::ArrayOf<Inspector::Protocol::CSS::PseudoIdMatches> > >&, WTF::RefPtr<WTF::JSONImpl::ArrayOf<Inspector::Protocol::CSS::InheritedStyleEntry>, WTF::DumbPtrTraits<WTF::JSONImpl::ArrayOf<Inspector::Protocol::CSS::InheritedStyleEntry> > >&) + 252
6   com.apple.JavaScriptCore      	0x00007fff47f06a95 Inspector::CSSBackendDispatcher::getMatchedStylesForNode(long, WTF::RefPtr<WTF::JSONImpl::Object, WTF::DumbPtrTraits<WTF::JSONImpl::Object> >&&) + 965
7   com.apple.JavaScriptCore      	0x00007fff47f06274 Inspector::CSSBackendDispatcher::dispatch(long, WTF::String const&, WTF::Ref<WTF::JSONImpl::Object, WTF::DumbPtrTraits<WTF::JSONImpl::Object> >&&) + 564
8   com.apple.JavaScriptCore      	0x00007fff47f02e3d Inspector::BackendDispatcher::dispatch(WTF::String const&) + 2349
9   com.apple.WebKit              	0x00007fff533178ed WebKit::WebPage::didReceiveWebPageMessage(IPC::Connection&, IPC::Decoder&) + 6055
...
----

[Joseph Pecoraro]

<rdar://problem/47628191>

[Joseph Pecoraro]

Created attachment 362059 [details]
[PATCH] Proposed Fix

[Devin Rousso]

Comment on attachment 362059 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=362059&action=review

rs=me

> Source/WebCore/ChangeLog:10
> +        (WebCore::CSSStyleSheet::item):

I think you could add more of the explanation from the bug comments in the ChangeLog.  Right now, what you have here doesn't really explain "how" this "might" happen, and what you investigated to arrive at this point.

> Source/WebCore/css/CSSStyleSheet.cpp:234
> +    if (m_childRuleCSSOMWrappers.size() != ruleCount)

NIT: I think it's "smarter" to only expand if we don't have enough room, not if we don't have exactly the right amount of room.

    if (m_childRuleCSSOMWrappers.size() < ruleCount)

[Joseph Pecoraro]

Comment on attachment 362059 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=362059&action=review

>> Source/WebCore/css/CSSStyleSheet.cpp:234
>> +    if (m_childRuleCSSOMWrappers.size() != ruleCount)
> 
> NIT: I think it's "smarter" to only expand if we don't have enough room, not if we don't have exactly the right amount of room.
> 
>     if (m_childRuleCSSOMWrappers.size() < ruleCount)

Fair enough.

[Joseph Pecoraro]

Created attachment 362066 [details]
[PATCH] For Landing

[WebKit Commit Bot]

Comment on attachment 362066 [details]
[PATCH] For Landing

Clearing flags on attachment: 362066

Committed r241567: <https://trac.webkit.org/changeset/241567>