We added the VIDEO_AS_CONTENT_SOURCE telemetry in bug 1299718, which is used to record the information of usage of a {visible / invisible} video element as the source of {drawImage(), createPattern(), createImageBitmap() and captureStream()} APIs. In this bug, I would like to extend the resolution of VIDEO_AS_CONTENT_SOURCE to know an invisible video element is in the document tree or not.

So, I'm wondering how do web developers draw an invisible video element. I guess that most of them create the video element by JavaScript and do NOT append the video element to the document because they don't need to show those video element at all. If the telemetry data supports my guess, we can then never suspend video decoders of those video elements which are not in the tree. In this way, the situation of bug 1295921 will be really rare and it will be much more acceptable to leak some fake frames while resuming video decoders.

Comment on attachment 8834331 [details] Bug 1337301 part 1 - Histogram for VIDEO_AS_CONTENT_SOURCE_IN_TREE_OR_NOT; https://reviewboard.mozilla.org/r/110306/#review111548 data-r=me ::: toolkit/components/telemetry/Histograms.json:9224 (Diff revision 1) > "bug_numbers": [1299718] > }, > + "VIDEO_AS_CONTENT_SOURCE_IN_TREE_OR_NOT" : { > + "alert_emails": ["ajones@mozilla.com", "kaku@mozilla.com"], > + "expires_in_version": "58", > + "description": "Usage of an invisible {in tree / not in tree} video element as the source of {drawImage(), createPattern(), createImageBitmap() and captureStream()} APIs. (0 = ALL_IN_TREE, 1 = ALL_NOT_IN_TREE, 2 = drawImage_IN_TREE, 3 = drawImage_NOT_IN_TREE, 4 = createPattern_IN_TREE, 5 = createPattern_NOT_IN_TREE, 6 = createImageBitmap_IN_TREE, 7 = createImageBitmap_NOT_IN_TREE, 8 = captureStream_IN_TREE, 9 = captureStream_NOT_IN_TREE)", The new categorical histogram type would be perfect for this. But you're welcome to keep this an enumeration if that's easier.

Comment on attachment 8834331 [details] Bug 1337301 part 1 - Histogram for VIDEO_AS_CONTENT_SOURCE_IN_TREE_OR_NOT; https://reviewboard.mozilla.org/r/110306/#review111710

Comment on attachment 8834332 [details] Bug 1337301 part 2 - modify the MarkAsContentSource() API; https://reviewboard.mozilla.org/r/110308/#review111712 r+ after comments are fixed: ::: dom/html/HTMLMediaElement.cpp:7154 (Diff revision 1) > } else { > // 3 = drawImage_INVISIBLE > Telemetry::Accumulate(Telemetry::VIDEO_AS_CONTENT_SOURCE, 3); > + > + if (IsInUncomposedDoc()) { > + // 0 = drawImage_IN_TREE This and all comments below start with "0 =" or "1 =" when it should be 2 or more.

Comment on attachment 8834331 [details] Bug 1337301 part 1 - Histogram for VIDEO_AS_CONTENT_SOURCE_IN_TREE_OR_NOT; https://reviewboard.mozilla.org/r/110306/#review111548 > The new categorical histogram type would be perfect for this. But you're welcome to keep this an enumeration if that's easier. Then let's keep it as an enumeration for now. Thanks for the review!

Comment on attachment 8834332 [details] Bug 1337301 part 2 - modify the MarkAsContentSource() API; https://reviewboard.mozilla.org/r/110308/#review111712 > This and all comments below start with "0 =" or "1 =" when it should be 2 or more. Right..., thanks, will modify them.

Thanks for the review!

Pushed by ihsiao@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bec479e9ea4a part 1 - Histogram for VIDEO_AS_CONTENT_SOURCE_IN_TREE_OR_NOT; r=bsmedberg,gerald https://hg.mozilla.org/integration/autoland/rev/0be683b20bf1 part 2 - modify the MarkAsContentSource() API; r=gerald

https://hg.mozilla.org/mozilla-central/rev/bec479e9ea4a https://hg.mozilla.org/mozilla-central/rev/0be683b20bf1

Comment on attachment 8834331 [details] Bug 1337301 part 1 - Histogram for VIDEO_AS_CONTENT_SOURCE_IN_TREE_OR_NOT; Approval Request Comment [Feature/Bug causing the regression]: This is not a regression, this bug adds telemetry to collect data. [User impact if declined]: We need the collected data to make development decisions, so the sooner we have the data, the sooner we can ship our feature. [Is this code covered by automated tests?]: No functional codes. Telemetry should already be covered by their tests. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: other patches in this bug. [Is the change risky?]: Not at all. [Why is the change risky/not risky?]: Because it is just a Telemetry. [String changes made/needed]: No.

Comment on attachment 8834332 [details] Bug 1337301 part 2 - modify the MarkAsContentSource() API; Approval Request Comment Same as comment 13.

Comment on attachment 8834332 [details] Bug 1337301 part 2 - modify the MarkAsContentSource() API; HTMLMediaElement telemetry, aurora53+, beta52+

https://hg.mozilla.org/releases/mozilla-aurora/rev/832f5ddaa6d5 https://hg.mozilla.org/releases/mozilla-aurora/rev/04dcfae8a165

https://hg.mozilla.org/releases/mozilla-beta/rev/cc6f316cc3e8 https://hg.mozilla.org/releases/mozilla-beta/rev/3f04711ccc4f

https://hg.mozilla.org/releases/mozilla-esr52/rev/cc6f316cc3e8 https://hg.mozilla.org/releases/mozilla-esr52/rev/3f04711ccc4f