Bug 121102

Summary:

MediaStream API: Update RTCDataChannel

Product:

WebKit

Reporter:

Eric Carlson <eric.carlson>

Component:

WebCore Misc.

Assignee:

Eric Carlson <eric.carlson>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, commit-queue, esprehn+autocc, glenn, hta, jer.noble, kondapallykalyan, tommyw

Priority:

Keywords:

BlinkMergeCandidate

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

121101

Attachments:

Description	Flags
Proposed patch	none

Description Eric Carlson 2013-09-10 10:58:17 PDT

The spec was updated to include a "DataChannelInit" parameter in the RTCDataChannelInit dictionary: http://dev.w3.org/2011/webrtc/editor/webrtc.html#idl-def-RTCDataChannelInit

Blink/Chromium changes here - https://chromium.googlesource.com/chromium/blink/+/c6975c41956acded7cf0363012d7d6b69d0c6d96 and https://chromium.googlesource.com/chromium/blink/+/214dab0bd6385f573c918ba5ce58a5aa206ce186

Comment 1 Eric Carlson 2013-09-13 22:39:17 PDT

It make sense to  also merge https://chromium.googlesource.com/chromium/blink/+/c3862b0a83e20fc8b1f770c7e4a886a7cceb80d2 (https://webkit.org/b/121104) here.

Comment 2 Eric Carlson 2013-09-13 23:39:08 PDT

Created attachment 211627 [details]
Proposed patch

Comment 3 Eric Carlson 2013-09-14 16:11:30 PDT

I will need to update this patch once bug 120889 lands.

Comment 4 Sam Weinig 2013-09-14 16:59:59 PDT

Comment on attachment 211627 [details]
Proposed patch

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?

Comment 5 Eric Carlson 2013-09-14 19:32:17 PDT

Committed r155794: https://trac.webkit.org/r155794