Closed
Bug 574522
Opened 14 years ago
Closed 14 years ago
crash [@nsAccEventQueue::WillRefresh]
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: davidb, Assigned: davidb)
Details
Attachments
(2 files)
1.19 KB,
patch
|
surkov
:
review+
|
Details | Diff | Splinter Review |
2.20 KB,
patch
|
surkov
:
review+
|
Details | Diff | Splinter Review |
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 1•14 years ago
|
||
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.
Assignee | ||
Comment 2•14 years ago
|
||
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?
Comment 3•14 years ago
|
||
In general I think we shouldn't check presshell, because no presshell means no accessible document, no accessible document means nsEventQueue is shutdown.
Assignee | ||
Comment 4•14 years ago
|
||
Can the presshell go away while we are processing the queue?
Comment 5•14 years ago
|
||
(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.
Assignee | ||
Comment 6•14 years ago
|
||
Attachment #454058 -
Flags: review?(surkov.alexander)
Assignee | ||
Updated•14 years ago
|
Attachment #453931 -
Attachment description: patch → part 1 (move check)
Comment 7•14 years ago
|
||
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+
Updated•14 years ago
|
Attachment #453931 -
Attachment is obsolete: true
Attachment #453931 -
Flags: review?(surkov.alexander)
Comment 8•14 years ago
|
||
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+
Assignee | ||
Comment 9•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/b5d427799427 http://hg.mozilla.org/mozilla-central/rev/1ad472e69168
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•