[webkit-reviews] review granted: [Bug 179186] SW: Implement "Update Registration State" algorithm (unused for now) : [Attachment 325797] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Brady Eidson <beidson at apple.com>'s
request for review:
Bug 179186: SW: Implement "Update Registration State" algorithm (unused for
now)
https://bugs.webkit.org/show_bug.cgi?id=179186

Attachment 325797: Patch

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




--- Comment #10 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 325797
  --> https://bugs.webkit.org/attachment.cgi?id=325797
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() ?