Bug 194671 - Web Inspector: Occasional crash under WebCore::CSSStyleSheet::item called from Inspector
Summary: Web Inspector: Occasional crash under WebCore::CSSStyleSheet::item called fro...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-02-14 14:00 PST by Joseph Pecoraro
Modified: 2019-02-15 16:30 PST (History)
5 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (1.69 KB, patch)
2019-02-14 14:15 PST, Joseph Pecoraro
drousso: review+
Details | Formatted Diff | Diff
[PATCH] For Landing (2.04 KB, patch)
2019-02-14 15:01 PST, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2019-02-14 14:00:13 PST
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
...
----
Comment 1 Joseph Pecoraro 2019-02-14 14:00:22 PST
<rdar://problem/47628191>
Comment 2 Joseph Pecoraro 2019-02-14 14:15:31 PST
Created attachment 362059 [details]
[PATCH] Proposed Fix
Comment 3 Devin Rousso 2019-02-14 14:19:41 PST
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)
Comment 4 Joseph Pecoraro 2019-02-14 14:58:21 PST
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.
Comment 5 Joseph Pecoraro 2019-02-14 15:01:25 PST
Created attachment 362066 [details]
[PATCH] For Landing
Comment 6 WebKit Commit Bot 2019-02-14 15:40:05 PST
Comment on attachment 362066 [details]
[PATCH] For Landing

Clearing flags on attachment: 362066

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