Bug 163812: incorrect caching in accessibility leads to null ptr deref. r=jgaunt, sr=alecf. Bug 163815: Accessibility should not use nsCOMPtr with frames. r=jgaunt, sr=alecf

accessible/src/base/nsAccessible.cpp

@@ -132,6 +132,9 @@ nsAccessibleTreeWalker::nsAccessibleTreeWalker(nsIWeakReference* aPresShell, nsI
 
   if (mState.siblingIndex < 0)
     mState.siblingList = nsnull;
+  NS_ASSERTION(mState.siblingList || mState.siblingIndex < 0,
+    "Accessible tree walker initialization error, "
+    "can't have index into null sibling list");
 
   if (aWalkAnonContent) {
     nsCOMPtr<nsIPresShell> shell(do_QueryReferent(mPresShell));
@@ -142,6 +145,7 @@ nsAccessibleTreeWalker::nsAccessibleTreeWalker(nsIWeakReference* aPresShell, nsI
     }
   }
   MOZ_COUNT_CTOR(nsAccessibleTreeWalker);
+  mInitialState = mState;  // deep copy
 }
 
 nsAccessibleTreeWalker::~nsAccessibleTreeWalker()
@@ -205,6 +209,8 @@ void nsAccessibleTreeWalker::GetSiblings(nsIDOMNode *aOneOfTheSiblings)
   if (NS_SUCCEEDED(GetFullTreeParentNode(aOneOfTheSiblings, getter_AddRefs(node)))) {
     GetKids(node);
     if (mState.siblingList) {      // Init index by seeing how far we are into list
+      if (mState.domNode == mInitialState.domNode)
+        mInitialState = mState; // deep copy, we'll use sibling info for caching
       while (NS_SUCCEEDED(mState.siblingList->Item(mState.siblingIndex, getter_AddRefs(node))) && node != mState.domNode) {
         NS_ASSERTION(node, "Something is terribly wrong - the child is not in it's parent's children!");
         ++mState.siblingIndex;
@@ -595,8 +601,12 @@ NS_IMETHODIMP nsAccessible::GetAccNextSibling(nsIAccessible * *aAccNextSibling)
   if (NS_SUCCEEDED(walker.GetNextSibling())) {
     *aAccNextSibling = walker.mState.accessible;
     NS_ADDREF(*aAccNextSibling);
-    mSiblingList = walker.mState.siblingList;
-    (*aAccNextSibling)->CacheOptimizations(mParent, walker.mState.siblingIndex, mSiblingList);
+    // Use last walker state to cache data on next accessible
+    (*aAccNextSibling)->CacheOptimizations(mParent, walker.mState.siblingIndex, 
+                                           walker.mState.siblingList);
+    // Use first walker state to cache data on this accessible
+    CacheOptimizations(mParent, walker.mInitialState.siblingIndex,
+                       walker.mInitialState.siblingList);
   }
 
   return NS_OK;  
@@ -612,8 +622,12 @@ NS_IMETHODIMP nsAccessible::GetAccPreviousSibling(nsIAccessible * *aAccPreviousS
   if (NS_SUCCEEDED(walker.GetPreviousSibling())) {
     *aAccPreviousSibling = walker.mState.accessible;
     NS_ADDREF(*aAccPreviousSibling);
-    mSiblingList = walker.mState.siblingList;
-    (*aAccPreviousSibling)->CacheOptimizations(mParent, walker.mState.siblingIndex, mSiblingList);
+    // Use last walker state to cache data on prev accessible
+    (*aAccPreviousSibling)->CacheOptimizations(mParent, walker.mState.siblingIndex, 
+                                           walker.mState.siblingList);
+    // Use first walker state to cache data on this accessible
+    CacheOptimizations(mParent, walker.mInitialState.siblingIndex,
+                       walker.mInitialState.siblingList);
   }
 
   return NS_OK;  

accessible/src/base/nsAccessible.h

@@ -158,6 +158,7 @@ public:
   NS_IMETHOD GetLastChild();
   PRInt32 GetChildCount();
   WalkState mState;
+  WalkState mInitialState;
 
 protected:
   NS_IMETHOD GetChildBefore(nsIDOMNode* aParent, nsIDOMNode* aChild);

accessible/src/html/nsHTMLFormControlAccessible.cpp

@@ -328,7 +328,8 @@ nsFormControlAccessible(aNode, aShell)
   nsCOMPtr<nsIContent> content(do_QueryInterface(mDOMNode));
   nsIFrame *frame = nsnull;
   shell->GetPrimaryFrameFor(content, &frame);
-  nsCOMPtr<nsITextControlFrame> tframe(do_QueryInterface(frame));
+  nsITextControlFrame *tframe = nsnull;
+  frame->QueryInterface(NS_GET_IID(nsITextControlFrame), (void**)&tframe);
   if (!tframe)
     return;
 
@@ -468,7 +469,8 @@ NS_IMETHODIMP nsHTMLTextFieldAccessible::MakeSelection(PRInt32 aStartPos, PRInt3
   nsCOMPtr<nsIContent> content(do_QueryInterface(mDOMNode));
   nsIFrame *frame = nsnull;
   shell->GetPrimaryFrameFor(content, &frame);
-  nsCOMPtr<nsITextControlFrame> tframe(do_QueryInterface(frame));
+  nsITextControlFrame *tframe = nsnull;
+  frame->QueryInterface(NS_GET_IID(nsITextControlFrame), (void**)&tframe);
   if (!tframe)
     return NS_ERROR_FAILURE;  
 

accessible/src/html/nsHTMLImageAccessible.cpp

@@ -89,7 +89,8 @@ NS_IMETHODIMP nsHTMLImageAccessible::GetAccState(PRUint32 *_retval)
   if (content && shell) 
     shell->GetPrimaryFrameFor(content, &frame);
 
-  nsCOMPtr<nsIImageFrame> imageFrame(do_QueryInterface(frame));
+  nsIImageFrame *imageFrame = nsnull;
+  frame->QueryInterface(NS_GET_IID(nsIImageFrame), (void**)&imageFrame);
 
   nsCOMPtr<imgIRequest> imageRequest;
   if (imageFrame) 

accessible/src/html/nsHTMLSelectAccessible.cpp

@@ -602,7 +602,8 @@ NS_IMETHODIMP nsHTMLSelectOptionAccessible::AccDoAction(PRUint8 index)
 
     nsIFrame *selectFrame = nsnull;
     presShell->GetPrimaryFrameFor(selectContent, &selectFrame);
-    nsCOMPtr<nsIComboboxControlFrame> comboBoxFrame(do_QueryInterface(selectFrame));
+    nsIComboboxControlFrame *comboBoxFrame = nsnull;
+    selectFrame->QueryInterface(NS_GET_IID(nsIComboboxControlFrame), (void**)&comboBoxFrame);
     if (comboBoxFrame) {
       nsIFrame *listFrame = nsnull;
       comboBoxFrame->GetDropDown(&listFrame);
@@ -610,7 +611,8 @@ NS_IMETHODIMP nsHTMLSelectOptionAccessible::AccDoAction(PRUint8 index)
       comboBoxFrame->IsDroppedDown(&isDroppedDown);
       if (isDroppedDown && listFrame) {
         // use this list control frame to roll up the list
-        nsCOMPtr<nsIListControlFrame> listControlFrame(do_QueryInterface(listFrame));
+        nsIListControlFrame *listControlFrame = nsnull;
+        listFrame->QueryInterface(NS_GET_IID(nsIListControlFrame), (void**)&listControlFrame);
         if (listControlFrame) {
           PRInt32 newIndex = 0;
           option->GetIndex(&newIndex);
@@ -645,6 +647,8 @@ nsresult nsHTMLSelectOptionAccessible::GetFocusedOptionNode(nsIDOMNode *aListNod
 
   nsIFrame *frame = nsnull;
   shell->GetPrimaryFrameFor(content, &frame);
+  if (!frame)
+    return NS_ERROR_FAILURE;
 
   PRInt32 focusedOptionIndex = 0;
 
@@ -656,7 +660,8 @@ nsresult nsHTMLSelectOptionAccessible::GetFocusedOptionNode(nsIDOMNode *aListNod
   nsresult rv = selectElement->GetOptions(getter_AddRefs(options));
   
   if (NS_SUCCEEDED(rv)) {
-    nsCOMPtr<nsIListControlFrame> listFrame(do_QueryInterface(frame));
+    nsIListControlFrame *listFrame = nsnull;
+    frame->QueryInterface(NS_GET_IID(nsIListControlFrame), (void**)&listFrame);
     if (listFrame) {
       // Get what's focused in listbox by asking frame for "selected item". 
       // Can't use dom interface for this, because it will always return the first selected item
@@ -840,7 +845,9 @@ NS_IMETHODIMP nsHTMLComboboxAccessible::GetAccState(PRUint32 *_retval)
 
   // we are open or closed
   PRBool isOpen = PR_FALSE;
-  nsCOMPtr<nsIComboboxControlFrame> comboFrame(do_QueryInterface(GetBoundsFrame()));
+  nsIFrame *frame = GetBoundsFrame();
+  nsIComboboxControlFrame *comboFrame = nsnull;
+  frame->QueryInterface(NS_GET_IID(nsIComboboxControlFrame), (void**)&comboFrame);
   if (comboFrame)
     comboFrame->IsDroppedDown(&isOpen);