1199901 - shutdown crash in nsIPrincipal::GetAppStatus()

Status: VERIFIED FIXED

(Core :: DOM: Workers, defect)

Description

[Ekanan Ketunuti]

This bug was filed from the Socorro interface and is 
report bp-df7870a8-9fc8-4a39-9aea-fd2e02150829.
=============================================================

STR
1. Open Gmail & enable desktop notifications.
2. Close firefox when new mail notification show on the bottom right.


[Tracking Requested - why for this release]: crash, this will get annoying for users if close firefox on notification.

Comment 1

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: MozReview Request: Bug 1199901 - Clear mObserver when WorkerNotificationObserver is destroyed. r?wchen

Bug 1199901 - Clear mObserver when WorkerNotificationObserver is destroyed. r?wchen

Comment 2

[William Chen [:wchen]]

The patch doesn't look related to the crash. Is this the right patch or can you explain why this solves the problem?

Comment 3

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

(In reply to William Chen [:wchen] from comment #2)
> The patch doesn't look related to the crash. Is this the right patch or can
> you explain why this solves the problem?

Sorry, I completely forgot about patch 2. I'll have that up in some time. This patch is for another bug I noticed on Linux that is triggered by the same STR, so please review this too. This one is caused because we fail to clear the mObserver when the observer goes away.

Comment 4

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8655037 [details]
MozReview Request: Bug 1199901 - Clear mObserver when WorkerNotificationObserver is destroyed. r?wchen

Bug 1199901 - Clear mObserver when WorkerNotificationObserver is destroyed. r?wchen

Comment 5

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: MozReview Request: Bug 1199901 - GetOrigin() fails cleanly instead of asserting principal. r?wchen

Bug 1199901 - GetOrigin() fails cleanly instead of asserting principal. r?wchen

When we use the XUL based alerts and the main firefox window is closed, the XUL
window still keeps the process running, but as the window closes it calls
DisconnectFromOwner() on the Notification. Later, when the XUL alert closes
(either due to timeout or due to script) attempts to get the principal can
fail. This patch allows that to happen and will just skip deleting the
Notification from persistent storage.

Comment 6

[William Chen [:wchen]]

Comment on attachment 8655037 [details]
MozReview Request: Bug 1199901 - Clear mObserver when WorkerNotificationObserver is destroyed. r?wchen

https://reviewboard.mozilla.org/r/17787/#review16079

Trailing whitespace

Comment 7

[William Chen [:wchen]]

Comment on attachment 8655625 [details]
MozReview Request: Bug 1199901 - GetOrigin() fails cleanly instead of asserting principal. r?wchen

https://reviewboard.mozilla.org/r/17951/#review16085

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7f7c010fd294
https://hg.mozilla.org/integration/mozilla-inbound/rev/17a5de3b6eaa

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/50fd13696678

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7f7c010fd294
https://hg.mozilla.org/mozilla-central/rev/17a5de3b6eaa
https://hg.mozilla.org/mozilla-central/rev/50fd13696678

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Do you want to request uplift to aurora for these patches? Are older version (41 for example) significantly affected? Thanks!

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Forgot to needinfo you. Nikhil what do you think?

Comment 13

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #11)
> Do you want to request uplift to aurora for these patches? Are older version
> (41 for example) significantly affected? Thanks!

So the GetOrigin() failure was a main thread issue which means we may have already been shipping it for several releases (IIRC we have had notifications enabled for more than a year). Unless it is showing up in crash stats for older releases, I don't think it is worth uplifting.

Comment 14

[Ekanan Ketunuti]

I can't reproduce in 41 and I don't see that 41 is affected from crash-stats.
I guess it's regressed by bug 1114554 so this should be uplift to aurora, yes?

Comment 15

[Ekanan Ketunuti]

Verified fixed in Nightly 9/4/2015.

Comment 16

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8655625 [details]
MozReview Request: Bug 1199901 - GetOrigin() fails cleanly instead of asserting principal. r?wchen

Ekanan, you are right, Bug 1114554 caused this.

Approval Request Comment
[Feature/regressing bug #]: Bug 1114554
[User impact if declined]: Crash when closing the browser while a Notification is open.
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: No risk.
[String/UUID change made/needed]: None.

Comment 17

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8655625 [details]
MozReview Request: Bug 1199901 - GetOrigin() fails cleanly instead of asserting principal. r?wchen

Fix a crash, taking it.

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a04096cb27cc
https://hg.mozilla.org/releases/mozilla-aurora/rev/77863b95e7d6
https://hg.mozilla.org/releases/mozilla-aurora/rev/f37badf7f491

Comment 19

[Ekanan Ketunuti]

Verified fixed in aurora 20150911004112.