492961 - Crash [@ nsHTMLTableHeaderAccessible::GetRoleInternal(unsigned int*) ]

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

http://crash-stats.mozilla.com/report/index/a1827418-9a41-4edd-881d-c0bf82090514

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

Just crach fix, not actual problem what lies in invalidation problems I would guess.

aja: surkov: noticed latest crash when tab with tindexboxpushlog finished a load.

Comment 2

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 377401 [details] [diff] [review]
patch

r=me

(Still need to figure out underlying issue of course)

Comment 3

[alexander :surkov (:asurkov)]

(In reply to comment #2)
> (From update of attachment 377401 [details] [diff] [review])
> r=me
> 
> (Still need to figure out underlying issue of course)

Wanna play with Orca on Linux? If I get right then originally problem was reproduced by this way.

Comment 4

[alexander :surkov (:asurkov)]

http://crash-stats.mozilla.com/report/index/dac056e8-2a1b-4b0a-b189-7514c2090514
http://crash-stats.mozilla.com/report/index/a1827418-9a41-4edd-881d-c0bf82090514
http://crash-stats.mozilla.com/report/index/26afa9cd-1ea7-4710-8907-d08f32090514
http://crash-stats.mozilla.com/report/pending/3aceafde-9d3d-4241-bf0e-c49d72090514

installed extension:
http://firefox.cita.uiuc.edu/
http://firefox.cita.uiuc.edu/downloads/accessext1.5.5.xpi

Comment 5

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #1)
> Just crach fix, not actual problem what lies in invalidation problems I would
> guess.

Let's IRC about this.

Comment 6

[aja+bugzilla]

Crashes 
* with aforementioned extension,disabled
* with aforementioned extension uninstalled
* with all other extensions disabled via -safe-mode startup parm
* even with Orca uninstalled: http://crash-stats.mozilla.com/report/index/6004fbf7-3d8c-4564-b0fe-1c0f92090514

No crashes with prior nitely: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090513 Minefield/3.6a1pre ID:20090513030859

Comment 7

[aja+bugzilla]

Did some additional testing to narrow down STR.

Seems that running
http://tests.themasta.com/tinderboxpushlog/
til it updates seems to be all it takes.

Removing tab with that URL allowed running for hours without crashing.

Comment 8

[aja+bugzilla]

Results of regression tests on trunk hourlies:

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090513 Minefield/3.6a1pre ID:20090513204509 no crashes

20090513213927 crashes

Regression range URL:
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2009-05-13+20%3A45%3A09+&enddate=2009-05-13+21%3A39%3A27

I'd bet it's regression from
changeset 2322a5800d59	 Bug 491851 - implement relations inside HTML table

Comment 9

[timeless]

so, you really shouldn't have both an ASSERTION and an ENSURE_. Instead after the assertion, use a normal:
 if (...)
  return ...;

It's also a bit odd to use an assertion for something you *know* will happen. There should be a bug number open for fixing it. assertions should be limited to things you don't think can happen....

Comment 10

[David Bolter [:davidb] (NeedInfo me for attention)]

CC'ing Roc

Roc, it seems we have a problem in nsHTMLTableHeaderAccessible::GetRoleInternal (see Alex's attachment 377401 [details] [diff] [review]) where our nsIContent doesn't have a parent. Do you think this is a situation where we need to call FlushPendingNotifications on the doc pres shell? Can you think of other reasons GetParent might fail?

Comment 11

[David Bolter [:davidb] (NeedInfo me for attention)]

Really cc'ing Roc this time. Please see comment #10

Comment 12

[Marco Zehe (:MarcoZ)]

Comment on attachment 377401 [details] [diff] [review]
patch

r=me. Sorry for the dealy, was out of the office.

Comment 13

[alexander :surkov (:asurkov)]

(In reply to comment #9)
> so, you really shouldn't have both an ASSERTION and an ENSURE_. Instead after
> the assertion, use a normal:
>  if (...)
>   return ...;

ok

> It's also a bit odd to use an assertion for something you *know* will happen.
> There should be a bug number open for fixing it. assertions should be limited
> to things you don't think can happen....

Just assertions shows dialog on windows and it makes easier to see when the problem happens.

Comment 14

[Martijn Wargers (dead)]

Attachment: testcase (uses enhanced privileges)

I get this crash with this testcase.

Comment 15

[alexander :surkov (:asurkov)]

Thanks Martijn for testcase. The testcase clearly shows the problem is we can't operate with accessible if it's pending to be invalidated. I think we should follow idea from bug 398021 to fix the bug.

Comment 16

[Daniel Holbert [:dholbert]]

BTW, I consistently hit this crash from simply:
 1. load http://tests.themasta.com/tinderboxpushlog/ (in a fresh profile)
 2. Wait 2-3 minutes
 3. *crash*
http://crash-stats.mozilla.com/report/index/ab51770f-1c04-4bdf-8a12-002762090522
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090522 Minefield/3.6a1pre

 --> adding 'regression' keyword because tinderboxpushlog didn't used to crash
 --> adding 'dogfood' keyword because it's a pretty important tool for developers

Comment 17

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #15)
> Thanks Martijn for testcase. The testcase clearly shows the problem is we can't
> operate with accessible if it's pending to be invalidated. I think we should
> follow idea from bug 398021 to fix the bug.

Tree freezing?

Comment 18

[alexander :surkov (:asurkov)]

(In reply to comment #17)
> (In reply to comment #15)
> > Thanks Martijn for testcase. The testcase clearly shows the problem is we can't
> > operate with accessible if it's pending to be invalidated. I think we should
> > follow idea from bug 398021 to fix the bug.
> 
> Tree freezing?

Exactly. In the mantime we should check parent on null and return ROLE_CELL as stub I think

Comment 19

[David Bolter [:davidb] (NeedInfo me for attention)]

This is crashing 3.6beta1 (~80th crasher)

(Maybe a null check?)

Comment 20

[alexander :surkov (:asurkov)]

(In reply to comment #19)
> This is crashing 3.6beta1 (~80th crasher)
> 
> (Maybe a null check?)

Let's land the patch and leave the bug open to fix real problem later.

Comment 21

[alexander :surkov (:asurkov)]

landed on 1.9.3 - http://hg.mozilla.org/mozilla-central/rev/259c8a2fbef4

Comment 23

[Marco Zehe (:MarcoZ)]

Comment on attachment 377401 [details] [diff] [review]
patch

See chofman's comments in bug 529233 which was since marked as a duplicate of this one.

Comment 24

[Marco Zehe (:MarcoZ)]

Requesting blocking since this is the #5 topcrasher on 1.9.2b2 and later.

Comment 25

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 377401 [details] [diff] [review]
patch

a192=beltzner; we need to figure out why this is such a high topcrash if it's in a11y code :(

Comment 26

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #25)
> (From update of attachment 377401 [details] [diff] [review])
> a192=beltzner; we need to figure out why this is such a high topcrash if it's
> in a11y code :(

Agreed and the a11y code is being exercised in unfortunate ways. Some evangelism is needed. Would a minidump help here?

Comment 27

[Marco Zehe (:MarcoZ)]

Pushed to 1.9.2 on Alexander's behalf in changeset: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/bfb57bd90f06