169389 – Support PeerConnectionStates::BundlePolicy::MaxBundle when setting rtc configuration

RESOLVED FIXED 169389

Support PeerConnectionStates::BundlePolicy::MaxBundle when setting rtc configuration

https://bugs.webkit.org/show_bug.cgi?id=169389

Summary Support PeerConnectionStates::BundlePolicy::MaxBundle when setting rtc config...

youenn fablet

Reported 2017-03-08 15:48:43 PST

libwebrtc does not like it currently. Setting MaxBundle will then remove the possibility to set other fields.

Attachments
Patch (2.30 KB, patch) 2017-03-08 15:50 PST, youenn fablet	no flags	Details Formatted Diff Diff
Patch (13.50 KB, patch) 2017-06-27 11:00 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews105 for mac-elcapitan-wk2 (1.43 MB, application/zip) 2017-06-27 12:19 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews124 for ios-simulator-wk2 (1.03 MB, application/zip) 2017-06-27 12:42 PDT, Build Bot	no flags	Details
Patch (27.34 KB, patch) 2017-06-28 10:33 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch (23.22 KB, patch) 2017-06-28 11:16 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Correcting style and adding bug link (24.21 KB, patch) 2017-06-28 14:28 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch (1.58 KB, patch) 2017-06-29 14:16 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Disabling ice candidate pool size (24.61 KB, patch) 2017-06-30 07:22 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

youenn fablet

Comment 1 2017-03-08 15:50:50 PST

Created attachment 303855 [details] Patch

WebKit Commit Bot

Comment 2 2017-03-08 17:35:41 PST

Comment on attachment 303855 [details] Patch Clearing flags on attachment: 303855 Committed r213613: <http://trac.webkit.org/changeset/213613>

WebKit Commit Bot

Comment 3 2017-03-08 17:35:45 PST

All reviewed patches have been landed. Closing bug.

youenn fablet

Comment 4 2017-03-08 20:09:57 PST

We should find a way to support this option.

youenn fablet

Comment 5 2017-06-27 10:55:10 PDT

rdar://problem/33006091

youenn fablet

Comment 6 2017-06-27 11:00:10 PDT

Created attachment 313929 [details] Patch

Build Bot

Comment 7 2017-06-27 12:19:00 PDT

Comment on attachment 313929 [details] Patch Attachment 313929 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4007096 New failing tests: fast/mediastream/RTCSessionDescription.html imported/w3c/web-platform-tests/webrtc/RTCDataChannel-id.html imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-constructor.html

Build Bot

Comment 8 2017-06-27 12:19:01 PDT

Created attachment 313932 [details] Archive of layout-test-results from ews105 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6

Build Bot

Comment 9 2017-06-27 12:42:53 PDT

Comment on attachment 313929 [details] Patch Attachment 313929 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4007114 New failing tests: imported/w3c/web-platform-tests/webrtc/RTCDataChannel-id.html imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-createAnswer.html imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-constructor.html

Build Bot

Comment 10 2017-06-27 12:42:55 PDT

Created attachment 313934 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.5

youenn fablet

Comment 11 2017-06-28 10:33:06 PDT

Created attachment 314038 [details] Patch

Alex Christensen

Comment 12 2017-06-28 10:35:41 PDT

Comment on attachment 314038 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=314038&action=review > LayoutTests/fast/mediastream/RTCPeerConnection-getConfiguration-expected.txt:56 > -PASS successfullyParsed is true > +FAIL iceServers.length should be 1. Was 2. This seems like a test regression.

youenn fablet

Comment 13 2017-06-28 11:04:02 PDT

(In reply to Alex Christensen from comment #12) > Comment on attachment 314038 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=314038&action=review > > > LayoutTests/fast/mediastream/RTCPeerConnection-getConfiguration-expected.txt:56 > > -PASS successfullyParsed is true > > +FAIL iceServers.length should be 1. Was 2. > > This seems like a test regression. Right. Previously, we were trying to set the configuration, failing to do so at libwebrtc backend but reporting that this was done at RTCPeerConnection level. With the changes done to RTCPeerConnection::setConfiguration, we will report more accurately the actual configuration.

Alex Christensen

Comment 14 2017-06-28 11:10:53 PDT

Comment on attachment 314038 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=314038&action=review > Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp:83 > + else if (configuration.bundlePolicy == RTCBundlePolicy::MaxBundle) This should probably be a switch with no default so we will see compiler failures if another one is added.

youenn fablet

Comment 15 2017-06-28 11:16:52 PDT

Created attachment 314043 [details] Patch

youenn fablet

Comment 16 2017-06-28 11:17:28 PDT

(In reply to youenn fablet from comment #15) > Created attachment 314043 [details] > Patch Reduced patch without the setConfiguration behavior change.

youenn fablet

Comment 17 2017-06-28 11:18:06 PDT

(In reply to Alex Christensen from comment #14) > Comment on attachment 314038 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=314038&action=review > > > Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp:83 > > + else if (configuration.bundlePolicy == RTCBundlePolicy::MaxBundle) > > This should probably be a switch with no default so we will see compiler > failures if another one is added. Will do that as a follow-up or at landing time. I probably also need to update the fixes about the setConfiguration issue with a bugzilla entry.

youenn fablet

Comment 18 2017-06-28 14:27:16 PDT

(In reply to youenn fablet from comment #16) > (In reply to youenn fablet from comment #15) > > Created attachment 314043 [details] > > Patch > > Reduced patch without the setConfiguration behavior change. Filed bug 173938 for setConfiguration issue

youenn fablet

Comment 19 2017-06-28 14:28:09 PDT

Created attachment 314056 [details] Correcting style and adding bug link

WebKit Commit Bot

Comment 20 2017-06-28 16:24:45 PDT

Comment on attachment 314056 [details] Correcting style and adding bug link Clearing flags on attachment: 314056 Committed r218903: <http://trac.webkit.org/changeset/218903>

WebKit Commit Bot

Comment 21 2017-06-28 16:24:47 PDT

All reviewed patches have been landed. Closing bug.

youenn fablet

Comment 22 2017-06-29 13:31:27 PDT

This patch makes more tests flaky in the bots. The only change I see is that we are now really disabling CPU overuse detection with this patch. This might cause issues with the bots. If that is the case, we might actually want to activate CPU overuse detection, at least on the bots.

youenn fablet

Comment 23 2017-06-29 14:16:26 PDT

Reopening to attach new patch.

youenn fablet

Comment 24 2017-06-29 14:16:27 PDT

Created attachment 314163 [details] Patch

youenn fablet

Comment 25 2017-06-29 14:17:18 PDT

(In reply to youenn fablet from comment #24) > Created attachment 314163 [details] > Patch Patch to disable cpu overuse detection. This will allow verifying whether this is what is causing issues with bots.

WebKit Commit Bot

Comment 26 2017-06-29 15:04:45 PDT

The commit-queue encountered the following flaky tests while processing attachment 314163 [details]: workers/bomb.html bug 171985 (author: fpizlo@apple.com) The commit-queue is continuing to process your patch.

WebKit Commit Bot

Comment 27 2017-06-29 15:05:13 PDT

Comment on attachment 314163 [details] Patch Clearing flags on attachment: 314163 Committed r218963: <http://trac.webkit.org/changeset/218963>

WebKit Commit Bot

Comment 28 2017-06-29 15:05:14 PDT

All reviewed patches have been landed. Closing bug.

Matt Lewis

Comment 29 2017-06-29 16:58:22 PDT

I looks like the patch did not fix the issue with the test it is still failing on all WK2 testers. https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=imported%2Fw3c%2Fweb-platform-tests%2Fwebrtc%2Fsimplecall.html https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK2%20(Tests)/builds/2628 Talked with Youenn, rolling out both patches.

Matt Lewis

Comment 30 2017-06-29 17:14:59 PDT

Reverted r218963 for reason: This patch and its fix cause immediate flakiness on all WK2 testers Committed r218972: <http://trac.webkit.org/changeset/218972>

Matt Lewis

Comment 31 2017-06-29 17:16:01 PDT

Reverted r218903 for reason: This patch and its fix cause immediate flakiness on all WK2 testers Committed r218973: <http://trac.webkit.org/changeset/218973>

youenn fablet

Comment 32 2017-06-30 07:22:50 PDT

Created attachment 314265 [details] Disabling ice candidate pool size

youenn fablet

Comment 33 2017-06-30 07:23:36 PDT

(In reply to youenn fablet from comment #32) > Created attachment 314265 [details] > Disabling ice candidate pool size WK2 testers might have issues with ice candidate pool size

WebKit Commit Bot

Comment 34 2017-06-30 08:00:46 PDT

Comment on attachment 314265 [details] Disabling ice candidate pool size Clearing flags on attachment: 314265 Committed r218994: <http://trac.webkit.org/changeset/218994>

WebKit Commit Bot

Comment 35 2017-06-30 08:00:48 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware Unspecified

OS Unspecified

Product WebKit

Component WebCore Misc.

Assignee

youenn fablet

Reported

2017-03-08 15:48 PST

Modified

2017-06-30 08:00 PDT History

CC List

7 users Show

URL

Keywords InRadar

Depends on

Blocks