Cleanup tests related to controls hiding/fading in/out

Cleanup tests related to controls hiding/fading in/out

Add a helper function runAfterControlsHidden() that computes a reasonable
timeout for when the controls should be invisible.
Add controlsFade{In,Out}DurationMs to avoid having to redefine them in
every test that needs them.
Convert tests to use the helper and the variables, and also perform some
opportunity simplifications to lookups of the panel shadow element.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175365

[fs, 2014-05-26 13:21:17]

Followup to https://codereview.chromium.org/297783004/ .

@philipj: Feel free to comment early, otherwise this is pending for next week
(when acolwell is back).

[philipj_slow, 2014-05-26 13:38:47]

LGTM % nits

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/audio-cont...
File LayoutTests/media/audio-controls-do-not-fade-out.html (right):

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/audio-cont...
LayoutTests/media/audio-controls-do-not-fade-out.html:23:
document.getElementById("result").innerText = opacity < 1 ? "FAIL" : "PASS";
testExpected("getComputedStyle(controls).opacity", 1); would make for more a
more readable -expected.txt in case of failure I think.

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/media-cont...
File LayoutTests/media/media-controls.js (right):

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/media-cont...
LayoutTests/media/media-controls.js:5: // mirror the values
'fadeInDuration'/'fadeOutDuration' in MediaControlElements.cpp.
Add a comment in MediaControlsElements.cpp to point back here, to improve
changes of staying in sync.

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/media-cont...
LayoutTests/media/media-controls.js:11: // in MediaControls.cpp.
Same.

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/media-cont...
LayoutTests/media/media-controls.js:125: if (mediaElement &&
mediaElement.paused)
It looks like the second argument is always included and never intentionally
null, so why check it here?

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/media-cont...
LayoutTests/media/media-controls.js:131: var hideTimeoutMs =
controlsMouseMovementTimeoutMs + controlsFadeOutDurationMs + 500;
Maybe check that hideTimeoutMs >= 1000 * (mediaElement.duration -
mediaElement.currentTime)? Otherwise picking the wrong media file for a test
that uses this could test the wrong thing.

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/video-cont...
File LayoutTests/media/video-controls-show-on-focus.html (right):

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/video-cont...
LayoutTests/media/video-controls-show-on-focus.html:24: setTimeout(function() {
{ on new line

[fs, 2014-05-26 15:02:09]

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/audio-cont...
File LayoutTests/media/audio-controls-do-not-fade-out.html (right):

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/audio-cont...
LayoutTests/media/audio-controls-do-not-fade-out.html:23:
document.getElementById("result").innerText = opacity < 1 ? "FAIL" : "PASS";
On 2014/05/26 13:38:47, philipj wrote:
> testExpected("getComputedStyle(controls).opacity", 1); would make for more a
> more readable -expected.txt in case of failure I think.

Done.

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/media-cont...
File LayoutTests/media/media-controls.js (right):

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/media-cont...
LayoutTests/media/media-controls.js:5: // mirror the values
'fadeInDuration'/'fadeOutDuration' in MediaControlElements.cpp.
On 2014/05/26 13:38:47, philipj wrote:
> Add a comment in MediaControlsElements.cpp to point back here, to improve
> changes of staying in sync.

Done.

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/media-cont...
LayoutTests/media/media-controls.js:11: // in MediaControls.cpp.
On 2014/05/26 13:38:47, philipj wrote:
> Same.

Done.

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/media-cont...
LayoutTests/media/media-controls.js:125: if (mediaElement &&
mediaElement.paused)
On 2014/05/26 13:38:47, philipj wrote:
> It looks like the second argument is always included and never intentionally
> null, so why check it here?

Maybe a bit too forward-thinking of me (I.e. allow skipping the test by not
passing a context element.) Dropped.

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/media-cont...
LayoutTests/media/media-controls.js:131: var hideTimeoutMs =
controlsMouseMovementTimeoutMs + controlsFadeOutDurationMs + 500;
On 2014/05/26 13:38:47, philipj wrote:
> Maybe check that hideTimeoutMs >= 1000 * (mediaElement.duration -
> mediaElement.currentTime)? Otherwise picking the wrong media file for a test
> that uses this could test the wrong thing.

Makes sense. Added (w/ added .loop).

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/video-cont...
File LayoutTests/media/video-controls-show-on-focus.html (right):

https://codereview.chromium.org/302603003/diff/1/LayoutTests/media/video-cont...
LayoutTests/media/video-controls-show-on-focus.html:24: setTimeout(function() {
On 2014/05/26 13:38:47, philipj wrote:
> { on new line

Done.

[philipj_slow, 2014-05-27 07:54:03]

LGTM. If you have further changes depending on this I think it's OK to land and
let acolwell review after the fact, otherwise just wait.

[acolwell GONE FROM CHROMIUM, 2014-06-02 21:29:50]

lgtm

[fs, 2014-06-03 13:13:31]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2014-06-03 13:14:04]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fs@opera.com/302603003/10001

[commit-bot: I haz the power, 2014-06-03 13:26:30]

Change committed as 175365