Re-enable the activation behavior of media elements (click to play/pause)

Re-enable the activation behavior of media elements (click to play/pause)

This revives most of the code and tests from an earlier revert:
https://codereview.chromium.org/249483002

The "Don't fire events when interacting with <video controls>" bug has
been fixed, so the native controls and scripted controls will no
longer compete for control:
https://codereview.chromium.org/406213002

The definition of "restrained media controller" has been simplified in
spec and implementation since the last time:
https://codereview.chromium.org/262463003

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

[philipj_slow, 2015-01-08 13:42:08]

update tests

[philipj_slow, 2015-01-08 14:07:32]

simplify test

[philipj_slow, 2015-01-08 14:24:49]

philipj@opera.com changed reviewers:
+ fs@opera.com

[philipj_slow, 2015-01-08 14:24:50]

PTAL. If it LGTY, I'll also ask aberent to take a look before landing.

[fs, 2015-01-08 15:03:59]

On 2015/01/08 14:24:50, philipj wrote:
> PTAL. If it LGTY, I'll also ask aberent to take a look before landing.

LGTM

[philipj_slow, 2015-01-08 15:49:37]

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

[philipj_slow, 2015-01-08 15:49:37]

aberent@, can you please take a look as well? This retry is made possible by
your earlier fix so you'll be familiar with the problem.

[aberent, 2015-01-08 16:22:16]

On 2015/01/08 15:49:37, philipj wrote:
> aberent@, can you please take a look as well? This retry is made possible by
> your earlier fix so you'll be familiar with the problem.

LGTM - one possible nit.

[aberent, 2015-01-08 16:22:36]

https://codereview.chromium.org/830183005/diff/40001/LayoutTests/media/activa...
File LayoutTests/media/activation-behavior-shadow.html (right):

https://codereview.chromium.org/830183005/diff/40001/LayoutTests/media/activa...
LayoutTests/media/activation-behavior-shadow.html:23: // when not visible
violates sound assumptions made in HTMLMediaElement.
Should you apply similar logic to the closed captions button, which is also
sometimes invisible? I am not sure what happens if this is clicked when there
are no closed captions available.

[philipj_slow, 2015-01-08 22:59:56]

https://codereview.chromium.org/830183005/diff/40001/LayoutTests/media/activa...
File LayoutTests/media/activation-behavior-shadow.html (right):

https://codereview.chromium.org/830183005/diff/40001/LayoutTests/media/activa...
LayoutTests/media/activation-behavior-shadow.html:23: // when not visible
violates sound assumptions made in HTMLMediaElement.
On 2015/01/08 16:22:36, aberent wrote:
> Should you apply similar logic to the closed captions button, which is also
> sometimes invisible? I am not sure what happens if this is clicked when there
> are no closed captions available.

HTMLMediaElement::setClosedCaptionsVisible has an early return that makes it
harmless. If that were changed to ASSERT(hasClosedCaptions()) then avoiding it
here would make sense, but in the current state I don't think there's any
reason.

[aberent, 2015-01-09 09:51:48]

lgtm

[philipj_slow, 2015-01-09 10:50:25]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2015-01-09 10:50:53]

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

[commit-bot: I haz the power, 2015-01-09 10:54:46]

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

[jbudorick, 2015-01-09 17:57:51]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/839293002/ by jbudorick@chromium.org.

The reason for reverting is: breaks androidwebview_instrumentation_tests on
Android Tests (dbg) on chromium.webkit starting with
http://build.chromium.org/p/chromium.webkit/builders/Android%20Tests%20%28dbg....

[philipj_slow, 2015-01-12 14:37:42]

Trying again in https://codereview.chromium.org/829393004/