Comment on attachment 625571 [details] [diff] [review] patch >+ PRUint64 state = nsDocAccessibleWrap::NativeState(); >+ if (state & states::DEFUNCT) >+ return state; what's it for?

(In reply to Trevor Saunders (:tbsaunde) from comment #1) > Comment on attachment 625571 [details] [diff] [review] > patch > > >+ PRUint64 state = nsDocAccessibleWrap::NativeState(); > >+ if (state & states::DEFUNCT) > >+ return state; > > what's it for? basically to not have a check for mDocument, in general I think we are allowed to not spend a time for states computation if accessible is defunct.

(In reply to alexander :surkov from comment #2) > (In reply to Trevor Saunders (:tbsaunde) from comment #1) > > Comment on attachment 625571 [details] [diff] [review] > > patch > > > > >+ PRUint64 state = nsDocAccessibleWrap::NativeState(); > > >+ if (state & states::DEFUNCT) > > >+ return state; > > > > what's it for? > > basically to not have a check for mDocument, in general I think we are > allowed to not spend a time for states computation if accessible is defunct. yeah, was just wondering why it was possible we were dealing with a defunct accessible since I forgot about the atk crash.

(In reply to Trevor Saunders (:tbsaunde) from comment #3) > (In reply to alexander :surkov from comment #2) > > (In reply to Trevor Saunders (:tbsaunde) from comment #1) > > > Comment on attachment 625571 [details] [diff] [review] > > > patch > > > > > > >+ PRUint64 state = nsDocAccessibleWrap::NativeState(); > > > >+ if (state & states::DEFUNCT) > > > >+ return state; > > > > > > what's it for? > > > > basically to not have a check for mDocument, in general I think we are > > allowed to not spend a time for states computation if accessible is defunct. > > yeah, was just wondering why it was possible we were dealing with a defunct > accessible since I forgot about the atk crash. It might be cleaner to only return states::DEFUNCT here.

Which probably happens nearly all the time?

(In reply to David Bolter [:davidb] from comment #5) > Which probably happens nearly all the time? what? being called on a defunct accessible? hopefully not

(In reply to alexander :surkov from comment #2) > (In reply to Trevor Saunders (:tbsaunde) from comment #1) > > Comment on attachment 625571 [details] [diff] [review] > > patch > > > > >+ PRUint64 state = nsDocAccessibleWrap::NativeState(); > > >+ if (state & states::DEFUNCT) > > >+ return state; > > > > what's it for? > > basically to not have a check for mDocument, in general I think we are > allowed to not spend a time for states computation if accessible is defunct. actually, I'm not sure how it works either, since we only set the DEFUNCT state in nsAccessible::NativeState()

(In reply to Trevor Saunders (:tbsaunde) from comment #6) > (In reply to David Bolter [:davidb] from comment #5) > > Which probably happens nearly all the time? > > what? being called on a defunct accessible? No. I'm wondering how often is returning "state" equivalent to returning states::DEFUNCT (how often are other bits flipped).