Closed
Bug 1333082
Opened 8 years ago
Closed 8 years ago
Crash in mozilla::dom::workers::ServiceWorkerRegistrationInfo::TransitionInstallingToWaiting
Categories
(Core :: DOM: Service Workers, defect)
Tracking
()
RESOLVED
FIXED
mozilla54
People
(Reporter: bkelly, Assigned: bkelly)
References
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
|
1.60 KB,
patch
|
asuth
:
review+
jcristau
:
approval-mozilla-aurora+
jcristau
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is
report bp-e2eef747-f0dd-48c1-b2f8-156282170123.
=============================================================
It appears ServiceWorkerUpdateJob::ContinueAfterInstallEvent() is getting called when mRegistration is nullptr. I don't see how this could be happening, but lets try to protect against it and assert it doesn't happen.
| Assignee | ||
Comment 1•8 years ago
|
||
Somehow we are triggering a FailUpdate() call and then also still calling ContinueAfterInstallEvent(). This makes us try to continue on a nullptr mRegistration. I can't find which path triggers this, but lets try not to crash here and add an assertion.
Attachment #8829466 -
Flags: review?(bugmail)
Updated•8 years ago
|
Attachment #8829466 -
Flags: review?(bugmail) → review+
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/323cb4def9fd
Handle nullptr mRegistration in ServiceWorkerUpdateJob::ContinueAfterInstallEvent(). r=asuth
| Assignee | ||
Updated•8 years ago
|
status-firefox50:
--- → wontfix
status-firefox51:
--- → wontfix
status-firefox52:
--- → affected
status-firefox53:
--- → affected
status-firefox-esr45:
--- → wontfix
| Assignee | ||
Comment 3•8 years ago
|
||
Comment on attachment 8829466 [details] [diff] [review]
Handle nullptr mRegistration in ServiceWorkerUpdateJob::ContinueAfterInstallEvent(). r=asuth
Approval Request Comment
[Feature/Bug causing the regression]: Service workers
[User impact if declined]: Low frequency crashes. This patch will mitigate some of the crashes and help narrow down the problem.
[Is this code covered by automated tests?]: We have extensive tests, but they don't trigger this condition.
[Has the fix been verified in Nightly?]: We do not have steps to reproduce, so we can't manually verify.
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Minimal risk
[Why is the change risky/not risky?]: It only adds a nullptr check and a diagnostic assertion. The chance of a regression is very low.
[String changes made/needed]:
Attachment #8829466 -
Flags: approval-mozilla-beta?
Attachment #8829466 -
Flags: approval-mozilla-aurora?
Comment 4•8 years ago
|
||
| bugherder | ||
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 5•8 years ago
|
||
Comment on attachment 8829466 [details] [diff] [review]
Handle nullptr mRegistration in ServiceWorkerUpdateJob::ContinueAfterInstallEvent(). r=asuth
service worker crash fix, aurora53+, beta52+
Attachment #8829466 -
Flags: approval-mozilla-beta?
Attachment #8829466 -
Flags: approval-mozilla-beta+
Attachment #8829466 -
Flags: approval-mozilla-aurora?
Attachment #8829466 -
Flags: approval-mozilla-aurora+
| Assignee | ||
Comment 6•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/005d7fcffdd3103ef2504c086614c61e40d9c50f
Aurora is closed at the moment.
Comment 7•8 years ago
|
||
| bugherder uplift | ||
You need to log in
before you can comment on or make changes to this bug.
Description
•