1333289 - Add a web API to retrieve debugging data from the media element in an asynchronous way

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P3)

Description

[JW Wang [:jwwang] [:jw_wang]]

This is an asyn version of bug 1236703 because some media objects (ex: MDSM) can only be accessed off the main thread.

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1333289. Part 1 - add a function to the webidl to retrieve debugging data in an asynchronous way.

Review commit: https://reviewboard.mozilla.org/r/106756/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/106756/

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1333289. Part 2 - retrieve debugging data from MediaDecoder/MDSM.

Review commit: https://reviewboard.mozilla.org/r/106758/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/106758/

Comment 3

[Jean-Yves Avenard [:jya]]

Comment on attachment 8829751 [details]
Bug 1333289. Part 2 - retrieve debugging data from MediaDecoder/MDSM.

https://reviewboard.mozilla.org/r/106758/#review107776

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8829750 [details]
Bug 1333289. Part 1 - add a function to the webidl to retrieve debugging data in an asynchronous way.

https://reviewboard.mozilla.org/r/106756/#review108010

::: dom/html/HTMLMediaElement.cpp:1480
(Diff revision 1)
> +HTMLMediaElement::MozRequestDebugInfo()
> +{
> +  ErrorResult rv;
> +  RefPtr<Promise> promise = CreateDOMPromise(rv);
> +  if (NS_WARN_IF(rv.Failed())) {
> +    return nullptr;

r-, returns null without saying so in the IDL.

What you _probably_ wanted was to mark it [NewObject] and then throw on the incoming ErrorResult...

Comment 5

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8829750 [details]
Bug 1333289. Part 1 - add a function to the webidl to retrieve debugging data in an asynchronous way.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/106756/diff/1-2/

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8829751 [details]
Bug 1333289. Part 2 - retrieve debugging data from MediaDecoder/MDSM.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/106758/diff/1-2/

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8829750 [details]
Bug 1333289. Part 1 - add a function to the webidl to retrieve debugging data in an asynchronous way.

https://reviewboard.mozilla.org/r/106756/#review108122

r=me

Comment 8

[JW Wang [:jwwang] [:jw_wang]]

Thanks for the reviews!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5a910c074e14
Part 1 - add a function to the webidl to retrieve debugging data in an asynchronous way. r=bz
https://hg.mozilla.org/integration/autoland/rev/e72c737ab6ed
Part 2 - retrieve debugging data from MediaDecoder/MDSM. r=jya

Comment 10

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/autoland/rev/95df6209f125 for ASan mda thread-related leaks, https://treeherder.mozilla.org/logviewer.html#?job_id=71765321&repo=autoland

Comment 11

[JW Wang [:jwwang] [:jw_wang]]

(In reply to Phil Ringnalda (:philor) from comment #10)
> Backed out in https://hg.mozilla.org/integration/autoland/rev/95df6209f125
> for ASan mda thread-related leaks,
> https://treeherder.mozilla.org/logviewer.html#?job_id=71765321&repo=autoland

I don't think my patches are responsible for the leaks which are produced by webrtc. Furthermore, the code added by this bug will only be called by the about:media plug-in.

Comment 12

[Phil Ringnalda (:philor)]

Well, https://treeherder.mozilla.org/#/jobs?repo=autoland&tochange=95df6209f125277b6db62402897dd125f4fda5ae&fromchange=5346a3afb0364d70861dc732e66e087dc42b5e81&filter-searchStr=41d5f9ef579931d7a8ed9ede19057d71458c3baf&group_state=expanded seems to say there was a 10% chance of hitting that leak while your patches were in the tree, and a 0% chance of hitting it while they weren't.

Comment 13

[Phil Ringnalda (:philor)]

Or, maybe you made a new flavor of the bug 1326399 leak more common, or, maybe it was pure coincidence that it was more common right then, dunno.

Comment 14

[JW Wang [:jwwang] [:jw_wang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=203d11285cd7068435e021897e449fdb5edc3b53
5/121 failures with my patches.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bfbc3a54f08485c58b5b7a5cf0aea60e22d3b1f4
4/121 failures without my patches.

I don't really see a significant difference.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b68c0c1ae045
Part 1 - add a function to the webidl to retrieve debugging data in an asynchronous way. r=bz
https://hg.mozilla.org/integration/autoland/rev/0f69481eb5ad
Part 2 - retrieve debugging data from MediaDecoder/MDSM. r=jya

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b68c0c1ae045
https://hg.mozilla.org/mozilla-central/rev/0f69481eb5ad