Remove LocalMediaStream (and its Stop()) from js

RESOLVED FIXED in Firefox 64

Status

()

defect
P2
normal
Rank:
19
RESOLVED FIXED
3 years ago
5 months ago

People

(Reporter: RyanVM, Assigned: pehrsons)

Tracking

({dev-doc-complete, site-compat})

47 Branch
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
Saw some logspam fly by recently warning about the deprecation. Seems trivial enough to change.
Rank: 25
Priority: -- → P2
(Assignee)

Comment 1

3 years ago
Before we can do this we have to

1 change HTMLMediaElement to end when a stream becomes inactive
2 let all tracks (including remote ones) be stoppable. This was recently changed in the spec iirc.
(In reply to Andreas Pehrson [:pehrsons] (Telenor) from comment #1)
> Before we can do this we have to
> 
> 1 change HTMLMediaElement to end when a stream becomes inactive
> 2 let all tracks (including remote ones) be stoppable. This was recently
> changed in the spec iirc.

Have both of these things happened now?

fippo just alerted me to that Chrome no longer supports MediaStream.stop() and we've had deprecation warnings since 44, so I think it's time to remove this feature from the platform (as well as remove tests, hijacking this bug #).
Rank: 25 → 19
Flags: needinfo?(pehrson)
Priority: P2 → P1
Summary: Remove mediaStream.stop() from the media tests → Remove mediaStream.stop() support entirely (as well as from media tests)
(Assignee)

Comment 3

3 years ago
No. But we're close.
Depends on: 1208316, 1301675
Flags: needinfo?(pehrson)
(Assignee)

Comment 4

3 years ago
We're good now. Do you want to kill it already in 52 or wait for 53?

The reason I ask is because bug 1208316 landed in 52 and changed how media elements end when they play MediaStreams. They used to end when the input stream ended, either by chrome or stop(). Though we only had stop() on LocalMediaStream, which only getUserMedia used. Now a media element ends as the MediaStream goes inactive.

We should also kill LocalMediaStream as part of this bug.
Flags: needinfo?(jib)
Would love to get it in for ESR
Flags: needinfo?(jib)
Mass change P1->P2 to align with new Mozilla triage process
Priority: P1 → P2
(Assignee)

Updated

7 months ago
Assignee: nobody → apehrson
Status: NEW → ASSIGNED
Summary: Remove mediaStream.stop() support entirely (as well as from media tests) → Remove LocalMediaStream (and its Stop()) from js
Duplicate of this bug: 1494213

Comment 11

7 months ago
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/905c871bcf03
Remove LocalMediaStream. r=jib,smaug
https://hg.mozilla.org/integration/autoland/rev/514420f15a67
Test that MediaStream is the primary interface of a getUserMedia-stream. r=jib
Backed out 2 changesets (Bug 1258143) for dom/media/tests/crashtests/791330.html failures

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=205078511&revision=514420f15a67df92c218191b23a3b4bcd4b7ea95

Backout link: https://hg.mozilla.org/integration/autoland/rev/2fe9ae64504697968b89d5ec4ef0d027a0dc46fa

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=205078511&repo=autoland&lineNumber=26647

07:03:32     INFO - --DOMWINDOW == 6 (0x1220bb000) [pid = 795] [serial = 3076] [outer = 0x0] [url = file:///Users/cltbld/tasks/task_1539352695/build/tests/reftest/tests/dom/media/test/crashtests/oscillator-ended-2.html]
07:03:32     INFO - --DOMWINDOW == 5 (0x121582c00) [pid = 795] [serial = 3090] [outer = 0x0] [url = file:///Users/cltbld/tasks/task_1539352695/build/tests/reftest/tests/dom/media/tests/crashtests/791278.html]
07:03:32     INFO - --DOMWINDOW == 4 (0x12b0c0400) [pid = 795] [serial = 3081] [outer = 0x0] [url = data:text/html;charset=UTF-8,%3C%21%2D%2DCLEAR%2D%2D%3E]
07:03:40     INFO - [Parent 789, Main Thread] WARNING: Suboptimal indexes for the SQL statement 0x1223f8c70 (http://mzl.la/1FuID0j).: file /builds/worker/workspace/build/src/storage/mozStoragePrivateHelpers.cpp, line 114
07:08:24     INFO - REFTEST TEST-UNEXPECTED-FAIL | file:///Users/cltbld/tasks/task_1539352695/build/tests/reftest/tests/dom/media/tests/crashtests/791330.html | load failed: timed out waiting for reftest-wait to be removed
07:08:24     INFO - REFTEST INFO | Saved log: START file:///Users/cltbld/tasks/task_1539352695/build/tests/reftest/tests/dom/media/tests/crashtests/791330.html
07:08:24     INFO - REFTEST INFO | Saved log: [CONTENT] OnDocumentLoad triggering WaitForTestEnd
07:08:24     INFO - REFTEST INFO | Saved log: [CONTENT] WaitForTestEnd: Adding listeners
07:08:24     INFO - REFTEST INFO | Saved log: Initializing canvas snapshot
07:08:24     INFO - REFTEST INFO | Saved log: [CONTENT] MakeProgress: STATE_WAITING_TO_FIRE_INVALIDATE_EVENT
07:08:24     INFO - REFTEST INFO | Saved log: [CONTENT] MakeProgress: dispatching MozReftestInvalidate
07:08:24     INFO - REFTEST INFO | Saved log: [CONTENT] MakeProgress: STATE_WAITING_FOR_REFTEST_WAIT_REMOVAL
07:08:24     INFO - REFTEST INFO | Saved log: [CONTENT] MakeProgress: waiting for reftest-wait to be removed
07:08:24     INFO - REFTEST INFO | Saved log: [CONTENT] AfterPaintListener in file:///Users/cltbld/tasks/task_1539352695/build/tests/reftest/tests/dom/media/tests/crashtests/791330.html
07:08:24     INFO - REFTEST INFO | Saved log: [CONTENT] SendUpdateCanvasForEvent with 0 rects
07:08:24     INFO - REFTEST INFO | Saved log: Updating canvas for invalidation
07:08:24     INFO - REFTEST INFO | Saved log: [CONTENT] AfterPaintListener in file:///Users/cltbld/tasks/task_1539352695/build/tests/reftest/tests/dom/media/tests/crashtests/791330.html
07:08:24     INFO - REFTEST INFO | Saved log: [CONTENT] SendUpdateCanvasForEvent with 1 rects
07:08:24     INFO - REFTEST INFO | Saved log: [CONTENT] Rect: 0 0 800 1000
07:08:24     INFO - REFTEST INFO | Saved log: Updating canvas for invalidation
07:08:24     INFO - REFTEST INFO | Saved log: [CONTENT] MakeProgress: STATE_WAITING_FOR_REFTEST_WAIT_REMOVAL
07:08:24     INFO - REFTEST INFO | Saved log: [CONTENT] MakeProgress: waiting for reftest-wait to be removed
07:08:24     INFO - REFTEST INFO | Saved log: [CONTENT] MakeProgress: STATE_WAITING_FOR_REFTEST_WAIT_REMOVAL
07:08:24     INFO - REFTEST INFO | Saved log: [CONTENT] MakeProgress: waiting for reftest-wait to be removed
07:08:24     INFO - REFTEST TEST-END | file:///Users/cltbld/tasks/task_1539352695/build/tests/reftest/tests/dom/media/tests/crashtests/791330.html
Flags: needinfo?(apehrson)
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13496 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Upstream PR was closed without merging
(Assignee)

Comment 16

6 months ago
Thanks Bogdan. Looks like I forgot to run crashtests when testing this.
Flags: needinfo?(apehrson)

Comment 17

6 months ago
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/9508a03be7c2
Remove LocalMediaStream. r=jib,smaug
https://hg.mozilla.org/integration/autoland/rev/92b4fac6ea6c
Test that MediaStream is the primary interface of a getUserMedia-stream. r=jib
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 19

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9508a03be7c2
https://hg.mozilla.org/mozilla-central/rev/92b4fac6ea6c
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Upstream PR merged
Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/13496
* Taskcluster (pull_request) (https://tools.taskcluster.net/task-group-inspector/#/H9zp5O1xTdO203eD7tpITw)
Note to docs team:

I've added a note about this to the Fx64 rel notes:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/64#APIs

The LocalMediaStream page has already been marked as obsolete. You should delete the page and update BCD.
I would prefer not to delete the page outright at this time. Being marked as obsolete is good, and I've added a big note, made some other additions, and changed the page's SEO summary to direct people to information about updating old code to further emphasize matters. I've also updated Firefox 64 for developers to link to that same information.

I've submitted a PR to update BCD. That closes out this issue.
You need to log in before you can comment on or make changes to this bug.