Add new "resize" event to HTMLMediaElement.

Add new "resize" event to HTMLMediaElement.
This event was added in http://html5.org/r/8347 and so I'm claiming "trivial" for the blink web-facing policy (since this is a minor change to a shipping spec that improves compliance to the standard).

TESTED=./webkit/tools/layout_tests/run_webkit_tests.sh -f --debug media http/tests/media
BUG=328140
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=163900

[Ami GONE FROM CHROMIUM, 2013-12-13 07:59:45]

@acolwell: PTAL
@vsevik: OWNERS plz (git cl owners chose you; feel free to redirect as
appropriate).

I've basically forgotten everything I used to know about WebKit programming, so
please treat this as if it's coming from \noideadog.

[vsevik, 2013-12-13 13:04:43]

I don't know this code.
Adding adamk@ who reviewed patches in this file recently.

https://codereview.chromium.org/103913010/diff/1/LayoutTests/http/tests/media...
File LayoutTests/http/tests/media/media-source/mediasource-config-changes.js
(right):

https://codereview.chromium.org/103913010/diff/1/LayoutTests/http/tests/media...
LayoutTests/http/tests/media/media-source/mediasource-config-changes.js:2:
function resolutionFromFilename(filename) {
nit: { on the next line

https://codereview.chromium.org/103913010/diff/1/LayoutTests/http/tests/media...
LayoutTests/http/tests/media/media-source/mediasource-config-changes.js:5:
return resolution;
nit: 4 spaces indent

https://codereview.chromium.org/103913010/diff/1/Source/core/html/HTMLMediaEl...
File Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/103913010/diff/1/Source/core/html/HTMLMediaEl...
Source/core/html/HTMLMediaElement.cpp:3148: if (m_readyState > HAVE_NOTHING) {
nit: drop braces for consistency.

[acolwell GONE FROM CHROMIUM, 2013-12-13 15:58:11]

lgtm % comments

https://codereview.chromium.org/103913010/diff/1/LayoutTests/http/tests/media...
File LayoutTests/http/tests/media/media-source/mediasource-config-changes.js
(right):

https://codereview.chromium.org/103913010/diff/1/LayoutTests/http/tests/media...
LayoutTests/http/tests/media/media-source/mediasource-config-changes.js:69: if
(mediaElementForResizeExpectation) {
This doesn't appear to be defined anywhere. I'm assuming this was just
accidentally left in here. Please remove.

https://codereview.chromium.org/103913010/diff/1/LayoutTests/media/video-fram...
File LayoutTests/media/video-frame-size-change.html (right):

https://codereview.chromium.org/103913010/diff/1/LayoutTests/media/video-fram...
LayoutTests/media/video-frame-size-change.html:25: function onresize() {
nit: { is supposed to be on the next line. Could you please fix all the other
instances of a function's opening { being on the same line in this file while
you are in here please.

[acolwell GONE FROM CHROMIUM, 2013-12-13 16:02:38]

Actually not lgtm. I believe you need to send out an Intent to Implement & Ship
for this change since it is a web visible change.

[Ami GONE FROM CHROMIUM, 2013-12-13 17:48:42]

Thanks for the comments; all addressed.  PTAL.

Updated CL description to claim "trivial" change to the web platform and will
PSA to blink-dev (per advice from eseidel@).

https://codereview.chromium.org/103913010/diff/1/LayoutTests/http/tests/media...
File LayoutTests/http/tests/media/media-source/mediasource-config-changes.js
(right):

https://codereview.chromium.org/103913010/diff/1/LayoutTests/http/tests/media...
LayoutTests/http/tests/media/media-source/mediasource-config-changes.js:2:
function resolutionFromFilename(filename) {
On 2013/12/13 13:04:43, vsevik wrote:
> nit: { on the next line

Done.

https://codereview.chromium.org/103913010/diff/1/LayoutTests/http/tests/media...
LayoutTests/http/tests/media/media-source/mediasource-config-changes.js:5:
return resolution;
On 2013/12/13 13:04:43, vsevik wrote:
> nit: 4 spaces indent

Done.

https://codereview.chromium.org/103913010/diff/1/LayoutTests/http/tests/media...
LayoutTests/http/tests/media/media-source/mediasource-config-changes.js:69: if
(mediaElementForResizeExpectation) {
On 2013/12/13 15:58:12, acolwell wrote:
> This doesn't appear to be defined anywhere. I'm assuming this was just
> accidentally left in here. Please remove.

Done.
(had to rework this bit a bit to get the test to actually wait for the events to
fire)

https://codereview.chromium.org/103913010/diff/1/LayoutTests/media/video-fram...
File LayoutTests/media/video-frame-size-change.html (right):

https://codereview.chromium.org/103913010/diff/1/LayoutTests/media/video-fram...
LayoutTests/media/video-frame-size-change.html:25: function onresize() {
On 2013/12/13 15:58:12, acolwell wrote:
> nit: { is supposed to be on the next line. Could you please fix all the other
> instances of a function's opening { being on the same line in this file while
> you are in here please.

Done.

https://codereview.chromium.org/103913010/diff/1/Source/core/html/HTMLMediaEl...
File Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/103913010/diff/1/Source/core/html/HTMLMediaEl...
Source/core/html/HTMLMediaElement.cpp:3148: if (m_readyState > HAVE_NOTHING) {
On 2013/12/13 13:04:43, vsevik wrote:
> nit: drop braces for consistency.

Done.

[eseidel, 2013-12-13 17:59:15]

I'm OK with this counting as a "trivial" change and skipping the intent to ship
process.  Thanks for the PSA.

[acolwell GONE FROM CHROMIUM, 2013-12-13 18:05:20]

lgtm

[Ami GONE FROM CHROMIUM, 2013-12-13 18:23:55]

eseidel: would you like to OWNERS review this, as well, since adamk@ is at an
off-site?

[eseidel, 2013-12-13 18:28:27]

Code change lgtm.

[bemasc1, 2013-12-13 21:37:45]

LGTM, tested with webrtc and variable remote stream resolution.  Events fire
when the remote stream's frame dimensions change, as desired.

One note: The video tag doesn't get an "onresize" attribute in the console's
autocomplete list, like "onplaying", "onpause", etc..  I don't know if this is a
problem.

[Ami GONE FROM CHROMIUM, 2013-12-13 21:47:51]

On 2013/12/13 21:37:45, bemasc1 wrote:
> LGTM, tested with webrtc and variable remote stream resolution.  Events fire
> when the remote stream's frame dimensions change, as desired.
> 
> One note: The video tag doesn't get an "onresize" attribute in the console's
> autocomplete list, like "onplaying", "onpause", etc..  I don't know if this is
a
> problem.

Thanks for verifying, Ben.

AFAICT the onfoo attributes on the video tag are not spec'd (they are spec'd on
MediaController), so I think this is OK.

eseidel: can you confirm that we don't willy-nilly add onfoo attributes for foo
events?

[eseidel, 2013-12-13 22:11:44]

I don't know what the current best-practice for onfoo attributes corresponding
to "foo" events are.  That's a spec question.

[Ami GONE FROM CHROMIUM, 2013-12-13 22:41:57]

Clarification courtesy of #whatwg: onfoo attributes are defined across
everything
(http://www.whatwg.org/specs/web-apps/current-work/multipage/webappapis.html#e...),
not restricted to e.g. media elements (support bubbling, and generic
dispatchEvent()).  onresize is missing from the list and Hixie said he'd add it.
 I think that's a separate issue from this one though so I'll CQ this now.

[commit-bot: I haz the power, 2013-12-13 22:42:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fischman@chromium.org/103913010/20001

[commit-bot: I haz the power, 2013-12-14 00:00:23]

Change committed as 163900