From ef0b55f96beaf467ce9927b24ded470a6033af55 Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Mon, 4 Apr 2022 15:18:45 +0200 Subject: [PATCH 1/3] ice-restart: fix functionality by listening to the transport selectedcandidatepairchange event. This sample has been relying on the somewhat buggy but useful behavior of the "legacy" iceconnectionstatechange firing a "connected" event even if the previous state was already connected. This behavior is no longer supported in the spec iceconnectionstatechange event. Alternatively listen to the selectedcandidatepairchange event and determine the availability of new candidates based on that --- package.json | 2 +- .../peerconnection/restart-ice/js/main.js | 23 +++++++++++++++---- .../peerconnection/restart-ice/js/test.js | 3 +-- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index 401bcad8e..662696be1 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ ], "devDependencies": { "chai": "^4.3.6", - "chromedriver": "^98.0.1", + "chromedriver": ">98.0.1", "eslint": "^8.9.0", "eslint-config-google": "^0.14.0", "geckodriver": "^3.0.1", diff --git a/src/content/peerconnection/restart-ice/js/main.js b/src/content/peerconnection/restart-ice/js/main.js index ffb7b92d7..6e996789a 100644 --- a/src/content/peerconnection/restart-ice/js/main.js +++ b/src/content/peerconnection/restart-ice/js/main.js @@ -32,6 +32,8 @@ remoteVideo.addEventListener('loadedmetadata', function() { console.log(`Remote video videoWidth: ${this.videoWidth}px, videoHeight: ${this.videoHeight}px`); }); +const useSelectedCandidatePairChange = window.RTCIceTransport && 'onselectedcandidatepairchange' in RTCIceTransport.prototype; + remoteVideo.onresize = () => { console.log(`Remote video size changed to ${remoteVideo.videoWidth}x${remoteVideo.videoHeight}`); // We'll use the first onsize callback as an indication that video has started @@ -170,6 +172,18 @@ function onCreateAnswerSuccess(desc) { pc2.setLocalDescription(desc).then(() => onSetLocalSuccess(pc2), onSetSessionDescriptionError); console.log('pc1 setRemoteDescription start'); pc1.setRemoteDescription(desc).then(() => onSetRemoteSuccess(pc1), onSetSessionDescriptionError); + + if (useSelectedCandidatePairChange) { + pc1.getSenders()[0].transport.iceTransport.onselectedcandidatepairchange = () => { + checkStats(pc1); + if (pc1.iceConnectionState === 'connected') { + restartButton.disabled = false; + } + }; + pc2.getSenders()[0].transport.iceTransport.onselectedcandidatepairchange = () => { + checkStats(pc2); + }; + } } function onIceCandidate(pc, event) { @@ -191,10 +205,11 @@ function onIceStateChange(pc, event) { if (pc) { console.log(`${getName(pc)} ICE state: ${pc.iceConnectionState}`); console.log('ICE state change event: ', event); - // TODO: get rid of this in favor of http://w3c.github.io/webrtc-pc/#widl-RTCIceTransport-onselectedcandidatepairchange - if (pc.iceConnectionState === 'connected' || - pc.iceConnectionState === 'completed') { - checkStats(pc); + if (!useSelectedCandidatePairChange) { + if (pc.iceConnectionState === 'connected' || + pc.iceConnectionState === 'completed') { + checkStats(pc); + } } } } diff --git a/src/content/peerconnection/restart-ice/js/test.js b/src/content/peerconnection/restart-ice/js/test.js index 5e0ec5acc..133a0ed67 100644 --- a/src/content/peerconnection/restart-ice/js/test.js +++ b/src/content/peerconnection/restart-ice/js/test.js @@ -16,7 +16,7 @@ let driver; const path = '/src/content/peerconnection/restart-ice/index.html'; const url = `${process.env.BASEURL ? process.env.BASEURL : ('file://' + process.cwd())}${path}`; -describe.skip('peerconnection ice restart', () => { +describe('peerconnection ice restart', () => { before(() => { driver = seleniumHelpers.buildDriver(); }); @@ -58,7 +58,6 @@ describe.skip('peerconnection ice restart', () => { await driver.wait(() => driver.findElement(webdriver.By.id('restartButton')).isEnabled()); await driver.findElement(webdriver.By.id('restartButton')).click(); - await driver.wait(() => !driver.findElement(webdriver.By.id('restartButton')).isEnabled()); await driver.wait(() => driver.findElement(webdriver.By.id('restartButton')).isEnabled()); const secondCandidateIds = await Promise.all([ From 90c750b96981db9f995e174d26c7a9b2a1a08927 Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Tue, 5 Apr 2022 10:00:18 +0200 Subject: [PATCH 2/3] fix test description --- src/content/datachannel/filetransfer/js/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/content/datachannel/filetransfer/js/test.js b/src/content/datachannel/filetransfer/js/test.js index f25444840..21dfacb15 100644 --- a/src/content/datachannel/filetransfer/js/test.js +++ b/src/content/datachannel/filetransfer/js/test.js @@ -15,7 +15,7 @@ let driver; const path = '/src/content/datachannel/filetransfer/index.html'; const url = `${process.env.BASEURL ? process.env.BASEURL : ('file://' + process.cwd())}${path}`; -describe('datachannel basic', () => { +describe('datachannel filetransfer', () => { before(() => { driver = seleniumHelpers.buildDriver(); }); From b83125d8fd000081cef875c634a83de439fe42f1 Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Tue, 5 Apr 2022 09:51:57 +0200 Subject: [PATCH 3/3] specify test timeouts a total 5 minute job timeout should be enough and will prevent jobs from hanging for 30 minutes. Increase the mocha per-test timeout to five seconds (from the default 2s) --- .github/workflows/test.yml | 1 + package.json | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f56491aa0..fae3fe9d3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -14,6 +14,7 @@ jobs: test: needs: lint runs-on: ubuntu-latest + timeout-minutes: 5 strategy: matrix: browser: [chrome] diff --git a/package.json b/package.json index 662696be1..bd2ceaca1 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "start": "http-server . -c-1", "test": "npm run eslint && npm run stylelint", "eslint": "eslint 'src/content/**/*.js'", - "mocha": "mocha 'src/content/**/test.js'", + "mocha": "mocha --timeout 5000 'src/content/**/test.js'", "stylelint": "stylelint 'src/**/*.css'" }, "eslintIgnore": [