Re-land: Don't keep recreating AXMenuListPopup

Source/modules/accessibility/AXMenuListPopup.cpp

Index: Source/modules/accessibility/AXMenuListPopup.cpp
diff --git a/Source/modules/accessibility/AXMenuListPopup.cpp b/Source/modules/accessibility/AXMenuListPopup.cpp
index cdefe82992c8dd9c4f135b1edc69d9478a286a18..c52856145ca6728450084b010d3fa306651c5251 100644
--- a/Source/modules/accessibility/AXMenuListPopup.cpp
+++ b/Source/modules/accessibility/AXMenuListPopup.cpp
@@ -71,17 +71,19 @@ AXMenuListOption* AXMenuListPopup::menuListOptionAXObject(HTMLElement* element)
     if (!isHTMLOptionElement(*element))
         return 0;
 
-    AXObject* object = axObjectCache()->getOrCreate(MenuListOptionRole);
-    ASSERT_WITH_SECURITY_IMPLICATION(object->isMenuListOption());
-
-    AXMenuListOption* option = toAXMenuListOption(object);
-    option->setElement(element);
+    AXObject* object = axObjectCache()->getOrCreate(element);
+    if (!object)
+        return 0;
 
-    return option;
+    ASSERT_WITH_SECURITY_IMPLICATION(object->isMenuListOption());
+    return toAXMenuListOption(object);
 }
 
 int AXMenuListPopup::getSelectedIndex() const
 {
+    if (!m_parent)
+        return -1;
+
     Node* selectNode = m_parent->node();
     if (!selectNode)
         return -1;
@@ -111,7 +113,9 @@ void AXMenuListPopup::addChildren()
     HTMLSelectElement* htmlSelectElement = toHTMLSelectElement(selectNode);
     m_haveChildren = true;
 
-    m_activeIndex = getSelectedIndex();
+    if (m_activeIndex == -1)
+        m_activeIndex = getSelectedIndex();
+
     const WillBeHeapVector<RawPtrWillBeMember<HTMLElement>>& listItems = htmlSelectElement->listItems();
     unsigned length = listItems.size();
     for (unsigned i = 0; i < length; i++) {
@@ -123,23 +127,10 @@ void AXMenuListPopup::addChildren()
     }
 }
 
-void AXMenuListPopup::childrenChanged()
+void AXMenuListPopup::updateChildrenIfNecessary()

[je_julie(Not used), 2015/04/06 08:42:50]

From previous code, I can see childrenChanged() is called when children is
updated.
When I search 'updateChildrenIfNecessary()', it seems to be called when
'AXObject::children()' is called.
I'm not sure but do we need to clear and add children when AXObject::children()
is called?

[dmazzoni, 2015/04/06 15:42:12]

Hmmm, good point.

Looking again at AXLayoutObject::updateChildrenIfNecessary, it already does the
logic we need - we can just rely on it to call clearChildren only when necessary
and addChildren will be called automatically.

I deleted this override, the new test still passes.

 {
-    AXObjectCacheImpl* cache = axObjectCache();
-    if (!cache)
-        return;
-    for (size_t i = m_children.size(); i > 0 ; --i) {
-        AXObject* child = m_children[i - 1].get();
-        // FIXME: How could children end up in here that have no actionElement(), the check
-        // in menuListOptionAXObject would seem to prevent that.
-        if (child->actionElement()) {
-            child->detachFromParent();
-            cache->remove(child->axObjectID());
-        }
-    }
-
-    m_children.clear();
-    m_haveChildren = false;
+    clearChildren();
+    addChildren();
 }
 
 void AXMenuListPopup::didUpdateActiveOption(int optionIndex)
@@ -149,18 +140,18 @@ void AXMenuListPopup::didUpdateActiveOption(int optionIndex)
     if (!m_haveChildren)
         addChildren();
 
-    ASSERT_ARG(optionIndex, optionIndex >= 0);
-    ASSERT_ARG(optionIndex, optionIndex < static_cast<int>(m_children.size()));
-
     AXObjectCacheImpl* cache = axObjectCache();
     if (m_activeIndex != optionIndex && m_activeIndex >= 0 && m_activeIndex < static_cast<int>(m_children.size())) {
         RefPtr<AXObject> previousChild = m_children[m_activeIndex].get();
         cache->postNotification(previousChild.get(), AXObjectCacheImpl::AXMenuListItemUnselected);
     }
 
-    RefPtr<AXObject> child = m_children[optionIndex].get();
-    cache->postNotification(child.get(), AXObjectCacheImpl::AXFocusedUIElementChanged);
-    cache->postNotification(child.get(), AXObjectCacheImpl::AXMenuListItemSelected);
+    if (optionIndex >= 0 && optionIndex < static_cast<int>(m_children.size())) {
+        RefPtr<AXObject> child = m_children[optionIndex].get();
+        cache->postNotification(child.get(), AXObjectCacheImpl::AXFocusedUIElementChanged);
+        cache->postNotification(child.get(), AXObjectCacheImpl::AXMenuListItemSelected);
+    }
+
     m_activeIndex = optionIndex;
 }