[webkit-reviews] review granted: [Bug 92380] Introduce a minimal RTCPeerConnection together with Dictionary changes : [Attachment 155296] Patch

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has granted Tommy Widenflycht
<tommyw at google.com>'s request for review:
Bug 92380: Introduce a minimal RTCPeerConnection together with Dictionary
changes
https://bugs.webkit.org/show_bug.cgi?id=92380

Attachment 155296: Patch
https://bugs.webkit.org/attachment.cgi?id=155296&action=review

------- Additional Comments from Kentaro Hara <haraken at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=155296&action=review


Looks good to me. Please confirm that build bots get green before landing.

> Source/WebCore/bindings/js/ArrayValue.cpp:44
> +    if (!value.isUndefinedOrNull() && isJSArray(value))

Nit: To keep the implementation consistent between JSC and V8, I might want to
move this check to JSDictionary::convertValue(). You can write ASSERT() here.

> Source/WebCore/bindings/v8/ArrayValue.cpp:44
> +    if (m_array.IsEmpty())
> +	   return true;
> +    return WebCore::isUndefinedOrNull(m_array);

Nit: This could be:

    return m_array.IsEmpty() || WebCore::isUndefinedOrNull(m_array);

Let's keep the implementation consistent between JSC and V8.