649409 - Firefox 4.2a1pre Crash Report [@ nsLinkableAccessible::NativeState() ]

Status: RESOLVED DUPLICATE of bug 653584

(Firefox :: Disability Access, defect)

Description

[Sheila Mooney]

New crash on the trunk appeared on April 11, 2011

Signature	nsLinkableAccessible::NativeState()
UUID	83f9ec81-5dec-42f1-803c-f8d392110412
Time 	2011-04-12 08:47:45.206690
Uptime	74
Last Crash	77 seconds before submission
Install Age	75585 seconds (21.0 hours) since version was first installed.
Product	Firefox
Version	4.2a1pre
Build ID	20110411030525
Branch	2.2
OS	Windows NT
OS Version	6.0.6002 Service Pack 2
CPU	x86
CPU Info	GenuineIntel family 6 model 15 stepping 13
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x0
User Comments	
App Notes 	AdapterVendorID: 10de, AdapterDeviceID: 0427, AdapterDriverVersion: 8.16.11.8766
D3D10 Layers? D3D10 Layers-
D3D9 Layers? D3D9 Layers-
Processor Notes 	
EMCheckCompatibility	False

Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsLinkableAccessible::NativeState 	accessible/src/base/nsBaseWidgetAccessible.cpp:124
1 	xul.dll 	nsHTMLTextAccessible::NativeState 	accessible/src/html/nsHTMLTextAccessible.cpp:91
2 	xul.dll 	nsAccessible::State 	accessible/src/base/nsAccessible.cpp:1530
3 	xul.dll 	nsAccessibleWrap::get_accState 	accessible/src/msaa/nsAccessibleWrap.cpp:448

Crash Reports:
https://crash-stats.mozilla.com/report/list?product=Firefox&branch=2.2&platform=windows&build_id=20110411030525&query_search=signature&query_type=exact&query=&date=04%2F12%2F2011%2010%3A45%3A53&range_value=30&range_unit=days&hang_type=crash&process_type=browser&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=&signature=nsLinkableAccessible%3A%3ANativeState%28%29

Comment 1

[Marco Zehe (:MarcoZ)]

I suspect this is a dupe, or different incarnation, of bug 648988, which was fixed for the April 12 nightly builds. Watch out for it, but I believe this is fixed already.

Comment 2

[alexander :surkov (:asurkov)]

maybe, but it can crash due to different reason: no action accessible (for generic text node accessible). Prior bug 634218 we had null check.

Trevor, please fix it?

Comment 3

[Trevor Saunders (:tbsaunde)]

Attachment: Bug 649409 - crash in [@ nsLinkableAccessible::NativeState() ]

Comment 4

[alexander :surkov (:asurkov)]

(In reply to comment #2)
> maybe, but it can crash due to different reason: no action accessible (for
> generic text node accessible). Prior bug 634218 we had null check.

This shouldn't happen because we are under mIsLink condition. So if that happens then something goes really wrong. I tend to agree with Marco comment #1.

Comment 5

[Trevor Saunders (:tbsaunde)]

I'm not 100% sure GetActionAccessible can return NULL for a non defunct accessible but it seems pretty possible reading GetActionAccessible.

Comment 6

[Marco Zehe (:MarcoZ)]

Comment on attachment 525609 [details] [diff] [review]
Bug 649409 - crash in [@ nsLinkableAccessible::NativeState() ]

r=me. I like the approach of returning early if we determine we don#t have anything else to do here.

Comment 7

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

Comment on attachment 525609 [details] [diff] [review]
Bug 649409 - crash in [@ nsLinkableAccessible::NativeState() ]

This seems ok to me, but I'm not sure if !mIsLink indicates the object simply hasn't been bound to its parent yet (in our tree) or if it indicates this is a linkable accessible that doesn't actually have a link in its ancestry chain. I'd like to understand the cause of this bug; let's get Alexander's thoughts as well.

>+    state |= actionAcc->State() & states::TRAVERSED;

This was a nice change.

Comment 8

[Trevor Saunders (:tbsaunde)]

(In reply to comment #7)
> Comment on attachment 525609 [details] [diff] [review]
> Bug 649409 - crash in [@ nsLinkableAccessible::NativeState() ]
> 
> This seems ok to me, but I'm not sure if !mIsLink indicates the object simply
> hasn't been bound to its parent yet (in our tree) or if it indicates this is a
> linkable accessible that doesn't actually have a link in its ancestry chain.
> I'd like to understand the cause of this bug; let's get Alexander's thoughts as
> well.

sure,  note that  I didn't change what we did if mIsLink is false.  I'm not sure what it being false means, but I do know we crashed when it was true.  The crash has to do with actionAcc being null, which may be related to mIsLink though.

Comment 9

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

Comment on attachment 525609 [details] [diff] [review]
Bug 649409 - crash in [@ nsLinkableAccessible::NativeState() ]

Actually I'm not sure what state might hold at this point. Seems better to return 0 I think. Note intrinsic states don't map directly to our a11y states...

> PRUint64
> nsLinkableAccessible::NativeState()
> {

>+  PRUint64 state = nsAccessibleWrap::NativeState();
>+  if (!mIsLink)
>+    return state;
>+

Comment 10

[Trevor Saunders (:tbsaunde)]

(In reply to comment #9)
> Comment on attachment 525609 [details] [diff] [review]
> Bug 649409 - crash in [@ nsLinkableAccessible::NativeState() ]
> 
> Actually I'm not sure what state might hold at this point. Seems better to
> return 0 I think. Note intrinsic states don't map directly to our a11y
> states...

hmm, I'm not sure where your refering to now ;)

Comment 11

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

Actually I took a closer look at the base NativeState method and we don't just pass instrinsic states through of course, so don't worry about comment 9. Thanks.

Comment 12

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

(In reply to comment #8)
> (In reply to comment #7)
> > Comment on attachment 525609 [details] [diff] [review]
> > Bug 649409 - crash in [@ nsLinkableAccessible::NativeState() ]
> > 
> > This seems ok to me, but I'm not sure if !mIsLink indicates the object simply
> > hasn't been bound to its parent yet (in our tree) or if it indicates this is a
> > linkable accessible that doesn't actually have a link in its ancestry chain.
> > I'd like to understand the cause of this bug; let's get Alexander's thoughts as
> > well.
> 
> sure,  note that  I didn't change what we did if mIsLink is false.  I'm not
> sure what it being false means, but I do know we crashed when it was true.  The
> crash has to do with actionAcc being null, which may be related to mIsLink
> though.

Weird. It is worth figuring out if you have time.

Comment 13

[alexander :surkov (:asurkov)]

(In reply to comment #12)

> > sure,  note that  I didn't change what we did if mIsLink is false.  I'm not
> > sure what it being false means, but I do know we crashed when it was true.  The
> > crash has to do with actionAcc being null, which may be related to mIsLink
> > though.
> 
> Weird. It is worth figuring out if you have time.

Nothing weird here. If node is defunct then this scenario is possible. I don't see other scenarios. If you can point me it then it's great and I thumb up to take a patch.

Comment 14

[alexander :surkov (:asurkov)]

Comment on attachment 525609 [details] [diff] [review]
Bug 649409 - crash in [@ nsLinkableAccessible::NativeState() ]

r- to prevent the landing of this patch just in case. I don't see a scenario how it may be null. So if you are pretty sure we need to have null check here (in the case we've got completely broken tree) then please add an assertion since the !actionAcc condition is never true.

Comment 15

[Trevor Saunders (:tbsaunde)]

(In reply to comment #14)
> Comment on attachment 525609 [details] [diff] [review]
> Bug 649409 - crash in [@ nsLinkableAccessible::NativeState() ]
> 
> r- to prevent the landing of this patch just in case. I don't see a scenario
> how it may be null. So if you are pretty sure we need to have null check here
> (in the case we've got completely broken tree) then please add an assertion
> since the !actionAcc condition is never true.

Looking deeper I don't think it can be null if the object isn't defunct

Comment 16

[Marco Zehe (:MarcoZ)]

Are we going to make any progress on this bug for Firefox 6?

Comment 17

[Trevor Saunders (:tbsaunde)]

(In reply to comment #16)
> Are we going to make any progress on this bug for Firefox 6?

I suspect this was infact a different form of bug  648988.  bug 653584 may be related, but I doubt it, I think that one is linux specific, but maybe not.  Anyway I don't think we've seen any crashes like this in a pretty good while so I think we can consider this  fixed  unless I've been missing some crashes :)

Comment 18

[alexander :surkov (:asurkov)]

Trevor, if you mark the bug fixed then you should point what is a fix (patch or bug), otherwise worksforme.

Comment 19

[Trevor Saunders (:tbsaunde)]

(In reply to comment #18)
> Trevor, if you mark the bug fixed then you should point what is a fix (patch
> or bug), otherwise worksforme.

sure, sorry about that

Comment 20

[James Teh [:Jamie]]

Should this have been fixed in the 20110517 build? I saw it twice last night:
bp-7d2ee591-eacb-4d64-984f-006af2110518
bp-17852a35-244e-4185-87e8-661702110518
Unfortunately, I can't find the URL that caused the crash. Damn.

Comment 21

[Marco Zehe (:MarcoZ)]

Reopening based on comment 20.

Comment 22

[alexander :surkov (:asurkov)]

(In reply to comment #21)
> Reopening based on comment 20.

for me it sounds as a dupe of bug 653584

Comment 23

[alexander :surkov (:asurkov)]

(In reply to comment #22)
> (In reply to comment #21)
> > Reopening based on comment 20.
> 
> for me it sounds as a dupe of bug 653584

yep, must a dupe, bug 653584 was landed 21 May, last crash was 21 May
https://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=exact&query=nsLinkableAccessible%3A%3ANativeState%28%29&reason_type=contains&date=05%2F24%2F2011%2002%3A27%3A54&range_value=1&range_unit=weeks&hang_type=any&process_type=any&do_query=1&signature=nsLinkableAccessible%3A%3ANativeState%28%29