[webkit-reviews] review granted: [Bug 121102] MediaStream API: Update RTCDataChannel : [Attachment 211627] Proposed patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Eric Carlson <eric.carlson at apple.com>'s
request for review:
Bug 121102: MediaStream API: Update RTCDataChannel
https://bugs.webkit.org/show_bug.cgi?id=121102

Attachment 211627: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=211627&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=211627&action=review


> Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:68
> +    if (options.get("id", value))
> +	   initData.id = value;
> +    if (options.get("maxRetransmits", value))
> +	   initData.maxRetransmits = value;
> +    if (options.get("maxRetransmitTime", value))
> +	   initData.maxRetransmitTime = value;

Can these be written without the if? Just options.get("id", initData.id)), etc?


> Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:72
> +    String protocolString;
> +    options.get("protocol", protocolString);
> +    initData.protocol = protocolString;

Can this be written as options.get("protocol", initData.protocol); ?

> Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:143
> +    DEFINE_STATIC_LOCAL(String, connectingState, ("connecting",
AtomicString::ConstructFromLiteral));
> +    DEFINE_STATIC_LOCAL(String, openState, ("open",
AtomicString::ConstructFromLiteral));
> +    DEFINE_STATIC_LOCAL(String, closingState, ("closing",
AtomicString::ConstructFromLiteral));
> +    DEFINE_STATIC_LOCAL(String, closedState, ("closed",
AtomicString::ConstructFromLiteral));

Can we use NeverDestroyed<> instead?