Bug 56497 – Add client callback on seeing and audio and video tags

• Component: WebCore Misc.
• Status: RESOLVED WONTFIX

[Shishir Agrawal]

Proposal: Add client callbacks when <audio> and <video> tags are encountered while parsing a page. 

This could be useful in multiple cases, and right now we would like to use for the pre-rendering use case in chrome.

[Shishir Agrawal]

Created attachment 85995 [details]
Patch

[WebKit Review Bot]

Attachment 85995 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebKit/chromium/ChangeLog:13:  Line contains tab character.  [whitespace/tab] [5]
Source/WebKit/chromium/ChangeLog:15:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:14:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:16:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 4 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Shishir Agrawal]

Created attachment 85997 [details]
Patch

Fixing the tabs that showed up in the ChangeLog files causing style check failures.

[Alexey Proskuryakov]

This doesn't sound good to me.

What exactly do you need to do from such a callback? What's so special about audio and video that they need callbacks that other elements don't?

[Eric Seidel (no email)]

Comment on attachment 85997 [details]
Patch

r- per alexey.  Can't you ask the page via getElementsByTagName?

[Shishir Agrawal]

The proposed change is to let the FrameLoaderClient know that the parser has encountered an audio / video tag (as against it being a javascript callback). We want to handle the audio and video case separately while prerendering because we dont want the audio / video to start playing until the page is visible to the user.

[Eric Seidel (no email)]

There is a bool which is passed to elements during parsing (or at least was) called createdByParser.  Would that be sufficient for your needs here?

Are <audio> <video> really started immediately after parse?  SVG animation, for example, is started when the containing </svg> close tag is seen and the SVGLoad event is fired IIRC.

[Shishir Agrawal]

In the prerendering case, we want to be able to either cancel prerendering or pause media till a a prerendered tab is visible, if a page has an audio or video tag. Whether the tag is created by the parser or script is not important for us. Which also indicates that the patch does not cover the case where the elements are created by js. So I need to perhaps move the calls from the HTMLTreeBuilder to the Video element constructor.

Also we would definitely like to know about the presence of such tags before the playback begins.

[Shishir Agrawal]

Created attachment 86527 [details]
Patch

The callback will now happen for audio/video tags inserted by script too.

[WebKit Review Bot]

Attachment 86527 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:13:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Shishir Agrawal]

Created attachment 86528 [details]
Patch

Fixing tab issue.

[Alexey Proskuryakov]

It still feels hugely wrong to me to make client callbacks during normal parsing. If anything, you care about loading and playback here, not about parsing.

[Eric Seidel (no email)]

Comment on attachment 86528 [details]
Patch

I still dont' understand why this is the right approach.  Why do you need to tell the client that you saw specifically a video or audio tag?  Why can't we just disconnect the media system while parsing or something?

[Eric Seidel (no email)]

Comment on attachment 86528 [details]
Patch

This design just totally feels wrong.

[James Robinson]

I have to agree - prerendering should perhaps interact with the media engines to tell them not to play video or audio, but there definitely shouldn't be any interaction with parsing or inserting tags into the DOM.

[Shishir Agrawal]

Closing this bug, since we managed to get around it in the chrome codebase.