1071623 - make the browser_devices_get_user_media.js test pass with e10s

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Florian Quèze [:florian]]

The UI for getUserMedia has been fixed in bug 973001 to work with e10s enabled, but the test for this piece of UI doesn't work with e10s and is currently disabled. We should fix it.

Comment 1

[Florian Quèze [:florian]]

Attachment: Patch v1

The things that prevented this test from working with e10s weren't exactly what I expected. I assumed all the things touching content.* and especially the things using content.wrappedJSObject.* to call functions from the page loaded in the tab would be broken, but it seems to be working just fine using CPOWs. Are CPOWs still OK to use in tests?

Comment 2

[Florian Quèze [:florian]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=daab5576d253

Comment 3

[:Felipe Gomes (needinfo for replies!)]

They are still OK as long as they don't escape the add-on compartment.. I think most usage here looks ok, but it would be nice to test with the patches from bug 1233497 to make sure it doesn't break when that lands.

Comment 4

[:Felipe Gomes (needinfo for replies!)]

Although it's unclear how `MediaManagerService.mediaCaptureWindowState(content, hasVideo, hasAudio)` is working, with our without bug 1233497, as you can't pass CPOWs to native code.. Do you know why that works?

Comment 5

[Florian Quèze [:florian]]

(In reply to :Felipe Gomes (needinfo me!) from comment #4)
> Although it's unclear how
> `MediaManagerService.mediaCaptureWindowState(content, hasVideo, hasAudio)`
> is working, with our without bug 1233497, as you can't pass CPOWs to native
> code.. Do you know why that works?

That doesn't work with e10s. My patch here moves this code to a framescript.

Comment 6

[Florian Quèze [:florian]]

(In reply to Florian Quèze [:florian] [:flo] from comment #2)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=daab5576d253

This shows several different intermittent failures when the test runs with e10s :-(.

Comment 7

[Florian Quèze [:florian]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=59c5dedc1000

Comment 8

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8705743 [details] [diff] [review]
Patch v1

Ok, I've taking a look but I'll finish the review once these failures are figured out.  It was easy to miss the `let frameScript = function() {` at the beginning of the file, and since this is a big file, it's easy to not realize that and think this is running in the parent because it is a browser_*.js file.

that is to say, could you move the framescript part to a separate file (through support_files) and load it instead of keeping it all together?

Comment 9

[Florian Quèze [:florian]]

Attachment: Patch v2

After using ContentTask instead of CPOWs for the code touching content.wrappedJSObject.*, this is green on try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=119e98811f5b

I also added a few enableDevice calls, these let the test run correctly in a loop (--run-until-failure).

I agree the framescript would be better in a separate file, but I would like to do this in a follow-up bug at the same time as fixing the other two browser_devices_get_user_media*.js tests.

Comment 10

[Florian Quèze [:florian]]

https://hg.mozilla.org/integration/fx-team/rev/a2997ea0c867abb7d53254e0125fdd452a91022f
Bug 1071623 - make the browser_devices_get_user_media.js test pass with e10s, r=felipe.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a2997ea0c867

Comment 12

[Mayur Patil]

[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

STR: Not clear.
Addon Developer specific testing.

Component: 
Name 			Firefox
Version 		46.0b4
Build ID 		20160322075646
Update Channel 	        beta
User Agent 		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS                      Windows 7 SP1 x86_64

Comment 13

[:Gijs (he/him)]

https://bugzilla.mozilla.org/buglist.cgi?quicksearch=browser_devices_get_user_media.js&list_id=12942609

has 3 e10s failures, so it's clearly running, so marking verified. Dunno to what degree the e10s failures are because of this bug or because of other changes.