Closed Bug 1160527 Opened 10 years ago Closed 9 years ago

Other ServiceWorker events should inherit from ExtendableEvent

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: bkelly, Assigned: nsm)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file, 1 obsolete file)

The ServiceWorker spec has changed such that FetchEvent and other functional events now inherit from ExtendableEvent. The purpose of this is to provide waitUntil() on all events. We should implement this and the requisite logic to support waitUntil() on these events. Note, MessageEvent is not currently spec'd to inherit from ExtendableEvent, but I wrote this issue to possibly fix that: https://github.com/slightlyoff/ServiceWorker/issues/690 See bug 1134841 for some of the annoying intermittents that can happen from early worker shutdown.
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Bug 1160527 - Patch 1 - Support PushEvent.waitUntil(). r=baku.
Attachment #8614387 - Flags: review?(amarchesini)
Attached patch patchSplinter Review
Assignee: ehsan → nsm.nikhil
Attachment #8614387 - Attachment is obsolete: true
Attachment #8614387 - Flags: review?(amarchesini)
Attachment #8615712 - Flags: review?(amarchesini)
Blocks: ServiceWorkers-postv1
No longer blocks: ServiceWorkers-v1
Comment on attachment 8615712 [details] [diff] [review] patch Review of attachment 8615712 [details] [diff] [review]: ----------------------------------------------------------------- Sorry if it took so long to review this code. ::: dom/workers/ServiceWorkerManager.cpp @@ +1453,5 @@ > PendingOperation* opt = mPendingOperations.AppendElement(); > opt->mRunnable = aRunnable; > } > > +namespace { Don't we already have an anonymous namespace? Can we merge them? @@ +1460,5 @@ > +{ > + nsMainThreadPtrHandle<ServiceWorker> mServiceWorker; > + > + virtual > + ~KeepAliveHandler() same line? @@ +1503,5 @@ > + WidgetEvent* internalEvent = aEvent->GetInternalNSEvent(); > + > + ErrorResult result; > + result = aWorkerScope->DispatchDOMEvent(nullptr, aEvent, nullptr, nullptr); > + if (!result.Failed() && !internalEvent->mFlags.mExceptionHasBeenRisen) { can you do a quick return here? if (result.Failed() || ... ) { return nullptr; }
Attachment #8615712 - Flags: review?(amarchesini) → review+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Depends on: 1180274
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: