Closed Bug 1315858 Opened 3 years ago Closed 3 years ago

Test ability to detect screensharing sources that are firefox.

Categories

(Core :: WebRTC: Audio/Video, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: jib, Assigned: jib)

References

Details

Attachments

(2 files)

Comment on attachment 8808456 [details]
Bug 1315858 - Test ability to detect screensharing sources that are firefox.

https://reviewboard.mozilla.org/r/91260/#review91234

Looks reasonable to me, thanks for adding a test :-).

::: dom/media/tests/mochitest/test_getUserMedia_scarySources.html:38
(Diff revision 1)
> +
> +runTest(async () => {
> +  try {
> +    await pushPrefs(["media.navigator.permission.disabled", true],
> +                     ["media.navigator.permission.fake", true],
> +                     ["media.navigator.permission.force", true]);

nit: these 2 lines have one more space of indent than they need.
Attachment #8808456 - Flags: review?(florian) → review+
Comment on attachment 8808456 [details]
Bug 1315858 - Test ability to detect screensharing sources that are firefox.

https://reviewboard.mozilla.org/r/91260/#review91394
Attachment #8808456 - Flags: review?(rjesup) → review+
Pushed by jbruaroey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/621860d37e72
Test ability to detect screensharing sources that are firefox. r=florian,jesup
Why is XP off by default on try, when we're backing out over XP?
For the same reason that Win8 is off by default on try even though we back out over it: they both run on a limited pool of actual hardware, bought years ago by people who didn't have a good enough crystal ball to predict the ridiculous number of try runs people would do, or to predict how long we would keep dragging the corpse of WinXP around with us (the answer to that being: until next Monday, when it leaves the trunk, though we'll keep dragging it around on esr52 what will seem like forever).
 
We run Win7 on AWS, and will eventually run Win10 there as well, but if we (when we) had WinXP and Win8 on by default on try so that everybody who can't be bothered to think about what tests on what platforms they actually need to run and just run -p all -u all on everything ran it, then it would (did) take 24-36 hours to get results, by which time most of them had stopped looking without doing the right thing and cancelling, and just landed and been backed out already before their try jobs even ran.
Pushed by jbruaroey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c0d2974a8537
Test ability to detect screensharing sources that are firefox. r=florian,jesup
https://hg.mozilla.org/mozilla-central/rev/c0d2974a8537
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Hi, we should not add any new enablePrivilege() calls. Why did you need enablePrivilege() in test_getUserMedia_scarySources.html?
Flags: needinfo?(jib)
Sorry didn't know that. It's needed to call mozGetUserMediaDevices [1], a ChromeOnly method.

Would appreciate pointers on better ways to do this.

[1] https://dxr.mozilla.org/mozilla-central/rev/8103c612b79c2587ea4ca1b0a9f9f82db4b185b8/dom/webidl/Navigator.webidl#328-329
Flags: needinfo?(jib)
* Using SpecialPowers.wrapCallback() instead of enablePrivilege().
* I had to remove try-catch to spot the offending line. You should not use try-catch to swallow exceptions. Uncaught exceptions will cause a test failure automatically.

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=966ead0013c552d5ebf07647973bb30e738a9d8e
Attachment #8819465 - Flags: review?(jib)
Comment on attachment 8819465 [details] [diff] [review]
Stop using enablePrivilege() in test_getUserMedia_scarySources.html

Review of attachment 8819465 [details] [diff] [review]:
-----------------------------------------------------------------

Lgtm, thanks! Are you landing it off this bug, or opening a new one since this one is resolved already? Or do you want me to do it?
Attachment #8819465 - Flags: review?(jib) → review+
OK, I'll land it as a part of bug 1149966.
You need to log in before you can comment on or make changes to this bug.