1524154 - Re-enable flow control for wpt that bug 1280629 disabled

Status: RESOLVED FIXED

(Core :: Networking, defect, P1)

Description

[Masatoshi Kimura [:emk]]

Flow control should not have been disabled for those tests in the first place.

Comment 1

[Masatoshi Kimura [:emk]]

Honestly, I don't understand why bug 1280629 has been landed with all failing tests disabled. It defeated the sole purpose of automated tests.

Comment 2

[Junior [inactive]]

Honestly, I don't understand why bug 1280629 has been landed with all failing tests disabled. It defeated the sole purpose of automated tests.

See bug 1280629 comment 51
I believe it should be under Testing/web-platform-tests

Comment 3

[Honza Bambas (:mayhemer)]

Junior, I think this needs some attention. It doesn't seem right to just disable test because of flow control in necko.

If there were a resume problem, then it was a serious regressing bug we caused.

P2 if flow control is now disabled on Release. This may even be P1 if it's enabled by default on Release.

Comment 4

[Junior [inactive]]

Let's see current status first
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a36ef32eba11c08feee9e93fb11ee016f0f83763

Comment 5

[Junior [inactive]]

too many noises, rebase and try again
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4e2ef3787e929297500c4e510f9ebd2c8594af6e

Comment 6

[Junior [inactive]]

Attachment: Bug 1524154 - Re-enable flow control for wpt

Comment 7

[Junior [inactive]]

(In reply to Junior [:junior] from comment #5)

too many noises, rebase and try again
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4e2ef3787e929297500c4e510f9ebd2c8594af6e

Note for myself: need to take care of
/html/semantics/embedded-content/the-video-element/video_initially_paused.html
/html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-move-to-other-document.html

Comment 8

[Junior [inactive]]

It's related to pause a <video>.
Theory: The nsHttpChannel seems failing to cancel or close after suspend (pause), and isn't removed from the loadgroup. MediaStream initiates another nsHttpChannel for range request afterward.

It's easier to reproduce in mac but not in linux.

Comment 9

[Honza Bambas (:mayhemer)]

P1 as the flow control feature is now enabled in Release and apparently we shipped a bug with it, not necessarily in Necko, but somewhere.

Comment 10

[Junior [inactive]]

findings:
(a) when it fails, the console showes NotAllowedError: The play method is not allowed by the user agent or the platform in the current context, possibly because the user denied permission.

(b) ./mach wpt can't reproduce. firefox-release can't reproduce. moz-regression can't reproduce
Firefox nightly (or self debug build) with local served wpt can reproduce.
And it's not clearly tangled with pref network.http.send_window_size

Comment 11

[Junior [inactive]]

Attachment: Bug 1524154 - Re-enable flow control for some wpt that bug 1280629 disabled

Enable those tests already pass in the treehereder

Comment 12

[Junior [inactive]]

pref media.autoplay.default is related to /html/semantics/embedded-content/media-elements/playing-the-media-resource/*

Those tests are with v.play() and v.onplaying, and the latter one is not called with e10s-bp.

Theory:
If we block autoplay, suspend will send at some time by media stream
(a) without e10s-bp, enough data could be sent before the media suspension. Hence, v.onplaying could be called.
(b) with e10s-bp, no data more than 1MB would be sent if the listener didn't handle.

Let's see treehereder for /html/semantics/embedded-content/the-video-element/video_initially_paused.html
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce146413b7e62105cfbef264c8271e4efef0dd09

Comment 13

[Junior [inactive]]

Hello Alastor,
We thought some autoplay problem in web-platform test are exposed by e10s-backpressure flow control.
e10s-backpressure flow control will suspend the underlying HTTP if the listener in child process doesn't handle OnDataAvailable on time.
Therefore, only 1/4 MB is sent before the HTTP channel is suspended.

Please see comment 12.
v.onplaying should not be triggered if the v.play is blocked. But these tests are still rely on the unreliable onplaying:

/html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-move-to-other-document.html
/html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-remove-from-document.html
/html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-move-within-document.html
/html/semantics/embedded-content/media-elements/video_008.htm

After we flip the media.autoplay.default to zero, those tests are fixed in local test and the intermittent rate goes down.

For this one,
/html/semantics/embedded-content/the-video-element/video_initially_paused.html

We try to allow the autoplay by setting pref media.autoplay.default to zero.
No timeout happen. (EXPECTED-FAIL is fixed in some platform, not sure why)

To me, I'd rather disable those tests with unreliable setup. What do you think?

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/52494bc29687
Re-enable flow control for some wpt that bug 1280629 disabled r=kershaw

Comment 15

[Alastor Wu [:alwu]]

Hi, Chris,
Do we have a pref list just for wpt test (like what we have for mochitest)? If not, it's possible to disable blocking autoplay for wpt by default?
Thank you.

Comment 16

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/52494bc29687

Comment 17

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Alastor Wu [:alwu] from comment #15)

Hi, Chris,
Do we have a pref list just for wpt test (like what we have for mochitest)? If not, it's possible to disable blocking autoplay for wpt by default?
Thank you.

Maybe:
https://searchfox.org/mozilla-central/source/testing/profiles/web-platform/user.js

... but I'd have though that

https://searchfox.org/mozilla-central/source/testing/profiles/common/user.js#54

.. would be included in the WPT user.js, and so we'd already have autoplay unblocked for WPT?

Comment 18

[Alastor Wu [:alwu]]

Hmm, I tested it on my local environment, it seems that we didn't block autoplay for wpt. Are you sure that we're blocking autoplay for those test you mentioned?

Comment 19

[Junior [inactive]]

(In reply to Alastor Wu [:alwu] from comment #18)

Hmm, I tested it on my local environment, it seems that we didn't block autoplay for wpt. Are you sure that we're blocking autoplay for those test you mentioned?

I can reproduce it with local served wpt. (i.e., clone wpt, and host a local wpt server) However, it would be a dup of bug 1539766. Stay tuned.

Comment 20

[Junior [inactive]]

Seems like a dup of bug 1539766, let's enable wpts once we land it.
https://hg.mozilla.org/try/rev/fe819d5d1dbee45c8a1d25f01fdfefecc0d936ee

Comment 21

[Junior [inactive]]

Attachment: Bug 1524154 - Re-enable flow control for media wpt

Comment 22

[Junior [inactive]]

bug 1539766 landed. treeherder looks good. Let's give it a try.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/799599db47f1
Re-enable flow control for media wpt r=kershaw

Comment 24

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/799599db47f1

Comment 25

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0b4923e63c70