Reland "[Media Controls] Show volume slider only if user hover mute button for a while (200ms)"

Change-Id: I7b8e4c989b0ec0d21b5620ab2ece577fc61ce11b
Reviewed-on: https://chromium-review.googlesource.com/c/1336060
Commit-Queue: Jazz Xu <jazzhsu@chromium.org>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609098}

third_party/WebKit/LayoutTests/media/controls/volume-slider.html

@@ -57,7 +57,7 @@ function testMediaElement(t, element) {
 
   // Hovering over the mute button before preloading should not open the volume
   // slider.
-  hoverOverControl(muteBtn, t.step_func(() => {
+  hoverMuteButton(element, t.step_func(() => {
     assert_false(isVolumeSliderOpen(element), element.id + ': volume slider should not open before metadata load');
 
     // Move the cursor away from the mute button and start testing the loaded
@@ -72,7 +72,7 @@ function testMediaElement(t, element) {
 
     // Hovering over the mute button after preloading should open the volume
     // slider.
-    hoverOverControl(muteBtn, t.step_func(() => {
+    hoverMuteButton(element, t.step_func(() => {
       assert_true(isVolumeSliderOpen(element), element.id + ': volume slider should open when hovering the mute button');
 
       // Moving the cursor onto the volume slider should not hide the volume slider.

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

@@ -427,6 +427,14 @@ function runAfterDoubleTapTimerFired(func) {
   setTimeout(func, doubleTapTimeoutMs);
 }
 
+function hoverMuteButton(video, func) {
+  // 300ms slack because the test could be flaky on Mac Test build
+  // even with volume slider delay set to 0ms
+  const delayedCallback = function() { setTimeout(func, 300); };
+  const muteBtn = muteButton(video);
+  hoverOverControl(muteBtn, delayedCallback);
+}
+
 function hasEnabledFullscreenButton(element) {
   var button = fullscreenButton(element);
   return !button.disabled && button.style.display != "none";

third_party/blink/renderer/core/layout/layout_theme_default.cc

@@ -162,6 +162,9 @@ int LayoutThemeDefault::SliderTickOffsetFromTrackCenter() const {
 }
 
 void LayoutThemeDefault::AdjustSliderThumbSize(ComputedStyle& style) const {
+  if (!Platform::Current()->ThemeEngine())
+    return;
+
   IntSize size = Platform::Current()->ThemeEngine()->GetSize(
       WebThemeEngine::kPartSliderThumb);
 

third_party/blink/renderer/modules/media_controls/media_controls_impl.cc

@@ -85,6 +85,7 @@
 #include "third_party/blink/renderer/modules/remoteplayback/html_media_element_remote_playback.h"
 #include "third_party/blink/renderer/modules/remoteplayback/remote_playback.h"
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
+#include "third_party/blink/renderer/platform/layout_test_support.h"
 #include "third_party/blink/renderer/platform/runtime_enabled_features.h"
 #include "third_party/blink/renderer/platform/text/platform_locale.h"
 
@@ -136,6 +137,14 @@ const char kImmersiveModeCSSClass[] = "immersive-mode";
 // The delay between two taps to be recognized as a double tap gesture.
 constexpr WTF::TimeDelta kDoubleTapDelay = TimeDelta::FromMilliseconds(300);
 
+// The time user have to hover on mute button to show volume slider.
+// If this value is changed, you need to change the corresponding value in
+// media_controls_impl_test.cc
+constexpr WTF::TimeDelta kTimeToShowVolumeSlider =
+    TimeDelta::FromMilliseconds(200);
+constexpr WTF::TimeDelta kTimeToShowVolumeSliderTest =
+    TimeDelta::FromMilliseconds(0);
+
 // The number of seconds to jump when double tapping.
 constexpr int kNumberOfSecondsToJump = 10;
 
@@ -395,7 +404,11 @@ MediaControlsImpl::MediaControlsImpl(HTMLMediaElement& media_element)
       tap_timer_(
           media_element.GetDocument().GetTaskRunner(TaskType::kInternalMedia),
           this,
-          &MediaControlsImpl::TapTimerFired) {
+          &MediaControlsImpl::TapTimerFired),
+      volume_slider_wanted_timer_(
+          media_element.GetDocument().GetTaskRunner(TaskType::kInternalMedia),
+          this,
+          &MediaControlsImpl::VolumeSliderWantedTimerFired) {
   // On touch devices, start with the assumption that the user will interact via
   // touch events.
   Settings* settings = media_element.GetDocument().GetSettings();
@@ -2162,14 +2175,26 @@ void MediaControlsImpl::StartHideMediaControlsIfNecessary() {
     StartHideMediaControlsTimer();
 }
 
+void MediaControlsImpl::VolumeSliderWantedTimerFired(TimerBase*) {
+  volume_slider_->OpenSlider();
+}
+
 void MediaControlsImpl::OpenVolumeSliderIfNecessary() {
-  if (ShouldOpenVolumeSlider())
-    volume_slider_->OpenSlider();
+  if (ShouldOpenVolumeSlider()) {
+    volume_slider_wanted_timer_.StartOneShot(
+        LayoutTestSupport::IsRunningLayoutTest() ? kTimeToShowVolumeSliderTest
+                                                 : kTimeToShowVolumeSlider,
+        FROM_HERE);
+  }
 }
 
 void MediaControlsImpl::CloseVolumeSliderIfNecessary() {
-  if (ShouldCloseVolumeSlider())
+  if (ShouldCloseVolumeSlider()) {
     volume_slider_->CloseSlider();
+
+    if (volume_slider_wanted_timer_.IsActive())
+      volume_slider_wanted_timer_.Stop();
+  }
 }
 
 bool MediaControlsImpl::ShouldOpenVolumeSlider() const {

third_party/blink/renderer/modules/media_controls/media_controls_impl.h

@@ -142,6 +142,7 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement,
   void ToggleOverflowMenu();
   bool OverflowMenuVisible();
 
+  void VolumeSliderWantedTimerFired(TimerBase*);
   void OpenVolumeSliderIfNecessary();
   void CloseVolumeSliderIfNecessary();
 
@@ -415,6 +416,10 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement,
   TaskRunnerTimer<MediaControlsImpl> tap_timer_;
   bool is_paused_for_double_tap_ = false;
 
+  // Timer to delay showing the volume slider to avoid accidental triggering
+  // of the slider
+  TaskRunnerTimer<MediaControlsImpl> volume_slider_wanted_timer_;
+
   bool is_test_mode_ = false;
 };
 

third_party/blink/renderer/modules/media_controls/media_controls_impl_test.cc

@@ -18,6 +18,7 @@
 #include "third_party/blink/renderer/core/css/document_style_environment_variables.h"
 #include "third_party/blink/renderer/core/css/style_engine.h"
 #include "third_party/blink/renderer/core/dom/document.h"
+#include "third_party/blink/renderer/core/dom/dom_token_list.h"
 #include "third_party/blink/renderer/core/dom/element_traversal.h"
 #include "third_party/blink/renderer/core/dom/events/event.h"
 #include "third_party/blink/renderer/core/dom/node_computed_style.h"
@@ -41,6 +42,7 @@
 #include "third_party/blink/renderer/modules/remoteplayback/html_media_element_remote_playback.h"
 #include "third_party/blink/renderer/modules/remoteplayback/remote_playback.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
+#include "third_party/blink/renderer/platform/layout_test_support.h"
 #include "third_party/blink/renderer/platform/testing/empty_web_media_player.h"
 #include "third_party/blink/renderer/platform/testing/histogram_tester.h"
 #include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
@@ -73,10 +75,12 @@ class MockWebMediaPlayerForImpl : public EmptyWebMediaPlayer {
   // WebMediaPlayer overrides:
   WebTimeRanges Seekable() const override { return seekable_; }
   bool HasVideo() const override { return true; }
+  bool HasAudio() const override { return has_audio_; }
   SurfaceLayerMode GetVideoSurfaceLayerMode() const override {
     return SurfaceLayerMode::kAlways;
   }
 
+  bool has_audio_ = false;
   WebTimeRanges seekable_;
 };
 
@@ -269,6 +273,8 @@ class MediaControlsImplTest : public PageTestBase,
     SimulateLoadedMetadata();
   }
 
+  void SetHasAudio(bool has_audio) { WebMediaPlayer()->has_audio_ = has_audio; }
+
   void ClickOverflowButton() {
     MediaControls()
         .download_button_->OverflowElementForTests()
@@ -999,7 +1005,16 @@ TEST_F(MediaControlsImplTest, TimeIsCorrectlyFormatted) {
 
 namespace {
 
-class MediaControlsImplTestWithMockScheduler : public MediaControlsImplTest {
+class ModernMediaControlsImplTest : public MediaControlsImplTest {
+ public:
+  void SetUp() override {
+    RuntimeEnabledFeatures::SetModernMediaControlsEnabled(true);
+    MediaControlsImplTest::SetUp();
+  }
+};
+
+class MediaControlsImplTestWithMockScheduler
+    : public ModernMediaControlsImplTest {
  public:
   MediaControlsImplTestWithMockScheduler() { EnablePlatform(); }
 
@@ -1008,7 +1023,7 @@ class MediaControlsImplTestWithMockScheduler : public MediaControlsImplTest {
     // DocumentParserTiming has DCHECKS to make sure time > 0.0.
     platform()->AdvanceClockSeconds(1);
 
-    MediaControlsImplTest::SetUp();
+    ModernMediaControlsImplTest::SetUp();
   }
 
   bool IsCursorHidden() {
@@ -1265,6 +1280,55 @@ TEST_F(MediaControlsImplTest, OverflowMenuMetricsTimeToDismiss) {
   GetHistogramTester().ExpectTotalCount(kTimeToActionHistogramName, 0);
 }
 
+TEST_F(MediaControlsImplTestWithMockScheduler,
+       ShowVolumeSliderAfterHoverTimerFired) {
+  const double kTimeToShowVolumeSlider = 0.2;
+
+  EnsureSizing();
+  MediaControls().MediaElement().SetSrc("https://example.com/foo.mp4");
+  platform()->RunForPeriodSeconds(1);
+  SetHasAudio(true);
+  SimulateLoadedMetadata();
+
+  LayoutTestSupport::SetIsRunningLayoutTest(false);
+
+  Element* volume_slider = GetElementByShadowPseudoId(
+      MediaControls(), "-webkit-media-controls-volume-slider");
+  Element* mute_btn = GetElementByShadowPseudoId(
+      MediaControls(), "-webkit-media-controls-mute-button");
+
+  ASSERT_NE(nullptr, volume_slider);
+  ASSERT_NE(nullptr, mute_btn);
+
+  EXPECT_TRUE(IsElementVisible(*mute_btn));
+  EXPECT_TRUE(volume_slider->classList().contains("closed"));
+
+  DOMRect* mute_btn_rect = mute_btn->getBoundingClientRect();
+  WebFloatPoint mute_btn_center(
+      mute_btn_rect->left() + mute_btn_rect->width() / 2,
+      mute_btn_rect->top() + mute_btn_rect->height() / 2);
+  WebFloatPoint edge(0, 0);
+
+  // Hover on mute button and stay
+  MouseMoveTo(mute_btn_center);
+  platform()->RunForPeriodSeconds(kTimeToShowVolumeSlider - 0.001);
+  EXPECT_TRUE(volume_slider->classList().contains("closed"));
+
+  platform()->RunForPeriodSeconds(0.002);
+  EXPECT_FALSE(volume_slider->classList().contains("closed"));
+
+  MouseMoveTo(edge);
+  EXPECT_TRUE(volume_slider->classList().contains("closed"));
+
+  // Hover on mute button and move away before timer fired
+  MouseMoveTo(mute_btn_center);
+  platform()->RunForPeriodSeconds(kTimeToShowVolumeSlider - 0.001);
+  EXPECT_TRUE(volume_slider->classList().contains("closed"));
+
+  MouseMoveTo(edge);
+  EXPECT_TRUE(volume_slider->classList().contains("closed"));
+}
+
 TEST_F(MediaControlsImplTest, CastOverlayDefaultHidesOnTimer) {
   MediaControls().MediaElement().SetBooleanAttribute(html_names::kControlsAttr,
                                                      false);
@@ -1325,14 +1389,6 @@ TEST_F(MediaControlsImplTest, isConnected) {
   EXPECT_FALSE(MediaControls().isConnected());
 }
 
-class ModernMediaControlsImplTest : public MediaControlsImplTest {
- public:
-  void SetUp() override {
-    RuntimeEnabledFeatures::SetModernMediaControlsEnabled(true);
-    MediaControlsImplTest::SetUp();
-  }
-};
-
 TEST_F(ModernMediaControlsImplTest, ControlsShouldUseSafeAreaInsets) {
   UpdateAllLifecyclePhasesForTest();
   {