Closed Bug 574522 Opened 14 years ago Closed 14 years ago

crash [@nsAccEventQueue::WillRefresh]

Categories

(Core :: Disability Access APIs, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: davidb, Assigned: davidb)

Details

Attachments

(2 files)

I just ran into this while debugging an unrelated issue.

(gdb) bt
#0  0x03c56d1f in nsCOMPtr<nsIWeakReference>::get (this=0x10) at nsCOMPtr.h:800
#1  0x03c56d35 in nsCOMPtr<nsIWeakReference>::operator nsIWeakReference* (this=0x10) at nsCOMPtr.h:813
#2  0x04e970cd in nsAccessNode::GetPresShell (this=0x0) at /Users/dtb/mozworkspace/src/accessible/src/base/nsAccessNode.cpp:264
#3  0x04ecd6c8 in nsAccEventQueue::WillRefresh (this=0x21064060, aTime={mValue = 6114205932}) at /Users/dtb/mozworkspace/src/accessible/src/base/nsEventShell.cpp:222
#4  0x03efbc3b in nsRefreshDriver::Notify (this=0x1aa0ede0) at /Users/dtb/mozworkspace/src/layout/base/nsRefreshDriver.cpp:209
#5  0x051018d9 in nsTimerImpl::Fire (this=0x1aaba230) at /Users/dtb/mozworkspace/src/xpcom/threads/nsTimerImpl.cpp:430
#6  0x05101af6 in nsTimerEvent::Run (this=0x2103fd40) at /Users/dtb/mozworkspace/src/xpcom/threads/nsTimerImpl.cpp:519
#7  0x050fa854 in nsThread::ProcessNextEvent (this=0xb2bd40, mayWait=0, result=0xbfffd5d4) at /Users/dtb/mozworkspace/src/xpcom/threads/nsThread.cpp:547
#8  0x0508924a in NS_ProcessPendingEvents_P (thread=0xb2bd40, timeout=20) at nsThreadUtils.cpp:200
#9  0x04e75455 in nsBaseAppShell::NativeEventCallback (this=0x1882a4c0) at /Users/dtb/mozworkspace/src/widget/src/xpwidgets/nsBaseAppShell.cpp:126
#10 0x04e25a6d in nsAppShell::ProcessGeckoEvents (aInfo=0x1882a4c0) at ../../../../widget/src/cocoa/nsAppShell.mm:394
#11 0x93d0b40f in CFRunLoopRunSpecific ()
#12 0x93d0baa8 in CFRunLoopRunInMode ()
#13 0x951cd2ac in RunCurrentEventLoopInMode ()
#14 0x951ccffe in ReceiveNextEventCommon ()
#15 0x951ccf39 in BlockUntilNextEventMatchingListInMode ()
#16 0x905706d5 in _DPSNextEvent ()
#17 0x9056ff88 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#18 0x90568f9f in -[NSApplication run] ()
#19 0x04e2481f in nsAppShell::Run (this=0x1882a4c0) at ../../../../widget/src/cocoa/nsAppShell.mm:747
#20 0x04b9f0be in nsAppStartup::Run (this=0x18848490) at /Users/dtb/mozworkspace/src/toolkit/components/startup/src/nsAppStartup.cpp:192
#21 0x03c13d9f in XRE_main (argc=3, argv=0xbfffeb20, aAppData=0xb0e6f0) at ../../../toolkit/xre/nsAppRunner.cpp:3623
#22 0x00002822 in main (argc=3, argv=0xbfffeb20) at ../../../browser/app/nsBrowserApp.cpp:158
Attachment #453931 - Flags: review?(surkov.alexander)
Comment on attachment 453931 [details] [diff] [review]
part 1 (move check)


>   for (PRUint32 index = 0; index < length; index ++) {
> 
>+    nsAccEvent *accEvent = events[index];
>+    if (accEvent->mEventRule != nsAccEvent::eDoNotEmit)
>+      mDocument->ProcessPendingEvent(accEvent);
>+
>     // No presshell means the document was shut down during event handling
>     // by AT.
>     if (!mDocument || !mDocument->HasWeakShell())
>+      return;
>   }

this should be ok, but

if !mDocument then it's fine (document and queue was shutdown)
if !mDocument->HasWeakShell() then it should mean 1) presshell gone away or 2) document gone away and 3) event queue isn't shutdown (because mDocument is not null) and that's a problem since it won't be ever shutdown. 

If document goes away (either Shutdown or Object desctruction) then we guaranteed event queue is shutdown, and the document goes away if presshell goes away. So I don't think this is a real problem.
The problem case I encountered seemed to be that the document was shutdown after the last event was processed. It seemed to happen when putting focus back to the FF window (where we usually queue and fire one event). You chopped it out above in your comment but the check used to be before the ProcessPendingEvent call inside the loop, which means on the final ProcessPendingEvent call we didn't check it. It seems to me that we should have that final check right?
In general I think we shouldn't check presshell, because no presshell means no accessible document, no accessible document means nsEventQueue is shutdown.
Can the presshell go away while we are processing the queue?
(In reply to comment #4)
> Can the presshell go away while we are processing the queue?

It could be I guess, but if it does then it goes away with document accessible. Therefore I'm saying mDocument check should be enough now.
Attachment #454058 - Flags: review?(surkov.alexander)
Attachment #453931 - Attachment description: patch → part 1 (move check)
Comment on attachment 454058 [details] [diff] [review]
part 2 (remove shell check)


>-    if (!mDocument || !mDocument->HasWeakShell())
>+    // No document means it was shut down during event handling by AT

nit: dot please in the end
Attachment #454058 - Flags: review?(surkov.alexander) → review+
Attachment #453931 - Attachment is obsolete: true
Attachment #453931 - Flags: review?(surkov.alexander)
Comment on attachment 453931 [details] [diff] [review]
part 1 (move check)

r=me with part2
Attachment #453931 - Attachment is obsolete: false
Attachment #453931 - Flags: review+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: