platform/mac/accessibility/aria-multiline.html sometimes asserts on Mac WebKit1: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x000000010674845a WTFCrash + 42 (Assertions.cpp:333) 1 DumpRenderTree 0x0000000105f4e71a AccessibilityController::removeNotificationListener() + 74 (AccessibilityControllerMac.mm:141) 2 DumpRenderTree 0x0000000105f4db06 removeNotificationListenerCallback(OpaqueJSContext const*, OpaqueJSValue*, OpaqueJSValue*, unsigned long, OpaqueJSValue const* const*, OpaqueJSValue const**) + 54 (AccessibilityController.cpp:130) 3 com.apple.JavaScriptCore 0x0000000106456007 long long JSC::APICallbackFunction::call<JSC::JSCallbackFunction>(JSC::ExecState*) + 455 (APICallbackFunction.h:61) 4 com.apple.JavaScriptCore 0x000000010641b7d5 handleHostCall + 293 (JITOperations.cpp:654) 5 com.apple.JavaScriptCore 0x000000010641c175 linkFor + 133 (JITOperations.cpp:697) 6 com.apple.JavaScriptCore 0x00000001064170ca operationLinkCall + 26 (JITOperations.cpp:730) Looks like this started today, perhaps with <http://trac.webkit.org/changeset/162132>.
<rdar://problem/15839077>
now that all the notifications are perhaps being sent, there might be a test that is not cleaning up after itself
(In reply to comment #2) > now that all the notifications are perhaps being sent, there might be a test that is not cleaning up after itself looking into it
(In reply to comment #3) > (In reply to comment #2) > > now that all the notifications are perhaps being sent, there might be a test that is not cleaning up after itself > > looking into it this would indicate some test didn't call removeNotificationListener() when it finished, which means that maybe a test timed out as well? if we can't figure this out and it becomes an issue, we can probably remove this assert ASSERT(m_globalNotificationHandler); in DRT and be ok as well
Would it be possible to remove the listeners automatically? It is nearly always the responsibility of DumpRenderTree to ensure that tests don't affect each other, regardless of what they do.
(In reply to comment #5) > Would it be possible to remove the listeners automatically? It is nearly always the responsibility of DumpRenderTree to ensure that tests don't affect each other, regardless of what they do. Seems reasonable. I forget the issues why I couldn't do it before like that
Affects other tests too: <http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK1%20(Tests)/r162151%20(12285)/platform/mac/accessibility/aria-menubar-crash-log.txt> (this one was reported as a failure in a different test from what CrashLog itself says).
Created attachment 221429 [details] patch
Comment on attachment 221429 [details] patch Clearing flags on attachment: 221429 Committed r162179: <http://trac.webkit.org/changeset/162179>
All reviewed patches have been landed. Closing bug.
Hmm, while this still looks like a good change to me, I'm not sure how it could fix the assertions - and indeed, they still occur: http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK1%20(Tests)/r162180%20(1934)/platform/mac/accessibility/aria-menubar-crash-log.txt
(In reply to comment #11) > Hmm, while this still looks like a good change to me, I'm not sure how it could fix the assertions - and indeed, they still occur: > > http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK1%20(Tests)/r162180%20(1934)/platform/mac/accessibility/aria-menubar-crash-log.txt Looks like there are two asserts we need to investigate.
Created attachment 221478 [details] patch
(In reply to comment #13) > Created an attachment (id=221478) [details] > patch So i was under the impression that AXController would dealloc when the page went away. That is not correct. We need to clean up this stuff on the resetToConsistentState() method
Comment on attachment 221478 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=221478&action=review r=me > Tools/DumpRenderTree/ios/AccessibilityControllerIOS.mm:126 > +{ > +} Not needed on iOS? > Tools/DumpRenderTree/mac/AccessibilityControllerMac.mm:145 > + // No longer a need to cleanup for tests, since resetToConsistentState will remove the listener. Should we remove this function completely, including from all tests?
(In reply to comment #15) > (From update of attachment 221478 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=221478&action=review > > r=me > > > Tools/DumpRenderTree/ios/AccessibilityControllerIOS.mm:126 > > +{ > > +} > > Not needed on iOS? It looks like iOS doesn't allow notifications on the axController (yet) > > > Tools/DumpRenderTree/mac/AccessibilityControllerMac.mm:145 > > + // No longer a need to cleanup for tests, since resetToConsistentState will remove the listener. > > Should we remove this function completely, including from all tests? I think so. I'll file a new bug https://bugs.webkit.org/show_bug.cgi?id=127187
Comment on attachment 221478 [details] patch Clearing flags on attachment: 221478 Committed r162222: <http://trac.webkit.org/changeset/162222>