Make video.webkitSupportsFullscreen an alias of document.fullscreenEnabled

Make video.webkitSupportsFullscreen an alias of document.fullscreenEnabled

The removed checks both originate from the earliest revision of this
code, for ChromeClient::supportsFullscreenForNode() and
MediaPlayer::supportsFullscren() calls respectively:
http://trac.webkit.org/changeset/49136

Both were conservatively left in later cleanup:
https://codereview.chromium.org/13851023
https://codereview.chromium.org/139943006

Fullscreen requests in detached documents will instead fail silently in
Fullscreen::requestFullscreen() due to the document()->isActive() check.

As for the webMediaPlayer() check, this will change the value of
video.webkitSupportsFullscreen before a video is loaded, as seen in
LayoutTests/media/video-prefixed-fullscreen.html. However, at worst
this ought to cause custom fullscreen buttons to be shown too early,
just as they would if using one of the other fullscreen APIs without
checking that the video is ready to play.

Overall risk ought to be low given the low usage of these APIs:
https://www.chromestatus.com/metrics/feature/timeline/popularity/166
https://www.chromestatus.com/metrics/feature/timeline/popularity/168
https://www.chromestatus.com/metrics/feature/timeline/popularity/170

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

[philipj_slow, 2015-07-14 14:35:14]

The CQ bit was checked by philipj@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-07-14 14:35:25]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1240573005/1

[philipj_slow, 2015-07-14 14:37:32]

philipj@opera.com changed reviewers:
+ falken@chromium.org

[philipj_slow, 2015-07-14 14:37:33]

PTAL

[commit-bot: I haz the power, 2015-07-14 14:37:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-14 14:37:53]

Dry run: Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/7...)

(exceeded global retry quota)

[falken, 2015-07-15 10:16:46]

On 2015/07/14 14:37:53, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   linux_blink_rel on tryserver.blink (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/7...)
> 
> (exceeded global retry quota)

Sorry I read these mails in the morning but then forgot to come back to them.
I'll review tomorrow.

[philipj_slow, 2015-07-15 10:24:18]

Looks like running the bots with dependent patchsets didn't work, I guess the
first will have to land first.

[falken, 2015-07-16 04:54:36]

On 2015/07/15 10:24:18, philipj wrote:
> Looks like running the bots with dependent patchsets didn't work, I guess the
> first will have to land first.

So this does the "change webkitSupportsFullscreen to include
document.fullscreenEnabled as part of its definition and stop ever throwing
exceptions in webkitEnterFullscreen()" part as stated in the new plan
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/3NukIuOaU4c/LW...

lgtm

[philipj_slow, 2015-07-21 15:24:07]

Patchset #2 (id:20001) has been deleted

[philipj_slow, 2015-07-22 11:07:24]

The CQ bit was checked by philipj@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-07-22 11:07:35]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1240573005/40001

[commit-bot: I haz the power, 2015-07-22 12:28:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-22 12:28:52]

Dry run: This issue passed the CQ dry run.

[philipj_slow, 2015-07-22 13:58:25]

Matt, can you take another look at this? I forgot to update the media controls
logic, so with PS1 the fullscreen button would have been shown in some cases
where it could not work. The Fullscreen::fullscreenEnabled() check includes both
the setting and the allowfullscreen attribut check.

[falken, 2015-07-22 23:14:56]

lgtm

https://codereview.chromium.org/1240573005/diff/40001/LayoutTests/media/video...
File LayoutTests/media/video-controls-fullscreen.js (right):

https://codereview.chromium.org/1240573005/diff/40001/LayoutTests/media/video...
LayoutTests/media/video-controls-fullscreen.js:57: // allowfullscreen attribute
Would this comment be better as the third argument to assert_equals?

https://codereview.chromium.org/1240573005/diff/40001/LayoutTests/media/video...
LayoutTests/media/video-controls-fullscreen.js:72:
v.addEventListener("loadeddata", t.step_func_done(function()
why does load become loadeddata?

https://codereview.chromium.org/1240573005/diff/40001/LayoutTests/media/video...
LayoutTests/media/video-controls-fullscreen.js:76:
assert_false(hasFullscreenButton(v));
ditto

[falken, 2015-07-22 23:14:57]

lgtm

[philipj_slow, 2015-07-23 07:02:55]

https://codereview.chromium.org/1240573005/diff/40001/LayoutTests/media/video...
File LayoutTests/media/video-controls-fullscreen.js (right):

https://codereview.chromium.org/1240573005/diff/40001/LayoutTests/media/video...
LayoutTests/media/video-controls-fullscreen.js:57: // allowfullscreen attribute
On 2015/07/22 23:14:56, falken wrote:
> Would this comment be better as the third argument to assert_equals?

Sure, I can do that.

https://codereview.chromium.org/1240573005/diff/40001/LayoutTests/media/video...
LayoutTests/media/video-controls-fullscreen.js:72:
v.addEventListener("loadeddata", t.step_func_done(function()
On 2015/07/22 23:14:56, falken wrote:
> why does load become loadeddata?

The load event on window doesn't work in the iframe case, so I changed this for
consistency. The window load event is most useful to avoid waiting for two
loadeddata events in the first test.

[philipj_slow, 2015-07-23 07:24:11]

move comments to assert descriptions

[philipj_slow, 2015-07-23 07:30:26]

The CQ bit was checked by philipj@opera.com

[philipj_slow, 2015-07-23 07:30:26]

The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org
Link to the patchset: https://codereview.chromium.org/1240573005/#ps60001
(title: "move comments to assert descriptions")

[commit-bot: I haz the power, 2015-07-23 07:30:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1240573005/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1240573005/60001

[commit-bot: I haz the power, 2015-07-23 08:44:08]

Committed patchset #3 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=199362