[webkit-reviews] review granted: [Bug 178882] Implement Service Worker Matching Registration algorithm : [Attachment 325802] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted youenn fablet <youennf at gmail.com>'s
request for review:
Bug 178882: Implement Service Worker Matching Registration algorithm
https://bugs.webkit.org/show_bug.cgi?id=178882

Attachment 325802: Patch

https://bugs.webkit.org/attachment.cgi?id=325802&action=review




--- Comment #43 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 325802
  --> https://bugs.webkit.org/attachment.cgi?id=325802
Patch

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

r=me with comments.

> Source/WebCore/page/SecurityOriginData.h:86
> +inline bool operator!=(const SecurityOriginData& first, const
SecurityOriginData& second) { return !(first == second); }

This is a bit inefficient. If you implemented a real !=, we would be able to
abort early as soon as one of the component does not match. I'd prefer we
provide a proper implementation.

> Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:38
> +    URL scope;

As mentioned earlier, I think this is error prone. It makes it looks like we
have the full scopeURL while this does not container the fragment. I think we
should either:
1. Use String scopeString; as in the spec
or
2. Rename to scopeWithoutFragment.