386580 - Crash [@nsAccessibleWrap::GetAtkObject]

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Ginn Chen]

I got this crash, when I use accerciser to monitor events, and then quit Firefox.

It looks like we missed something in nsAppRootAccessibleWrap

#0  0x00000000 in ?? ()
#1  0xb6db1b54 in nsAccessibleWrap::GetAtkObject (this=0x8814090)
    at nsAccessibleWrap.cpp:396
#2  0xb6db3b16 in GetAccessibleWrap (aAtkObj=0x8171af0)
    at nsAccessibleWrap.cpp:1078
#3  0xb6db5319 in getRoleCB (aAtkObj=0x8171af0) at nsAccessibleWrap.cpp:768
#4  0xb78acabc in atk_object_get_role (accessible=0x8171af0) at atkobject.c:730
#5  0xb2168bc3 in impl_accessibility_accessible_get_role_name (
    servant=0x85c2d74, ev=0xbfd772cc) at accessible.c:413

frame 2
(gdb) print *aAtkObj
$2 = {parent = {g_type_instance = {g_class = 0x8817770}, ref_count = 3, 
    qdata = 0x0}, description = 0x8c881d8 "Minefield Application Accessible", 
  name = 0x8800870 "Minefield", accessible_parent = 0x0, 
  role = ATK_ROLE_APPLICATION, relation_set = 0x84a5570, 
  layer = ATK_LAYER_INVALID}

Comment 1

[Aaron Leventhal]

Ginn, can you still reproduce this?

Comment 2

[Ginn Chen]

Still reproducible.

I will take a look.

Comment 3

[Ginn Chen]

The root cause is, ShutdownAtkObject is not called for nsAppRootAccessibleWrap.

Because in nsAccessNode::LastRelease(), it checks mWeakShell before calling Shutdown();

To solve this, we can either
#1, remove the mWeakShell check in nsAccessNode::LastRelease().
I think it may cause performance regression.
or
#2, In nsAccessNode::LastRelease(), if (!mWeakShell) { ((nsAccessibleWrap*)this)->Shutdown();
looks ugly
or
#3, call ShutdownAtkObject() in ~nsAppAccessibleWrap()
I think nsApplicationAccessible is the only object that initialized with null mWeakShell, right? 

Comment 4

[Ginn Chen]

Attachment: patch

Comment 5

[Aaron Leventhal]

> I think nsApplicationAccessible is the only object that initialized with null
mWeakShell, right? 

Correct.

Comment 6

[Aaron Leventhal]

Damon meant to mark a+ not r+

Comment 7

[Aaron Leventhal]

Checked in for Ginn.