Bug 171927

Summary: ASSERTION FAILED in WebCore::AccessibilityNodeObject::insertChild()
Product: WebKit Reporter: zalan <zalan>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, bfulgham, buildbot, cfleizach, dmazzoni, jcraig, jdiggs, n_wang, rniwa, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
patch
none
patch
cfleizach: review+, buildbot: commit-queue-
Archive of layout-test-results from ews103 for mac-elcapitan none

Description zalan 2017-05-10 09:44:08 PDT
in WebCore::AccessibilityNodeObject::insertChild() 

run-webkit-test 
 editing/execCommand/align-in-span.html 
 fullscreen/full-screen-render-inline.html
with AX enabled.
Comment 1 Radar WebKit Bug Importer 2017-05-10 09:44:32 PDT
<rdar://problem/32109781>
Comment 2 zalan 2017-05-10 09:52:46 PDT
0   com.apple.JavaScriptCore      	0x000000011d0de234 WTFCrash + 36 (Assertions.cpp:292)
1   com.apple.WebCore             	0x000000010fa9b1b1 WebCore::AccessibilityNodeObject::insertChild(WebCore::AccessibilityObject*, unsigned int) + 529 (AccessibilityNodeObject.cpp:372)
2   com.apple.WebCore             	0x000000010fa9b5bd WebCore::AccessibilityNodeObject::addChild(WebCore::AccessibilityObject*) + 77 (AccessibilityNodeObject.cpp:383)
3   com.apple.WebCore             	0x000000010fadf929 WebCore::AccessibilityRenderObject::addChildren() + 217 (AccessibilityRenderObject.cpp:3219)
4   com.apple.WebCore             	0x000000010fab0281 WebCore::AccessibilityObject::updateChildrenIfNecessary() + 81 (AccessibilityObject.cpp:1785)
5   com.apple.WebCore             	0x000000010fadeba6 WebCore::AccessibilityRenderObject::updateChildrenIfNecessary() + 70 (AccessibilityRenderObject.cpp:2999)
6   com.apple.WebCore             	0x000000010faa902c WebCore::AccessibilityObject::updateBackingStore() + 124 (AccessibilityObject.cpp:1727)
7   com.apple.WebCore             	0x0000000112753751 -[WebAccessibilityObjectWrapperBase updateObjectBackingStore] + 113
8   com.apple.WebCore             	0x000000011276b2ab -[WebAccessibilityObjectWrapper accessibilityIsIgnored] + 43 (WebAccessibilityObjectWrapperMac.mm:3282)
9   com.apple.WebCore             	0x000000010fc8f839 WebCore::AXObjectCache::postPlatformNotification(WebCore::AccessibilityObject*, WebCore::AXObjectCache::AXNotification) + 601 (AXObjectCacheMac.mm:351)
10  com.apple.WebCore             	0x000000010fc5f57f WebCore::AXObjectCache::notificationPostTimerFired() + 447 (AXObjectCache.cpp:923)
11  com.apple.WebCore             	0x000000010fc7876b void std::__1::__invoke_void_return_wrapper<void>::__call<std::__1::__bind<void (WebCore::AXObjectCache::*&)(), WebCore::AXObjectCache*>&>(std::__1::__bind<void (WebCore::AXObjectCache::*&)(), WebCore::AXObjectCache*>&&&) + 235 (__functional_base:469)
12  com.apple.WebCore             	0x000000010fc78659 std::__1::__function::__func<std::__1::__bind<void (WebCore::AXObjectCache::*&)(), WebCore::AXObjectCache*>, std::__1::allocator<std::__1::__bind<void (WebCore::AXObjectCache::*&)(), WebCore::AXObjectCache*> >, void ()>::operator()() + 41 (functional:1437)
13  com.apple.WebCore             	0x000000010fae4eba std::__1::function<void ()>::operator()() const + 26 (functional:1817)
14  com.apple.WebCore             	0x000000010fae4dd9 WebCore::Timer::fired() + 25 (Timer.h:135)
15  com.apple.WebCore             	0x00000001126031c0 WebCore::ThreadTimers::sharedTimerFiredInternal() + 480 (ThreadTimers.cpp:121)
16  com.apple.WebCore             	0x0000000112604401 WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0::operator()() const + 33 (ThreadTimers.cpp:70)
17  com.apple.WebCore             	0x00000001126043cd void std::__1::__invoke_void_return_wrapper<void>::__call<WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0&>(WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0&&&) + 45 (__functional_base:469)
18  com.apple.WebCore             	0x0000000112604379 std::__1::__function::__func<WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0, std::__1::allocator<WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0>, void ()>::operator()() + 41 (functional:1437)
19  com.apple.WebCore             	0x000000010fae4eba std::__1::function<void ()>::operator()() const + 26 (functional:1817)
20  com.apple.WebCore             	0x00000001118a5a88 WebCore::MainThreadSharedTimer::fired() + 104 (MainThreadSharedTimer.cpp:53)
21  com.apple.WebCore             	0x00000001118a5e19 WebCore::timerFired(__CFRunLoopTimer*, void*) + 41 (MainThreadSharedTimerCF.cpp:74)
22  com.apple.CoreFoundation      	0x00007fff8ccaede4 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 20
23  com.apple.CoreFoundation      	0x00007fff8ccaea73 __CFRunLoopDoTimer + 1075
24  com.apple.CoreFoundation      	0x00007fff8ccae5ca __CFRunLoopDoTimers + 298
25  com.apple.CoreFoundation      	0x00007fff8cca5fa1 __CFRunLoopRun + 2081
26  com.apple.CoreFoundation      	0x00007fff8cca5524 CFRunLoopRunSpecific + 420
27  com.apple.HIToolbox           	0x00007fff8c205ebc RunCurrentEventLoopInMode + 240
28  com.apple.HIToolbox           	0x00007fff8c205cf1 ReceiveNextEventCommon + 432
29  com.apple.HIToolbox           	0x00007fff8c205b26 _BlockUntilNextEventMatchingListInModeWithFilter + 71
30  com.apple.AppKit              	0x00007fff8a7a0e24 _DPSNextEvent + 1120
31  com.apple.AppKit              	0x00007fff8af1c85e -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 2796
32  com.apple.AppKit              	0x00007fff8a7957ab -[NSApplication run] + 926
33  com.apple.AppKit              	0x00007fff8a7601de NSApplicationMain + 1237
34  libxpc.dylib                  	0x00007fffa2aa88c7 _xpc_objc_main + 775
35  libxpc.dylib                  	0x00007fffa2aa72e4 xpc_main + 494
36  com.apple.WebKit.WebContent   	0x000000010b5be115 main + 1189 (XPCServiceMain.mm:148)
37  libdyld.dylib                 	0x00007fffa284f235 start + 1
Comment 3 Nan Wang 2017-05-16 16:49:33 PDT
Created attachment 310317 [details]
patch
Comment 4 chris fleizach 2017-05-16 16:57:34 PDT
Comment on attachment 310317 [details]
patch

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

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:371
> +    } else if (child->parentObject() == this)

should we leave this check in place and instead return nil for this case in nextSibling()?
Comment 5 Nan Wang 2017-05-16 17:03:29 PDT
Comment on attachment 310317 [details]
patch

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

>> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:371
>> +    } else if (child->parentObject() == this)
> 
> should we leave this check in place and instead return nil for this case in nextSibling()?

I think there might be case somewhere we actually want the sibling element.
Comment 6 chris fleizach 2017-05-16 17:06:55 PDT
(In reply to Nan Wang from comment #5)
> Comment on attachment 310317 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=310317&action=review
> 
> >> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:371
> >> +    } else if (child->parentObject() == this)
> > 
> > should we leave this check in place and instead return nil for this case in nextSibling()?
> 
> I think there might be case somewhere we actually want the sibling element.

can we detect the bad case and return nil from nextSibling()? then we can leave the assert in place that finds elements with bad parents.

because with this we are going to essentially throw away those elements anyway since they don't get inserted into the tree

or do we have an issue where we don't want this current nextSibling() but if we did

nextSibling()->nextSibling() that's a good element?

if that's the case, can we just do nextSibling()->nextSibling() inside nextSibling() ?
Comment 7 Nan Wang 2017-05-16 17:23:26 PDT
Created attachment 310326 [details]
patch

updated from review.

I think for AXRenderObject, siblings should share the same parent object since we already considered those cases in renderParentObject().
Comment 8 Build Bot 2017-05-16 17:26:06 PDT
Attachment 310326 [details] did not pass style-queue:


ERROR: Source/WebCore/accessibility/AccessibilityRenderObject.cpp:414:  Declaration has space between type name and * in AccessibilityObject *nextObj  [whitespace/declaration] [3]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Nan Wang 2017-05-16 17:27:35 PDT
Created attachment 310327 [details]
patch

fix style
Comment 10 Build Bot 2017-05-17 02:51:56 PDT
Comment on attachment 310327 [details]
patch

Attachment 310327 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3760330

New failing tests:
http/tests/appcache/404-resource-with-slow-main-resource.php
http/tests/media/hls/video-controls-live-stream.html
Comment 11 Build Bot 2017-05-17 02:51:57 PDT
Created attachment 310366 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 12 Nan Wang 2017-05-17 10:33:56 PDT
I don't think these failures are related.
Comment 13 Nan Wang 2017-05-17 10:34:29 PDT
Committed r216980: <http://trac.webkit.org/changeset/216980>