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

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

The previous attempt broke the overlay play button:
https://codereview.chromium.org/830183005/#msg15

Drop the changes to MediaControlOverlayPlayButtonElement and add a test.

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

[philipj_slow, 2015-01-12 14:25:34]

Drop the changes to MediaControlOverlayPlayButtonElement and add a test

[philipj_slow, 2015-01-12 14:31:18]

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

[philipj_slow, 2015-01-12 14:31:19]

PTAL. I'll land when "Android Tests (dbg)" is green, which I'm hoping it will
be.

[fs, 2015-01-12 14:41:18]

On 2015/01/12 14:31:19, philipj wrote:
> PTAL. I'll land when "Android Tests (dbg)" is green, which I'm hoping it will
> be.

Was the problem that the overlay play button wouldn't hide, or? Anyway, LGTM,
third time lucky right? =)

[philipj_slow, 2015-01-12 14:47:21]

On 2015/01/12 14:41:18, fs wrote:
> On 2015/01/12 14:31:19, philipj wrote:
> > PTAL. I'll land when "Android Tests (dbg)" is green, which I'm hoping it
will
> > be.
> 
> Was the problem that the overlay play button wouldn't hide, or? Anyway, LGTM,
> third time lucky right? =)

Not quite, it actually stopped doing what a play button should do, because I
removed MediaControlOverlayPlayButtonElement::defaultEventHandler but left
MediaControlOverlayPlayButtonElement::keepEventInNode so the event wouldn't
reach HTMLMediaElement. I think removing
MediaControlOverlayPlayButtonElement::keepEventInNode as well would have worked,
but instead opted to keep these things separate and not be so clever.

[fs, 2015-01-12 14:50:16]

On 2015/01/12 14:47:21, philipj wrote:
> On 2015/01/12 14:41:18, fs wrote:
> > On 2015/01/12 14:31:19, philipj wrote:
> > > PTAL. I'll land when "Android Tests (dbg)" is green, which I'm hoping it
> will
> > > be.
> > 
> > Was the problem that the overlay play button wouldn't hide, or? Anyway,
LGTM,
> > third time lucky right? =)
> 
> Not quite, it actually stopped doing what a play button should do, because I
> removed MediaControlOverlayPlayButtonElement::defaultEventHandler but left
> MediaControlOverlayPlayButtonElement::keepEventInNode so the event wouldn't
> reach HTMLMediaElement. I think removing
> MediaControlOverlayPlayButtonElement::keepEventInNode as well would have
worked,
> but instead opted to keep these things separate and not be so clever.

Aha, ok, so the event didn't make it out of the shadow tree...

[philipj_slow, 2015-01-12 15:49:53]

Android Tests (dbg) (started from the command line by guessing) doesn't seem to
work, but android_dbg_tests_recipe ran the two previously failing tests:
org.chromium.android_webview.test.AwContentsClientFullScreenTest#testOnShowCustomViewAndPlayWithHtmlControl_video
org.chromium.android_webview.test.AwContentsClientFullScreenTest#testPowerSaveBlockerIsEnabledDuringFullscreenPlayback_video

Seems OK then.

[philipj_slow, 2015-01-12 15:49:57]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2015-01-12 15:50:27]

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

[commit-bot: I haz the power, 2015-01-12 17:27:57]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=188233

[philipj_slow, 2015-01-23 09:48:19]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/867063003/ by philipj@opera.com.

The reason for reverting is: On Android:
1. The entire video is now clickable, leading more often to ambigous
   touches and zooming.
2. When the controls are not visible, touching the video will both pause
   the video and show the controls, where they would previously only
   show the controls..