657296 - Crash [@ nsAccessibilityService::GetAccessible]

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Ginn Chen]

Run firefox -P, create a lots of profiles. I created 12 profiles.
Use Accerciser to expose Firefox a11y tree.
It crashed.

The stack is:
=>[1] nsCOMPtr<nsINodeInfo>::operator->(this = 0x8), line 819 in "nsCOMPtr.h"
  [2] nsINode::GetOwnerDoc(this = (nil)), line 415 in "nsINode.h"
  [3] nsAccessibilityService::GetAccessible(this = 0x8662e28, aNode = (nil)), line 848 in "nsAccessibilityService.cpp"
  [4] nsAccUtils::GetPositionAndSizeForXULSelectControlItem(aContent = 0x865ead0, aPosInSet = 0x804556c, aSetSize = 0x8045568), line 163 in "nsAccUtils.cpp"
  [5] nsXULListitemAccessible::GetPositionAndSizeInternal(this = 0x867dea8, aPosInSet = 0x804556c, aSetSize = 0x8045568), line 967 in "nsXULListboxAccessible.cpp"
  [6] nsAccessible::GroupPosition(this = 0x867dea8, aGroupLevel = 0x80459b0, aSimilarItemsInGroup = 0x80459a8, aPositionInGroup = 0x80459ac), line 1507 in "nsAccessible.cpp"
  [7] nsAccessible::GetAttributes(this = 0x867dea8, aAttributes = 0x8045ce8), line 1333 in "nsAccessible.cpp"
  [8] GetAttributeSet(aAccessible = 0x867dea8), line 802 in "nsAccessibleWrap.cpp"
  [9] getAttributesCB(aAtkObj = 0x8695f90), line 824 in "nsAccessibleWrap.cpp"
  [10] atk_object_get_attributes(0x8695f90, 0x0, 0xfeec3cfc, 0xf6032dd2), at 0xf829c556

Not sure if it relates to Bug 656225.

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

null check

Comment 2

[Ginn Chen]

Can we also null check and assert at
http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/nsAccessibilityService.cpp#843 ?

Comment 3

[alexander :surkov (:asurkov)]

(In reply to comment #2)
> Can we also null check and assert at
> http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/
> nsAccessibilityService.cpp#843 ?

I'm fine to add precondition (assertion). I'm not sure about extra null check, double check is not what we want.

Comment 4

[Ginn Chen]

How do we know other calls to GetAccessible are all fine?

Comment 5

[alexander :surkov (:asurkov)]

(In reply to comment #4)
> How do we know other calls to GetAccessible are all fine?

Crash reports or code analysis. There's second option is to move the check into the method, but this way makes sense if the callers do a check in most cases.

Comment 6

[alexander :surkov (:asurkov)]

landed - http://hg.mozilla.org/mozilla-central/rev/d02f592810cd