Bug 183023 – AX: AOM: More accessibility events support

• Component: Accessibility
• Status: RESOLVED FIXED

[Nan Wang]

Spec: https://wicg.github.io/aom/spec/phase2.html

Add support for:
accessiblecontextmenu
accessibledecrement
accessibledismiss
accessiblefocus
accessibleincrement
accessiblescrollintoview
accessibleselect

[Radar WebKit Bug Importer]

<rdar://problem/37764380>

[Nan Wang]

Created attachment 335012 [details]
patch

[Nan Wang]

Created attachment 335015 [details]
patch

This patch should work

[EWS Watchlist]

Comment on attachment 335015 [details]
patch

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

New failing tests:
accessibility/mac/AOM-events-all.html
js/dom/dom-static-property-for-in-iteration.html

[EWS Watchlist]

Created attachment 335019 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 335015 [details]
patch

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

New failing tests:
js/dom/dom-static-property-for-in-iteration.html

[EWS Watchlist]

Created attachment 335020 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Nan Wang]

Created attachment 335021 [details]
patch

fix the test

[EWS Watchlist]

Comment on attachment 335021 [details]
patch

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

New failing tests:
accessibility/mac/AOM-events-all.html

[EWS Watchlist]

Created attachment 335031 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 335021 [details]
patch

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

New failing tests:
accessibility/mac/AOM-events-all.html
accessibility/scroll-to-make-visible-iframe.html

[EWS Watchlist]

Created attachment 335039 [details]
Archive of layout-test-results from ews115 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Nan Wang]

Created attachment 335053 [details]
patch

[chris fleizach]

Comment on attachment 335053 [details]
patch

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

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3542
> +    if (m_object->dispatchAccessibilityEventWithType(AccessibilityEventType::ContextMenu))

Are we gonna implement escape action for iOS now?

[Nan Wang]

(In reply to chris fleizach from comment #14)
> Comment on attachment 335053 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=335053&action=review
> 
> > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3542
> > +    if (m_object->dispatchAccessibilityEventWithType(AccessibilityEventType::ContextMenu))
> 
> Are we gonna implement escape action for iOS now?

I think I'll file a new bug for that.

[Nan Wang]

(In reply to Nan Wang from comment #15)
> (In reply to chris fleizach from comment #14)
> > Comment on attachment 335053 [details]
> > patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=335053&action=review
> > 
> > > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3542
> > > +    if (m_object->dispatchAccessibilityEventWithType(AccessibilityEventType::ContextMenu))
> > 
> > Are we gonna implement escape action for iOS now?
> 
> I think I'll file a new bug for that.

filed:
https://bugs.webkit.org/show_bug.cgi?id=183352

[Nan Wang]

Created attachment 335064 [details]
patch

Fixed a crashing test

[WebKit Commit Bot]

Comment on attachment 335064 [details]
patch

Clearing flags on attachment: 335064

Committed r229310: <https://trac.webkit.org/changeset/229310>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

LayoutTest accessibility/mac/AOM-events.html is consistently crashing on High Sierra and timing out on Sierra after this change:

https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r229323%20(3270)/results.html

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=accessibility%2Fmac%2FAOM-events.html

[Nan Wang]

(In reply to Ryan Haddad from comment #20)
> LayoutTest accessibility/mac/AOM-events.html is consistently crashing on
> High Sierra and timing out on Sierra after this change:
> 
> https://build.webkit.org/results/
> Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r229323%20(3270)/results.html
> 
> https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=accessibility%2Fmac%2FAOM-events.html

I'm going to look at this soon

[Nan Wang]

Going to fix the test in https://bugs.webkit.org/show_bug.cgi?id=183376

[Nan Wang]

Reopen this since it's causing a crash

>  1 com.apple.WebCore              0x00d6b0ab WebCore::EventDispatcher::dispatchEvent(WTF::Vector<WebCore::Element*, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::Event&) + 43
   2 com.apple.WebCore              0x00add5eb WebCore::AccessibilityObject::dispatchAccessibilityEvent(WebCore::Event&) const + 651
   3 com.apple.WebCore              0x00acec32 WebCore::AccessibilityObject::dispatchAccessibilityEventWithType(WebCore::AccessibilityEventType) const + 290
   4 com.apple.WebCore              0x00ae7ac3 WebCore::AccessibilityRenderObject::setFocused(bool) + 35
   5 libdispatch.dylib              0x0000848b _dispatch_call_block_and_release + 12
   6 libdispatch.dylib              0x0000185f _dispatch_client_callout + 8
   7 libdispatch.dylib              0x0000c6ae _dispatch_main_queue_callback_4CF + 1199
   8 com.apple.CoreFoundation       0x000c2959 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/CoreFoundation/Foundation-1524.10/CoreFoundation/RunLoop.subproj/CFRunLoop.c:1815)
   9 com.apple.CoreFoundation       0x000845dc __CFRunLoopRun + 2556 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/CoreFoundation/Foundation-1524.10/CoreFoundation/RunLoop.subproj/CFRunLoop.c:3115)
  10 com.apple.CoreFoundation       0x00083940 CFRunLoopRunSpecific + 736 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/CoreFoundation/Foundation-1524.10/CoreFoundation/RunLoop.subproj/CFRunLoop.c:3249)
  11 com.apple.Foundation           0x000215ea -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 277 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/Foundation/Foundation-1524.10/Foundation/Soil.subproj/NSRunLoop.m:367)
  12 com.apple.Foundation           0x000214c2 -[NSRunLoop(NSRunLoop) run] + 76 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/Foundation/Foundation-1524.10/Foundation/Soil.subproj/NSRunLoop.m:389)
  13 libxpc.dylib                   0x0001068d _xpc_objc_main + 547
  14 libxpc.dylib                   0x0000f306 xpc_main + 417
  15 com.apple.WebKit.WebContent    0x000016b5  + 0
  16 libdyld.dylib                  0x00000f09 start + 1

[Nan Wang]

Created attachment 335381 [details]
patch to fix crash

[WebKit Commit Bot]

Comment on attachment 335381 [details]
patch to fix crash

Clearing flags on attachment: 335381

Committed r229452: <https://trac.webkit.org/changeset/229452>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

(In reply to WebKit Commit Bot from comment #25)
> Comment on attachment 335381 [details]
> patch to fix crash
> 
> Clearing flags on attachment: 335381
> 
> Committed r229452: <https://trac.webkit.org/changeset/229452>

The test added with this change seems to be a flaky crash. In this test run, the test results blame accessibility/mac/AOM-events.html, but the crashlog blames ccessibility/mac/AOM-events-webarea-crash.html:
https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r229462%20(3354)/results.html

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x00000004a88ac8a4 WTF::KeyValuePair<WebCore::RenderObject*, unsigned int>* WTF::HashTable<WebCore::RenderObject*, WTF::KeyValuePair<WebCore::RenderObject*, unsigned int>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::RenderObject*, unsigned int> >, WTF::PtrHash<WebCore::RenderObject*>, WTF::HashMap<WebCore::RenderObject*, unsigned int, WTF::PtrHash<WebCore::RenderObject*>, WTF::HashTraits<WebCore::RenderObject*>, WTF::HashTraits<unsigned int> >::KeyValuePairTraits, WTF::HashTraits<WebCore::RenderObject*> >::lookup<WTF::HashMapTranslatorAdapter<WTF::HashMap<WebCore::RenderObject*, unsigned int, WTF::PtrHash<WebCore::RenderObject*>, WTF::HashTraits<WebCore::RenderObject*>, WTF::HashTraits<unsigned int> >::KeyValuePairTraits, WTF::IdentityHashTranslator<WTF::HashMap<WebCore::RenderObject*, unsigned int, WTF::PtrHash<WebCore::RenderObject*>, WTF::HashTraits<WebCore::RenderObject*>, WTF::HashTraits<unsigned int> >::KeyValuePairTraits, WTF::PtrHash<WebCore::RenderObject*> > >, WebCore::RenderObject*>(WebCore::RenderObject* const&) + 4 (HashTable.h:601)
1   com.apple.WebCore             	0x00000004a889d8c3 WebCore::AXObjectCache::get(WebCore::RenderObject*) + 35 (AXObjectCache.cpp:386)
2   com.apple.WebCore             	0x00000004a889dfdf WebCore::AXObjectCache::getOrCreate(WebCore::RenderObject*) + 47 (AXObjectCache.cpp:621)
3   com.apple.WebCore             	0x00000004a88d6380 WebCore::AccessibilityRenderObject::addChildren() + 48 (AccessibilityRenderObject.cpp:3262)
4   com.apple.WebCore             	0x00000004a88d5d3e WebCore::AccessibilityRenderObject::updateChildrenIfNecessary() + 94 (AccessibilityRenderObject.cpp:3044)
5   com.apple.WebCore             	0x00000004a88c0401 WebCore::AccessibilityObject::updateBackingStore() + 97 (RefCounted.h:98)
6   com.apple.WebCore             	0x00000004a94f73fd -[WebAccessibilityObjectWrapperBase updateObjectBackingStore] + 61 (WebAccessibilityObjectWrapperBase.mm:294)
7   com.apple.WebCore             	0x00000004a9509377 -[WebAccessibilityObjectWrapper _accessibilitySetValue:forAttribute:] + 39 (WebAccessibilityObjectWrapperMac.mm:3620)
8   libdispatch.dylib             	0x00007fff76964591 _dispatch_call_block_and_release + 12
9   libdispatch.dylib             	0x00007fff7695cd50 _dispatch_client_callout + 8
10  libdispatch.dylib             	0x00007fff7696832d _dispatch_main_queue_callback_4CF + 1148
11  com.apple.CoreFoundation      	0x00007fff4f02f839 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9
12  com.apple.CoreFoundation      	0x00007fff4eff208a __CFRunLoopRun + 2586
13  com.apple.CoreFoundation      	0x00007fff4eff13d7 CFRunLoopRunSpecific + 487
14  com.apple.HIToolbox           	0x00007fff4e2fee26 RunCurrentEventLoopInMode + 286
15  com.apple.HIToolbox           	0x00007fff4e2feb96 ReceiveNextEventCommon + 613
16  com.apple.HIToolbox           	0x00007fff4e2fe914 _BlockUntilNextEventMatchingListInModeWithFilter + 64
17  com.apple.AppKit              	0x00007fff4c5c9f5f _DPSNextEvent + 2085
18  com.apple.AppKit              	0x00007fff4cd5fb4c -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 3044
19  com.apple.AppKit              	0x00007fff4c5bed6d -[NSApplication run] + 764
20  com.apple.AppKit              	0x00007fff4c58df1a NSApplicationMain + 804
21  libxpc.dylib                  	0x00007fff76c6342f _xpc_objc_main + 580
22  libxpc.dylib                  	0x00007fff76c62082 xpc_main + 417
23  com.apple.WebKit.WebContent   	0x000000010eba26a1 main + 490
24  libdyld.dylib                 	0x00007fff76996115 start + 1

[Nan Wang]

Ok I think the problem is that takeFocus() is async and the webpage closes before it finishes calling updateObjectBackingStore

[Nan Wang]

Created attachment 335449 [details]
patch to fix test crash

Fixed the test crash

[chris fleizach]

Comment on attachment 335449 [details]
patch to fix test crash

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

> LayoutTests/ChangeLog:9
> +        The test is crashing during updateObjectBackingStore. The problem

is it a crash or an assert

[Nan Wang]

Comment on attachment 335449 [details]
patch to fix test crash

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

>> LayoutTests/ChangeLog:9
>> +        The test is crashing during updateObjectBackingStore. The problem
> 
> is it a crash or an assert

It's a crash during hash table lookup

[chris fleizach]

seems like we should have an assert so we don't get into that state. don't think we should hit a crash for those cases.

[Nan Wang]

(In reply to chris fleizach from comment #32)
> seems like we should have an assert so we don't get into that state. don't
> think we should hit a crash for those cases.

I can reproduce this crash on a release build. And this patch fixed the crash.

[chris fleizach]

(In reply to Nan Wang from comment #33)
> (In reply to chris fleizach from comment #32)
> > seems like we should have an assert so we don't get into that state. don't
> > think we should hit a crash for those cases.
> 
> I can reproduce this crash on a release build. And this patch fixed the
> crash.

I understand that the test change can fix the patch, but I'm more interesting in fixing the actual crash

[Nan Wang]

(In reply to chris fleizach from comment #34)
> (In reply to Nan Wang from comment #33)
> > (In reply to chris fleizach from comment #32)
> > > seems like we should have an assert so we don't get into that state. don't
> > > think we should hit a crash for those cases.
> > 
> > I can reproduce this crash on a release build. And this patch fixed the
> > crash.
> 
> I understand that the test change can fix the patch, but I'm more
> interesting in fixing the actual crash

Ok I can take some further look at it

[Nan Wang]

Created attachment 335491 [details]
patch to fix test crash

[chris fleizach]

Comment on attachment 335491 [details]
patch to fix test crash

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

> Source/WebCore/ChangeLog:9
> +        The test is crashing that we call updateBackingStore when 

test is crashing "when" we

> Source/WebCore/ChangeLog:12
> +        Modified the test by using the right format of setTimtout and extended the delay.

setTimeout

> Source/WebCore/accessibility/AccessibilityObject.cpp:1780
> +    AXObjectCache* cache = this->axObjectCache();

if (!axObjectCache())
   return;

[Nan Wang]

Created attachment 335492 [details]
patch to fix test crash

update from review

[WebKit Commit Bot]

Comment on attachment 335492 [details]
patch to fix test crash

Clearing flags on attachment: 335492

Committed r229499: <https://trac.webkit.org/changeset/229499>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.