755402 - Desktop notification callback never gets called after small delay

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: General, defect, P1)

Description

[Etienne Segonzac (:etienne)]

Attachment: Test file

Even when attaching the callback function to a window kept open.

If we don't (really) hurry to send the desktop-notification-click/close mozContentEvent the callback never gets called.

So looks like a garbage collection issue.

Comment 1

[Dietrich Ayala (:dietrich)]

Blocking. Who's the right person to work on this?

Comment 2

[[:fabrice] Fabrice Desré]

I'm gonna take a look.

Comment 3

[[:fabrice] Fabrice Desré]

Ok, so I investigated and here are the results :

When you create the notification, we instanciate an nsDOMDesktopNotification object in c++. This object holds a reference to the callbacks.

If you don't keep the notification object in scope in JS, it is garbage collected, and we disconnect the callbacks here : http://mxr.mozilla.org/mozilla-central/source/dom/src/notification/nsDesktopNotification.cpp#125 (relevant code also at http://mxr.mozilla.org/mozilla-central/source/dom/src/notification/nsDesktopNotification.h#168)

It looks like this is what's happening there, so I'm not sure if the bug is in gaia or not.

Comment 4

[Etienne Segonzac (:etienne)]

I will do some more testing a keep this bug updated.

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Doug do you know if that an expected behavior or not?

Comment 6

[Doug Turner (:dougt)]

Major bug.

the nsIAlertsService holds a reference to the nsDesktopNotification via a nsIObserver.  This is so that when the user clicks on the dialog, we can get back to content and call the onclick handler.


* Is the gaia implementation of the nsIAlertService losing that reference?
* Does this work on Android?

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Doug Turner (:dougt) from comment #6)
> Major bug.
> 
> the nsIAlertsService holds a reference to the nsDesktopNotification via a
> nsIObserver.  This is so that when the user clicks on the dialog, we can get
> back to content and call the onclick handler.
> 
> 
> * Is the gaia implementation of the nsIAlertService losing that reference?

Looking at the code I would say not:
http://mxr.mozilla.org/mozilla-central/source/b2g/components/AlertsService.js
http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#406

> * Does this work on Android?

Dunno.

Comment 8

[Doug Turner (:dougt)]

is "alertfinished" being fired?

Comment 9

[JP Rosevear [:jpr]]

(In reply to Vivien Nicolas (:vingtetun) from comment #7)
> (In reply to Doug Turner (:dougt) from comment #6)
> > * Does this work on Android?
> 
> Dunno.

Notification works on release fennec native.

Comment 10

[[:fabrice] Fabrice Desré]

fwiw, I just tested on fennec native (current nightly) and the bug happens there also.

Comment 11

[Etienne Segonzac (:etienne)]

FYI we're currently working around this bug by keeping a reference on the mozNotification objects.

https://github.com/mozilla-b2g/gaia/blob/master/apps/dialer/js/notification_helper.js#L8

Comment 12

[Doug Turner (:dougt)]

I do not think that this is a blocker.

1) we have a work around for apps
2) for the browser, web content isn't really using desktop notifications
3) we are working on getting rid of this web api in favor of the w3c version (http://www.w3.org/TR/notifications/)

Comment 13

[Andrew Overholt [:overholt]]

Based on comment #12, not a blocker.

Comment 14

[Doug Turner (:dougt)]

william, can you see if this is still an issue after bug 782211 is fixed?