<html>
    <head>
      <base href="https://bugs.webkit.org/" />
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - WebRTC: Add support for RTCPeerConnection legacy MediaStream-based API"
   href="https://bugs.webkit.org/show_bug.cgi?id=158940#c4">Comment # 4</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - WebRTC: Add support for RTCPeerConnection legacy MediaStream-based API"
   href="https://bugs.webkit.org/show_bug.cgi?id=158940">bug 158940</a>
              from <span class="vcard"><a class="email" href="mailto:adam.bergkvist&#64;ericsson.com" title="Adam Bergkvist &lt;adam.bergkvist&#64;ericsson.com&gt;"> <span class="fn">Adam Bergkvist</span></a>
</span></b>
        <pre>(In reply to <a href="show_bug.cgi?id=158940#c3">comment #3</a>)
<span class="quote">&gt; Comment on <span class=""><a href="attachment.cgi?id=281658&amp;action=diff" name="attach_281658" title="Proposed patch">attachment 281658</a> <a href="attachment.cgi?id=281658&amp;action=edit" title="Proposed patch">[details]</a></span>
&gt; Proposed patch</span >

Thanks for reviewing Youenn.

<span class="quote">&gt; View in context:
&gt; <a href="https://bugs.webkit.org/attachment.cgi?id=281658&amp;action=review">https://bugs.webkit.org/attachment.cgi?id=281658&amp;action=review</a>
&gt; 
&gt; &gt; Source/WebCore/Modules/mediastream/RTCPeerConnection.js:57
&gt; &gt; +    return <a href="mailto:this.&#64;localStreams.slice">this.&#64;localStreams.slice</a>();
&gt; 
&gt; Array.slice may be corrupted by user scripts.
&gt; The same applies to find findIndex, push, forEach, splice methods.
&gt; This should be robustified.</span >

Looking at the array prototype implementation, we seem to have a private name for push, but the rest needs fixing. New bug: [1].

[1] <a href="http://webkit.org/b/158978">http://webkit.org/b/158978</a>

<span class="quote">&gt; &gt; Source/WebCore/Modules/mediastream/RTCPeerConnection.js:94
&gt; &gt; +    stream.getTracks().forEach(track =&gt; this.&#64;addTrack(track, stream));
&gt; 
&gt; Potentially, we know that this, track and stream are of the right type.
&gt; So we could replace if statements by assert statements in the binding
&gt; generated code.
&gt; That said, this might be too early to do so.</span >

MediaStream.getTracks is actually part of the public API so we need a safe way to call it. New bug [2].

[2] webkit.org/b/158979

I agree that assertions would be sufficient for [PrivateOnly] bindings. The bug [3] suggests that one might want to use the type checking of a [PrivateOnly] function as part of a public API call, but that might be unlikely now when we have [PrivateAlso]. I wouldn't mind closing [3] as a WONTFIX.

[3] webkit.org/b/158778

<span class="quote">&gt; 
&gt; &gt; Source/WebCore/bindings/js/JSDOMGlobalObject.cpp:87
&gt; &gt; +        JSDOMGlobalObject::GlobalPropertyInfo(clientData.builtinNames().MediaStreamPrivateName(), JSMediaStream::getConstructor(vm, this), DontDelete | ReadOnly),
&gt; 
&gt; It might be good to add a keyword similar to PrivateAlso for exposing DOM
&gt; constructors safely to JS builtins.
&gt; Ideally, we should not need to create the constructor but just pass a getter
&gt; function that would create it if needed</span >

That would be useful indeed.</pre>
        </div>
      </p>
      <hr>
      <span>You are receiving this mail because:</span>
      
      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>