Last Comment Bug 666337 - "ASSERTION: null data pointer" in nsAccessibleWrap::FirePlatformEvent with <audio>
: "ASSERTION: null data pointer" in nsAccessibleWrap::FirePlatformEvent with <a...
Status: RESOLVED FIXED
: assertion, regression, testcase
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: 1.9.2 Branch
: x86_64 Linux
: -- normal (vote)
: mozilla7
Assigned To: Trevor Saunders (:tbsaunde)
:
: alexander :surkov
Mentors:
Depends on:
Blocks: 659018
  Show dependency treegraph
 
Reported: 2011-06-22 11:38 PDT by Jesse Ruderman
Modified: 2011-06-30 10:47 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (175 bytes, text/html)
2011-06-22 11:38 PDT, Jesse Ruderman
no flags Details
add null check (883 bytes, patch)
2011-06-24 22:01 PDT, Trevor Saunders (:tbsaunde)
surkov.alexander: review+
Details | Diff | Splinter Review

Description Jesse Ruderman 2011-06-22 11:38:23 PDT
Created attachment 541130 [details]
testcase

1. Enable accessibility, e.g. by pasting the following into the js console:

Components.classes["@mozilla.org/accessibilityService;1"]
      .getService(Components.interfaces.nsIAccessibleRetrieval);

2. Load the testcase.

3. Wait for the audio to finish? Reload the page?

Result:

###!!! ASSERTION: null data pointer: 'Not Reached', file /builds/slave/cen-lnx64-dbg/build/xpcom/string/src/nsTSubstring.cpp, line 615

nsACString_internal::Equals [xpcom/string/src/nsTSubstring.cpp:616]
nsAccessibleWrap::FirePlatformEvent [accessible/src/atk/nsAccessibleWrap.cpp:1085]
nsAccessibleWrap::HandleAccEvent [accessible/src/atk/nsAccessibleWrap.cpp:1033]
nsEventShell::FireEvent [accessible/src/base/nsEventShell.cpp:65]
nsDocAccessible::ProcessPendingEvent [accessible/src/base/nsDocAccessible.cpp:1720]
NotificationController::WillRefresh [accessible/src/base/NotificationController.cpp:306]
nsRefreshDriver::Notify [layout/base/nsRefreshDriver.cpp:328]
nsTimerImpl::Fire [xpcom/threads/nsTimerImpl.cpp:428]
nsTimerEvent::Run [xpcom/threads/nsTimerImpl.cpp:522]
nsThread::ProcessNextEvent [xpcom/threads/nsThread.cpp:618]
NS_ProcessNextEvent_P [obj-firefox/xpcom/build/nsThreadUtils.cpp:245]
mozilla::ipc::MessagePump::Run [ipc/glue/MessagePump.cpp:107]
MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:219]
MessageLoop::RunHandler [ipc/chromium/src/base/message_loop.cc:203]
MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:175]
nsBaseAppShell::Run [widget/src/xpwidgets/nsBaseAppShell.cpp:191]
nsAppStartup::Run [toolkit/components/startup/nsAppStartup.cpp:222]
XRE_main [toolkit/xre/nsAppRunner.cpp:3565]
do_main [browser/app/nsBrowserApp.cpp:198]
main [browser/app/nsBrowserApp.cpp:281]
libc.so.6 + 0x1eeff
firefox-bin + 0x1139

Regression from bug 659018?
Comment 1 Trevor Saunders (:tbsaunde) 2011-06-22 14:06:01 PDT
(In reply to comment #0)
> Created attachment 541130 [details]
> testcase
> 
> 1. Enable accessibility, e.g. by pasting the following into the js console:
> 
> Components.classes["@mozilla.org/accessibilityService;1"]
>       .getService(Components.interfaces.nsIAccessibleRetrieval);

yeah, we know you can save yourself the time :)

> ###!!! ASSERTION: null data pointer: 'Not Reached', file
> /builds/slave/cen-lnx64-dbg/build/xpcom/string/src/nsTSubstring.cpp, line 615
> 
> nsACString_internal::Equals [xpcom/string/src/nsTSubstring.cpp:616]
> nsAccessibleWrap::FirePlatformEvent
> [accessible/src/atk/nsAccessibleWrap.cpp:1085]
> nsAccessibleWrap::HandleAccEvent
> [accessible/src/atk/nsAccessibleWrap.cpp:1033]

ok, so atkobj->name can reasonably be null.  So the fix is trivial just atkObj->name && utf8Name.Equals(atkObj->name).  However   I don't understand why we assert that the arg to nsTSubstring::Equal() isn't Null.  dbaron asked for that assertion in bug  236003.  Off hand I don't understand why comparing to a null string is unreasonable, but I'm willing to accept that it is given a small amount of reason :-)

> Regression from bug 659018?

well yes, in some sense of the word regression ;)
Comment 2 alexander :surkov 2011-06-22 23:46:23 PDT
trevor, trivial patch?
Comment 3 Trevor Saunders (:tbsaunde) 2011-06-23 00:02:07 PDT
(In reply to comment #2)
> trevor, trivial patch?

yes, I basically mentioned what it was in my comment, but see the other part too.

> atkObj->name && utf8Name.Equals(atkObj->name).  However   I don't understand
> why we assert that the arg to nsTSubstring::Equal() isn't Null.  dbaron
> asked for that assertion in bug  236003.  Off hand I don't understand why
> comparing to a null string is unreasonable, but I'm willing to accept that
> it is given a small amount of reason :-)
Comment 4 alexander :surkov 2011-06-23 00:15:54 PDT
maybe because they want to deal with stings making null pointer out of law. Anyway it's xpcom guys decision, until we have real objections we should follow it.
Comment 5 Trevor Saunders (:tbsaunde) 2011-06-24 22:01:33 PDT
Created attachment 541893 [details] [diff] [review]
add null check

this makes me a bit sad, but I gues if we need to fix this in code we control we need this patch.
Comment 6 Trevor Saunders (:tbsaunde) 2011-06-30 10:34:50 PDT
landed http://hg.mozilla.org/mozilla-central/rev/e5a39bc7d5d6

Note You need to log in before you can comment on or make changes to this bug.