Bug 348158. Accessibility cache gets out of sync. Provide assertions so we catch this in the future. r=ginn.chen

accessible/public/nsPIAccessible.idl

@@ -46,6 +46,8 @@ interface nsPIAccessible : nsISupports
   [noscript] void setParent(in nsIAccessible aAccParent);
   [noscript] void setFirstChild(in nsIAccessible aAccFirstChild);
   [noscript] void setNextSibling(in nsIAccessible aAccNextSibling);
+  // Return parent only if cached
+  [noscript] void getCachedParent(out nsIAccessible aAccParent);
 
   /**
    * Set the child count to -1 (unknown) and null out cached child pointers
@@ -58,5 +60,8 @@ interface nsPIAccessible : nsISupports
    */
   [noscript] readonly attribute boolean allowsAnonChildAccessibles;
 
+#ifdef DEBUG_A11Y
+  [noscript] void testChildCache(in nsIAccessible aCachedChild); // Assert if child not in parent's cache
+#endif
 };
 

accessible/src/base/nsAccessNode.cpp

@@ -100,7 +100,7 @@ NS_IMPL_ISUPPORTS2(nsAccessNode, nsIAccessNode, nsPIAccessNode)
 nsAccessNode::nsAccessNode(nsIDOMNode *aNode, nsIWeakReference* aShell): 
   mDOMNode(aNode), mWeakShell(aShell)
 {
-#ifdef DEBUG
+#ifdef DEBUG_A11Y
   mIsInitialized = PR_FALSE;
 #endif
 }
@@ -121,7 +121,9 @@ NS_IMETHODIMP nsAccessNode::Init()
   // we don't have the virtual GetUniqueID() method for the hash key.
   // We need that for accessibles that don't have DOM nodes
 
+#ifdef DEBUG_A11Y
   NS_ASSERTION(!mIsInitialized, "Initialized twice!");
+#endif
   nsCOMPtr<nsIAccessibleDocument> docAccessible(GetDocAccessible());
   if (!docAccessible) {
     // No doc accessible yet for this node's document. 
@@ -152,7 +154,7 @@ NS_IMETHODIMP nsAccessNode::Init()
     do_QueryInterface(docAccessible);
   NS_ASSERTION(privateDocAccessible, "No private docaccessible for docaccessible");
   privateDocAccessible->CacheAccessNode(uniqueID, this);
-#ifdef DEBUG
+#ifdef DEBUG_A11Y
   mIsInitialized = PR_TRUE;
 #endif
 
@@ -567,7 +569,7 @@ void nsAccessNode::PutCacheEntry(nsInterfaceHashtable<nsVoidHashKey, nsIAccessNo
                                  void* aUniqueID, 
                                  nsIAccessNode *aAccessNode)
 {
-#ifdef DEBUG
+#ifdef DEBUG_A11Y
   nsCOMPtr<nsIAccessNode> oldAccessNode;
   GetCacheEntry(aCache, aUniqueID, getter_AddRefs(oldAccessNode));
   NS_ASSERTION(!oldAccessNode, "This cache entry shouldn't exist already");

accessible/src/base/nsAccessNode.h

@@ -158,7 +158,7 @@ protected:
     nsCOMPtr<nsIDOMNode> mDOMNode;
     nsCOMPtr<nsIWeakReference> mWeakShell;
 
-#ifdef DEBUG
+#ifdef DEBUG_A11Y
     PRBool mIsInitialized;
 #endif
 

accessible/src/base/nsAccessibilityService.cpp

@@ -266,7 +266,7 @@ nsAccessibilityService::GetInfo(nsISupports* aFrame, nsIFrame** aRealFrame, nsIW
   if (!document)
     return NS_ERROR_FAILURE;
 
-#ifdef DEBUG
+#ifdef DEBUG_A11Y
   PRInt32 shells = document->GetNumberOfShells();
   NS_ASSERTION(shells > 0,"Error no shells!");
 #endif
@@ -1020,7 +1020,7 @@ NS_IMETHODIMP nsAccessibilityService::GetAccessible(nsIDOMNode *aNode,
 
   *aIsHidden = PR_FALSE;
 
-#ifdef DEBUG_aleventhal
+#ifdef DEBUG_A11Y
   // Please leave this in for now, it's a convenient debugging method
   nsAutoString name;
   aNode->GetLocalName(name);
@@ -1031,7 +1031,7 @@ NS_IMETHODIMP nsAccessibilityService::GetAccessible(nsIDOMNode *aNode,
   nsCOMPtr<nsIDOMElement> element(do_QueryInterface(aNode));
   if (element) {
     element->GetAttribute(NS_LITERAL_STRING("type"), attrib);
-    if (attrib.EqualsLiteral("checkbox"))
+    if (attrib.EqualsLiteral("statusbarpanel"))
       printf("## aaronl debugging attribute\n");
   }
 #endif
@@ -1098,7 +1098,7 @@ NS_IMETHODIMP nsAccessibilityService::GetAccessible(nsIDOMNode *aNode,
 
   // We have a content node
   nsIFrame *frame = *aFrameHint;
-#ifdef DEBUG_aleventhal
+#ifdef DEBUG_A11Y
   static int frameHintFailed, frameHintTried, frameHintNonexistant, frameHintFailedForText;
   ++frameHintTried;
 #endif
@@ -1106,7 +1106,7 @@ NS_IMETHODIMP nsAccessibilityService::GetAccessible(nsIDOMNode *aNode,
     // Frame hint not correct, get true frame, we try to optimize away from this
     frame = aPresShell->GetPrimaryFrameFor(content);
     if (frame) {
-#ifdef DEBUG_aleventhal_
+#ifdef DEBUG_A11Y_FRAME_OPTIMIZATION
       // Frame hint debugging
       ++frameHintFailed;
       if (content->IsNodeOfType(nsINode::eTEXT)) {
@@ -1188,7 +1188,7 @@ NS_IMETHODIMP nsAccessibilityService::GetAccessible(nsIDOMNode *aNode,
     nsresult rv = GetAccessibleByType(aNode, getter_AddRefs(newAcc));
     NS_ENSURE_SUCCESS(rv, rv);
 
-    if (newAcc) {
+    if (!newAcc) {
       if (content->GetNameSpaceID() == kNameSpaceID_SVG &&
                content->Tag() == nsAccessibilityAtoms::svg) {
         newAcc = new nsEnumRoleAccessible(aNode, aWeakShell, nsIAccessible::ROLE_DIAGRAM);
@@ -1275,7 +1275,7 @@ nsresult nsAccessibilityService::GetAccessibleByType(nsIDOMNode *aNode,
 
   nsCOMPtr<nsIAccessibleProvider> node(do_QueryInterface(aNode));
   if (!node)
-    return NS_ERROR_FAILURE;
+    return NS_OK;
 
   PRInt32 type;
   nsresult rv = node->GetAccessibleType(&type);

accessible/src/base/nsAccessible.cpp

@@ -495,18 +495,11 @@ NS_IMETHODIMP nsAccessible::InvalidateChildren()
 
 NS_IMETHODIMP nsAccessible::GetParent(nsIAccessible **  aParent)
 {
-  if (!mWeakShell) {
-    // This node has been shut down
-    *aParent = nsnull;
-    return NS_ERROR_FAILURE;
-  }
-  if (mParent) {
-    *aParent = mParent;
-    NS_ADDREF(*aParent);
-    return NS_OK;
+  nsresult rv = GetCachedParent(aParent);
+  if (NS_FAILED(rv) || *aParent) {
+    return rv;
   }
 
-  *aParent = nsnull;
   // Last argument of PR_TRUE indicates to walk anonymous content
   nsAccessibleTreeWalker walker(mWeakShell, mDOMNode, PR_TRUE); 
   if (NS_SUCCEEDED(walker.GetParent())) {
@@ -518,6 +511,17 @@ NS_IMETHODIMP nsAccessible::GetParent(nsIAccessible **  aParent)
   return NS_OK;
 }
 
+NS_IMETHODIMP nsAccessible::GetCachedParent(nsIAccessible **  aParent)
+{
+  *aParent = nsnull;
+  if (!mWeakShell) {
+    // This node has been shut down
+    return NS_ERROR_FAILURE;
+  }
+  NS_IF_ADDREF(*aParent = mParent);
+  return NS_OK;
+}
+
   /* readonly attribute nsIAccessible nextSibling; */
 NS_IMETHODIMP nsAccessible::GetNextSibling(nsIAccessible * *aNextSibling) 
 { 
@@ -660,17 +664,18 @@ void nsAccessible::CacheChildren()
     walker.mState.frame = GetFrame();
 
     nsCOMPtr<nsPIAccessible> privatePrevAccessible;
-    mAccChildCount = 0;
+    PRInt32 childCount = 0;
     walker.GetFirstChild();
     SetFirstChild(walker.mState.accessible);
 
     while (walker.mState.accessible) {
-      ++mAccChildCount;
+      ++ childCount;
       privatePrevAccessible = do_QueryInterface(walker.mState.accessible);
       privatePrevAccessible->SetParent(this);
       walker.GetNextSibling();
       privatePrevAccessible->SetNextSibling(walker.mState.accessible);
     }
+    mAccChildCount = childCount;
   }
 }
 
@@ -724,6 +729,31 @@ NS_IMETHODIMP nsAccessible::GetIndexInParent(PRInt32 *aIndexInParent)
   return NS_OK;
 }
 
+#ifdef DEBUG_A11Y
+NS_IMETHODIMP nsAccessible::TestChildCache(nsIAccessible *aCachedChild)
+{
+  // All cached accessible nodes should be in the parent
+  // It will assert if not all the children were created
+  // when they were first cached, and no invalidation
+  // ever corrected parent accessible's child cache.
+  if (mAccChildCount == eChildCountUninitialized) {
+    return NS_OK;
+  }
+  nsCOMPtr<nsIAccessible> sibling = mFirstChild;
+
+  while (sibling != aCachedChild) {
+    NS_ASSERTION(sibling, "[TestChildCache] Never ran into the same child that we started from");
+    if (!sibling)
+      return NS_ERROR_FAILURE;
+
+    nsCOMPtr<nsIAccessible> tempAccessible;
+    sibling->GetNextSibling(getter_AddRefs(tempAccessible));
+    sibling = tempAccessible;
+  }
+  return NS_OK;
+}
+#endif
+
 nsresult nsAccessible::GetTranslatedString(const nsAString& aKey, nsAString& aStringOut)
 {
   nsXPIDLString xsValue;

accessible/src/base/nsCaretAccessible.cpp

@@ -207,9 +207,6 @@ NS_IMETHODIMP nsCaretAccessible::NotifySelectionChanged(nsIDOMDocument *aDoc, ns
   PRInt32 caretOffset;
   if (NS_SUCCEEDED(textAcc->GetCaretOffset(&caretOffset))) {
     mRootAccessible->FireToolkitEvent(nsIAccessibleEvent::EVENT_ATK_TEXT_CARET_MOVE, accessible, &caretOffset);
-#ifdef DEBUG_aleventhal
-    printf("Caret move to offset %d\n", caretOffset);
-#endif
   }
   if (!isCollapsed) {
     mRootAccessible->FireToolkitEvent(nsIAccessibleEvent::EVENT_ATK_TEXT_SELECTION_CHANGE, accessible, nsnull);

accessible/src/base/nsCaretAccessible.h

@@ -81,7 +81,7 @@ public:
   /* ----- nsIAccessNode ----- */
   NS_IMETHOD Init()
   {
-#ifdef DEBUG
+#ifdef DEBUG_A11Y
     mIsInitialized = PR_TRUE;
 #endif
     return NS_OK;

accessible/src/base/nsDocAccessible.cpp

@@ -391,6 +391,21 @@ void nsDocAccessible::CheckForEditor()
 NS_IMETHODIMP nsDocAccessible::GetCachedAccessNode(void *aUniqueID, nsIAccessNode **aAccessNode)
 {
   GetCacheEntry(mAccessNodeCache, aUniqueID, aAccessNode); // Addrefs for us
+#ifdef DEBUG_A11Y
+  // All cached accessible nodes should be in the parent
+  // It will assert if not all the children were created
+  // when they were first cached, and no invalidation
+  // ever corrected parent accessible's child cache.
+  nsCOMPtr<nsIAccessible> accessible = do_QueryInterface(*aAccessNode);
+  if (accessible) {
+    nsCOMPtr<nsIAccessible> parent;
+    accessible->GetParent(getter_AddRefs(parent));
+    nsCOMPtr<nsPIAccessible> privateParent(do_QueryInterface(parent));
+    if (privateParent) {
+      privateParent->TestChildCache(accessible);
+    }
+  }
+#endif
   return NS_OK;
 }
 
@@ -1082,21 +1097,39 @@ NS_IMETHODIMP nsDocAccessible::InvalidateCacheSubtree(nsIContent *aChild,
   // Otherwise we might miss the nsAccessNode's that are not nsAccessible's.
 
   nsCOMPtr<nsIDOMNode> childNode = aChild ? do_QueryInterface(aChild) : mDOMNode;
-
   if (!mIsContentLoaded && mAccessNodeCache.Count() <= 1) {
     return NS_OK; // Still loading and nothing to invalidate yet
   }
 
-  else if (aChangeEventType == nsIAccessibleEvent::EVENT_HIDE) {
+  nsCOMPtr<nsIAccessNode> childAccessNode;
+  GetCachedAccessNode(childNode, getter_AddRefs(childAccessNode));
+  nsCOMPtr<nsIAccessible> childAccessible = do_QueryInterface(childAccessNode);
+  if (!childAccessible && mIsContentLoaded && aChangeEventType != nsIAccessibleEvent::EVENT_HIDE) {
+    // If not about to hide it, make sure there's an accessible so we can fire an
+    // event for it
+    GetAccService()->GetAccessibleFor(childNode, getter_AddRefs(childAccessible));
+  }
+  nsCOMPtr<nsPIAccessible> privateChildAccessible =
+    do_QueryInterface(childAccessible);
+#ifdef DEBUG_A11Y
+  nsAutoString localName;
+  childNode->GetLocalName(localName);
+  const char *hasAccessible = childAccessible ? " (acc)" : "";
+  if (aChangeEventType == nsIAccessibleEvent::EVENT_HIDE) {
+    printf("[Hide %s %s]\n", NS_ConvertUTF16toUTF8(localName).get(), hasAccessible);
+  }
+  else if (aChangeEventType == nsIAccessibleEvent::EVENT_SHOW) {
+    printf("[Show %s %s]\n", NS_ConvertUTF16toUTF8(localName).get(), hasAccessible);
+  }
+  else if (aChangeEventType == nsIAccessibleEvent::EVENT_REORDER) {
+    printf("[Reorder %s %s]\n", NS_ConvertUTF16toUTF8(localName).get(), hasAccessible);
+  }
+#endif
+
+  if (aChangeEventType == nsIAccessibleEvent::EVENT_HIDE) {
     // Fire EVENT_HIDE or EVENT_MENUPOPUPEND if previous accessible existed
     // for node being hidden. Fire this before the accessible goes away
-    nsCOMPtr<nsIAccessNode> childAccessNode;
-    GetCachedAccessNode(childNode, getter_AddRefs(childAccessNode));
-    nsCOMPtr<nsIAccessible> childAccessible = do_QueryInterface(childAccessNode);
-    if (childAccessible) {
-      nsCOMPtr<nsPIAccessible> privateChildAccessible =
-        do_QueryInterface(childAccessible);
-      NS_ASSERTION(privateChildAccessible, "No nsPIAccessible for an nsIAccessible");
+    if (privateChildAccessible) {
       privateChildAccessible->FireToolkitEvent(nsIAccessibleEvent::EVENT_HIDE,
                                                childAccessible, nsnull);
     }
@@ -1127,8 +1160,8 @@ NS_IMETHODIMP nsDocAccessible::InvalidateCacheSubtree(nsIContent *aChild,
       containerAccessible = this;  // At the root of UI or content
     }
   }
-  if (!containerAccessible) {
-    GetAccessibleInParentChain(childNode, getter_AddRefs(containerAccessible));
+  if (!containerAccessible && privateChildAccessible) {
+    privateChildAccessible->GetCachedParent(getter_AddRefs(containerAccessible));
   }
   nsCOMPtr<nsPIAccessible> privateContainerAccessible =
     do_QueryInterface(containerAccessible);

accessible/src/base/nsOuterDocAccessible.cpp

@@ -95,7 +95,6 @@ void nsOuterDocAccessible::CacheChildren()
     return;
   }
 
-  mAccChildCount = 0;
   SetFirstChild(nsnull);
 
   // In these variable names, "outer" relates to the nsOuterDocAccessible
@@ -108,12 +107,14 @@ void nsOuterDocAccessible::CacheChildren()
 
   nsCOMPtr<nsIDocument> outerDoc = content->GetDocument();
   if (!outerDoc) {
+    mAccChildCount = 0;
     return;
   }
 
   nsIDocument *innerDoc = outerDoc->GetSubDocumentFor(content);
   nsCOMPtr<nsIDOMNode> innerNode(do_QueryInterface(innerDoc));
   if (!innerNode) {
+    mAccChildCount = 0;
     return;
   }
 
@@ -124,6 +125,7 @@ void nsOuterDocAccessible::CacheChildren()
   nsCOMPtr<nsPIAccessible> privateInnerAccessible = 
     do_QueryInterface(innerAccessible);
   if (!privateInnerAccessible) {
+    mAccChildCount = 0;
     return;
   }
 

accessible/src/base/nsRootAccessible.cpp

@@ -527,9 +527,9 @@ NS_IMETHODIMP nsRootAccessible::HandleEventWithTarget(nsIDOMEvent* aEvent,
   aEvent->GetType(eventType);
   nsAutoString localName;
   targetNode->GetLocalName(localName);
-#ifdef DEBUG_aleventhal
+#ifdef DEBUG_A11Y
   // Very useful for debugging, please leave this here.
-  if (eventType.LowerCaseEqualsLiteral("focus")) {
+  if (eventType.LowerCaseEqualsLiteral("alertactive")) {
     printf("\ndebugging %s events for %s", NS_ConvertUTF16toUTF8(eventType).get(), NS_ConvertUTF16toUTF8(localName).get());
   }
   if (localName.LowerCaseEqualsLiteral("textbox")) {

accessible/src/html/nsHTMLFormControlAccessible.cpp

@@ -504,7 +504,7 @@ void nsHTMLGroupboxAccessible::CacheChildren()
     GetAllowsAnonChildAccessibles(&allowsAnonChildren);
     nsAccessibleTreeWalker walker(mWeakShell, mDOMNode, allowsAnonChildren);
     walker.mState.frame = GetFrame();
-    mAccChildCount = 0;
+    PRInt32 childCount = 0;
     walker.GetFirstChild();
     // Check for <legend> and skip it if it's there
     if (walker.mState.accessible && walker.mState.domNode) {
@@ -518,11 +518,12 @@ void nsHTMLGroupboxAccessible::CacheChildren()
     SetFirstChild(walker.mState.accessible);
     nsCOMPtr<nsPIAccessible> privatePrevAccessible;
     while (walker.mState.accessible) {
-      ++mAccChildCount;
+      ++ childCount;
       privatePrevAccessible = do_QueryInterface(walker.mState.accessible);
       privatePrevAccessible->SetParent(this);
       walker.GetNextSibling();
       privatePrevAccessible->SetNextSibling(walker.mState.accessible);
     }
+    mAccChildCount = childCount;
   }
 }

accessible/src/html/nsHTMLImageAccessible.cpp

@@ -182,22 +182,23 @@ void nsHTMLImageAccessible::CacheChildren()
     return;
   }
 
-  mAccChildCount = 0;
   nsCOMPtr<nsIDOMHTMLCollection> mapAreas;
   if (mMapElement) {
     mMapElement->GetAreas(getter_AddRefs(mapAreas));
   }
   if (!mapAreas) {
+    mAccChildCount = 0;
     return;
   }
 
   PRUint32 numMapAreas;
   mapAreas->GetLength(&numMapAreas);
-
+  PRInt32 childCount = 0;
+  
   nsCOMPtr<nsIAccessible> areaAccessible;
   nsCOMPtr<nsPIAccessible> privatePrevAccessible;
-  while (mAccChildCount < (PRInt32)numMapAreas && 
-         (areaAccessible = CreateAreaAccessible(mAccChildCount)) != nsnull) {
+  while (childCount < (PRInt32)numMapAreas && 
+         (areaAccessible = CreateAreaAccessible(childCount)) != nsnull) {
     if (privatePrevAccessible) {
       privatePrevAccessible->SetNextSibling(areaAccessible);
     }
@@ -205,12 +206,13 @@ void nsHTMLImageAccessible::CacheChildren()
       SetFirstChild(areaAccessible);
     }
 
-    ++mAccChildCount;
+    ++ childCount;
 
     privatePrevAccessible = do_QueryInterface(areaAccessible);
     NS_ASSERTION(privatePrevAccessible, "nsIAccessible impl's should always support nsPIAccessible as well");
     privatePrevAccessible->SetParent(this);
   }
+  mAccChildCount = childCount;
 }
 
 NS_IMETHODIMP nsHTMLImageAccessible::DoAction(PRUint8 index)

accessible/src/html/nsHTMLSelectAccessible.cpp

@@ -343,7 +343,8 @@ NS_IMETHODIMP nsHTMLSelectListAccessible::GetRole(PRUint32 *_retval)
 already_AddRefed<nsIAccessible>
 nsHTMLSelectListAccessible::AccessibleForOption(nsIAccessibilityService *aAccService,
                                                 nsIContent *aContent,
-                                                nsIAccessible *aLastGoodAccessible)
+                                                nsIAccessible *aLastGoodAccessible,
+                                                PRInt32 *aChildCount)
 {
   nsCOMPtr<nsIDOMNode> domNode(do_QueryInterface(aContent));
   NS_ASSERTION(domNode, "DOM node is null");
@@ -355,7 +356,7 @@ nsHTMLSelectListAccessible::AccessibleForOption(nsIAccessibilityService *aAccSer
     return nsnull;
   }
 
-  ++ mAccChildCount;
+  ++ *aChildCount;
   privateAccessible->SetParent(this);
   nsCOMPtr<nsPIAccessible> privatePrevAccessible(do_QueryInterface(aLastGoodAccessible));
   if (privatePrevAccessible) {
@@ -372,7 +373,8 @@ nsHTMLSelectListAccessible::AccessibleForOption(nsIAccessibilityService *aAccSer
 already_AddRefed<nsIAccessible>
 nsHTMLSelectListAccessible::CacheOptSiblings(nsIAccessibilityService *aAccService,
                                              nsIContent *aParentContent,
-                                             nsIAccessible *aLastGoodAccessible)
+                                             nsIAccessible *aLastGoodAccessible,
+                                             PRInt32 *aChildCount)
 {
   // Recursive helper for CacheChildren()
 
@@ -388,10 +390,12 @@ nsHTMLSelectListAccessible::CacheOptSiblings(nsIAccessibilityService *aAccServic
     if (tag == nsAccessibilityAtoms::option || tag == nsAccessibilityAtoms::optgroup) {
       lastGoodAccessible = AccessibleForOption(aAccService,
                                                childContent,
-                                               lastGoodAccessible);
+                                               lastGoodAccessible,
+                                               aChildCount);
       if (tag == nsAccessibilityAtoms::optgroup) {
         lastGoodAccessible = CacheOptSiblings(aAccService, childContent,
-                                              lastGoodAccessible);
+                                              lastGoodAccessible,
+                                              aChildCount);
       }
     }
   }
@@ -422,9 +426,10 @@ void nsHTMLSelectListAccessible::CacheChildren()
     return;
   }
 
-  mAccChildCount = 0;
+  PRInt32 childCount = 0;
   nsCOMPtr<nsIAccessible> lastGoodAccessible =
-    CacheOptSiblings(accService, selectContent, nsnull);
+    CacheOptSiblings(accService, selectContent, nsnull, &childCount);
+  mAccChildCount = childCount;
 }
 
 /** ----- nsHTMLSelectOptionAccessible ----- */

accessible/src/html/nsHTMLSelectAccessible.h

@@ -136,11 +136,13 @@ protected:
   already_AddRefed<nsIAccessible>
     AccessibleForOption(nsIAccessibilityService *aAccService,
                         nsIContent *aContent,
-                        nsIAccessible *aLastGoodAccessible);
+                        nsIAccessible *aLastGoodAccessible,
+                        PRInt32 *aChildCount);
   already_AddRefed<nsIAccessible>
     CacheOptSiblings(nsIAccessibilityService *aAccService,
                      nsIContent *aParentContent,
-                     nsIAccessible *aLastGoodAccessible);
+                     nsIAccessible *aLastGoodAccessible,
+                     PRInt32 *aChildCount);
 };
 
 /*

accessible/src/html/nsHTMLTableAccessible.cpp

@@ -611,7 +611,7 @@ NS_IMETHODIMP nsHTMLTableAccessible::GetDescription(nsAString& aDescription)
   PRBool isProbablyForLayout;
   IsProbablyForLayout(&isProbablyForLayout);
   aDescription = mLayoutHeuristic;
-#ifdef DEBUG
+#ifdef DEBUG_A11Y
   printf("\nTABLE: %s\n", NS_ConvertUTF16toUTF8(mLayoutHeuristic).get());
 #endif
   return NS_OK;

accessible/src/html/nsHTMLTextAccessible.cpp

@@ -262,7 +262,7 @@ void nsHTMLLIAccessible::CacheChildren()
   if (mAccChildCount == eChildCountUninitialized) {
     SetFirstChild(mBulletAccessible);
     mBulletAccessible->SetParent(this); // Set weak parent;
-    mAccChildCount = 1;
+    PRInt32 childCount = 1;
     PRBool allowsAnonChildren = PR_FALSE;
     GetAllowsAnonChildAccessibles(&allowsAnonChildren);
     nsAccessibleTreeWalker walker(mWeakShell, mDOMNode, allowsAnonChildren);
@@ -271,12 +271,13 @@ void nsHTMLLIAccessible::CacheChildren()
 
     nsCOMPtr<nsPIAccessible> privatePrevAccessible = mBulletAccessible.get();
     while (walker.mState.accessible) {
-      ++mAccChildCount;
+      ++ childCount;
       privatePrevAccessible->SetNextSibling(walker.mState.accessible);
       privatePrevAccessible = do_QueryInterface(walker.mState.accessible);
       privatePrevAccessible->SetParent(this);
       walker.GetNextSibling();
     }
+    mAccChildCount = childCount;
   }
 }
 

accessible/src/html/nsHyperTextAccessible.cpp

@@ -362,8 +362,9 @@ nsresult nsHyperTextAccessible::DOMPointToOffset(nsIDOMNode* aNode, PRInt32 aNod
   if (nodeType != nsIDOMNode::TEXT_NODE) {
     // If not text, aNodeOffset is a child
     if (aNode != mDOMNode) {
-#ifdef DEBUG
+#ifdef DEBUG_A11Y
       // This should only happen in the empty plaintext case, when there is no text child yet
+      // XXX What's with this check, the assertion is commented out, why?
       nsCOMPtr<nsIEditor> editor = GetEditor();
       nsCOMPtr<nsIPlaintextEditor> plaintextEditor = do_QueryInterface(editor);
       // NS_ASSERTION(plaintextEditor, "DOM Point is not in this nsHyperTextAccessible");
@@ -1130,7 +1131,7 @@ nsresult nsHyperTextAccessible::FireTextChangeEvent(AtkTextChange *aTextData)
   nsCOMPtr<nsIAccessible> accessible(do_QueryInterface(NS_STATIC_CAST(nsIAccessibleText*, this)));
   nsCOMPtr<nsPIAccessible> privAccessible(do_QueryInterface(accessible));
   if (privAccessible) {
-#ifdef DEBUG
+#ifdef DEBUG_A11Y
     printf("  [start=%d, length=%d, add=%d]\n", aTextData->start, aTextData->length, aTextData->add);
 #endif
     privAccessible->FireToolkitEvent(nsIAccessibleEvent::EVENT_ATK_TEXT_CHANGE, accessible, aTextData);

accessible/src/msaa/nsAccessibleWrap.cpp

@@ -277,7 +277,9 @@ STDMETHODIMP nsAccessibleWrap::get_accName(
     if (!name.IsVoid()) {
       *pszName = ::SysAllocString(name.get());
     }
+#ifdef DEBUG_A11Y
     NS_ASSERTION(mIsInitialized, "Access node was not initialized");
+#endif
   }
 
   return S_OK;

accessible/src/msaa/nsDocAccessibleWrap.cpp

@@ -174,7 +174,7 @@ NS_IMETHODIMP nsDocAccessibleWrap::Shutdown()
 
 NS_IMETHODIMP nsDocAccessibleWrap::FireToolkitEvent(PRUint32 aEvent, nsIAccessible* aAccessible, void* aData)
 {
-#ifdef DEBUG
+#ifdef DEBUG_A11Y
   // Ensure that we're only firing events that we intend to
   PRUint32 supportedEvents[] = {
     nsIAccessibleEvent::EVENT_SHOW,
@@ -207,7 +207,7 @@ NS_IMETHODIMP nsDocAccessibleWrap::FireToolkitEvent(PRUint32 aEvent, nsIAccessib
     }
   }
   if (!found) {
-    NS_WARNING("Event not supported!");
+    // NS_WARNING("Event not supported!");
   }
 #endif
   if (!mWeakShell) {   // Means we're not active
@@ -526,6 +526,9 @@ STDMETHODIMP nsDocAccessibleWrap::get_docType(/* [out] */ BSTR __RPC_FAR *aDocTy
 STDMETHODIMP nsDocAccessibleWrap::get_nameSpaceURIForID(/* [in] */  short aNameSpaceID,
   /* [out] */ BSTR __RPC_FAR *aNameSpaceURI)
 {
+  if (aNameSpaceID < 0) {
+    return E_FAIL;  // -1 is kNameSpaceID_Unknown
+  }
   *aNameSpaceURI = NULL;
   nsAutoString nameSpaceURI;
   if (NS_SUCCEEDED(GetNameSpaceURIForID(aNameSpaceID, nameSpaceURI))) {

accessible/src/xul/nsXULMenuAccessible.cpp

@@ -214,11 +214,16 @@ NS_IMETHODIMP nsXULMenuitemAccessible::GetRole(PRUint32 *aRole)
   return NS_OK;
 }
 
-NS_IMETHODIMP nsXULMenuitemAccessible::GetChildCount(PRInt32 *aAccChildCount)
+void nsXULMenuitemAccessible::CacheChildren()
 {
+  if (!mWeakShell) {
+    // This node has been shut down
+    mAccChildCount = eChildCountUninitialized;
+    return;
+  }
+
   if (mAccChildCount != eChildCountUninitialized) {
-    *aAccChildCount = mAccChildCount;
-    return NS_OK;
+    return;
   }
 
   // Set menugenerated="true" on the menupopup node to generate the
@@ -246,21 +251,9 @@ NS_IMETHODIMP nsXULMenuitemAccessible::GetChildCount(PRInt32 *aAccChildCount)
           element->SetAttribute(NS_LITERAL_STRING("menugenerated"), NS_LITERAL_STRING("true"));
         }
       }
-      // fire a popup dom event
-      nsEventStatus status = nsEventStatus_eIgnore;
-      nsMouseEvent event(PR_TRUE, NS_XUL_POPUP_SHOWING, nsnull,
-                         nsMouseEvent::eReal);
-
-      nsCOMPtr<nsIPresShell> presShell(do_QueryReferent(mWeakShell));
-      nsCOMPtr<nsIContent> content(do_QueryInterface(childNode));
-      if (presShell && content)
-        presShell->HandleDOMEventWithTarget(content, &event, &status);
     }
   }
-
-  CacheChildren();
-  *aAccChildCount = mAccChildCount;
-  return NS_OK;  
+  nsAccessibleWrap::CacheChildren();
 }
 
 NS_IMETHODIMP

accessible/src/xul/nsXULMenuAccessible.h

@@ -57,11 +57,11 @@ public:
   NS_IMETHOD GetKeyBinding(nsAString& _retval);
   NS_IMETHOD GetState(PRUint32 *_retval); 
   NS_IMETHOD GetRole(PRUint32 *_retval); 
-  NS_IMETHOD GetChildCount(PRInt32 *aAccChildCount);
   NS_IMETHOD DoAction(PRUint8 index);
   NS_IMETHOD GetActionName(PRUint8 index, nsAString& _retval);
   NS_IMETHOD GetNumActions(PRUint8 *_retval);
   NS_IMETHOD GetAllowsAnonChildAccessibles(PRBool *aAllowsAnonChildren);
+  void CacheChildren();
 };
 
 class nsXULMenuSeparatorAccessible : public nsXULMenuitemAccessible

accessible/src/xul/nsXULSelectAccessible.cpp

@@ -602,8 +602,18 @@ NS_IMETHODIMP nsXULComboboxAccessible::GetDescription(nsAString& aDescription)
   return NS_OK;
 }
 
-NS_IMETHODIMP nsXULComboboxAccessible::GetChildCount(PRInt32 *aAccChildCount)
+void nsXULComboboxAccessible::CacheChildren()
 {
+  if (!mWeakShell) {
+    // This node has been shut down
+    mAccChildCount = -1;
+    return;
+  }
+
+  if (mAccChildCount != eChildCountUninitialized) {
+    return;
+  }
+
   // Set menugenerated="true" on the menupopup node to generate the
   // sub-menu items if they have not been generated
   PRUint32 childIndex, numChildren = 0;
@@ -631,10 +641,7 @@ NS_IMETHODIMP nsXULComboboxAccessible::GetChildCount(PRInt32 *aAccChildCount)
       }
     }
   }
-
-  CacheChildren();
-  *aAccChildCount = mAccChildCount;
-  return NS_OK;
+  nsXULSelectableAccessible::CacheChildren();
 }
 
 NS_IMETHODIMP

accessible/src/xul/nsXULSelectAccessible.h

@@ -135,7 +135,6 @@ public:
   NS_IMETHOD GetRole(PRUint32 *_retval);
   NS_IMETHOD GetState(PRUint32 *_retval);
   NS_IMETHOD GetValue(nsAString& _retval);
-
 };
 
 /**
@@ -157,7 +156,7 @@ public:
   NS_IMETHOD GetState(PRUint32 *_retval);
   NS_IMETHOD GetActionName(PRUint8 index, nsAString& _retval);
   // Don't use XUL menu's special child aggregator, this can be a rich list item
-  NS_IMETHOD GetChildCount(PRInt32 *aAccChildCount) { return nsAccessibleWrap::GetChildCount(aAccChildCount); }
+  void CacheChildren() { nsAccessibleWrap::CacheChildren(); }
 
 private:
   PRBool mIsCheckbox;
@@ -178,7 +177,7 @@ public:
   virtual ~nsXULComboboxAccessible() {}
 
   /* ----- nsIAccessible ----- */
-  NS_IMETHOD GetChildCount(PRInt32 *_retval);
+  void CacheChildren();
   NS_IMETHOD GetRole(PRUint32 *_retval);
   NS_IMETHOD GetState(PRUint32 *_retval);