Bug 121102 - MediaStream API: Update RTCDataChannel
Summary: MediaStream API: Update RTCDataChannel
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: BlinkMergeCandidate
Depends on:
Blocks: 121101
  Show dependency treegraph
 
Reported: 2013-09-10 10:58 PDT by Eric Carlson
Modified: 2013-09-14 19:32 PDT (History)
8 users (show)

See Also:


Attachments
Proposed patch (16.60 KB, patch)
2013-09-13 23:39 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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