Closed Bug 691443 Opened 14 years ago Closed 14 years ago

crash [@ nsAccessibleWrap::GetHWNDFor]

Categories

(Core :: Disability Access APIs, defect)

Product:
Core
Component:
Disability Access APIs
Platform:
x86
Windows 7
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla10

People

(Reporter: nhirata, Assigned: surkov)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

patch
2.07 KB, patch
davidb
: review+
Details | Diff | Splinter Review
This bug was filed from the Socorro interface and is report bp-972f2e40-c739-40d6-8e87-926442111001 . ============================================================= 0 xul.dll nsAccessibleWrap::GetHWNDFor accessible/src/msaa/nsAccessibleWrap.cpp:1621 1 xul.dll nsAccessibleWrap::FirePlatformEvent accessible/src/msaa/nsAccessibleWrap.cpp:1572 2 xul.dll nsAccessibleWrap::HandleAccEvent accessible/src/msaa/nsAccessibleWrap.cpp:1528 3 xul.dll nsHyperTextAccessibleWrap::HandleAccEvent accessible/src/msaa/nsHyperTextAccessibleWrap.cpp:77 4 xul.dll nsEventShell::FireEvent accessible/src/base/nsEventShell.cpp:63 5 xul.dll nsDocAccessible::ProcessLoad accessible/src/base/nsDocAccessible.cpp:1551 6 xul.dll NotificationController::WillRefresh accessible/src/base/NotificationController.cpp:286 7 xul.dll nsRefreshDriver::Notify layout/base/nsRefreshDriver.cpp:326 8 xul.dll nsTimerImpl::Fire xpcom/threads/nsTimerImpl.cpp:427 9 xul.dll nsTimerEvent::Run xpcom/threads/nsTimerImpl.cpp:520 10 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:631 11 xul.dll NS_ProcessNextEvent_P obj-firefox/xpcom/build/nsThreadUtils.cpp:245 12 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:134 13 xul.dll MessageLoop::RunInternal ipc/chromium/src/base/message_loop.cc:208 14 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:201 15 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:175 16 xul.dll nsBaseAppShell::Run widget/src/xpwidgets/nsBaseAppShell.cpp:189 17 xul.dll nsAppShell::Run widget/src/windows/nsAppShell.cpp:261 18 xul.dll XRE_RunAppShell toolkit/xre/nsEmbedFunctions.cpp:677 19 xul.dll mozilla::ipc::MessagePumpForChildProcess::Run ipc/glue/MessagePump.cpp:215 20 xul.dll MessageLoop::RunInternal ipc/chromium/src/base/message_loop.cc:208 21 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:201 22 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:175 23 xul.dll XRE_InitChildProcess toolkit/xre/nsEmbedFunctions.cpp:516 24 plugin-container.exe NS_internal_main ipc/app/MozillaRuntimeMain.cpp:81 25 plugin-container.exe wmain toolkit/xre/nsWindowsWMain.cpp:107 26 plugin-container.exe __tmainCRTStartup crtexe.c:594 27 kernel32.dll BaseThreadInitThunk 28 ntdll.dll __RtlUserThreadStart 29 ntdll.dll _RtlUserThreadStart More crashes : https://crash-stats.mozilla.com/report/list?range_value=7&range_unit=days&date=2011-10-03%2009%3A00%3A00&signature=nsAccessibleWrap%3A%3AGetHWNDFor%28nsAccessible*%29&version=Fennec%3A10.0a1 #5 on top 10 crashes in Fennec Nightly. Windows Only.
Crash Signature: [@ nsAccessibleWrap::GetHWNDFor]
Robert, when nsIFrame::GetNearestWidget can return null? Presumably we get nearest widget on root frame of tab document (nsIPresShell::GetRootFrame).
Can we get regression range please?
(In reply to alexander surkov from comment #1) > Robert, when nsIFrame::GetNearestWidget can return null? Presumably we get > nearest widget on root frame of tab document (nsIPresShell::GetRootFrame). That can sometimes happen when the presentation has been detached from the window while something is being torn down. It can also happen during printing.
It does not occur on Win XP; It does occur on Win 7 Found a repro steps : 1. turn on an Accessibility application such as Narrator on Windows 7 2. go to http://people.mozilla.com/~nhirata/html_tp/formsninput.html 3. type a couple of lines of nonsense in the text area 4. hit the submit button Expected: no content crash Actual: content crash Note: 1. after a crash happens the Accessibility application becomes unstable as well. Regression range : 9/28 build works, 9/29 build crashes occasionally, 9/30 build crashes reliably.
Keywords: regressionwindow-wanted
I cannot reproduce this with NVDA and the current nightly of 2011-10-04. When I hit submit after typing in a few lines in the textarea, I get a "Redirecting back to forms", and the doorhanger saying that Firefox prevented this page from automatically redirecting... I then hit "allow", and am returned to the page. No crash.
Addition:L This is on Win7 x64 with the 32 bit version of Nightly.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #3) > (In reply to alexander surkov from comment #1) > > Robert, when nsIFrame::GetNearestWidget can return null? Presumably we get > > nearest widget on root frame of tab document (nsIPresShell::GetRootFrame). > > That can sometimes happen when the presentation has been detached from the > window while something is being torn down. It can also happen during > printing. We fire accessibility events when refresh driver notifies us (mPresShell->AddRefreshObserver(this, Flush_Display) and this point we assume that is nsIFrame::GetNearestWidget is never null, otherwise we're not able to fire events. At what time we are ok to request GetNearestWidget and don't get a null? Is there any hint? Because otherwise we are basically faced to either crash or missed events.
This is on the nightly of Fennec windows desktop; which probably should have a lower priority. http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/latest-mozilla-central-win32/
(In reply to alexander surkov from comment #7) > We fire accessibility events when refresh driver notifies us > (mPresShell->AddRefreshObserver(this, Flush_Display) and this point we > assume that is nsIFrame::GetNearestWidget is never null, otherwise we're not > able to fire events. Hmm. > At what time we are ok to request GetNearestWidget and don't get a null? Is > there any hint? Because otherwise we are basically faced to either crash or > missed events. I think you can probably just miss the event. Hmm, this is Fennec? Maybe you can get null GetNearestWidget more often there, I don't know exactly how widget setup works there.
Fennec Windows Desktop to be specific. It allows for windows users that don't have android devices to test on Fennec, however it's way low percentage of the population that uses this method for testing Fennec. I do know Martijn sometimes does use this as a testing platform. I'm unsure if he does use any of the accessibility though. Looping in mw22.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #9) > > At what time we are ok to request GetNearestWidget and don't get a null? Is > > there any hint? Because otherwise we are basically faced to either crash or > > missed events. > > I think you can probably just miss the event. That's not an option because it must break screen reader user experience. We could do not emit events while GetNearestWidget is null collecting them. But this should be a problem if GetNearestWidget stays null for a long time. > Hmm, this is Fennec? Maybe you can get null GetNearestWidget more often > there, I don't know exactly how widget setup works there. Can we cc somebody who knows please?
(In reply to alexander surkov from comment #11) > (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #9) > > > > At what time we are ok to request GetNearestWidget and don't get a null? Is > > > there any hint? Because otherwise we are basically faced to either crash or > > > missed events. > > > > I think you can probably just miss the event. > > That's not an option because it must break screen reader user experience. We > could do not emit events while GetNearestWidget is null collecting them. But > this should be a problem if GetNearestWidget stays null for a long time. If it's null, it's unlikely to ever become non-null. The document is either going away or it's never going to have a widget, which probably means it's being printed.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #12) > If it's null, it's unlikely to ever become non-null. The document is either > going away or it's never going to have a widget, which probably means it's > being printed. Is it print preview documents that never have a widget? Anyway this crash is not about printing. It's likely we crash for tab document. Maybe it is temporary document that was loaded and about to destroy. Don't you know whether presshell lives longer than widget associated with document?
(In reply to Naoki Hirata :nhirata from comment #8) > This is on the nightly of Fennec windows desktop; which probably should have > a lower priority. > > http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/latest-mozilla-central- > win32/ it doesn't crash for me (with NVDA running) with steps to reproduce from comment #4.
(In reply to alexander surkov from comment #13) > Anyway this crash is not about printing. It's likely we crash for tab > document. Maybe it is temporary document that was loaded and about to > destroy. Don't you know whether presshell lives longer than widget > associated with document? On Fennec, I don't know. If this is only on a configuration hardly anyone uses, we shouldn't worry about it. I'd just null-check, drop the event, and maybe emit a warning in debug builds.
what about desktop? We could shutdown document accessible earlier and don't do excess processing.
Yes, i think you could do that, although it probably doesn't matter.
Attached patch patch — — Splinter Review
null check, no warnings since the code is popups support and is not a case of the crash.
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #565160 - Flags: review?(bolterbugz)
(In reply to alexander surkov from comment #18) > Created attachment 565160 [details] [diff] [review] [diff] [details] [review] > patch > > null check, no warnings since the code is popups support and is not a case > of the crash. anyway we'll warn when empty GetHWND returns.
(In reply to Naoki Hirata :nhirata from comment #10) > I do know Martijn sometimes does use this as a testing platform. I'm unsure > if he does use any of the accessibility though. Looping in mw22. I'm not doing any (or hardly) fuzz testing anymore.
Comment on attachment 565160 [details] [diff] [review] patch ok r=me
Attachment #565160 - Flags: review?(bolterbugz) → review+
Would you like me to test on a debug build before you land the patch? I'm willing to do that.
(In reply to Naoki Hirata :nhirata from comment #22) > Would you like me to test on a debug build before you land the patch? > I'm willing to do that. please, just in case try build is http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/surkov.alexander@gmail.com-ee16bfda13f9/
Crash Signature: [@ nsAccessibleWrap::GetHWNDFor] → [@ nsAccessibleWrap::GetHWNDFor] [@ nsAccessibleWrap::GetHWNDFor(nsAccessible*) ]
I couldn't reproduce the issue with the try build. Just looking at the patch, is the patch only doing a null check on widget? I'd like to understand so that I might be able to create fixes in the future.
(In reply to Naoki Hirata :nhirata from comment #24) > I couldn't reproduce the issue with the try build. > > Just looking at the patch, is the patch only doing a null check on widget? yes, it's null check > I'd like to understand so that I might be able to create fixes in the future. it might be more complicated than this case but thanks.
https://hg.mozilla.org/mozilla-central/rev/f4fa6df431be
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: