Bug 753093 - Fix crasher when virtual cursor's position becomes defunct and a move method is called. r=surkov

2012-05-14 14:21:59 -07:00 · 2012-05-14 14:21:59 -07:00 · c66e58f0e5
--- a/accessible/src/base/nsAccessiblePivot.cpp
+++ b/accessible/src/base/nsAccessiblePivot.cpp
@ -42,6 +42,7 @@
 #include "Accessible-inl.h"
 #include "nsAccUtils.h"
 #include "nsHyperTextAccessible.h"
+#include "nsDocAccessible.h"
 #include "States.h"

 #include "nsArrayUtils.h"
@ -217,6 +218,10 @@ nsAccessiblePivot::MoveNext(nsIAccessibleTraversalRule* aRule, bool* aResult)
  NS_ENSURE_ARG(aResult);
  NS_ENSURE_ARG(aRule);

+  if (mPosition && (mPosition->IsDefunct() ||
+                    !mPosition->Document()->IsInDocument(mPosition)))
+    return NS_ERROR_NOT_IN_TREE;
+
  nsresult rv = NS_OK;
  nsAccessible* accessible = SearchForward(mPosition, aRule, false, &rv);
  NS_ENSURE_SUCCESS(rv, rv);
@ -234,6 +239,10 @@ nsAccessiblePivot::MovePrevious(nsIAccessibleTraversalRule* aRule, bool* aResult
  NS_ENSURE_ARG(aResult);
  NS_ENSURE_ARG(aRule);

+  if (mPosition && (mPosition->IsDefunct() ||
+                    !mPosition->Document()->IsInDocument(mPosition)))
+    return NS_ERROR_NOT_IN_TREE;
+
  nsresult rv = NS_OK;
  nsAccessible* accessible = SearchBackward(mPosition, aRule, false, &rv);
  NS_ENSURE_SUCCESS(rv, rv);
@ -250,6 +259,10 @@ nsAccessiblePivot::MoveFirst(nsIAccessibleTraversalRule* aRule, bool* aResult)
 {
  NS_ENSURE_ARG(aResult);
  NS_ENSURE_ARG(aRule);
+
+  if (mRoot && mRoot->IsDefunct())
+    return NS_ERROR_NOT_IN_TREE;
+
  nsresult rv = NS_OK;
  nsAccessible* accessible = SearchForward(mRoot, aRule, true, &rv);
  NS_ENSURE_SUCCESS(rv, rv);
@ -267,6 +280,9 @@ nsAccessiblePivot::MoveLast(nsIAccessibleTraversalRule* aRule, bool* aResult)
  NS_ENSURE_ARG(aResult);
  NS_ENSURE_ARG(aRule);

+  if (mRoot && mRoot->IsDefunct())
+    return NS_ERROR_NOT_IN_TREE;
+
  *aResult = false;
  nsresult rv = NS_OK;
  nsAccessible* lastAccessible = mRoot;
@ -334,6 +350,9 @@ nsAccessiblePivot::RemoveObserver(nsIAccessiblePivotObserver* aObserver)
 bool
 nsAccessiblePivot::IsRootDescendant(nsAccessible* aAccessible)
 {
+  if (!mRoot || mRoot->IsDefunct())
+    return false;
+
  nsAccessible* accessible = aAccessible;
  do {
    if (accessible == mRoot)
--- a/accessible/src/base/nsAccessiblePivot.h
+++ b/accessible/src/base/nsAccessiblePivot.h
@ -49,6 +49,10 @@
 class nsAccessible;
 class nsIAccessibleTraversalRule;

+// raised when current pivot's position is needed but it is not in the tree.
+#define NS_ERROR_NOT_IN_TREE \
+NS_ERROR_GENERATE_FAILURE(NS_ERROR_MODULE_GENERAL, 0x26)
+
 /**
 * Class represents an accessible pivot.
 */
--- a/accessible/tests/mochitest/pivot.js
+++ b/accessible/tests/mochitest/pivot.js
@ -8,6 +8,8 @@ const FILTER_MATCH = nsIAccessibleTraversalRule.FILTER_MATCH;
 const FILTER_IGNORE = nsIAccessibleTraversalRule.FILTER_IGNORE;
 const FILTER_IGNORE_SUBTREE = nsIAccessibleTraversalRule.FILTER_IGNORE_SUBTREE;

+const NS_ERROR_NOT_IN_TREE = 0x80780026;
+
 ////////////////////////////////////////////////////////////////////////////////
 // Traversal rules

@ -68,13 +70,13 @@ var ObjectTraversalRule =
 /**
 * A checker for virtual cursor changed events.
 */
-function virtualCursorChangedChecker(aDocAcc, aIdOrNameOrAcc, aTextOffsets)
+function VCChangedChecker(aDocAcc, aIdOrNameOrAcc, aTextOffsets)
 {
  this.__proto__ = new invokerChecker(EVENT_VIRTUALCURSOR_CHANGED, aDocAcc);

-  this.check = function virtualCursorChangedChecker_check(aEvent)
+  this.check = function VCChangedChecker_check(aEvent)
  {
-    SimpleTest.info("virtualCursorChangedChecker_check");
+    SimpleTest.info("VCChangedChecker_check");

    var event = null;
    try {
@ -100,7 +102,7 @@ function virtualCursorChangedChecker(aDocAcc, aIdOrNameOrAcc, aTextOffsets)
                    "wrong end offset");
    }

-    var prevPosAndOffset = virtualCursorChangedChecker.
+    var prevPosAndOffset = VCChangedChecker.
      getPreviousPosAndOffset(aDocAcc.virtualCursor);

    if (prevPosAndOffset) {
@ -114,36 +116,36 @@ function virtualCursorChangedChecker(aDocAcc, aIdOrNameOrAcc, aTextOffsets)
  };
 }

-virtualCursorChangedChecker.prevPosAndOffset = {};
+VCChangedChecker.prevPosAndOffset = {};

-virtualCursorChangedChecker.storePreviousPosAndOffset =
+VCChangedChecker.storePreviousPosAndOffset =
  function storePreviousPosAndOffset(aPivot)
 {
-  virtualCursorChangedChecker.prevPosAndOffset[aPivot] =
+  VCChangedChecker.prevPosAndOffset[aPivot] =
    {position: aPivot.position,
     startOffset: aPivot.startOffset,
     endOffset: aPivot.endOffset};
 };

-virtualCursorChangedChecker.getPreviousPosAndOffset =
+VCChangedChecker.getPreviousPosAndOffset =
  function getPreviousPosAndOffset(aPivot)
 {
-  return virtualCursorChangedChecker.prevPosAndOffset[aPivot];
+  return VCChangedChecker.prevPosAndOffset[aPivot];
 };

 /**
 * Set a text range in the pivot and wait for virtual cursor change event.
 *
- * @param aDocAcc document that manages the virtual cursor
- * @param aTextAccessible accessible to set to virtual cursor's position
- * @param aTextOffsets start and end offsets of text range to set in virtual
- *   cursor
+ * @param aDocAcc         [in] document that manages the virtual cursor
+ * @param aTextAccessible [in] accessible to set to virtual cursor's position
+ * @param aTextOffsets    [in] start and end offsets of text range to set in
+ *                        virtual cursor.
 */
-function setVirtualCursorRangeInvoker(aDocAcc, aTextAccessible, aTextOffsets)
+function setVCRangeInvoker(aDocAcc, aTextAccessible, aTextOffsets)
 {
  this.invoke = function virtualCursorChangedInvoker_invoke()
  {
-    virtualCursorChangedChecker.
+    VCChangedChecker.
      storePreviousPosAndOffset(aDocAcc.virtualCursor);
    SimpleTest.info(prettyName(aTextAccessible) + " " + aTextOffsets);
    aDocAcc.virtualCursor.setTextRange(aTextAccessible,
@ -151,45 +153,44 @@ function setVirtualCursorRangeInvoker(aDocAcc, aTextAccessible, aTextOffsets)
                                       aTextOffsets[1]);
  };

-  this.getID = function setVirtualCursorRangeInvoker_getID()
+  this.getID = function setVCRangeInvoker_getID()
  {
    return "Set offset in " + prettyName(aTextAccessible) +
      " to (" + aTextOffsets[0] + ", " + aTextOffsets[1] + ")";
-  }
+  };

  this.eventSeq = [
-    new virtualCursorChangedChecker(aDocAcc, aTextAccessible, aTextOffsets)
+    new VCChangedChecker(aDocAcc, aTextAccessible, aTextOffsets)
  ];
 }

 /**
 * Move the pivot and wait for virtual cursor change event.
 *
- * @param aDocAcc document that manages the virtual cursor
- * @param aPivotMoveMethod method to test (ie. "moveNext", "moveFirst", etc.)
- * @param aRule traversal rule object
- * @param aIdOrNameOrAcc id, accessivle or accessible name to expect virtual
- *   cursor to land on after performing move method.
+ * @param aDocAcc          [in] document that manages the virtual cursor
+ * @param aPivotMoveMethod [in] method to test (ie. "moveNext", "moveFirst", etc.)
+ * @param aRule            [in] traversal rule object
+ * @param aIdOrNameOrAcc   [in] id, accessivle or accessible name to expect
+ *                         virtual cursor to land on after performing move method.
 */
-function setVirtualCursorPosInvoker(aDocAcc, aPivotMoveMethod, aRule,
-                                    aIdOrNameOrAcc)
+function setVCPosInvoker(aDocAcc, aPivotMoveMethod, aRule, aIdOrNameOrAcc)
 {
  this.invoke = function virtualCursorChangedInvoker_invoke()
  {
-    virtualCursorChangedChecker.
+    VCChangedChecker.
      storePreviousPosAndOffset(aDocAcc.virtualCursor);
    var moved = aDocAcc.virtualCursor[aPivotMoveMethod](aRule);
    SimpleTest.ok((aIdOrNameOrAcc && moved) || (!aIdOrNameOrAcc && !moved),
                  "moved pivot");
  };

-  this.getID = function setVirtualCursorPosInvoker_getID()
+  this.getID = function setVCPosInvoker_getID()
  {
    return "Do " + (aIdOrNameOrAcc ? "" : "no-op ") + aPivotMoveMethod;
-  }
+  };

  if (aIdOrNameOrAcc) {
-    this.eventSeq = [ new virtualCursorChangedChecker(aDocAcc, aIdOrNameOrAcc) ];
+    this.eventSeq = [ new VCChangedChecker(aDocAcc, aIdOrNameOrAcc) ];
  } else {
    this.eventSeq = [];
    this.unexpectedEventSeq = [
@ -202,45 +203,137 @@ function setVirtualCursorPosInvoker(aDocAcc, aPivotMoveMethod, aRule,
 * Add invokers to a queue to test a rule and an expected sequence of element ids
 * or accessible names for that rule in the given document.
 *
- * @param aQueue event queue in which to push invoker sequence.
- * @param aDocAcc the managing document of the virtual cursor we are testing
- * @param aRule the traversal rule to use in the invokers
- * @param aSequence a sequence of accessible names or elemnt ids to expect with
- *   the given rule in the given document
+ * @param aQueue    [in] event queue in which to push invoker sequence.
+ * @param aDocAcc   [in] the managing document of the virtual cursor we are testing
+ * @param aRule     [in] the traversal rule to use in the invokers
+ * @param aSequence [in] a sequence of accessible names or elemnt ids to expect with
+ *                  the given rule in the given document
 */
 function queueTraversalSequence(aQueue, aDocAcc, aRule, aSequence)
 {
  aDocAcc.virtualCursor.position = null;

  for (var i = 0; i < aSequence.length; i++) {
-    var invoker = new setVirtualCursorPosInvoker(aDocAcc, "moveNext",
-                                                 aRule, aSequence[i]);
+    var invoker =
+      new setVCPosInvoker(aDocAcc, "moveNext", aRule, aSequence[i]);
    aQueue.push(invoker);
  }

  // No further more matches for given rule, expect no virtual cursor changes.
-  aQueue.push(new setVirtualCursorPosInvoker(aDocAcc, "moveNext", aRule, null));
+  aQueue.push(new setVCPosInvoker(aDocAcc, "moveNext", aRule, null));

  for (var i = aSequence.length-2; i >= 0; i--) {
-    var invoker = new setVirtualCursorPosInvoker(aDocAcc, "movePrevious",
-                                                 aRule, aSequence[i])
+    var invoker =
+      new setVCPosInvoker(aDocAcc, "movePrevious", aRule, aSequence[i]);
    aQueue.push(invoker);
  }

  // No previous more matches for given rule, expect no virtual cursor changes.
-  aQueue.push(new setVirtualCursorPosInvoker(aDocAcc, "movePrevious", aRule, null));
+  aQueue.push(new setVCPosInvoker(aDocAcc, "movePrevious", aRule, null));

-  aQueue.push(new setVirtualCursorPosInvoker(
-    aDocAcc, "moveLast", aRule, aSequence[aSequence.length - 1]));
+  aQueue.push(new setVCPosInvoker(aDocAcc, "moveLast", aRule,
+                                  aSequence[aSequence.length - 1]));

  // No further more matches for given rule, expect no virtual cursor changes.
-  aQueue.push(new setVirtualCursorPosInvoker(aDocAcc, "moveNext", aRule, null));
+  aQueue.push(new setVCPosInvoker(aDocAcc, "moveNext", aRule, null));

-  aQueue.push(new setVirtualCursorPosInvoker(
-    aDocAcc, "moveFirst", aRule, aSequence[0]));
+  aQueue.push(new setVCPosInvoker(aDocAcc, "moveFirst", aRule, aSequence[0]));

  // No previous more matches for given rule, expect no virtual cursor changes.
-  aQueue.push(new setVirtualCursorPosInvoker(aDocAcc, "movePrevious", aRule, null));
+  aQueue.push(new setVCPosInvoker(aDocAcc, "movePrevious", aRule, null));
+}
+
+/**
+ * A checker for removing an accessible while the virtual cursor is on it.
+ */
+function removeVCPositionChecker(aDocAcc, aHiddenParentAcc)
+{
+  this.__proto__ = new invokerChecker(EVENT_REORDER, aHiddenParentAcc);
+
+  this.check = function removeVCPositionChecker_check(aEvent) {
+    var errorResult = 0;
+    try {
+      aDocAcc.virtualCursor.moveNext(ObjectTraversalRule);
+    } catch (x) {
+      errorResult = x.result;
+    }
+    SimpleTest.is(
+      errorResult, NS_ERROR_NOT_IN_TREE,
+      "Expecting NOT_IN_TREE error when moving pivot from invalid position.");
+  };
+}
+
+/**
+ * Put the virtual cursor's position on an object, and then remove it.
+ *
+ * @param aDocAcc     [in] document that manages the virtual cursor
+ * @param aPosNode    [in] DOM node to hide after virtual cursor's position is
+ *                    set to it.
+ */
+function removeVCPositionInvoker(aDocAcc, aPosNode)
+{
+  this.accessible = getAccessible(aPosNode);
+  this.invoke = function removeVCPositionInvoker_invoke()
+  {
+    aDocAcc.virtualCursor.position = this.accessible;
+    aPosNode.parentNode.removeChild(aPosNode);
+  };
+
+  this.getID = function removeVCPositionInvoker_getID()
+  {
+    return "Bring virtual cursor to accessible, and remove its DOM node.";
+  };
+
+  this.eventSeq = [
+    new removeVCPositionChecker(aDocAcc, this.accessible.parent)
+  ];
+}
+
+/**
+ * A checker for removing the pivot root and then calling moveFirst, and
+ * checking that an exception is thrown.
+ */
+function removeVCRootChecker(aPivot)
+{
+  this.__proto__ = new invokerChecker(EVENT_REORDER, aPivot.root.parent);
+
+  this.check = function removeVCRootChecker_check(aEvent) {
+    var errorResult = 0;
+    try {
+      aPivot.moveLast(ObjectTraversalRule);
+    } catch (x) {
+      errorResult = x.result;
+    }
+    SimpleTest.is(
+      errorResult, NS_ERROR_NOT_IN_TREE,
+      "Expecting NOT_IN_TREE error when moving pivot from invalid position.");
+  };
+}
+
+/**
+ * Create a pivot, remove its root, and perform an operation where the root is
+ * needed.
+ *
+ * @param aRootNode [in] DOM node of which accessible will be the root of the
+ *                       pivot. Should have more than one child.
+ */
+function removeVCRootInvoker(aRootNode)
+{
+  this.pivot = gAccRetrieval.createAccessiblePivot(getAccessible(aRootNode));
+  this.invoke = function removeVCRootInvoker_invoke()
+  {
+    this.pivot.position = this.pivot.root.firstChild;
+    aRootNode.parentNode.removeChild(aRootNode);
+  };
+
+  this.getID = function removeVCRootInvoker_getID()
+  {
+    return "Remove root of pivot from tree.";
+  };
+
+  this.eventSeq = [
+    new removeVCRootChecker(this.pivot)
+  ];
 }

 /**
--- a/accessible/tests/mochitest/pivot/doc_virtualcursor.html
+++ b/accessible/tests/mochitest/pivot/doc_virtualcursor.html
@ -17,10 +17,11 @@
  <iframe
     src="data:text/html,<html><body>An <i>embedded</i> document.</body></html>">
  </iframe>
-<p>
-  <a href="http://mozilla.org" title="Link 1 title">Link 1</a>
-  <a href="http://mozilla.org" title="Link 2 title">Link 2</a>
-  <a href="http://mozilla.org" title="Link 3 title">Link 3</a>
-</p>
+  <div id="hide-me">Hide me</div>
+  <p id="links">
+    <a href="http://mozilla.org" title="Link 1 title">Link 1</a>
+    <a href="http://mozilla.org" title="Link 2 title">Link 2</a>
+    <a href="http://mozilla.org" title="Link 3 title">Link 3</a>
+  </p>
 </body>
 </html>
--- a/accessible/tests/mochitest/pivot/test_virtualcursor.html
+++ b/accessible/tests/mochitest/pivot/test_virtualcursor.html
@ -60,15 +60,22 @@
         'dolor', ' sit amet. Integer vitae urna leo, id ',
         'semper', ' nulla. ', 'Second Section Title',
         'Sed accumsan luctus lacus, vitae mollis arcu tristique vulputate.',
-         'An ', 'embedded', ' document.', 'Link 1', 'Link 2', 'Link 3']);
+         'An ', 'embedded', ' document.', 'Hide me', 'Link 1', 'Link 2',
+         'Link 3']);

      // Just a random smoke test to see if our setTextRange works.
      gQueue.push(
-        new setVirtualCursorRangeInvoker(
+        new setVCRangeInvoker(
          docAcc,
          getAccessible(doc.getElementById('paragraph-2'), nsIAccessibleText),
          [2,6]));

+      gQueue.push(new removeVCPositionInvoker(
+        docAcc, doc.getElementById('hide-me')));
+
+      gQueue.push(new removeVCRootInvoker(
+        doc.getElementById('links')));
+
      gQueue.invoke();
    }