Closed Bug 500487 Opened 13 years ago Closed 13 years ago

Crash [@ nsDocAccessible::RemoveEventListeners() ]

Categories

(Core :: Disability Access APIs, defect)

x86
Windows Vista
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: davidb, Assigned: davidb)

Details

(Keywords: access)

Attachments

(1 file, 1 obsolete file)

We have a lot of checks for valid pointers in out shutdown code, but not for this. Shall I just null check?

If someone has any ideas why mDocument would sometimes be invalid during RemoveEventListeners please comment.
Oh course, STR would be awesome; but I'm doubtful we can get some.
Attached patch wip (obsolete) — Splinter Review
Attachment #385196 - Flags: review?(surkov.alexander)
Attachment #385196 - Flags: review?(marco.zehe)
Attachment #385196 - Flags: review?(marco.zehe) → review+
Comment on attachment 385196 [details] [diff] [review]
wip

Without STR it's hard to judge, but this seems like a reasonable approach.
Attachment #385196 - Flags: review?(surkov.alexander) → review+
Comment on attachment 385196 [details] [diff] [review]
wip

No ideas why.

>   // Remove document observer
>-  mDocument->RemoveObserver(this);
>+  if (mDocument)
>+    mDocument->RemoveObserver(this); //XXX todo: should we bail if !mDocument?

XXX is no needed I think, just warn if no document, uninitialize all we can.
Attached patch patchSplinter Review
Actually I think this is better, since we use mDocument in two places.
Attachment #385196 - Attachment is obsolete: true
Attachment #385452 - Flags: review?(surkov.alexander)
Attachment #385452 - Flags: review?(surkov.alexander) → review+
Comment on attachment 385452 [details] [diff] [review]
patch


>+  if (mDocument) {
>+    mDocument->RemoveObserver(this);

new line please

>+    nsCOMPtr<nsISupports> container = mDocument->GetContainer();
>+    nsCOMPtr<nsIDocShellTreeItem> docShellTreeItem(do_QueryInterface(container));
>+    NS_ENSURE_TRUE(docShellTreeItem, NS_ERROR_FAILURE);

warn and if (docShellTreeItem) otherwise you won't uninitialize another things.

r=me with these changes.
Thanks, pushed: http://hg.mozilla.org/mozilla-central/rev/4b4a99f918e4
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.