Closed
Bug 982749
Opened 10 years ago
Closed 10 years ago
RuntimeService should be able to create a ServiceWorker
Categories
(Core :: DOM: Workers, defect)
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: nsm, Assigned: nsm)
References
Details
Attachments
(1 file, 3 obsolete files)
11.56 KB,
patch
|
bent.mozilla
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → nsm.nikhil
Assignee | ||
Comment 2•10 years ago
|
||
Functions to allow creating a ServiceWorker and setting the relevant properties on the window facing ServiceWorker object. GetStatus() is used by other patches to avoid certain event dispatches.
Attachment #8400304 -
Flags: review?(bent.mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8389929 -
Attachment is obsolete: true
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8415980 -
Flags: review?(bent.mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8400304 -
Attachment is obsolete: true
Attachment #8400304 -
Flags: review?(bent.mozilla)
Comment on attachment 8415980 [details] [diff] [review] RuntimeService::CreateServiceWorker and associated. Review of attachment 8415980 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/RuntimeService.cpp @@ +2095,5 @@ > +{ > + nsresult rv; > + > + nsCOMPtr<nsPIDOMWindow> window = do_QueryInterface(aGlobal.GetAsSupports()); > + MOZ_ASSERT(window); Hrm, sicking says that we won't always have a window when we want to launch a service worker. Seems like this won't work. @@ +2172,5 @@ > + nsRefPtr<SharedWorker> sharedWorker; > + if (workerPrivate->IsSharedWorker() || workerPrivate->IsServiceWorker()) { > + sharedWorker = new SharedWorker(window, workerPrivate); > + } else { > + MOZ_CRASH("Expected SharedWorker or ServiceWorker!"); MOZ_ASSERT this at the start of the function, no need for release mode checks. ::: dom/workers/WorkerPrivate.cpp @@ +2192,5 @@ > } > > if (self->mStatus == Dead || > (!aSyncLoopTarget && ParentStatus() > Running)) { > + MOZ_ASSERT("A runnable was posted to a worker that is already shutting " Please revert this, this is a normal condition that happens sometimes and isn't fatal. ::: dom/workers/WorkerPrivate.h @@ +857,5 @@ > mFeatures.IsEmpty()); > } > > + Status > + GetStatus() This looks unused, but it's scary to expose anyway so I'd rather not do it.
Attachment #8415980 -
Flags: review?(bent.mozilla) → review-
Assignee | ||
Comment 5•10 years ago
|
||
CreateServiceWorker is only for when we have a window. A later patch adds a CreateServiceWorker overload that accepts a LoadInfo for the no-window cases.
Attachment #8419657 -
Flags: review?(bent.mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8415980 -
Attachment is obsolete: true
Comment on attachment 8419657 [details] [diff] [review] RuntimeService::CreateServiceWorker and associated. Review of attachment 8419657 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/RuntimeService.cpp @@ +1419,5 @@ > domainInfo->mActiveWorkers.RemoveElement(aWorkerPrivate); > } > > + bool isSharedOrServiceWorker = aWorkerPrivate->IsSharedWorker() || > + aWorkerPrivate->IsServiceWorker(); This temporary can go away, it's only used once right? @@ +2084,5 @@ > SharedWorker** aSharedWorker) > { > + return CreateSharedWorkerInternal(aGlobal, aScriptURL, aName, > + WorkerTypeShared, > + aSharedWorker); You might as well inline this in the header. @@ +2101,5 @@ > + > + nsRefPtr<SharedWorker> sharedWorker; > + rv = CreateSharedWorkerInternal(aGlobal, aScriptURL, aScope, > + WorkerTypeService, > + getter_AddRefs(sharedWorker)); You probably want to check that this succeeded, right? ::: dom/workers/RuntimeService.h @@ +29,2 @@ > class SharedWorker; > class WorkerPrivate; Nit: This doesn't need to be fwd-declared any longer
Attachment #8419657 -
Flags: review?(bent.mozilla) → review+
Assignee | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d1d3435bdbc
https://hg.mozilla.org/mozilla-central/rev/3d1d3435bdbc
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in
before you can comment on or make changes to this bug.
Description
•