SW: Implement "Update Registration State" algorithm From the server process back to the client process.
For our sanity we'll do Update Worker State at the same time.
NM, patches will get too insane. I'll land "Update Registration State" for now with no effect
Created attachment 325738 [details] WIP for EWS
Comment on attachment 325738 [details] WIP for EWS Attachment 325738 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5078551 Number of test failures exceeded the failure limit.
Created attachment 325745 [details] Archive of layout-test-results from ews107 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 325738 [details] WIP for EWS Attachment 325738 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5078827 Number of test failures exceeded the failure limit.
Created attachment 325755 [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 325790 [details] WIP2 for EWS
Created attachment 325797 [details] Patch
Comment on attachment 325797 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325797&action=review r=me with suggestions > Source/WebCore/workers/service/ServiceWorkerTypes.h:33 > + Installing = 0, Why the explicit values? default is 0-based. > Source/WebCore/workers/service/server/SWClientConnection.cpp:69 > + auto result = m_registrations.add(registration.data().key, nullptr); Could use ensure() > Source/WebCore/workers/service/server/SWClientConnection.cpp:83 > + ASSERT(iterator->value && iterator->value->contains(®istration)); We should probably ASSERT(iterator != m_registrations.end()); before that. > Source/WebCore/workers/service/server/SWServerRegistration.cpp:93 > + auto result = m_clientRegistrationsByConnection.add(connectionIdentifier, nullptr); Could use ensure(). > Source/WebCore/workers/service/server/SWServerRegistration.cpp:104 > + if (!iterator->value) Should we assert and iterator != m_clientRegistrationsByConnection.end() ?
(In reply to Chris Dumez from comment #10) > Comment on attachment 325797 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=325797&action=review > > r=me with suggestions > > > Source/WebCore/workers/service/ServiceWorkerTypes.h:33 > > + Installing = 0, > > Why the explicit values? default is 0-based. In an earlier version of this patch (and in a follow-up) I was going to use them as array indices. I agree it's not needed, but I get paranoid about "enums as indices" anyways. *sigh* > > > Source/WebCore/workers/service/server/SWClientConnection.cpp:69 > > + auto result = m_registrations.add(registration.data().key, nullptr); > > Could use ensure() Yah (I'd never seen this before) > > Source/WebCore/workers/service/server/SWServerRegistration.cpp:104 > > + if (!iterator->value) > > Should we assert and iterator != m_clientRegistrationsByConnection.end() ? Nah, but similar.
Created attachment 325821 [details] Patch
Comment on attachment 325821 [details] Patch EWS (haven't finished building locally...) then I'll cq+
Comment on attachment 325821 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325821&action=review > Source/WebCore/workers/service/server/SWClientConnection.cpp:67 > +// static NeverDestroyed<HashMap<String, String>> mimeTypeToLabelTitleMap; Pretty sure you did not mean to have this here :D
Comment on attachment 325821 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325821&action=review > Source/WebCore/workers/service/server/SWServerRegistration.cpp:95 > + } });
(In reply to Chris Dumez from comment #14) > Comment on attachment 325821 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=325821&action=review > > > Source/WebCore/workers/service/server/SWClientConnection.cpp:67 > > +// static NeverDestroyed<HashMap<String, String>> mimeTypeToLabelTitleMap; > > Pretty sure you did not mean to have this here :D I don't even know how in the heck it got there!
(In reply to Brady Eidson from comment #16) > (In reply to Chris Dumez from comment #14) > > Comment on attachment 325821 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=325821&action=review > > > > > Source/WebCore/workers/service/server/SWClientConnection.cpp:67 > > > +// static NeverDestroyed<HashMap<String, String>> mimeTypeToLabelTitleMap; > > > > Pretty sure you did not mean to have this here :D > > I don't even know how in the heck it got there! NM I do.
(In reply to Chris Dumez from comment #15) > Comment on attachment 325821 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=325821&action=review > > > Source/WebCore/workers/service/server/SWServerRegistration.cpp:95 > > + } > > }); And if UnifiedSource hadn't broken Xcode's look-ahead compiling... *sigh*
Created attachment 325827 [details] EWS then maybe CQ
Comment on attachment 325827 [details] EWS then maybe CQ Clearing flags on attachment: 325827 Committed r224379: <https://trac.webkit.org/changeset/224379>
All reviewed patches have been landed. Closing bug.
FYI, I saw this crash when running the SW tests locally in debug: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x000000010e080f04 WTFCrash + 36 (Assertions.cpp:270) 1 com.apple.WebCore 0x0000000116de1459 WebCore::SWServerRegistration::removeClientServiceWorkerRegistration(unsigned long long, unsigned long long) + 393 (SWServerRegistration.cpp:107) 2 com.apple.WebCore 0x0000000116de05b9 WebCore::SWServer::removeClientServiceWorkerRegistration(WebCore::SWServer::Connection&, WebCore::ServiceWorkerRegistrationKey const&, unsigned long long) + 137 (SWServer.cpp:239) 3 com.apple.WebCore 0x0000000116de0524 WebCore::SWServer::Connection::removeServiceWorkerRegistrationInServer(WebCore::ServiceWorkerRegistrationKey const&, unsigned long long) + 52 (SWServer.cpp:115) 4 com.apple.WebKit 0x0000000107a2e260 void IPC::callMemberFunctionImpl<WebKit::WebSWServerConnection, void (WebCore::SWServer::Connection::*)(WebCore::ServiceWorkerRegistrationKey const&, unsigned long long), std::__1::tuple<WebCore::ServiceWorkerRegistrationKey, unsigned long long>, 0ul, 1ul>(WebKit::WebSWServerConnection*, void (WebCore::SWServer::Connection::*)(WebCore::ServiceWorkerRegistrationKey const&, unsigned long long), std::__1::tuple<WebCore::ServiceWorkerRegistrationKey, unsigned long long>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul>) + 192 (HandleMessage.h:41) 5 com.apple.WebKit 0x0000000107a2dfd0 void IPC::callMemberFunction<WebKit::WebSWServerConnection, void (WebCore::SWServer::Connection::*)(WebCore::ServiceWorkerRegistrationKey const&, unsigned long long), std::__1::tuple<WebCore::ServiceWorkerRegistrationKey, unsigned long long>, std::__1::integer_sequence<unsigned long, 0ul, 1ul> >(std::__1::tuple<WebCore::ServiceWorkerRegistrationKey, unsigned long long>&&, WebKit::WebSWServerConnection*, void (WebCore::SWServer::Connection::*)(WebCore::ServiceWorkerRegistrationKey const&, unsigned long long)) + 96 (HandleMessage.h:47) 6 com.apple.WebKit 0x0000000107a2bfe3 void IPC::handleMessage<Messages::WebSWServerConnection::RemoveServiceWorkerRegistrationInServer, WebKit::WebSWServerConnection, void (WebCore::SWServer::Connection::*)(WebCore::ServiceWorkerRegistrationKey const&, unsigned long long)>(IPC::Decoder&, WebKit::WebSWServerConnection*, void (WebCore::SWServer::Connection::*)(WebCore::ServiceWorkerRegistrationKey const&, unsigned long long)) + 339 (HandleMessage.h:127) 7 com.apple.WebKit 0x0000000107a2b8d6 WebKit::WebSWServerConnection::didReceiveMessage(IPC::Connection&, IPC::Decoder&) + 502 (WebSWServerConnectionMessageReceiver.cpp:64) 8 com.apple.WebKit 0x000000010734bd0e WebKit::StorageToWebProcessConnection::didReceiveMessage(IPC::Connection&, IPC::Decoder&) + 734 9 com.apple.WebKit 0x0000000106dddc63 IPC::Connection::dispatchMessage(IPC::Decoder&) + 51 (Connection.cpp:902) 10 com.apple.WebKit 0x0000000106dd3248 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) + 712 11 com.apple.WebKit 0x0000000106dde26a IPC::Connection::dispatchOneMessage() + 1530 (Connection.cpp:959) 12 com.apple.WebKit 0x0000000106df654d IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14::operator()() + 29 (Connection.cpp:896) 13 com.apple.WebKit 0x0000000106df64a9 WTF::Function<void ()>::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14>::call() + 25 (Function.h:101) 14 com.apple.JavaScriptCore 0x000000010e0b654b WTF::Function<void ()>::operator()() const + 139 (Function.h:56) 15 com.apple.JavaScriptCore 0x000000010e0d6b83 WTF::RunLoop::performWork() + 211 (RunLoop.cpp:107) 16 com.apple.JavaScriptCore 0x000000010e0d7404 WTF::RunLoop::performWork(void*) + 36 (RunLoopCF.cpp:38) 17 com.apple.CoreFoundation 0x00007fff3cff46e1 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17 18 com.apple.CoreFoundation 0x00007fff3d0ae3ac __CFRunLoopDoSource0 + 108 19 com.apple.CoreFoundation 0x00007fff3cfd7180 __CFRunLoopDoSources0 + 208 20 com.apple.CoreFoundation 0x00007fff3cfd65fd __CFRunLoopRun + 1293 21 com.apple.CoreFoundation 0x00007fff3cfd5e63 CFRunLoopRunSpecific + 483 22 com.apple.Foundation 0x00007fff3f14eba6 -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 277 23 com.apple.Foundation 0x00007fff3f14ea7e -[NSRunLoop(NSRunLoop) run] + 76 24 libxpc.dylib 0x00007fff650fa3ff _xpc_objc_main + 536 25 libxpc.dylib 0x00007fff650f907e xpc_main + 417 26 com.apple.WebKit.Storage 0x0000000106c9b13b main + 1195 (XPCServiceMain.mm:148) 27 libdyld.dylib 0x00007fff64e2d115 start + 1
Created attachment 325905 [details] Fix the ASSERT
Reopening for CQ followup
Comment on attachment 325905 [details] Fix the ASSERT Rejecting attachment 325905 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 325905, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit /Volumes/Data/EWS/WebKit/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://webkit-queues.webkit.org/results/5091493
*** Bug 179246 has been marked as a duplicate of this bug. ***
Created attachment 325935 [details] Fix the ASSERT
Comment on attachment 325935 [details] Fix the ASSERT Clearing flags on attachment: 325935 Committed r224418: <https://trac.webkit.org/changeset/224418>
<rdar://problem/35567268>