Add support for "install" event in service workers.
Created attachment 326156 [details] WIP Patch
Created attachment 326170 [details] WIP Patch
Created attachment 326171 [details] WIP Patch
Created attachment 326182 [details] Patch
Created attachment 326185 [details] Patch
Created attachment 326188 [details] Patch
Created attachment 326189 [details] Patch
Comment on attachment 326189 [details] Patch At a glance on iPhone where I can never effectively review patches, you have some non ascii characters
Created attachment 326193 [details] Patch
Comment on attachment 326193 [details] Patch Attachment 326193 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/5131229 New failing tests: http/tests/loading/preload-picture-type.html
Created attachment 326197 [details] Archive of layout-test-results from ews115 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 326193 [details] Patch Attachment 326193 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5134002 New failing tests: imported/w3c/web-platform-tests/streams/readable-byte-streams/brand-checks.serviceworker.https.html imported/w3c/web-platform-tests/streams/readable-streams/general.serviceworker.https.html
Created attachment 326207 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Created attachment 326216 [details] Patch
Comment on attachment 326216 [details] Patch This all seems fine once EWS is happy
(In reply to Brady Eidson from comment #15) > Comment on attachment 326216 [details] > Patch > > This all seems fine once EWS is happy r=you?
Comment on attachment 326216 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=326216&action=review > Source/WebCore/workers/service/InstallEvent.cpp:36 > + : ExtendableEvent(eventNames().installEvent, false, false) Maybe we should make ExtendableEvent constructor have default values for the last two parameters? > Source/WebCore/workers/service/ServiceWorkerContainer.cpp:276 > + registration->dispatchEvent(Event::create(eventNames().updatefoundEvent, false, false)); What if ServiceWorkerContainer is stopped at that point. Should we refrain from dispatching the event? > Source/WebCore/workers/service/server/SWClientConnection.cpp:161 > + // FIXME: We should iterate over all service worker clients, not only documents. We have this FIXME elsewhere. Maybe we should introduce something like SWClientConnection::iterateClients and pass it a lambda so that we have just one FIXME? > LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/foreign-fetch-event.https-expected.txt:12 > +FAIL ForeignFetchEvent constructor with all init dict members Can't find variable: ForeignFetchEvent We should just skip this test for now
Comment on attachment 326216 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=326216&action=review >> Source/WebCore/workers/service/InstallEvent.cpp:36 >> + : ExtendableEvent(eventNames().installEvent, false, false) > > Maybe we should make ExtendableEvent constructor have default values for the last two parameters? This matches the pattern in Event: Event(const AtomicString& type, bool canBubble, bool cancelable); >> Source/WebCore/workers/service/ServiceWorkerContainer.cpp:276 >> + registration->dispatchEvent(Event::create(eventNames().updatefoundEvent, false, false)); > > What if ServiceWorkerContainer is stopped at that point. Should we refrain from dispatching the event? I'll add the check to be safe. I was hoping scriptExecutionContext would not post the task in such case but looking at the code, it does not look like it. >> Source/WebCore/workers/service/server/SWClientConnection.cpp:161 >> + // FIXME: We should iterate over all service worker clients, not only documents. > > We have this FIXME elsewhere. > Maybe we should introduce something like SWClientConnection::iterateClients and pass it a lambda so that we have just one FIXME? Ok.
(In reply to Chris Dumez from comment #18) > Comment on attachment 326216 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=326216&action=review > > >> Source/WebCore/workers/service/InstallEvent.cpp:36 > >> + : ExtendableEvent(eventNames().installEvent, false, false) > > > > Maybe we should make ExtendableEvent constructor have default values for the last two parameters? > > This matches the pattern in Event: > Event(const AtomicString& type, bool canBubble, bool cancelable); > > >> Source/WebCore/workers/service/ServiceWorkerContainer.cpp:276 > >> + registration->dispatchEvent(Event::create(eventNames().updatefoundEvent, false, false)); > > > > What if ServiceWorkerContainer is stopped at that point. Should we refrain from dispatching the event? > > I'll add the check to be safe. I was hoping scriptExecutionContext would not > post the task in such case but looking at the code, it does not look like it. > > >> Source/WebCore/workers/service/server/SWClientConnection.cpp:161 > >> + // FIXME: We should iterate over all service worker clients, not only documents. > > > > We have this FIXME elsewhere. > > Maybe we should introduce something like SWClientConnection::iterateClients and pass it a lambda so that we have just one FIXME? > > Ok. Brady says we should drop the InstallEvent idl/class before landing as it is about to be removed from the spec.
Created attachment 326235 [details] Patch
Comment on attachment 326235 [details] Patch Clearing flags on attachment: 326235 Committed r224542: <https://trac.webkit.org/changeset/224542>
All reviewed patches have been landed. Closing bug.
<rdar://problem/35567067>