Closed Bug 1071623 Opened 9 years ago Closed 7 years ago

make the browser_devices_get_user_media.js test pass with e10s


(Firefox :: General, defect)

Not set



Firefox 46
Tracking Status
e10s + ---
firefox46 --- fixed


(Reporter: florian, Assigned: florian)


(Blocks 2 open bugs)



(1 file, 1 obsolete file)

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.
Flags: qe-verify-
Flags: firefox-backlog+
Blocks: 1080801
Blocks: 1213241
Attached patch Patch v1 (obsolete) — Splinter Review
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?
Attachment #8705743 - Flags: review?(felipc)
Assignee: nobody → florian
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.
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?
(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.
(In reply to Florian Quèze [:florian] [:flo] from comment #2)

This shows several different intermittent failures when the test runs with e10s :-(.
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?
Attachment #8705743 - Flags: review?(felipc) → feedback+
Attached patch Patch v2Splinter Review
After using ContentTask instead of CPOWs for the code touching content.wrappedJSObject.*, this is green on try:

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.
Attachment #8707558 - Flags: review?(felipc)
Attachment #8705743 - Attachment is obsolete: true
Attachment #8707558 - Flags: review?(felipc) → review+
Bug 1071623 - make the browser_devices_get_user_media.js test pass with e10s, r=felipe.
Depends on: 1239827
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46


STR: Not clear.
Addon Developer specific testing.

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

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.
You need to log in before you can comment on or make changes to this bug.