AX: Crash at WebCore::AccessibilityMenuList::addChildren()

[chris fleizach]

It's possible to make AX code crash in AXMenuList when trying to access children after the object has been hidden

<rdar://problem/20774337>

[chris fleizach]

Created attachment 252845 [details]
patch

[chris fleizach]

Created attachment 252846 [details]
patch

[Mario Sanchez Prada]

Comment on attachment 252846 [details]
patch

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

> Source/WebCore/accessibility/AccessibilityMenuList.cpp:67
> -    m_haveChildren = true;
> -
> -    AXObjectCache* cache = m_renderer->document().axObjectCache();
> -
> +    if (!m_renderer)
> +        return;
> +    
> +    AXObjectCache* cache = axObjectCache();
> +    if (!cache)
> +        return;
> +    

Indeed. This was an accident waiting to happen.

[WebKit Commit Bot]

Comment on attachment 252846 [details]
patch

Clearing flags on attachment: 252846

Committed r184097: <http://trac.webkit.org/changeset/184097>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Andreas Kling]

This test is failing on Windows bots since it was added:

https://build.webkit.org/results/Apple%20Win%207%20Debug%20(Tests)/r184105%20(66146)/accessibility/menu-list-crash2-pretty-diff.html

--- /home/buildbot/slave/win-debug-tests/build/layout-test-results/accessibility/menu-list-crash2-expected.txt
+++ /home/buildbot/slave/win-debug-tests/build/layout-test-results/accessibility/menu-list-crash2-actual.txt
@@ -5,8 +5,8 @@
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-Role before removal: AXRole: AXPopUpButton
-Role after removal: AXRole: 
+Role before removal: AXRole: AXComboBox
+Role after removal: AXRole: AXComboBox
 PASS successfullyParsed is true
 
 TEST COMPLETE

[chris fleizach]

We need a windows expectation file

I can do that today but not for a few hours

(In reply to comment #6)
> This test is failing on Windows bots since it was added:
> 
> https://build.webkit.org/results/Apple%20Win%207%20Debug%20(Tests)/
> r184105%20(66146)/accessibility/menu-list-crash2-pretty-diff.html
> 
> ---
> /home/buildbot/slave/win-debug-tests/build/layout-test-results/accessibility/
> menu-list-crash2-expected.txt
> +++
> /home/buildbot/slave/win-debug-tests/build/layout-test-results/accessibility/
> menu-list-crash2-actual.txt
> @@ -5,8 +5,8 @@
>  On success, you will see a series of "PASS" messages, followed by "TEST
> COMPLETE".
>  
>  
> -Role before removal: AXRole: AXPopUpButton
> -Role after removal: AXRole: 
> +Role before removal: AXRole: AXComboBox
> +Role after removal: AXRole: AXComboBox
>  PASS successfullyParsed is true
>  
>  TEST COMPLETE

[Andreas Kling]

(In reply to comment #7)
> We need a windows expectation file
> 
> I can do that today but not for a few hours

I'll land one then :)