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

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has denied 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 155259: Patch
https://bugs.webkit.org/attachment.cgi?id=155259&action=review

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


> Source/WebCore/bindings/js/ArrayValue.cpp:41
> +ArrayValue::ArrayValue(JSC::ExecState* exec, JSC::JSValue value)

value => array (for clarification and consistency with V8)

> Source/WebCore/bindings/js/ArrayValue.cpp:43
> +    , m_value(value)

m_value => m_array (for clarification and consistency with V8)

> Source/WebCore/bindings/js/ArrayValue.cpp:57
> +    return !m_exec;

This should be something like this:

  if (m_value.IsEmpty())
    return true;
  return WebCore::isUndefinedOrNull(m_value);

> Source/WebCore/bindings/js/ArrayValue.cpp:65
> +    JSArray* array = asArray(m_value);

Let's store JSArray* to the ArrayValue object so that we can avoid calling
asArray every time.

Also you need to check m_value->isArray() before calling asArray(). Given that
you store JSArray* to the ArrayValue object, you can do the check in
JSDictionary::convertValue() before calling the ArrayValue constructor.

> Source/WebCore/bindings/js/ArrayValue.cpp:75
> +    JSArray* array = asArray(m_value);

Ditto.

> Source/WebCore/bindings/js/ArrayValue.h:29
> +#include <interpreter/CallFrame.h>

Remove this.