812717 - Crash when we don't pick up the phone.

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Gregor Wagner [:gwagner]]

On current tip we crash when we don't pick up the phone and the call ends.

(gdb) bt
#0  nsCycleCollectingAutoRefCnt::decr (this=0x4509aa74) at ../../../dist/include/nsISupportsImpl.h:183
#1  nsEventListenerManager::Release (this=0x4509aa74) at /Volumes/2mac/gaia/isrc/content/events/src/nsEventListenerManager.h:75
#2  0x405f396c in nsRefPtr<nsEventListenerManager>::assign_assuming_AddRef (this=0x4509aa70, rhs=0x4509aa60)
    at ../../../dist/include/nsAutoPtr.h:864
#3  nsRefPtr<nsEventListenerManager>::assign_with_AddRef (this=0x4509aa70, rhs=0x4509aa60) at ../../../dist/include/nsAutoPtr.h:848
#4  nsRefPtr<nsEventListenerManager>::operator= (this=0x4509aa70, rhs=0x4509aa60) at ../../../dist/include/nsAutoPtr.h:932
#5  0x4066c42e in nsDOMEventTargetHelper::GetListenerManager (this=0x4509aa60, aCreateIfNotFound=<value optimized out>)
    at /Volumes/2mac/gaia/isrc/content/events/src/nsDOMEventTargetHelper.cpp:298
#6  0x4066c660 in nsDOMEventTargetHelper::SetEventHandler (this=0x4509aa60, aType=0x4252f080, aCx=0x1, aValue=...)
    at /Volumes/2mac/gaia/isrc/content/events/src/nsDOMEventTargetHelper.h:99
#7  nsDOMEventTargetHelper::SetEventHandler (this=0x4509aa60, aType=0x4252f080, aCx=0x1, aValue=...)
    at /Volumes/2mac/gaia/isrc/content/events/src/nsDOMEventTargetHelper.cpp:252
#8  0x4077c06c in nsDOMDesktopNotification::SetOnclick (this=0x0, aCx=0x43d0a500, aValue=...)
    at /Volumes/2mac/gaia/isrc/dom/src/notification/nsDesktopNotification.cpp:62
#9  0x40b99a74 in NS_InvokeByIndex_P (that=0x4509aa7c, methodIndex=<value optimized out>, paramCount=<value optimized out>, 
    params=<value optimized out>) at /Volumes/2mac/gaia/isrc/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_arm.cpp:160

Comment 1

[Doug Turner (:dougt)]

where in gaia are they using desktop notifications?

Comment 2

[Gregor Wagner [:gwagner]]

(In reply to Doug Turner (:dougt) from comment #1)
> where in gaia are they using desktop notifications?

We come from: 
https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/oncall.js#L366

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Part of the way there

Things still crash with this patch, I'm going to file a new bug in a second with the final patch needed to make this work. We ended up finding this by code inspection. nsDOMDesktopNotification inherits nsISupports through both nsDOMEventTargetHelper and nsIDOMDesktopNotification. In addition, the actual reference count is contained in the nsDOMEventTargetHelper object in the multiple inheritance hierarchy. However, according to the QI implementation of nsDOMDesktopNotification, the canonical nsISupports pointer is through the nsIDOMDesktopNotification object. So, depending on which object we reference count through, we either change the real refcount object *or* we modify some random memory in the middle of the object. We should just let our inherited base class tell us where to find the nsISupports pointer so that everybody is on the same page.

While we were here, bent pointed out that empty traverse/unlink implementations are unnecessary and we can safely inherit them from our base class.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Without this patch, desktop notifications will crash meaning that things like the "missed call" notification won't show up.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 683342 [details] [diff] [review]
Part of the way there

https://hg.mozilla.org/integration/mozilla-inbound/rev/22f2faddc413

As a note: the crash wasn't actually caused by this. The cleanup was correct though.

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/22f2faddc413

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/09cb2493bbb8
https://hg.mozilla.org/releases/mozilla-beta/rev/4101138ae0c4