Bug 203144 – Move service worker registration matching for navigation loads to network process

• Component: Service Workers
• Status: RESOLVED FIXED

[youenn fablet]

Move service worker registration matching for navigation loads to network process

[youenn fablet]

Created attachment 381288 [details]
Patch

[youenn fablet]

Created attachment 381293 [details]
Patch

[youenn fablet]

Created attachment 381299 [details]
Win fix

[youenn fablet]

Ping review.

Mac-wk1 tests are not about service workers ( fast/scrolling/latching/scroll-select-bottom-test.html  and  fast/events/resize-subframe-in-rendering-update.html ), so I doubt this is a regression.
Perl bot is failing test262 tests which should not be affected by this patch either.

[youenn fablet]

Also previous patch was passing in WK1 and webkitperl and the changes were #ifdef for WIN compilation.

[Chris Dumez]

Comment on attachment 381299 [details]
Win fix

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

> Source/WebCore/ChangeLog:8
> +        For regular loads, we no longer match service worker registration explicitly.

"explicitly"

> Source/WebCore/ChangeLog:10
> +        We still need to explicitely match registrations in those two cases:

"explicitely"

-> Make up your mind :P

> Source/WebCore/workers/service/server/SWServer.cpp:144
> +    m_importCompletedCallbacks.append(WTFMove(callback));

I think we should ASSERT(!m_importCompleted);

> Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp:191
> +void WebSWClientConnection::documentIsControlled(DocumentIdentifier documentIdentifier, ServiceWorkerRegistrationData&& data, CompletionHandler<void(bool)>&& completionHandler)

The naming is bad IMO. It looks like a getter but is really a setter. I'd suggest something like setDocumentIsControlled() or controlDocument().

[Alex Christensen]

Created attachment 381469 [details]
Patch

[Alex Christensen]

http://trac.webkit.org/r251409

[Radar WebKit Bug Importer]

<rdar://problem/56485381>