Bug 194632 – Web Inspector: Crash when inspecting an element that constantly changes visibility

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

[Joseph Pecoraro]

Crash when inspecting an element that constantly changes visibility

Test:
<div id="box"></div>
<script>
setInterval(function() {
    box.hidden = !box.hidden;
}, 50);
</script>

Steps to Reproduce:
1. Inspect test page
2. Select the <div id="box"> in the Elements tab
  => Page crashes

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

Exception Type:        EXC_BAD_INSTRUCTION (SIGILL)
Exception Codes:       0x0000000000000001, 0x0000000000000000
Exception Note:        EXC_CORPSE_NOTIFY

Application Specific Information:
Pure virtual function called!
abort() called 

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libsystem_c.dylib             	0x00007fff608df766 __abort + 177
1   libsystem_c.dylib             	0x00007fff608df6b5 abort + 142
2   libc++abi.dylib               	0x00007fff5d98b641 abort_message + 231
3   libc++abi.dylib               	0x00007fff5d997ad2 __cxa_pure_virtual + 18
4   com.apple.WebCore             	0x0000000491d8feff WebCore::AccessibilityObject::matchedParent(WebCore::AccessibilityObject const&, bool, WTF::Function<bool (WebCore::AccessibilityObject const&)> const&) + 63 (AccessibilityObject.cpp:1901)
5   com.apple.WebCore             	0x0000000491d94e73 WebCore::AccessibilityObject::parentObjectUnignored() const + 51 (AccessibilityObject.cpp:468)
6   com.apple.WebCore             	0x0000000492a6b0da WebCore::InspectorDOMAgent::buildObjectForAccessibilityProperties(WebCore::Node*) + 3642 (InspectorDOMAgent.cpp:1956)
7   com.apple.WebCore             	0x0000000492a6a273 WebCore::InspectorDOMAgent::getAccessibilityPropertiesForNode(WTF::String&, int, WTF::RefPtr<Inspector::Protocol::DOM::AccessibilityProperties, WTF::DumbPtrTraits<Inspector::Protocol::DOM::AccessibilityProperties> >&) + 83 (InspectorDOMAgent.cpp:995)
8   com.apple.JavaScriptCore      	0x00000004a7395582 Inspector::DOMBackendDispatcher::getAccessibilityPropertiesForNode(long, WTF::RefPtr<WTF::JSONImpl::Object, WTF::DumbPtrTraits<WTF::JSONImpl::Object> >&&) + 418 (InspectorBackendDispatchers.cpp:1920)
9   com.apple.JavaScriptCore      	0x00000004a739249b Inspector::DOMBackendDispatcher::dispatch(long, WTF::String const&, WTF::Ref<WTF::JSONImpl::Object, WTF::DumbPtrTraits<WTF::JSONImpl::Object> >&&) + 891 (InspectorBackendDispatchers.cpp:1496)
10  com.apple.JavaScriptCore      	0x00000004a73854fb Inspector::BackendDispatcher::dispatch(WTF::String const&) + 1883 (InspectorBackendDispatcher.cpp:180)
11  com.apple.WebCore             	0x00000004929f728c WebCore::InspectorController::dispatchMessageFromFrontend(WTF::String const&) + 44 (InspectorController.cpp:426)
12  com.apple.WebKit              	0x000000010fe5a90c WebKit::WebPageInspectorTarget::sendMessageToTargetBackend(WTF::String const&) + 76 (WebPageInspectorTarget.cpp:63)
...

[Joseph Pecoraro]

Created attachment 361978 [details]
[PATCH] Proposed Fix

[Radar WebKit Bug Importer]

<rdar://problem/48060258>

[Matt Baker]

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

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

r=me

> Source/WebCore/inspector/agents/InspectorDOMAgent.cpp:1746
> +void InspectorDOMAgent::processAccessibilityChildren(AccessibilityObject& axObject, JSON::ArrayOf<int>& childNodeIds)

This can be const AccessibilityObject&, since the children method is const.

[Joseph Pecoraro]

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

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

>> Source/WebCore/inspector/agents/InspectorDOMAgent.cpp:1746
>> +void InspectorDOMAgent::processAccessibilityChildren(AccessibilityObject& axObject, JSON::ArrayOf<int>& childNodeIds)
> 
> This can be const AccessibilityObject&, since the children method is const.

In this case it surprisingly is not const:

   const AccessibilityChildrenVector& children(bool updateChildrenIfNeeded = true);

[Devin Rousso]

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

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

r=me (as well), nice fix :)

I refactored a lot of this code when building `InspectorAuditAccessibilityObject`, so you could copy/merge much of this code if you wanted to change anything else :P

> Source/WebCore/inspector/agents/InspectorDOMAgent.cpp:1834
> +                processAccessibilityChildren(*axObject, *childNodeIds.get());

Could you just use `*childNodeIds`?

[Joseph Pecoraro]

Created attachment 361983 [details]
[PATCH] For Landing

[WebKit Commit Bot]

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

Clearing flags on attachment: 361983

Committed r241495: <https://trac.webkit.org/changeset/241495>

[Matt Baker]

(In reply to Joseph Pecoraro from comment #4)
> Comment on attachment 361978 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=361978&action=review
> 
> >> Source/WebCore/inspector/agents/InspectorDOMAgent.cpp:1746
> >> +void InspectorDOMAgent::processAccessibilityChildren(AccessibilityObject& axObject, JSON::ArrayOf<int>& childNodeIds)
> > 
> > This can be const AccessibilityObject&, since the children method is const.
> 
> In this case it surprisingly is not const:
> 
>    const AccessibilityChildrenVector& children(bool updateChildrenIfNeeded =
> true);

Oops, I saw the const return value and my eyes played tricks on me.