Avoid Node references from AXObjectCache from leaking

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

Reviewed by Darin Adler.

Merge https://chromium.googlesource.com/chromium/blink/+/454f31497613b6d0fbcfb0df757254b64a177c06
without any tests since we don't have the same infrastructure to detect leaks in WebKit.

A real world example of this would be selecting an <option> item inside frame by keyboard. The node will not be deref()-ed until the topDocument() is detached.

The issue was that AccessibilityMenuListOption is created in childrenChanged()
hook called when its RenderObject is being destroyed. This patch modifies AccessibilityMenuListPopup so it won't create AccessibilityMenuListOption if its
element is already detached.

* accessibility/AccessibilityMenuListPopup.cpp:
(WebCore::AccessibilityMenuListPopup::menuListOptionAccessibilityObject):
* rendering/RenderMenuList.cpp:
(WebCore::RenderMenuList::didUpdateActiveOption):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@154859 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index bf60008893d31847de681f91abeb3c4c9d74073b..8d075552916e46ef71d010b3c734955c42b5a5b2 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,24 @@
+2013-08-29  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Avoid Node references from AXObjectCache from leaking
+        https://bugs.webkit.org/show_bug.cgi?id=120501
+
+        Reviewed by Darin Adler.
+
+        Merge https://chromium.googlesource.com/chromium/blink/+/454f31497613b6d0fbcfb0df757254b64a177c06
+        without any tests since we don't have the same infrastructure to detect leaks in WebKit.
+
+        A real world example of this would be selecting an <option> item inside frame by keyboard. The node will not be deref()-ed until the topDocument() is detached. 
+
+        The issue was that AccessibilityMenuListOption is created in childrenChanged()
+        hook called when its RenderObject is being destroyed. This patch modifies AccessibilityMenuListPopup so it won't create AccessibilityMenuListOption if its
+        element is already detached.
+
+        * accessibility/AccessibilityMenuListPopup.cpp:
+        (WebCore::AccessibilityMenuListPopup::menuListOptionAccessibilityObject):
+        * rendering/RenderMenuList.cpp:
+        (WebCore::RenderMenuList::didUpdateActiveOption):
+
 2013-08-26  Simon Fraser  <simon.fraser@apple.com>
 
         Implement object-fit CSS property

diff --git a/Source/WebCore/accessibility/AccessibilityMenuListPopup.cpp b/Source/WebCore/accessibility/AccessibilityMenuListPopup.cpp
index 6e67f2cf5c2f4fbc253aabcc8f6a2e36e40472e7..88badd18cf354c97d39a46a640cf768e4ddb8e2a 100644 (file)
--- a/Source/WebCore/accessibility/AccessibilityMenuListPopup.cpp
+++ b/Source/WebCore/accessibility/AccessibilityMenuListPopup.cpp
@@ -70,7 +70,7 @@ bool AccessibilityMenuListPopup::computeAccessibilityIsIgnored() const
 
 AccessibilityMenuListOption* AccessibilityMenuListPopup::menuListOptionAccessibilityObject(HTMLElement* element) const
 {
-    if (!element || !isHTMLOptionElement(element))
+    if (!element || !isHTMLOptionElement(element) || !element->attached())
         return 0;
 
     AccessibilityObject* object = document()->axObjectCache()->getOrCreate(MenuListOptionRole);

diff --git a/Source/WebCore/rendering/RenderMenuList.cpp b/Source/WebCore/rendering/RenderMenuList.cpp
index 47af9b92adcafc5ec0537c349502ab9457cde353..681fee472aa33540c2529b2f8071e387e621705b 100644 (file)
--- a/Source/WebCore/rendering/RenderMenuList.cpp
+++ b/Source/WebCore/rendering/RenderMenuList.cpp
@@ -392,10 +392,12 @@ void RenderMenuList::didUpdateActiveOption(int optionIndex)
     if (listIndex < 0 || listIndex >= static_cast<int>(select->listItems().size()))
         return;
 
-    ASSERT(select->listItems()[listIndex]);
-
-    if (AccessibilityMenuList* menuList = static_cast<AccessibilityMenuList*>(document().axObjectCache()->get(this)))
-        menuList->didUpdateActiveOption(optionIndex);
+    HTMLElement* listItem = select->listItems()[listIndex];
+    ASSERT(listItem);
+    if (listItem->attached()) {
+        if (AccessibilityMenuList* menuList = static_cast<AccessibilityMenuList*>(document().axObjectCache()->get(this)))
+            menuList->didUpdateActiveOption(optionIndex);
+    }
 }
 
 String RenderMenuList::itemText(unsigned listIndex) const