WebProcess should unregister its interest for a SWServerRegistration when all its corresponding ServiceWorkerRegistrations are destroyed

[youenn fablet]

WebProcess should unregister its interest for a SWServerRegistration when all its corresponding ServiceWorkerRegistrations are destroyed

[youenn fablet]

Created attachment 381913 [details]
Patch

[youenn fablet]

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

[youenn fablet]

<rdar://problem/56480245>

[youenn fablet]

Created attachment 381914 [details]
Patch

[youenn fablet]

Created attachment 381923 [details]
Patch

[Chris Dumez]

Comment on attachment 381923 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=381923&action=review

> Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp:89
>      send(Messages::WebSWServerConnection::AddServiceWorkerRegistrationInServer { identifier });

Why do we send unconditionally and and not when going from 0 to 1?

> Source/WebKit/WebProcess/WebProcess.cpp:1765
> +bool WebProcess::decrementServiceWorkerRegistrationCount(WebCore::ServiceWorkerRegistrationIdentifier identifier)

It is unclear from the name what the returned boolean means. I think it should be factored differently.

> Source/WebKit/WebProcess/WebProcess.h:585
> +    HashMap<WebCore::ServiceWorkerRegistrationIdentifier, size_t> m_swRegistrationCounts;

Seems like a perfect use case for a HashCountedSet?

[youenn fablet]

> > Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp:89
> >      send(Messages::WebSWServerConnection::AddServiceWorkerRegistrationInServer { identifier });
> 
> Why do we send unconditionally and and not when going from 0 to 1?

I preferred not doing that because we probably have an issue in case of network process crash where we should reregister them.
Currently, we keep the status quo which gives a chance for a reload to fix the issue.
Once we update the code to handle network process crash, we should indeed do that.

> > Source/WebKit/WebProcess/WebProcess.h:585
> > +    HashMap<WebCore::ServiceWorkerRegistrationIdentifier, size_t> m_swRegistrationCounts;
> 
> Seems like a perfect use case for a HashCountedSet?

OK, will do.

[youenn fablet]

Created attachment 381927 [details]
Patch

[Chris Dumez]

Comment on attachment 381927 [details]
Patch

r=me.

[WebKit Commit Bot]

Comment on attachment 381927 [details]
Patch

Clearing flags on attachment: 381927

Committed r251598: <https://trac.webkit.org/changeset/251598>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.