Bug 182643 – AX: defer focusedUIElement notifications

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[chris fleizach]

We should defer focus ui element notifications when they happen during layout. This has the effect of

1) reducing chances where we recurse into calling layout again
2) we coalesce multiple focus notifications that would be needlessly sent
3) improve performance by not calling out to the accessibility notification machinery during layout

[Radar WebKit Bug Importer]

<rdar://problem/37394310>

[chris fleizach]

Created attachment 333504 [details]
patch

[chris fleizach]

@zalan can you take a look when you get a chance?

[zalan]

Comment on attachment 333504 [details]
patch

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

> Source/WebCore/accessibility/AXObjectCache.cpp:1020
> +void AXObjectCache::deferFocusedUIElementChange(Node* oldNode, Node* newNode)

deferFocusedUIElementChangeIfNeeded

[chris fleizach]

https://trac.webkit.org/changeset/228376/webkit

[Matt Lewis]

This change introduced a failure on macOS.

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=accessibility%2Fmac%2Fselection-notification-focus-change.html


Diff:
--- /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/accessibility/mac/selection-notification-focus-change-expected.txt
+++ /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/accessibility/mac/selection-notification-focus-change-actual.txt
@@ -7,18 +7,18 @@
 eventSender.keyDown(tabCharacter);
 Received AXSelectedTextChanged
 PASS userInfo["AXTextSelectionChangedFocus"] is true
-Received AXFocusChanged
 Received AXSelectedTextChanged
 PASS userInfo["AXTextSelectionChangedFocus"] is true
+Received AXFocusChanged
 
 PASS accessibilityController.accessibleElementById("1").isFocusable is true
 accessibilityController.accessibleElementById("1").takeFocus()
 Received AXFocusChanged
 
 eventSender.keyDown(tabCharacter)
-Received AXFocusChanged
 Received AXSelectedTextChanged
 PASS userInfo["AXTextSelectionChangedFocus"] is true
+Received AXFocusChanged
 PASS successfullyParsed is true
 
 TEST COMPLETE

[Matt Lewis]

Reverted r228376 for reason:

This caused accessibility/mac/selection-notification-focus-change.html to become flaky on macOS.

Committed r228390: <https://trac.webkit.org/changeset/228390>

[Michael Catanzaro]

(In reply to Matt Lewis from comment #6)
> This change introduced a failure on macOS.

It caused a bunch of crashes for GTK too. Sorry the backtrace isn't very good:

Thread 1 (Thread 0x7f77ebb8fac0 (LWP 25343)):
#0  0x00007f77fa3ef1f0 in _ZN7WebCoreL32conditionallyAddNodeToFilterListEPNS_4NodeERKNS_8DocumentERN3WTF7HashSetIS1_NS5_7PtrHashIS1_EENS5_10HashTraitsIS1_EEEE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#1  0x00007f77fa3f13ba in _ZN7WebCore13AXObjectCache29prepareForDocumentDestructionERKNS_8DocumentE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#2  0x00007f77fa683b4c in _ZN7WebCore8Document21prepareForDestructionEv () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#3  0x00007f77faa2c410 in _ZN7WebCore5Frame7setViewEON3WTF6RefPtrINS_9FrameViewENS1_13DumbPtrTraitsIS3_EEEE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#4  0x00007f77faa41473 in _ZN7WebCore5Frame10createViewERKNS_7IntSizeERKNS_5ColorEbS3_RKNS_7IntRectEbNS_13ScrollbarModeEbSA_b () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#5  0x00007f77f9bf22b6 in _ZN6WebKit20WebFrameLoaderClient31transitionToCommittedForNewPageEv () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#6  0x00007f77fa980e46 in _ZN7WebCore11FrameLoader21transitionToCommittedEPNS_10CachedPageE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#7  0x00007f77fa980fa0 in _ZN7WebCore11FrameLoader21commitProvisionalLoadEv () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#8  0x00007f77fa966a24 in _ZN7WebCore14DocumentLoader15finishedLoadingEv () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#9  0x00007f77fa9687f1 in _ZN7WebCore14DocumentLoader14maybeLoadEmptyEv () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#10 0x00007f77fa96a6cc in _ZN7WebCore14DocumentLoader24startLoadingMainResourceEv () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#11 0x00007f77fa981a75 in _ZN7WebCore11FrameLoader33continueLoadAfterNavigationPolicyERKNS_15ResourceRequestEPNS_9FormStateEbNS_27AllowNavigationToInvalidURLE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#12 0x00007f77fa995e8d in _ZZN7WebCore13PolicyChecker21checkNavigationPolicyEONS_15ResourceRequestEbPNS_14DocumentLoaderEPNS_9FormStateEON3WTF17CompletionHandlerIFvS2_S6_bEEEENUlNS_12PolicyActionEE_clESC_ () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#13 0x00007f77f9c0cba2 in _ZN6WebKit8WebFrame24didReceivePolicyDecisionEmN7WebCore12PolicyActionEmNS_10DownloadIDEOSt8optionalINS_19WebsitePoliciesDataEE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#14 0x00007f77f9bfb4da in _ZN6WebKit20WebFrameLoaderClient39dispatchDecidePolicyForNavigationActionERKN7WebCore16NavigationActionERKNS1_15ResourceRequestEbPNS1_9FormStateEON3WTF8FunctionIFvNS1_12PolicyActionEEEE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#15 0x00007f77fa997848 in _ZN7WebCore13PolicyChecker21checkNavigationPolicyEONS_15ResourceRequestEbPNS_14DocumentLoaderEPNS_9FormStateEON3WTF17CompletionHandlerIFvS2_S6_bEEE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#16 0x00007f77fa981f16 in _ZN7WebCore11FrameLoader22loadWithDocumentLoaderEPNS_14DocumentLoaderENS_13FrameLoadTypeEPNS_9FormStateENS_27AllowNavigationToInvalidURLE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#17 0x00007f77fa9822a3 in _ZN7WebCore11FrameLoader24loadWithNavigationActionERKNS_15ResourceRequestERKNS_16NavigationActionENS_11LockHistoryENS_13FrameLoadTypeEPNS_9FormStateENS_27AllowNavigationToInvalidURLE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#18 0x00007f77fa982afd in _ZN7WebCore11FrameLoader7loadURLEONS_16FrameLoadRequestERKN3WTF6StringENS_13FrameLoadTypeEPNS_5EventEPNS_9FormStateE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#19 0x00007f77fa985471 in _ZN7WebCore11FrameLoader21loadURLIntoChildFrameERKNS_3URLERKN3WTF6StringEPNS_5FrameE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#20 0x00007f77f9bf44df in _ZN6WebKit20WebFrameLoaderClient11createFrameERKN7WebCore3URLERKN3WTF6StringERNS1_21HTMLFrameOwnerElementES8_bii () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#21 0x00007f77fa9a653a in _ZN7WebCore14SubframeLoader12loadSubframeERNS_21HTMLFrameOwnerElementERKNS_3URLERKN3WTF6StringES9_ () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#22 0x00007f77fa9a685d in _ZN7WebCore14SubframeLoader22loadOrRedirectSubframeERNS_21HTMLFrameOwnerElementERKNS_3URLERKN3WTF12AtomicStringENS_11LockHistoryENS_19LockBackForwardListE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#23 0x00007f77fa9a6a71 in _ZN7WebCore14SubframeLoader12requestFrameERNS_21HTMLFrameOwnerElementERKN3WTF6StringERKNS3_12AtomicStringENS_11LockHistoryENS_19LockBackForwardListE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#24 0x00007f77fa7f3108 in _ZN7WebCore20HTMLFrameElementBase7openURLENS_11LockHistoryENS_19LockBackForwardListE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#25 0x00007f77fa64a59c in _ZN7WebCore13ContainerNode43appendChildWithoutPreInsertionValidityCheckERNS_4NodeE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#26 0x00007f77fa64a6ee in _ZN7WebCore13ContainerNode11appendChildERNS_4NodeE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#27 0x00007f77fa6caee5 in _ZN7WebCore4Node11appendChildERS0_ () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#28 0x00007f77fa04f0a1 in _ZN7WebCore34jsNodePrototypeFunctionAppendChildEPN3JSC9ExecStateE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#29 0x00007f77a3bff178 in ?? ()
#30 0x00007ffe2b1695f0 in ?? ()
#31 0x00007f77f7733417 in llint_entry () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
Backtrace stopped: frame did not save the PC

[chris fleizach]

Thanks

Will address before landing again

(In reply to Michael Catanzaro from comment #8)
> (In reply to Matt Lewis from comment #6)
> > This change introduced a failure on macOS.
> 
> It caused a bunch of crashes for GTK too. Sorry the backtrace isn't very
> good:
> 
> Thread 1 (Thread 0x7f77ebb8fac0 (LWP 25343)):
> #0  0x00007f77fa3ef1f0 in
> _ZN7WebCoreL32conditionallyAddNodeToFilterListEPNS_4NodeERKNS_8DocumentERN3WT
> F7HashSetIS1_NS5_7PtrHashIS1_EENS5_10HashTraitsIS1_EEEE () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #1  0x00007f77fa3f13ba in
> _ZN7WebCore13AXObjectCache29prepareForDocumentDestructionERKNS_8DocumentE ()
> from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #2  0x00007f77fa683b4c in _ZN7WebCore8Document21prepareForDestructionEv ()
> from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #3  0x00007f77faa2c410 in
> _ZN7WebCore5Frame7setViewEON3WTF6RefPtrINS_9FrameViewENS1_13DumbPtrTraitsIS3_
> EEEE () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #4  0x00007f77faa41473 in
> _ZN7WebCore5Frame10createViewERKNS_7IntSizeERKNS_5ColorEbS3_RKNS_7IntRectEbNS
> _13ScrollbarModeEbSA_b () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #5  0x00007f77f9bf22b6 in
> _ZN6WebKit20WebFrameLoaderClient31transitionToCommittedForNewPageEv () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #6  0x00007f77fa980e46 in
> _ZN7WebCore11FrameLoader21transitionToCommittedEPNS_10CachedPageE () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #7  0x00007f77fa980fa0 in _ZN7WebCore11FrameLoader21commitProvisionalLoadEv
> () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #8  0x00007f77fa966a24 in _ZN7WebCore14DocumentLoader15finishedLoadingEv ()
> from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #9  0x00007f77fa9687f1 in _ZN7WebCore14DocumentLoader14maybeLoadEmptyEv ()
> from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #10 0x00007f77fa96a6cc in
> _ZN7WebCore14DocumentLoader24startLoadingMainResourceEv () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #11 0x00007f77fa981a75 in
> _ZN7WebCore11FrameLoader33continueLoadAfterNavigationPolicyERKNS_15ResourceRe
> questEPNS_9FormStateEbNS_27AllowNavigationToInvalidURLE () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #12 0x00007f77fa995e8d in
> _ZZN7WebCore13PolicyChecker21checkNavigationPolicyEONS_15ResourceRequestEbPNS
> _14DocumentLoaderEPNS_9FormStateEON3WTF17CompletionHandlerIFvS2_S6_bEEEENUlNS
> _12PolicyActionEE_clESC_ () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #13 0x00007f77f9c0cba2 in
> _ZN6WebKit8WebFrame24didReceivePolicyDecisionEmN7WebCore12PolicyActionEmNS_10
> DownloadIDEOSt8optionalINS_19WebsitePoliciesDataEE () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #14 0x00007f77f9bfb4da in
> _ZN6WebKit20WebFrameLoaderClient39dispatchDecidePolicyForNavigationActionERKN
> 7WebCore16NavigationActionERKNS1_15ResourceRequestEbPNS1_9FormStateEON3WTF8Fu
> nctionIFvNS1_12PolicyActionEEEE () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #15 0x00007f77fa997848 in
> _ZN7WebCore13PolicyChecker21checkNavigationPolicyEONS_15ResourceRequestEbPNS_
> 14DocumentLoaderEPNS_9FormStateEON3WTF17CompletionHandlerIFvS2_S6_bEEE ()
> from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #16 0x00007f77fa981f16 in
> _ZN7WebCore11FrameLoader22loadWithDocumentLoaderEPNS_14DocumentLoaderENS_13Fr
> ameLoadTypeEPNS_9FormStateENS_27AllowNavigationToInvalidURLE () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #17 0x00007f77fa9822a3 in
> _ZN7WebCore11FrameLoader24loadWithNavigationActionERKNS_15ResourceRequestERKN
> S_16NavigationActionENS_11LockHistoryENS_13FrameLoadTypeEPNS_9FormStateENS_27
> AllowNavigationToInvalidURLE () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #18 0x00007f77fa982afd in
> _ZN7WebCore11FrameLoader7loadURLEONS_16FrameLoadRequestERKN3WTF6StringENS_13F
> rameLoadTypeEPNS_5EventEPNS_9FormStateE () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #19 0x00007f77fa985471 in
> _ZN7WebCore11FrameLoader21loadURLIntoChildFrameERKNS_3URLERKN3WTF6StringEPNS_
> 5FrameE () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #20 0x00007f77f9bf44df in
> _ZN6WebKit20WebFrameLoaderClient11createFrameERKN7WebCore3URLERKN3WTF6StringE
> RNS1_21HTMLFrameOwnerElementES8_bii () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #21 0x00007f77fa9a653a in
> _ZN7WebCore14SubframeLoader12loadSubframeERNS_21HTMLFrameOwnerElementERKNS_3U
> RLERKN3WTF6StringES9_ () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #22 0x00007f77fa9a685d in
> _ZN7WebCore14SubframeLoader22loadOrRedirectSubframeERNS_21HTMLFrameOwnerEleme
> ntERKNS_3URLERKN3WTF12AtomicStringENS_11LockHistoryENS_19LockBackForwardListE
> () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #23 0x00007f77fa9a6a71 in
> _ZN7WebCore14SubframeLoader12requestFrameERNS_21HTMLFrameOwnerElementERKN3WTF
> 6StringERKNS3_12AtomicStringENS_11LockHistoryENS_19LockBackForwardListE ()
> from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #24 0x00007f77fa7f3108 in
> _ZN7WebCore20HTMLFrameElementBase7openURLENS_11LockHistoryENS_19LockBackForwa
> rdListE () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #25 0x00007f77fa64a59c in
> _ZN7WebCore13ContainerNode43appendChildWithoutPreInsertionValidityCheckERNS_4
> NodeE () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #26 0x00007f77fa64a6ee in _ZN7WebCore13ContainerNode11appendChildERNS_4NodeE
> () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #27 0x00007f77fa6caee5 in _ZN7WebCore4Node11appendChildERS0_ () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #28 0x00007f77fa04f0a1 in
> _ZN7WebCore34jsNodePrototypeFunctionAppendChildEPN3JSC9ExecStateE () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libwebkit2gtk-4.0.so.37
> #29 0x00007f77a3bff178 in ?? ()
> #30 0x00007ffe2b1695f0 in ?? ()
> #31 0x00007f77f7733417 in llint_entry () from
> /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/
> libjavascriptcoregtk-4.0.so.18
> Backtrace stopped: frame did not save the PC

[chris fleizach]

Created attachment 333652 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 333652 [details]
patch for landing

Clearing flags on attachment: 333652

Committed r228417: <https://trac.webkit.org/changeset/228417>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Michael Catanzaro]

Regression in bug #182763 :(