Turn on support for mochitests in webrtc on FxAndroid.

I've added a patch here so that we can grab this and run a try build run to find out what failures we get when we run the mochitests on Android.

Looking at the try run, looks like we passed on all gUM tests, so we can land a patch to turn those on right now. However, every single peer connection test failed here with a reference error of "mozRTCPeerConnection is not defined." I thought the build patch that landed for WebRTC enabled PC support, but the tests are failing here on reference errors. gcp - Any ideas why?

Does the test put the media.peerconnection.enabled flag on Android?

(In reply to Gian-Carlo Pascutto (:gcp) from comment #5) > Does the test put the media.peerconnection.enabled flag on Android? Yes. See the attached patch - it runs all tests with media.peerconnection.enabled set to true.

Talked this over with Randell in the weekly meeting - he thinks the problem is that the build be generated on try with the mochitests isn't being compiled with the MOZ_WEBRTC flag, so the Peer Connection bits aren't being picked up. I'll follow up with gcp in next week's meeting to find out why.

Putting needinfo on gcp for comment 7 in case he has ideas why the builds being ran in try are not building the WebRTC code on FxAndroid correctly.

>Yes. See the attached patch - it runs all tests with media.peerconnection.enabled >set to true. But it doesn't set media.navigator.enabled to true.

(In reply to Gian-Carlo Pascutto (:gcp) from comment #9) > >Yes. See the attached patch - it runs all tests with media.peerconnection.enabled > >set to true. > > But it doesn't set media.navigator.enabled to true. That shouldn't matter. We landed a patch originally that removed the requirement that media.navigator.enabled had to be set. If that was true, then I'd expect the gUM tests to be failing right now. But that's not happening here.

Thing is I'm chasing mochitest crashes, and I don't have this problem at all: https://tbpl.mozilla.org/?tree=Try&rev=43d69806d706

Hmm...okay. Maybe we changed something then after we preffed gUM or PC on in desktop. Okay, I'll modify the patch to enable both then.

So I had another try push today that didn't include the prefs flip: https://tbpl.mozilla.org/?tree=Try&rev=286bad8d4740 Don't see the problem there at all.

(In reply to Gian-Carlo Pascutto (:gcp) from comment #13) > So I had another try push today that didn't include the prefs flip: > https://tbpl.mozilla.org/?tree=Try&rev=286bad8d4740 > > Don't see the problem there at all. Okay. I'll update the above patch for something that can be landed then.

I just landed the code to make things pass on inbound, so a patch to enable them would be beat.

..neat.

For people flagged down for review/feedback: - review on gcp to ensure these tests then effectively will run on FxAndroid - feedback on whimboo to see if he's okay with the framework changes to move towards the exclude list model, rather than the weird UA stuff we were doing originally - feedback on mw22 to ensure I did the b2g.json setup correct

Comment on attachment 744701 [details] [diff] [review] Enable WebRTC mochitests on FxAndroid v1 Apparently the perm pref is defined incorrectly.

Comment on attachment 744706 [details] [diff] [review] Enable WebRTC mochitests on FxAndroid v2 This time, with the right config pref.

Comment on attachment 744706 [details] [diff] [review] Enable WebRTC mochitests on FxAndroid v2 Looks good to me, but you prolly want an r+ from an actual peer (I know next to nothing about the rests).

Comment on attachment 744706 [details] [diff] [review] Enable WebRTC mochitests on FxAndroid v2 Review of attachment 744706 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine to me except for the question you can find below. ::: dom/media/tests/mochitest/head.js @@ +116,3 @@ > SimpleTest.waitForExplicitFinish(); > + SpecialPowers.pushPrefEnv({'set': [ > + ['media.peerconnection.enabled', true], We should also remove this preference given that peer connection is enabled by default. ::: testing/mochitest/b2g.json @@ +203,5 @@ > "dom/file/test/test_workers.html":"", > "dom/file/test/test_write_read_data.html":"", > "dom/imptests/editing/conformancetest/test_event.html":"", > "dom/imptests/editing/conformancetest/test_runtest.html":"", > + "dom/media/tests/mochitest":"", Can we already run all our mochitests on B2G?

(In reply to Henrik Skupin (:whimboo) from comment #25) > Comment on attachment 744706 [details] [diff] [review] > Enable WebRTC mochitests on FxAndroid v2 > > Review of attachment 744706 [details] [diff] [review]: > ----------------------------------------------------------------- > > Looks fine to me except for the question you can find below. > > ::: dom/media/tests/mochitest/head.js > @@ +116,3 @@ > > SimpleTest.waitForExplicitFinish(); > > + SpecialPowers.pushPrefEnv({'set': [ > > + ['media.peerconnection.enabled', true], > > We should also remove this preference given that peer connection is enabled > by default. Actually, we can't right now. FxAndroid requires the preference to be set in order to use the mozRTCPeerConnection API right now (i.e. it's preffed off by default). > > ::: testing/mochitest/b2g.json > @@ +203,5 @@ > > "dom/file/test/test_workers.html":"", > > "dom/file/test/test_write_read_data.html":"", > > "dom/imptests/editing/conformancetest/test_event.html":"", > > "dom/imptests/editing/conformancetest/test_runtest.html":"", > > + "dom/media/tests/mochitest":"", > > Can we already run all our mochitests on B2G? Don't think so. The WebRTC APIs are not implemented on B2G yet, so the tests will fail if we run them on B2G right now. This is being worked on though.

Try run looks green outside of the one failure we already know about in bug 866093. That's been fixed, so we should be good to land so long as I get the r+ from jesup.

(In reply to Jason Smith [:jsmith] from comment #26) > > ::: testing/mochitest/b2g.json > > @@ +203,5 @@ > > > "dom/file/test/test_workers.html":"", > > > "dom/file/test/test_write_read_data.html":"", > > > "dom/imptests/editing/conformancetest/test_event.html":"", > > > "dom/imptests/editing/conformancetest/test_runtest.html":"", > > > + "dom/media/tests/mochitest":"", > > > > Can we already run all our mochitests on B2G? > > Don't think so. The WebRTC APIs are not implemented on B2G yet, so the tests > will fail if we run them on B2G right now. This is being worked on though. So what does it then mean when we add this line? Doesn't it include all of the mochitests in the current set of executable tests?

(In reply to Henrik Skupin (:whimboo) from comment #28) > (In reply to Jason Smith [:jsmith] from comment #26) > > > ::: testing/mochitest/b2g.json > > > @@ +203,5 @@ > > > > "dom/file/test/test_workers.html":"", > > > > "dom/file/test/test_write_read_data.html":"", > > > > "dom/imptests/editing/conformancetest/test_event.html":"", > > > > "dom/imptests/editing/conformancetest/test_runtest.html":"", > > > > + "dom/media/tests/mochitest":"", > > > > > > Can we already run all our mochitests on B2G? > > > > Don't think so. The WebRTC APIs are not implemented on B2G yet, so the tests > > will fail if we run them on B2G right now. This is being worked on though. > > So what does it then mean when we add this line? Doesn't it include all of > the mochitests in the current set of executable tests? It's the opposite. The line I'm adding here adds the directory of tests to the exclude list, not the include list. See http://hg.mozilla.org/mozilla-central/file/da429c311864/testing/mochitest/b2g.json.

Comment on attachment 744706 [details] [diff] [review] Enable WebRTC mochitests on FxAndroid v2 Thanks!

Comment on attachment 744706 [details] [diff] [review] Enable WebRTC mochitests on FxAndroid v2 Review of attachment 744706 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/tests/mochitest/head.js @@ +116,3 @@ > SimpleTest.waitForExplicitFinish(); > + SpecialPowers.pushPrefEnv({'set': [ > + ['media.peerconnection.enabled', true], We need to keep the ability for people to disable it, at least to provide a firestop for security issues