Bug 646369 - UpdateTree should rely on accessible tree walker rather than DOM tree traversal, r=davidb
authorAlexander Surkov <surkov.alexander@gmail.com>
Thu, 07 Apr 2011 14:17:29 +0900
changeset 67625 883645192a0697c875101af8ebacc6e17c64629b
parent 67590 4a3d11e05c8e6f87be21fd41ff6e8646fb4a0a98
child 67626 558fff008e746e4011790ad8dd5c6ed10a46f61f
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdavidb
bugs646369
milestone2.2a1pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 646369 - UpdateTree should rely on accessible tree walker rather than DOM tree traversal, r=davidb
accessible/src/base/NotificationController.cpp
accessible/src/base/NotificationController.h
accessible/src/base/nsAccTreeWalker.cpp
accessible/src/base/nsAccTreeWalker.h
accessible/src/base/nsDocAccessible.cpp
accessible/src/base/nsDocAccessible.h
accessible/tests/mochitest/events/Makefile.in
accessible/tests/mochitest/events/test_mutation.xhtml
--- a/accessible/src/base/NotificationController.cpp
+++ b/accessible/src/base/NotificationController.cpp
@@ -152,21 +152,22 @@ void
 NotificationController::ScheduleContentInsertion(nsAccessible* aContainer,
                                                  nsIContent* aStartChildNode,
                                                  nsIContent* aEndChildNode)
 {
   // Ignore content insertions until we constructed accessible tree.
   if (mTreeConstructedState == eTreeConstructionPending)
     return;
 
-  nsRefPtr<ContentInsertion> insertion =
-    new ContentInsertion(mDocument, aContainer, aStartChildNode, aEndChildNode);
-
-  if (insertion && mContentInsertions.AppendElement(insertion))
+  nsRefPtr<ContentInsertion> insertion = new ContentInsertion(mDocument,
+                                                              aContainer);
+  if (insertion && insertion->InitChildList(aStartChildNode, aEndChildNode) &&
+      mContentInsertions.AppendElement(insertion)) {
     ScheduleProcessing();
+  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // NotificationCollector: protected
 
 void
 NotificationController::ScheduleProcessing()
 {
@@ -676,25 +677,41 @@ NotificationController::TextEnumerator(n
   return PL_DHASH_NEXT;
 }
 
 
 ////////////////////////////////////////////////////////////////////////////////
 // NotificationController: content inserted notification
 
 NotificationController::ContentInsertion::
-  ContentInsertion(nsDocAccessible* aDocument, nsAccessible* aContainer,
-                   nsIContent* aStartChildNode, nsIContent* aEndChildNode) :
+  ContentInsertion(nsDocAccessible* aDocument, nsAccessible* aContainer) :
   mDocument(aDocument), mContainer(aContainer)
 {
+}
+
+bool
+NotificationController::ContentInsertion::
+  InitChildList(nsIContent* aStartChildNode, nsIContent* aEndChildNode)
+{
+  bool haveToUpdate = false;
+
   nsIContent* node = aStartChildNode;
   while (node != aEndChildNode) {
-    mInsertedContent.AppendElement(node);
+    // Notification triggers for content insertion even if no content was
+    // actually inserted, check if the given content has a frame to discard
+    // this case early.
+    if (node->GetPrimaryFrame()) {
+      if (mInsertedContent.AppendElement(node))
+        haveToUpdate = true;
+    }
+
     node = node->GetNextSibling();
   }
+
+  return haveToUpdate;
 }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(NotificationController::ContentInsertion)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_NATIVE(NotificationController::ContentInsertion)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mContainer)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
--- a/accessible/src/base/NotificationController.h
+++ b/accessible/src/base/NotificationController.h
@@ -316,23 +316,23 @@ private:
   nsTArray<nsRefPtr<nsDocAccessible> > mHangingChildDocuments;
 
   /**
    * Storage for content inserted notification information.
    */
   class ContentInsertion
   {
   public:
-    ContentInsertion(nsDocAccessible* aDocument, nsAccessible* aContainer,
-                     nsIContent* aStartChildNode, nsIContent* aEndChildNode);
+    ContentInsertion(nsDocAccessible* aDocument, nsAccessible* aContainer);
     virtual ~ContentInsertion() { mDocument = nsnull; }
 
     NS_INLINE_DECL_REFCOUNTING(ContentInsertion)
     NS_DECL_CYCLE_COLLECTION_NATIVE_CLASS(ContentInsertion)
 
+    bool InitChildList(nsIContent* aStartChildNode, nsIContent* aEndChildNode);
     void Process();
 
   private:
     ContentInsertion();
     ContentInsertion(const ContentInsertion&);
     ContentInsertion& operator = (const ContentInsertion&);
 
     // The document used to process content insertion, matched to document of
--- a/accessible/src/base/nsAccTreeWalker.cpp
+++ b/accessible/src/base/nsAccTreeWalker.cpp
@@ -61,18 +61,18 @@ struct WalkState
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 // nsAccTreeWalker
 ////////////////////////////////////////////////////////////////////////////////
 
 nsAccTreeWalker::
   nsAccTreeWalker(nsIWeakReference* aShell, nsIContent* aContent,
-                  PRBool aWalkAnonContent) :
-  mWeakShell(aShell), mState(nsnull)
+                  PRBool aWalkAnonContent, bool aWalkCache) :
+  mWeakShell(aShell), mState(nsnull), mWalkCache(aWalkCache)
 {
   NS_ASSERTION(aContent, "No node for the accessible tree walker!");
 
   if (aContent)
     mState = new WalkState(aContent);
 
   mChildFilter = aWalkAnonContent ? nsIContent::eAllChildren :
                                   nsIContent::eAllButXBL;
@@ -109,17 +109,18 @@ nsAccTreeWalker::NextChildInternal(bool 
   if (mState->childList)
     mState->childList->GetLength(&length);
 
   while (mState->childIdx < length) {
     nsIContent* childNode = mState->childList->GetNodeAt(mState->childIdx);
     mState->childIdx++;
 
     bool isSubtreeHidden = false;
-    nsAccessible* accessible =
+    nsAccessible* accessible = mWalkCache ?
+      GetAccService()->GetAccessibleInWeakShell(childNode, mWeakShell) :
       GetAccService()->GetOrCreateAccessible(childNode, presShell, mWeakShell,
                                              &isSubtreeHidden);
 
     if (accessible)
       return accessible;
 
     // Walk down into subtree to find accessibles.
     if (!isSubtreeHidden) {
--- a/accessible/src/base/nsAccTreeWalker.h
+++ b/accessible/src/base/nsAccTreeWalker.h
@@ -49,17 +49,17 @@ struct WalkState;
 
 /**
  * This class is used to walk the DOM tree to create accessible tree.
  */
 class nsAccTreeWalker
 {
 public:
   nsAccTreeWalker(nsIWeakReference *aShell, nsIContent *aNode, 
-                  PRBool aWalkAnonymousContent);
+                  PRBool aWalkAnonymousContent, bool aWalkCache = false);
   virtual ~nsAccTreeWalker();
 
   /**
    * Return the next child accessible.
    *
    * @note Returned accessible is bound to the document, if the accessible is
    *       rejected during tree creation then the caller should be unbind it
    *       from the document.
@@ -90,12 +90,13 @@ private:
 
   /**
    * Pop state from stack.
    */
   void PopState();
 
   nsCOMPtr<nsIWeakReference> mWeakShell;
   PRInt32 mChildFilter;
+  bool mWalkCache;
   WalkState* mState;
 };
 
 #endif 
--- a/accessible/src/base/nsDocAccessible.cpp
+++ b/accessible/src/base/nsDocAccessible.cpp
@@ -1422,36 +1422,36 @@ void
 nsDocAccessible::ContentRemoved(nsIContent* aContainerNode,
                                 nsIContent* aChildNode)
 {
   // Update the whole tree of this document accessible when the container is
   // null (document element is removed).
   nsAccessible* container = aContainerNode ?
     GetAccessibleOrContainer(aContainerNode) : this;
 
-  UpdateTree(container, aChildNode, PR_FALSE);
+  UpdateTree(container, aChildNode, false);
 }
 
 void
 nsDocAccessible::RecreateAccessible(nsIContent* aContent)
 {
   // XXX: we shouldn't recreate whole accessible subtree, instead we should
   // subclass hide and show events to handle them separately and implement their
   // coalescence with normal hide and show events. Note, in this case they
   // should be coalesced with normal show/hide events.
 
   // Check if the node is in DOM still.
   nsIContent* parentContent = aContent->GetParent();
   if (parentContent && parentContent->IsInDoc()) {
     nsAccessible* container = GetAccessibleOrContainer(parentContent);
 
     // Remove and reinsert.
-    UpdateTree(container, aContent, PR_FALSE);
+    UpdateTree(container, aContent, false);
     container->UpdateChildren();
-    UpdateTree(container, aContent, PR_TRUE);
+    UpdateTree(container, aContent, true);
   }
 }
 
 void
 nsDocAccessible::NotifyOfCachingStart(nsAccessible* aAccessible)
 {
   if (!mCacheRoot)
     mCacheRoot = aAccessible;
@@ -1792,26 +1792,38 @@ nsDocAccessible::ProcessContentInserted(
   // double processing, however generated events are coalesced and we don't
   // harm an AT.
   // Theoretically the element might be not in tree at all at this point what
   // means there's no container.
   for (PRUint32 idx = 0; idx < aInsertedContent->Length(); idx++) {
     nsAccessible* directContainer =
       GetContainerAccessible(aInsertedContent->ElementAt(idx));
     if (directContainer)
-      UpdateTree(directContainer, aInsertedContent->ElementAt(idx), PR_TRUE);
+      UpdateTree(directContainer, aInsertedContent->ElementAt(idx), true);
   }
 }
 
 void
 nsDocAccessible::UpdateTree(nsAccessible* aContainer, nsIContent* aChildNode,
-                            PRBool aIsInsert)
+                            bool aIsInsert)
 {
-  PRUint32 updateFlags =
-    UpdateTreeInternal(aChildNode, aChildNode->GetNextSibling(), aIsInsert);
+  PRUint32 updateFlags = eNoAccessible;
+
+  // If child node is not accessible then look for its accessible children.
+  nsAccessible* child = GetAccessible(aChildNode);
+  if (child) {
+    updateFlags |= UpdateTreeInternal(child, aIsInsert);
+
+  } else {
+    nsAccTreeWalker walker(mWeakShell, aChildNode,
+                           aContainer->GetAllowsAnonChildAccessibles(), true);
+
+    while ((child = walker.NextChild()))
+      updateFlags |= UpdateTreeInternal(child, aIsInsert);
+  }
 
   // Content insertion/removal is not cause of accessible tree change.
   if (updateFlags == eNoAccessible)
     return;
 
   // Check to see if change occurred inside an alert, and fire an EVENT_ALERT
   // if it did.
   if (aIsInsert && !(updateFlags & eAlertAccessible)) {
@@ -1844,107 +1856,87 @@ nsDocAccessible::UpdateTree(nsAccessible
   nsRefPtr<AccEvent> reorderEvent =
     new AccEvent(nsIAccessibleEvent::EVENT_REORDER, aContainer->GetNode(),
                  eAutoDetect, AccEvent::eCoalesceFromSameSubtree);
   if (reorderEvent)
     FireDelayedAccessibleEvent(reorderEvent);
 }
 
 PRUint32
-nsDocAccessible::UpdateTreeInternal(nsIContent* aStartNode,
-                                    nsIContent* aEndNode,
-                                    PRBool aIsInsert)
+nsDocAccessible::UpdateTreeInternal(nsAccessible* aChild, bool aIsInsert)
 {
-  PRUint32 updateFlags = eNoAccessible;
-  for (nsIContent* node = aStartNode; node != aEndNode;
-       node = node->GetNextSibling()) {
+  PRUint32 updateFlags = eAccessible;
+
+  nsINode* node = aChild->GetNode();
+  if (aIsInsert) {
+    // Create accessible tree for shown accessible.
+    CacheChildrenInSubtree(aChild);
+
+  } else {
+    // Fire menupopup end event before hide event if a menu goes away.
+
+    // XXX: We don't look into children of hidden subtree to find hiding
+    // menupopup (as we did prior bug 570275) because we don't do that when
+    // menu is showing (and that's impossible until bug 606924 is fixed).
+    // Nevertheless we should do this at least because layout coalesces
+    // the changes before our processing and we may miss some menupopup
+    // events. Now we just want to be consistent in content insertion/removal
+    // handling.
+    if (aChild->ARIARole() == nsIAccessibleRole::ROLE_MENUPOPUP) {
+      nsRefPtr<AccEvent> event =
+        new AccEvent(nsIAccessibleEvent::EVENT_MENUPOPUP_END, aChild);
 
-    // Tree update triggers for content insertion even if no content was
-    // inserted actually, check if the given content has a frame to discard
-    // this case early.
-    if (aIsInsert && !node->GetPrimaryFrame())
-      continue;
+      if (event)
+        FireDelayedAccessibleEvent(event);
+    }
+  }
+
+  // Fire show/hide event.
+  nsRefPtr<AccEvent> event;
+  if (aIsInsert)
+    event = new AccShowEvent(aChild, node);
+  else
+    event = new AccHideEvent(aChild, node);
 
-    nsAccessible* accessible = GetAccessible(node);
+  if (event)
+    FireDelayedAccessibleEvent(event);
 
-    if (!accessible) {
-      updateFlags |= UpdateTreeInternal(node->GetFirstChild(), nsnull,
-                                        aIsInsert);
-      continue;
+  if (aIsInsert) {
+    PRUint32 ariaRole = aChild->ARIARole();
+    if (ariaRole == nsIAccessibleRole::ROLE_MENUPOPUP) {
+      // Fire EVENT_MENUPOPUP_START if ARIA menu appears.
+      FireDelayedAccessibleEvent(nsIAccessibleEvent::EVENT_MENUPOPUP_START,
+                                 node, AccEvent::eRemoveDupes);
+
+    } else if (ariaRole == nsIAccessibleRole::ROLE_ALERT) {
+      // Fire EVENT_ALERT if ARIA alert appears.
+      updateFlags = eAlertAccessible;
+      FireDelayedAccessibleEvent(nsIAccessibleEvent::EVENT_ALERT, node,
+                                 AccEvent::eRemoveDupes);
     }
 
-    updateFlags |= eAccessible;
-
-    if (aIsInsert) {
-      // Create accessible tree for shown accessible.
-      CacheChildrenInSubtree(accessible);
-
-    } else {
-      // Fire menupopup end event before hide event if a menu goes away.
-
-      // XXX: We don't look into children of hidden subtree to find hiding
-      // menupopup (as we did prior bug 570275) because we don't do that when
-      // menu is showing (and that's impossible until bug 606924 is fixed).
-      // Nevertheless we should do this at least because layout coalesces
-      // the changes before our processing and we may miss some menupopup
-      // events. Now we just want to be consistent in content insertion/removal
-      // handling.
-      if (accessible->ARIARole() == nsIAccessibleRole::ROLE_MENUPOPUP) {
-        nsRefPtr<AccEvent> event =
-          new AccEvent(nsIAccessibleEvent::EVENT_MENUPOPUP_END, accessible);
-
-        if (event)
-          FireDelayedAccessibleEvent(event);
-      }
+    // If focused node has been shown then it means its frame was recreated
+    // while it's focused. Fire focus event on new focused accessible. If
+    // the queue contains focus event for this node then it's suppressed by
+    // this one.
+    if (node == gLastFocusedNode) {
+      FireDelayedAccessibleEvent(nsIAccessibleEvent::EVENT_FOCUS,
+                                 node, AccEvent::eCoalesceFromSameDocument);
     }
-
-    // Fire show/hide event.
-    nsRefPtr<AccEvent> event;
-    if (aIsInsert)
-      event = new AccShowEvent(accessible, node);
-    else
-      event = new AccHideEvent(accessible, node);
-
-    if (event)
-      FireDelayedAccessibleEvent(event);
+  } else {
+    // Update the tree for content removal.
+    // The accessible parent may differ from container accessible if
+    // the parent doesn't have own DOM node like list accessible for HTML
+    // selects.
+    nsAccessible* parent = aChild->GetParent();
+    NS_ASSERTION(parent, "No accessible parent?!");
+    if (parent)
+      parent->RemoveChild(aChild);
 
-    if (aIsInsert) {
-      PRUint32 ariaRole = accessible->ARIARole();
-      if (ariaRole == nsIAccessibleRole::ROLE_MENUPOPUP) {
-        // Fire EVENT_MENUPOPUP_START if ARIA menu appears.
-        FireDelayedAccessibleEvent(nsIAccessibleEvent::EVENT_MENUPOPUP_START,
-                                   node, AccEvent::eRemoveDupes);
-
-      } else if (ariaRole == nsIAccessibleRole::ROLE_ALERT) {
-        // Fire EVENT_ALERT if ARIA alert appears.
-        updateFlags = eAlertAccessible;
-        FireDelayedAccessibleEvent(nsIAccessibleEvent::EVENT_ALERT, node,
-                                   AccEvent::eRemoveDupes);
-      }
-
-      // If focused node has been shown then it means its frame was recreated
-      // while it's focused. Fire focus event on new focused accessible. If
-      // the queue contains focus event for this node then it's suppressed by
-      // this one.
-      if (node == gLastFocusedNode) {
-        FireDelayedAccessibleEvent(nsIAccessibleEvent::EVENT_FOCUS,
-                                   node, AccEvent::eCoalesceFromSameDocument);
-      }
-    } else {
-      // Update the tree for content removal.
-      // The accessible parent may differ from container accessible if
-      // the parent doesn't have own DOM node like list accessible for HTML
-      // selects.
-      nsAccessible* parent = accessible->GetParent();
-      NS_ASSERTION(parent, "No accessible parent?!");
-      if (parent)
-        parent->RemoveChild(accessible);
-
-      UncacheChildrenInSubtree(accessible);
-    }
+    UncacheChildrenInSubtree(aChild);
   }
 
   return updateFlags;
 }
 
 void
 nsDocAccessible::CacheChildrenInSubtree(nsAccessible* aRoot)
 {
--- a/accessible/src/base/nsDocAccessible.h
+++ b/accessible/src/base/nsDocAccessible.h
@@ -438,31 +438,29 @@ protected:
    */
   void ProcessContentInserted(nsAccessible* aContainer,
                               const nsTArray<nsCOMPtr<nsIContent> >* aInsertedContent);
 
   /**
    * Update the accessible tree for content insertion or removal.
    */
   void UpdateTree(nsAccessible* aContainer, nsIContent* aChildNode,
-                  PRBool aIsInsert);
+                  bool aIsInsert);
 
   /**
    * Helper for UpdateTree() method. Go down to DOM subtree and updates
    * accessible tree. Return one of these flags.
    */
   enum EUpdateTreeFlags {
     eNoAccessible = 0,
     eAccessible = 1,
     eAlertAccessible = 2
   };
 
-  PRUint32 UpdateTreeInternal(nsIContent* aStartNode,
-                              nsIContent* aEndNode,
-                              PRBool aIsInsert);
+  PRUint32 UpdateTreeInternal(nsAccessible* aChild, bool aIsInsert);
 
   /**
    * Create accessible tree.
    */
   void CacheChildrenInSubtree(nsAccessible* aRoot);
 
   /**
    * Remove accessibles in subtree from node to accessible map.
--- a/accessible/tests/mochitest/events/Makefile.in
+++ b/accessible/tests/mochitest/events/Makefile.in
@@ -64,16 +64,17 @@ include $(topsrcdir)/config/rules.mk
 		test_dragndrop.html \
 		test_flush.html \
 		test_focus.html \
 		test_focus.xul \
 		test_focus_name.html \
 		test_focusdoc.html \
 		test_menu.xul \
 		test_mutation.html \
+		test_mutation.xhtml \
 		test_scroll.xul \
 		test_selection.html \
 		test_statechange.html \
 		test_text_alg.html \
 		test_text.html \
 		test_textattrchange.html \
 		test_tree.xul \
 		test_valuechange.html \
new file mode 100644
--- /dev/null
+++ b/accessible/tests/mochitest/events/test_mutation.xhtml
@@ -0,0 +1,99 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+
+<head>
+  <title>Accessible mutation events testing</title>
+
+  <link rel="stylesheet" type="text/css"
+        href="chrome://mochikit/content/tests/SimpleTest/test.css" />
+
+  <bindings xmlns="http://www.mozilla.org/xbl" >
+    <binding id="button">
+      <content>
+        <button xmlns="http://www.w3.org/1999/xhtml">a button</button>
+      </content>
+    </binding>
+  </bindings>
+
+  <script type="application/javascript"
+          src="chrome://mochikit/content/MochiKit/packed.js"></script>
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>
+
+  <script type="application/javascript"
+          src="../common.js"></script>
+  <script type="application/javascript"
+          src="../events.js"></script>
+
+  <script type="application/javascript">
+
+    /**
+     * Insert a not accessible bound element containing an accessible element
+     * in anonymous content.
+     */
+    function insertBinding(aContainerID)
+    {
+      this.containerNode = getNode(aContainerID);
+
+      function getButtonFromBinding(aNode)
+      {
+        try { return document.getAnonymousNodes(aNode.firstChild)[0]; }
+        catch (e) { return null; }
+      }
+
+      this.eventSeq = [
+        new invokerChecker(EVENT_SHOW, getButtonFromBinding, this.containerNode),
+        new invokerChecker(EVENT_REORDER, this.containerNode)
+      ];
+
+      this.invoke = function insertBinding_invoke()
+      {
+        var span = document.createElement("span");
+        span.setAttribute("style", "-moz-binding:url(#button)");
+        this.containerNode.appendChild(span);
+      }
+
+      this.getID = function insertBinding_getID()
+      {
+        return "insert button binding";
+      }
+    }
+
+    /**
+     * Do tests.
+     */
+    var gQueue = null;
+    //gA11yEventDumpID = "eventdump"; // debug stuff
+    //gA11yEventDumpToConsole = true;
+
+    function doTests()
+    {
+      gQueue = new eventQueue();
+
+      gQueue.push(new insertBinding("testContainer"));
+
+      gQueue.invoke(); // Will call SimpleTest.finish();
+    }
+
+    SimpleTest.waitForExplicitFinish();
+    addA11yLoadEvent(doTests);
+  </script>
+</head>
+
+<body>
+
+  <a target="_blank"
+     href="https://bugzilla.mozilla.org/show_bug.cgi?id=646369"
+     title="UpdateTree should rely on accessible tree walker rather than DOM tree traversal">
+    Mozilla Bug 646369</a>
+
+  <p id="display"></p>
+  <div id="content" style="display: none"></div>
+  <pre id="test">
+  </pre>
+  <div id="eventdump"></div>
+
+  <div id="testContainer"></div>
+</body>
+</html>