<html>
    <head>
      <base href="https://bugs.webkit.org/">
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [SOUP] Use libsoup WebSockets API"
   href="https://bugs.webkit.org/show_bug.cgi?id=199151#c6">Comment # 6</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [SOUP] Use libsoup WebSockets API"
   href="https://bugs.webkit.org/show_bug.cgi?id=199151">bug 199151</a>
              from <span class="vcard"><a class="email" href="mailto:cgarcia@igalia.com" title="Carlos Garcia Campos <cgarcia@igalia.com>"> <span class="fn">Carlos Garcia Campos</span></a>
</span></b>
        <pre>(In reply to youenn fablet from <a href="show_bug.cgi?id=199151#c4">comment #4</a>)
<span class="quote">> Nice to see that.
> We have similar test issues with the NSURLSession code path that we will
> need to handle one way or the other.</span >

So, the NSURLSession impl is not used yet in production, and not tested by the bots either?

<span class="quote">> I run some of these tests with Chrome and Firefox and some of them fail with
> either one or both, which indicates that these tests might be WebKit
> implementation specific.</span >

I'll prioritize the ones that we knoe are bugs or missing features.

<span class="quote">> (In reply to Carlos Garcia Campos from <a href="show_bug.cgi?id=199151#c2">comment #2</a>)
> > As I said most of the tests are failing because of missing console messages,
> > so they are actually working and I won't mention them here.

> Right, I am not sure we will be able to get back the exact same missing
> console messages given that part of the messages are generated by processing
> done by libsoup/nsurlsession now.</span >

Indeed, if we keep using console messages for those events, we will need platform specific test results. I don't know if it's possible to ignore those messages (if we end up adding them back) in layout tests to not show them in the results.

<span class="quote">> > 
> > http/tests/websocket/tests/hybi/bad-handshake-crash.html
> > 
> > This needs investigation, "WebSocket is open" message is unexpectedly shown
> > in the output.

> I think we also have an issue there with NSURLSession.

> > http/tests/websocket/tests/hybi/client-close.html
> > 
> > I've debugged this and I don't understand why it fails, the diff is:
> > -PASS closeEvent.reason is "close_frame[:2]='\\x88\\x80'"
> > +FAIL closeEvent.reason should be close_frame[:2]='\x88\x80'. Was
> > close_frame[:2]='\x88\x82'.

> Same issue here with NSURLSession.</span >

Interesting, I debugged it for a while, but couldn't understand what's going on there.

<span class="quote">> > http/tests/websocket/tests/hybi/close-code-and-reason.html
> > 
> > This one expects error codes that the spec says shouldn't be used like 1006.
> > But close reason is also failing, so it needs some more investigation.
> > 
> > http/tests/websocket/tests/hybi/close.html
> > 
> > It seems onclose is not called in a couple of cases.
> > 
> > http/tests/websocket/tests/hybi/deflate-frame-parameter.html
> > http/tests/websocket/tests/hybi/extensions.html
> > 
> > x-webkit-deflate-frame extension not supported

> The idea would be to move from x-webkit-deflate-frame to the standard
> version permessage-deflate.</span >

Since it's in a spec now I think we could implement it directly in libsoup, otherwise I'll add new API to libsoup to implement extensions. I guess this doesn't cause any regression in functionality, only that frames are not compressed.

<span class="quote">> > 
> > http/tests/websocket/tests/hybi/handshake-fail-by-no-cr.html
> > 
> > Libsoup doesn't fail when status line finishes with \n since that's what the
> > http spec suggests.
> > 
> > http/tests/websocket/tests/hybi/handshake-ok-with-http-version-beyond-1_1.
> > html
> > 
> > There are a couple of cases where libsoup doesn't fail either.
> > 
> > http/tests/websocket/tests/hybi/inspector/*
> > 
> > It seems we are not notifying the inspector in some cases about websockets
> > with the new code path.

> I am not sure we will be ever able to get back the same level of inspector
> control with the new code path.</span >

I'll take a look.

<span class="quote">> > http/tests/websocket/tests/hybi/null-character.html
> > 
> > libsoup API always expect a null terminated string for text frames. I
> > haven't found anything in the spec about it, it just says that text frames
> > should contains valid utf8, and null character si perfectly valid, so maybe
> > we need to add new API in libsoup to allow passing char* + length.
> > 
> > http/tests/websocket/tests/hybi/send-object-tostring-check.html
> > 
> > It expects to be closed cleanly, but apparently it isn't.
> > 
> > http/tests/websocket/tests/hybi/simple-wss.html
> > 
> > We don't have a way to accept the invalid certificate only for tests.

> The plan is to reuse the existing NSURLSession code path for these.
> I would hope you could do the same for libsoup.</span >

I'll check it too. We should be able, since the handshake happens in the session like a normal load.

<span class="quote">> > http/tests/websocket/tests/hybi/inspector/binary.html
> > http/tests/websocket/tests/hybi/send-blob-onmessage-origin.html
> > http/tests/websocket/tests/hybi/send-blob.html
> > 
> > Blobs are not implemented in new code path.

> Yep, this will be needed to implement.
> This will require to queue additional outgoing messages until the blob is
> read/put in memory and sent.</span >

I'll take a look too.</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>