Bug 169216 – Expose WebRTC current/pending description getters

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[youenn fablet]

Following on bug 168234, we should tie current/pending description getters to libwebrtc

[youenn fablet]

Created attachment 303654 [details]
Patch

[Alex Christensen]

Comment on attachment 303654 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=303654&action=review

> LayoutTests/webrtc/descriptionGetters-expected.txt:2
> +PASS description getters when changing description from video to video & data channel 

I think we should print the description in the results so we can see what the description looks like and so we can see if we accidentally change the description format or something.

[youenn fablet]

The description will change all the time.
We could print out a sanitized SDP though.
We could also reuse the SDP parser from WebCore and add directly in the tests some SDP field checks.

[youenn fablet]

The SDP parser is at Source/WebCore/Modules/mediastream/sdp.js
It is a good idea to add some libwebrtc-specifc tests that will check more deeply the different parameters. I'll add some in LayoutTests/webrtc/libwebrtc

[WebKit Commit Bot]

Comment on attachment 303654 [details]
Patch

Clearing flags on attachment: 303654

Committed r213520: <http://trac.webkit.org/changeset/213520>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Carlos Alberto Lopez Perez]

The test webrtc/descriptionGetters.html added on r213520 it is failing on GTK+ (We use OpenWebRTC instead of libwebrtc), and it seems this test is created to match the behaviour of the the current implementation based on libwebrtc.


On GTK+ we are skipping all tests under webrtc/libwebrtc.

So if when you add a test for something that depends on libwebrtc, and you add it under webrtc/libwebrtc, then on the GTK+ port we won't run into new unexpected failures.


So.. do you think its a good idea if we move this test from webrtc/descriptionGetters.html to webrtc/libwebrtc/descriptionGetters.html ?

[youenn fablet]

(In reply to comment #7)
> The test webrtc/descriptionGetters.html added on r213520 it is failing on
> GTK+ (We use OpenWebRTC instead of libwebrtc), and it seems this test is
> created to match the behaviour of the the current implementation based on
> libwebrtc.

Ah right, it might be better to put it in libwebrtc for now.
At the end of the day, we want that very same test, but matching the spec, and working with both libwebrtc and OpenWebRTC :)

> So.. do you think its a good idea if we move this test from
> webrtc/descriptionGetters.html to webrtc/libwebrtc/descriptionGetters.html ?

yep

[Carlos Alberto Lopez Perez]

(In reply to comment #8)
> (In reply to comment #7)
> > The test webrtc/descriptionGetters.html added on r213520 it is failing on
> > GTK+ (We use OpenWebRTC instead of libwebrtc), and it seems this test is
> > created to match the behaviour of the the current implementation based on
> > libwebrtc.
> 
> Ah right, it might be better to put it in libwebrtc for now.
> At the end of the day, we want that very same test, but matching the spec,
> and working with both libwebrtc and OpenWebRTC :)
> 

Yes :)

> > So.. do you think its a good idea if we move this test from
> > webrtc/descriptionGetters.html to webrtc/libwebrtc/descriptionGetters.html ?
> 
> yep

Ok.. I will move it now. Thanks!

[Carlos Alberto Lopez Perez]

Committed r213526: <http://trac.webkit.org/changeset/213526>

[youenn fablet]

Thanks!