If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Make ServiceWorker keep its document and window alive

RESOLVED FIXED in Firefox 42

Status

()

Core
DOM: Service Workers
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Away for a while, Assigned: Away for a while)

Tracking

unspecified
mozilla42
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

2 years ago
Without this, ServiceWorker::PostMessage won't work when the window has
been cleaned up, since at that time the event targets are disconnected
from their owners and GetParentObject() will return null.
(Assignee)

Comment 1

2 years ago
Created attachment 8628570 [details] [diff] [review]
Make ServiceWorker keep its document and window alive
Attachment #8628570 - Flags: review?(amarchesini)
(Assignee)

Updated

2 years ago
Assignee: nobody → ehsan
Blocks: 1059784, 1179401
Comment on attachment 8628570 [details] [diff] [review]
Make ServiceWorker keep its document and window alive

Review of attachment 8628570 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/workers/ServiceWorker.cpp
@@ +45,5 @@
>                               SharedWorker* aSharedWorker)
>    : DOMEventTargetHelper(aWindow),
>      mInfo(aInfo),
> +    mSharedWorker(aSharedWorker),
> +    mDocument(aWindow ? aWindow->GetExtantDoc() : nullptr),

I prefer to have this 2 assignment into the CTOR code.
Something like:

if (aWindow) {
  mDocument = aWindow->GetExtantDoc();
  mWindow = aWindow->GetOuterWindow();
}
Attachment #8628570 - Flags: review?(amarchesini) → review+

Comment 3

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/daffc9a65ab0
https://hg.mozilla.org/mozilla-central/rev/daffc9a65ab0
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
I think the problem is that the WorkerPrivate associated with the underlying shared worker is destroyed when the document is detached. Keeping the document and the window alive doesn't prevent this.

Are there any tests that are fixed by this change?
Flags: needinfo?(ehsan)
(Assignee)

Comment 6

2 years ago
(In reply to Cătălin Badea (:catalinb) from comment #5)
> I think the problem is that the WorkerPrivate associated with the underlying
> shared worker is destroyed when the document is detached. Keeping the
> document and the window alive doesn't prevent this.
> 
> Are there any tests that are fixed by this change?

No, this is part of bug 1179401.  The rest of that bug talks about restarting the worker if the previous one has died.  Doing that would fix the test in that bug.
Flags: needinfo?(ehsan)
Created attachment 8646019 [details]
MozReview Request: Bug 1188545 - Disentangle service workers from shared workers and refactor event dispatching code into a separate class.  r?nsm,bkelly,khuey

Bug 1188545 - Disentangle service workers from shared workers and refactor event dispatching code into a separate class.  r?nsm,bkelly,khuey
Attachment #8646019 - Flags: review?(nsm.nikhil)
Attachment #8646019 - Flags: review?(khuey)
Attachment #8646019 - Flags: review?(bkelly)
Created attachment 8646020 [details]
MozReview Request: Bug 1188545: Cosmetic changes regarding workerPrivate properties shared between shared workers and service workers. r?nsm

Bug 1188545: Cosmetic changes regarding workerPrivate properties shared between shared workers and service workers. r?nsm
Attachment #8646020 - Flags: review?(nsm.nikhil)
Created attachment 8646021 [details]
MozReview Request: Bug 1188545: Terminate service workers that have been idle for some time. r?nsm

Bug 1188545: Terminate service workers that have been idle for some time. r?nsm
Attachment #8646021 - Flags: review?(nsm.nikhil)
(In reply to Cătălin Badea (:catalinb) from comment #7)
> Created attachment 8646019 [details]
> MozReview Request: Bug 1188545 - Disentangle service workers from shared
> workers and refactor event dispatching code into a separate class. 
> r?nsm,bkelly,khuey
> 
> Bug 1188545 - Disentangle service workers from shared workers and refactor
> event dispatching code into a separate class.  r?nsm,bkelly,khuey

bkelly, please have a look at how the worker load group is handled in SpawnServiceWorkerIfNeeded.
khuey, for the changes in RuntimeService and WorkerPrivate.
nsm, please have another look at the changes in ServiceWorkerPrivate, the part about AllowedWindowInteraction and notification click events.
ah, crap - reviewboard used the first commit to attach the reviews. sorry about that.
Attachment #8646019 - Attachment is obsolete: true
Attachment #8646019 - Flags: review?(nsm.nikhil)
Attachment #8646019 - Flags: review?(khuey)
Attachment #8646019 - Flags: review?(bkelly)
Attachment #8646020 - Attachment is obsolete: true
Attachment #8646020 - Flags: review?(nsm.nikhil)
Attachment #8646021 - Attachment is obsolete: true
Attachment #8646021 - Flags: review?(nsm.nikhil)
You need to log in before you can comment on or make changes to this bug.