Fix modern media controls visibility issues

This CL fixes a couple of visibility issues for the modern media
controls. First, it only toggles the controls visibility on touch
events instead of all click events. Second, it changes the
ShouldHideMediaControls hover check to only use the hover state of the
button panel and timeline instead of the panel itself, which in modern
media controls covers the entire video element. This fixes an issue on
desktop where the controls wouldn't hide during fullscreen since the
controls were always hovered.

Bug: 812890
Change-Id: I8cb30841020e7f3ffdadc53131a4fa374b4254c2
Reviewed-on: https://chromium-review.googlesource.com/938841
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542564}

third_party/WebKit/LayoutTests/TestExpectations

@@ -1211,6 +1211,7 @@ crbug.com/613672 [ Mac ] virtual/mouseevent_fractional/fast/events/pointerevents
 crbug.com/613672 [ Mac ] virtual/mouseevent_fractional/fast/events/pointerevents/pointer-event-in-slop-region.html [ Skip ]
 crbug.com/613672 [ Mac ] virtual/mouseevent_fractional/fast/events/synthetic-events/tap-on-scaled-screen.html  [ Skip ]
 crbug.com/613672 [ Mac ] virtual/scroll_customization/fast/scroll-behavior/scroll-customization/scroll-customization-property.html [ Skip ]
+crbug.com/613672 [ Mac ] virtual/modern-media-controls/media/controls/modern/tap-to-hide-controls.html [ Skip ]
 
 # We should send PointerLeave events for stylus devices.
 crbug.com/583413 external/wpt/pointerevents/pointerevent_pointerleave_pen-manual.html  [ Failure ]

third_party/WebKit/LayoutTests/media/controls/modern/tap-to-hide-controls.html

@@ -16,7 +16,7 @@ async_test(t => {
 
     // Single tap in the top right hand corner
     const c = coordinatesOutsideElement(mediaControlsOverlayPlayButton(video));
-    singleTapAtCoordinates(c[0] + 1, c[1] + 1, t.step_func(() => {
+    singleTouchAtCoordinates(c[0] + 1, c[1] + 1, t.step_func(() => {
       // Wait for the tap to expire.
       setTimeout(t.step_func_done(() => {
         assert_false(isControlsPanelVisible(video));

third_party/WebKit/LayoutTests/media/media-controls.js

@@ -394,6 +394,18 @@ function singleTapAtCoordinates(xPos, yPos, callback) {
   ], callback);
 }
 
+function singleTouchAtCoordinates(xPos, yPos, callback) {
+  chrome.gpuBenchmarking.pointerActionSequence([
+    {
+      source: 'touch',
+      actions: [
+        { name: 'pointerDown', x: xPos, y: yPos },
+        { name: 'pointerUp' }
+      ]
+    }
+  ], callback);
+}
+
 function enableDoubleTapToJumpForTest(t) {
   var doubleTapToJumpOnVideoEnabledValue =
       internals.runtimeFlags.doubleTapToJumpOnVideoEnabled;

third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.cpp

@@ -306,6 +306,11 @@ bool MediaControlsImpl::IsModern() {
   return RuntimeEnabledFeatures::ModernMediaControlsEnabled();
 }
 
+bool MediaControlsImpl::IsTouchEvent(Event* event) {
+  return event->IsTouchEvent() || event->IsGestureEvent() ||
+         (event->IsMouseEvent() && ToMouseEvent(event)->FromTouch());
+}
+
 MediaControlsImpl::MediaControlsImpl(HTMLMediaElement& media_element)
     : HTMLDivElement(media_element.GetDocument()),
       MediaControls(media_element),
@@ -846,7 +851,7 @@ bool MediaControlsImpl::ShouldHideMediaControls(unsigned behavior_flags) const {
 
   // Don't hide if the mouse is over the controls.
   const bool ignore_controls_hover = behavior_flags & kIgnoreControlsHover;
-  if (!ignore_controls_hover && panel_->IsHovered())
+  if (!ignore_controls_hover && AreVideoControlsHovered())
     return false;
 
   // Don't hide if the mouse is over the video area.
@@ -874,6 +879,15 @@ bool MediaControlsImpl::ShouldHideMediaControls(unsigned behavior_flags) const {
   return true;
 }
 
+bool MediaControlsImpl::AreVideoControlsHovered() const {
+  DCHECK(MediaElement().IsHTMLVideoElement());
+
+  if (IsModern())
+    return media_button_panel_->IsHovered() || timeline_->IsHovered();
+
+  return panel_->IsHovered();
+}
+
 void MediaControlsImpl::UpdatePlayState() {
   if (is_paused_for_scrubbing_)
     return;
@@ -1189,16 +1203,14 @@ void MediaControlsImpl::DefaultEventHandler(Event* event) {
   // event, to allow the hide-timer to do the right thing when it fires.
   // FIXME: Preferably we would only do this when we're actually handling the
   // event here ourselves.
-  bool is_touch_event =
-      event->IsTouchEvent() || event->IsGestureEvent() ||
-      (event->IsMouseEvent() && ToMouseEvent(event)->FromTouch());
+  bool is_touch_event = IsTouchEvent(event);
   hide_timer_behavior_flags_ |=
       is_touch_event ? kIgnoreControlsHover : kIgnoreNone;
 
   // Touch events are treated differently to avoid fake mouse events to trigger
   // random behavior. The expect behaviour for touch is that a tap will show the
   // controls and they will hide when the timer to hide fires.
-  if (is_touch_event && !IsModern()) {
+  if (is_touch_event) {
     if (event->type() != EventTypeNames::gesturetap)
       return;
 
@@ -1239,11 +1251,6 @@ void MediaControlsImpl::DefaultEventHandler(Event* event) {
     return;
   }
 
-  if (event->type() == EventTypeNames::click) {
-    MaybeToggleControlsFromTap();
-    return;
-  }
-
   // The pointer event has finished so we should clear the bit.
   if (event->type() == EventTypeNames::mouseout) {
     pointer_event_did_show_controls_ = false;

third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.h

@@ -80,6 +80,9 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement,
   // Returns whether the ModernMediaControlsEnabled runtime flag is on.
   static bool IsModern();
 
+  // Returns whether the event is considered a touch event.
+  static bool IsTouchEvent(Event*);
+
   // Node override.
   Node::InsertionNotificationRequest InsertedInto(ContainerNode*) override;
   void RemovedFrom(ContainerNode*) override;
@@ -226,6 +229,7 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement,
   };
 
   bool ShouldHideMediaControls(unsigned behavior_flags = 0) const;
+  bool AreVideoControlsHovered() const;
   void HideMediaControlsTimerFired(TimerBase*);
   void StartHideMediaControlsIfNecessary();
   void StartHideMediaControlsTimer();

third_party/WebKit/Source/modules/media_controls/elements/MediaControlOverlayPlayButtonElement.cpp

@@ -214,10 +214,12 @@ void MediaControlOverlayPlayButtonElement::DefaultEventHandler(Event* event) {
       // case their is a second tap.
       if (tap_timer_.IsActive())
         return;
+      tap_was_touch_event_ = MediaControlsImpl::IsTouchEvent(event);
       tap_timer_.StartOneShot(kDoubleTapDelay, FROM_HERE);
     } else {
       // Cancel the play pause event.
       tap_timer_.Stop();
+      tap_was_touch_event_.reset();
 
       if (RuntimeEnabledFeatures::DoubleTapToJumpOnVideoEnabled()) {
         // Jump forwards or backwards based on the position of the tap.
@@ -255,7 +257,9 @@ WebSize MediaControlOverlayPlayButtonElement::GetSizeOrDefault() const {
 }
 
 void MediaControlOverlayPlayButtonElement::TapTimerFired(TimerBase*) {
-  GetMediaControls().MaybeToggleControlsFromTap();
+  if (tap_was_touch_event_.value())
+    GetMediaControls().MaybeToggleControlsFromTap();
+  tap_was_touch_event_.reset();
 }
 
 void MediaControlOverlayPlayButtonElement::Trace(blink::Visitor* visitor) {

third_party/WebKit/Source/modules/media_controls/elements/MediaControlOverlayPlayButtonElement.h

@@ -79,6 +79,7 @@ class MODULES_EXPORT MediaControlOverlayPlayButtonElement final
   void MaybeJump(int);
 
   TaskRunnerTimer<MediaControlOverlayPlayButtonElement> tap_timer_;
+  WTF::Optional<bool> tap_was_touch_event_;
 
   Member<HTMLDivElement> internal_button_;
   Member<AnimatedArrow> left_jump_arrow_;