Bug 179186 – SW: Implement "Update Registration State" algorithm (unused for now)

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Brady Eidson]

SW: Implement "Update Registration State" algorithm

From the server process back to the client process.

[Brady Eidson]

For our sanity we'll do Update Worker State at the same time.

[Brady Eidson]

NM, patches will get too insane.

I'll land "Update Registration State" for now with no effect

[Brady Eidson]

Created attachment 325738 [details]
WIP for EWS

[Build Bot]

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.

[Build Bot]

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

[Build Bot]

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.

[Build Bot]

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

[Brady Eidson]

Created attachment 325790 [details]
WIP2 for EWS

[Brady Eidson]

Created attachment 325797 [details]
Patch

[Chris Dumez]

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(&registration));

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() ?

[Brady Eidson]

(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.

[Brady Eidson]

Created attachment 325821 [details]
Patch

[Brady Eidson]

Comment on attachment 325821 [details]
Patch

EWS (haven't finished building locally...) then I'll cq+

[Chris Dumez]

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

[Chris Dumez]

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
> +    }

});

[Brady Eidson]

(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!

[Brady Eidson]

(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.

[Brady Eidson]

(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*

[Brady Eidson]

Created attachment 325827 [details]
EWS then maybe CQ

[WebKit Commit Bot]

Comment on attachment 325827 [details]
EWS then maybe CQ

Clearing flags on attachment: 325827

Committed r224379: <https://trac.webkit.org/changeset/224379>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

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

[Brady Eidson]

Created attachment 325905 [details]
Fix the ASSERT

[Brady Eidson]

Reopening for CQ followup

[WebKit Commit Bot]

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

[Ryan Haddad]

*** Bug 179246 has been marked as a duplicate of this bug. ***

[Brady Eidson]

Created attachment 325935 [details]
Fix the ASSERT

[WebKit Commit Bot]

Comment on attachment 325935 [details]
Fix the ASSERT

Clearing flags on attachment: 325935

Committed r224418: <https://trac.webkit.org/changeset/224418>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35567268>