1313324 - Cover the screensharing UI with browser chrome tests

Status: RESOLVED FIXED

(Firefox :: Site Permissions, defect, P3)

Description

[Florian Quèze [:florian]]

The initial screensharing UI landed without any test. We are now polishing this UI with bug 1284877 and bug 1284878, and more importantly we are planning to remove the screensharing whitelist to make the feature available to all websites (bug 1127522). As part of this project I think we need to ensure the UI is well covered by tests.

Comment 1

[Florian Quèze [:florian]]

Attachment: WIP v1

Comment 2

[Florian Quèze [:florian]]

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

Comment 3

[Florian Quèze [:florian]]

Attachment: Patch v2

I think I'm done, but I'll wait to see the color of try results before requesting review.

Comment 4

[Florian Quèze [:florian]]

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

Comment 5

[Florian Quèze [:florian]]

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

Comment 6

[Florian Quèze [:florian]]

Attachment: Patch v3

Comment 7

[Florian Quèze [:florian]]

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

Comment 8

[Florian Quèze [:florian]]

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

Comment 9

[Florian Quèze [:florian]]

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

Comment 10

[Florian Quèze [:florian]]

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

Comment 11

[Florian Quèze [:florian]]

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

Comment 12

[Florian Quèze [:florian]]

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

Comment 13

[Florian Quèze [:florian]]

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

Comment 14

[Florian Quèze [:florian]]

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

Comment 15

[Florian Quèze [:florian]]

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

Comment 16

[Florian Quèze [:florian]]

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

Comment 17

[Florian Quèze [:florian]]

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

Comment 18

[Florian Quèze [:florian]]

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

Comment 19

[Florian Quèze [:florian]]

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

Comment 20

[Florian Quèze [:florian]]

Attachment: Patch v4

The colors I see on the try push from comment 19 are promising at this point, so I think it's time to request review. Panos, I picked you but feel free to redirect to whoever has cycles.

Before landing I intend to file bugs for the AppConstants.platform checks, to mention the bug numbers in the todo() comments.

Comment 21

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8816305 [details] [diff] [review]
Patch v4

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

::: browser/base/content/test/webrtc/browser_devices_get_user_media_screen.js
@@ +387,5 @@
> +    function* check(expected = {}) {
> +      let shared = Object.keys(expected).join(" and ");
> +      if (shared) {
> +        Assert.deepEqual((yield getMediaCaptureState()), expected,
> +                         "expected nothing to be shared");

"nothing" wouldn't be accurate when |expected| has a non-default value, right?

::: browser/base/content/test/webrtc/head.js
@@ +247,5 @@
> +  return new Promise((resolve, reject) => {
> +    let mm = _mm();
> +    mm.addMessageListener("Test:MessageReceived", function listener({data}) {
> +      resolve(data);
> +      mm.removeMessageListener("Test:MessageReceived", listener);

Nit: removing the listener before further action is the more common pattern to avoid inadvertent reentrancy. Is there a particular reason you want it like this here?

Comment 22

[Florian Quèze [:florian]]

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

Comment 23

[Florian Quèze [:florian]]

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

Comment 24

[Florian Quèze [:florian]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6e2e9bdc497579d2145440575a9954f3244feccf
Bug 1313324 - Cover the screensharing UI with browser chrome test, r=past.

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6e2e9bdc4975