Bug 1263188 - fix event tree building, part2, r=yzen
authorAlexander Surkov <surkov.alexander@gmail.com>
Wed, 13 Apr 2016 11:22:19 -0400
changeset 316817 8c8bded39b76cb9f32ded2f988209d46695d45ae
parent 316816 0c21f872515b2b49c53bd22b12d8a3e472dbd0b2
child 316818 ac43dab284afd189ed2b0f4c1906a45e0c1736dd
push id9480
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 17:12:58 +0000
treeherdermozilla-aurora@0d6a91c76a9e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyzen
bugs1263188
milestone48.0a1
Bug 1263188 - fix event tree building, part2, r=yzen
accessible/base/EventTree.cpp
accessible/base/EventTree.h
accessible/tests/mochitest/events/test_coalescence.html
--- a/accessible/base/EventTree.cpp
+++ b/accessible/base/EventTree.cpp
@@ -232,121 +232,66 @@ EventTree::Process()
     mDependentEvents.Clear();
   }
 }
 
 EventTree*
 EventTree::FindOrInsert(Accessible* aContainer)
 {
   if (!mFirst) {
-    return mFirst = new EventTree(aContainer);
+    return mFirst = new EventTree(aContainer, true);
   }
 
   EventTree* prevNode = nullptr;
   EventTree* node = mFirst;
   do {
     MOZ_ASSERT(!node->mContainer->IsApplication(),
                "No event for application accessible is expected here");
     MOZ_ASSERT(!node->mContainer->IsDefunct(), "An event target has to be alive");
 
     // Case of same target.
     if (node->mContainer == aContainer) {
       return node;
     }
 
     // Check if the given container is contained by a current node
-    Accessible* tailRoot = aContainer->Document();
-    Accessible* tailParent = aContainer;
-
-    EventTree* matchNode = nullptr;
-    Accessible* matchParent = nullptr;
-    while (true) {
+    Accessible* top = mContainer ? mContainer : aContainer->Document();
+    Accessible* parent = aContainer;
+    while (parent) {
       // Reached a top, no match for a current event.
-      if (tailParent == tailRoot) {
-        // If we have a match in parents then continue to look in siblings.
-        if (matchNode && node->mNext) {
-          node = node->mNext;
-          if (node->mContainer == aContainer) {
-            return node; // case of same target
-          }
-          tailParent = aContainer;
-          continue;
-        }
+      if (parent == top) {
         break;
       }
 
       // We got a match.
-      if (tailParent->Parent() == node->mContainer) {
-        matchNode = node;
-        matchParent = tailParent;
-
-        // Search the subtree for a better match.
-        if (node->mFirst) {
-          tailRoot = node->mContainer;
-          node = node->mFirst;
-          if (node->mContainer == aContainer) {
-            return node; // case of same target
-          }
-          tailParent = aContainer;
-          continue;
-        }
-        break;
+      if (parent->Parent() == node->mContainer) {
+        return node->FindOrInsert(aContainer);
       }
 
-      tailParent = tailParent->Parent();
-      MOZ_ASSERT(tailParent, "Wrong tree");
-      if (!tailParent) {
-        break;
-      }
+      parent = parent->Parent();
+      MOZ_ASSERT(parent, "Wrong tree");
     }
 
-    // The given node is contained by a current node
-    //   if hide of a current node contains the given node
-    //   then assert
-    //   if show of a current node contains the given node
-    //   then ignore the given node
-    //   otherwise ignore the given node, but not its show and hide events
-    if (matchNode) {
-      uint32_t eventType = 0;
-      uint32_t count = matchNode->mDependentEvents.Length();
-      for (uint32_t idx = count - 1; idx < count; idx--) {
-        if (matchNode->mDependentEvents[idx]->mAccessible == matchParent) {
-          eventType = matchNode->mDependentEvents[idx]->mEventType;
-        }
-      }
-      MOZ_ASSERT(eventType != nsIAccessibleEvent::EVENT_HIDE,
-                 "Accessible tree was modified after it was removed");
-
-      // If contained by show event target then no events are required.
-      if (eventType == nsIAccessibleEvent::EVENT_SHOW) {
-        return nullptr;
-      }
-
-      node->mFirst = new EventTree(aContainer);
-      node->mFirst->mFireReorder = false;
-      return node->mFirst;
-    }
-
-    // If the given node contains a current node
+    // If the given container contains a current node
     // then
     //   if show or hide of the given node contains a grand parent of the current node
     //   then ignore the current node and its show and hide events
     //   otherwise ignore the current node, but not its show and hide events
     Accessible* curParent = node->mContainer;
     while (curParent && !curParent->IsDoc()) {
       if (curParent->Parent() != aContainer) {
         curParent = curParent->Parent();
         continue;
       }
 
       // Insert the tail node into the hierarchy between the current node and
       // its parent.
       node->mFireReorder = false;
       nsAutoPtr<EventTree>& nodeOwnerRef = prevNode ? prevNode->mNext : mFirst;
-      nsAutoPtr<EventTree> newNode(new EventTree(aContainer));
+      nsAutoPtr<EventTree> newNode(new EventTree(aContainer, mDependentEvents.IsEmpty()));
       newNode->mFirst = Move(nodeOwnerRef);
       nodeOwnerRef = Move(newNode);
       nodeOwnerRef->mNext = Move(node->mNext);
 
       // Check if a next node is contained by the given node too, and move them
       // under the given node if so.
       prevNode = nodeOwnerRef;
       node = nodeOwnerRef->mNext;
@@ -378,17 +323,24 @@ EventTree::FindOrInsert(Accessible* aCon
 
       return nodeOwnerRef;
     }
 
     prevNode = node;
   } while ((node = node->mNext));
 
   MOZ_ASSERT(prevNode, "Nowhere to insert");
-  return prevNode->mNext = new EventTree(aContainer);
+  MOZ_ASSERT(!prevNode->mNext, "Taken by another node");
+
+  // If 'this' node contains the given container accessible, then
+  //   do not emit a reorder event for the container
+  //   if a dependent show event target contains the given container then do not
+  //      emit show / hide events (to be done)
+
+  return prevNode->mNext = new EventTree(aContainer, mDependentEvents.IsEmpty());
 }
 
 void
 EventTree::Clear()
 {
   mFirst = nullptr;
   mNext = nullptr;
   mContainer = nullptr;
--- a/accessible/base/EventTree.h
+++ b/accessible/base/EventTree.h
@@ -52,18 +52,19 @@ private:
 
 /**
  * A mutation events coalescence structure.
  */
 class EventTree final {
 public:
   EventTree() :
     mFirst(nullptr), mNext(nullptr), mContainer(nullptr), mFireReorder(false) { }
-  explicit EventTree(Accessible* aContainer) :
-    mFirst(nullptr), mNext(nullptr), mContainer(aContainer), mFireReorder(true) { }
+  explicit EventTree(Accessible* aContainer, bool aFireReorder) :
+    mFirst(nullptr), mNext(nullptr), mContainer(aContainer),
+    mFireReorder(aFireReorder) { }
   ~EventTree() { Clear(); }
 
   void Shown(Accessible* aChild)
   {
     RefPtr<AccShowEvent> ev = new AccShowEvent(aChild);
     Mutated(ev);
   }
 
--- a/accessible/tests/mochitest/events/test_coalescence.html
+++ b/accessible/tests/mochitest/events/test_coalescence.html
@@ -416,16 +416,54 @@
         testIsDefunct(this.o2fc);
       }
 
       this.getID = function test4_getID() {
         return "remove children, and then a parent of 2nd child";
       }
     }
 
+    /**
+     * Remove a child, remove a parent sibling, remove the parent
+     */
+    function test5()
+    {
+      this.o = getAccessible("t5_o");
+      this.ofc = this.o.firstChild;
+      this.b = getAccessible("t5_b");
+      this.lb = getAccessible("t5_lb");
+
+      this.eventSeq = [
+        new invokerChecker(EVENT_HIDE, this.o),
+        new invokerChecker(EVENT_HIDE, this.b),
+        new invokerChecker(EVENT_REORDER, "t5"),
+        new unexpectedInvokerChecker(EVENT_HIDE, this.ofc),
+        new unexpectedInvokerChecker(EVENT_REORDER, this.o),
+        new unexpectedInvokerChecker(EVENT_REORDER, this.lb)
+      ];
+
+      this.invoke = function test5_invoke()
+      {
+        getNode("t5_o").textContent = "";
+        getNode("t5").removeChild(getNode("t5_b"));
+        getNode("t5_lb").removeChild(getNode("t5_o"));
+      }
+
+      this.finalCheck = function test5_finalCheck()
+      {
+        testIsDefunct(this.ofc);
+        testIsDefunct(this.o);
+        testIsDefunct(this.b);
+      }
+
+      this.getID = function test5_getID() {
+        return "remove a child, remove a parent sibling, remove the parent";
+      }
+    }
+
     ////////////////////////////////////////////////////////////////////////////
     // Do tests.
 
     //gA11yEventDumpToConsole = true; // debug stuff
     //enableLogging("tree,eventTree,verbose");
 
     var gQueue = null;
     function doTests()
@@ -446,16 +484,17 @@
       gQueue.push(new showParentNChild("select9", "option9", false));
       gQueue.push(new showParentNChild("select10", "option10", true));
       gQueue.push(new showParentNAddChild("select11", false));
       gQueue.push(new showParentNAddChild("select12", true));
 
       gQueue.push(new removeGrandChildrenNHideParent("t1_child1", "t1_child2", "t1_parent"));
       gQueue.push(new test3());
       gQueue.push(new test4());
+      gQueue.push(new test5());
 
       gQueue.invoke(); // Will call SimpleTest.finish();
     }
 
     SimpleTest.waitForExplicitFinish();
     addA11yLoadEvent(doTests);
   </script>
 </head>
@@ -529,10 +568,17 @@
   </div>
 
   <div id="t4">
     <div role="listbox" id="t4_lb">
       <div role="option" id="t4_o1">opt1</div>
       <div role="option" id="t4_o2">opt2</div>
     </div>
   </div>
+
+  <div id="t5">
+    <div role="button" id="t5_b">btn</div>
+    <div role="listbox" id="t5_lb">
+      <div role="option" id="t5_o">opt</div>
+    </div>
+  </div>
 </body>
 </html>