[Media Controls] Fix volume slider jumping back and forth

This issue is cause by an empty space between volume slider
and the container. When user hover on mute button in a small
video/audio frame, the volume slider shoot out towards right,
which makes the mouse ended up in the empty space which will
close the volume slider. Fixed by adding a container class around
mute button and volume slider and close the volume slider
only if user is not hovered on the container rather than
mute button/volume slider.

Bug: 921918
Change-Id: Ida2d943104fca3ab74290e58674856ada25555d5
Reviewed-on: https://chromium-review.googlesource.com/c/1413395
Commit-Queue: Jazz Xu <jazzhsu@chromium.org>
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629758}

third_party/blink/renderer/modules/media_controls/BUILD.gn

@@ -72,6 +72,8 @@ blink_modules_sources("media_controls") {
     "elements/media_control_timeline_metrics.h",
     "elements/media_control_toggle_closed_captions_button_element.cc",
     "elements/media_control_toggle_closed_captions_button_element.h",
+    "elements/media_control_volume_control_container_element.cc",
+    "elements/media_control_volume_control_container_element.h",
     "elements/media_control_volume_slider_element.cc",
     "elements/media_control_volume_slider_element.h",
     "media_controls_display_cutout_delegate.cc",

third_party/blink/renderer/modules/media_controls/elements/media_control_mute_button_element.cc

@@ -70,15 +70,11 @@ void MediaControlMuteButtonElement::DefaultEventHandler(Event& event) {
   }
 
   if (!IsOverflowElement()) {
-    if (event.type() == event_type_names::kMouseover ||
-        event.type() == event_type_names::kFocus) {
+    if (event.type() == event_type_names::kFocus)
       GetMediaControls().OpenVolumeSliderIfNecessary();
-    }
 
-    if (event.type() == event_type_names::kMouseout ||
-        event.type() == event_type_names::kBlur) {
+    if (event.type() == event_type_names::kBlur)
       GetMediaControls().CloseVolumeSliderIfNecessary();
-    }
   }
 
   MediaControlInputElement::DefaultEventHandler(event);

third_party/blink/renderer/modules/media_controls/elements/media_control_volume_control_container_element.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "third_party/blink/renderer/modules/media_controls/elements/media_control_volume_control_container_element.h"
+
+#include "third_party/blink/renderer/core/dom/dom_token_list.h"
+#include "third_party/blink/renderer/core/dom/events/event.h"
+#include "third_party/blink/renderer/modules/media_controls/elements/media_control_consts.h"
+#include "third_party/blink/renderer/modules/media_controls/elements/media_control_elements_helper.h"
+#include "third_party/blink/renderer/modules/media_controls/media_controls_impl.h"
+
+namespace blink {
+
+MediaControlVolumeControlContainerElement::
+    MediaControlVolumeControlContainerElement(MediaControlsImpl& media_controls)
+    : MediaControlDivElement(media_controls, kMediaIgnore) {
+  SetShadowPseudoId(
+      AtomicString("-webkit-media-controls-volume-control-container"));
+  MediaControlElementsHelper::CreateDiv(
+      "-webkit-media-controls-volume-control-hover-background", this);
+
+  if (MediaControlsImpl::IsModern())
+    CloseContainer();
+}
+
+void MediaControlVolumeControlContainerElement::OpenContainer() {
+  classList().Remove(kClosedCSSClass);
+}
+
+void MediaControlVolumeControlContainerElement::CloseContainer() {
+  classList().Add(kClosedCSSClass);
+}
+
+void MediaControlVolumeControlContainerElement::DefaultEventHandler(
+    Event& event) {
+  if (event.type() == event_type_names::kMouseover)
+    GetMediaControls().OpenVolumeSliderIfNecessary();
+
+  if (event.type() == event_type_names::kMouseout)
+    GetMediaControls().CloseVolumeSliderIfNecessary();
+}
+
+}  // namespace blink

third_party/blink/renderer/modules/media_controls/elements/media_control_volume_control_container_element.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_MEDIA_CONTROLS_ELEMENTS_MEDIA_CONTROL_VOLUME_CONTROL_CONTAINER_ELEMENT_H_
+#define THIRD_PARTY_BLINK_RENDERER_MODULES_MEDIA_CONTROLS_ELEMENTS_MEDIA_CONTROL_VOLUME_CONTROL_CONTAINER_ELEMENT_H_
+
+#include "third_party/blink/renderer/modules/media_controls/elements/media_control_div_element.h"
+
+namespace blink {
+
+class MediaControlsImpl;
+
+class MediaControlVolumeControlContainerElement final
+    : public MediaControlDivElement {
+ public:
+  explicit MediaControlVolumeControlContainerElement(MediaControlsImpl&);
+
+  void OpenContainer();
+  void CloseContainer();
+
+ private:
+  void DefaultEventHandler(Event&) override;
+};
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_MODULES_MEDIA_CONTROLS_ELEMENTS_MEDIA_CONTROL_VOLUME_CONTROL_CONTAINER_ELEMENT_H_

third_party/blink/renderer/modules/media_controls/elements/media_control_volume_slider_element.cc

@@ -93,15 +93,11 @@ void MediaControlVolumeSliderElement::DefaultEventHandler(Event& event) {
     SetVolumeInternal(volume);
   }
 
-  if (event.type() == event_type_names::kMouseover ||
-      event.type() == event_type_names::kFocus) {
+  if (event.type() == event_type_names::kFocus)
     GetMediaControls().OpenVolumeSliderIfNecessary();
-  }
 
-  if (event.type() == event_type_names::kMouseout ||
-      event.type() == event_type_names::kBlur) {
+  if (event.type() == event_type_names::kBlur)
     GetMediaControls().CloseVolumeSliderIfNecessary();
-  }
 }
 
 void MediaControlVolumeSliderElement::SetVolumeInternal(double volume) {

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

@@ -79,6 +79,7 @@
 #include "third_party/blink/renderer/modules/media_controls/elements/media_control_text_track_list_element.h"
 #include "third_party/blink/renderer/modules/media_controls/elements/media_control_timeline_element.h"
 #include "third_party/blink/renderer/modules/media_controls/elements/media_control_toggle_closed_captions_button_element.h"
+#include "third_party/blink/renderer/modules/media_controls/elements/media_control_volume_control_container_element.h"
 #include "third_party/blink/renderer/modules/media_controls/elements/media_control_volume_slider_element.h"
 #include "third_party/blink/renderer/modules/media_controls/media_controls_display_cutout_delegate.h"
 #include "third_party/blink/renderer/modules/media_controls/media_controls_media_event_listener.h"
@@ -373,6 +374,7 @@ MediaControlsImpl::MediaControlsImpl(HTMLMediaElement& media_element)
       duration_display_(nullptr),
       mute_button_(nullptr),
       volume_slider_(nullptr),
+      volume_control_container_(nullptr),
       toggle_closed_captions_button_(nullptr),
       text_track_list_(nullptr),
       overflow_list_(nullptr),
@@ -507,11 +509,15 @@ MediaControlsImpl* MediaControlsImpl::Create(HTMLMediaElement& media_element,
 //     |  +-HTMLDivElement
 //     |  |    (-internal-media-controls-button-spacer)
 //     |  |    {if ModernMediaControls is enabled and is video element}
-//     |  +-MediaControlMuteButtonElement
-//     |  |    (-webkit-media-controls-mute-button)
-//     |  +-MediaControlVolumeSliderElement
-//     |  |    (-webkit-media-controls-volume-slider)
-//     |  |    {if not ModernMediaControlsEnabled}
+//     |  +-MediaControlVolumeControlContainerElement
+//     |  |  |  (-webkit-media-controls-volume-control-container)
+//     |  |  +-HTMLDivElement
+//     |  |  |    (-webkit-media-controls-volume-control-hover-background)
+//     |  |  +-MediaControlMuteButtonElement
+//     |  |  |    (-webkit-media-controls-mute-button)
+//     |  |  +-MediaControlVolumeSliderElement
+//     |  |       (-webkit-media-controls-volume-slider)
+//     |  |       {if not ModernMediaControlsEnabled}
 //     |  +-MediaControlPictureInPictureButtonElement
 //     |  |    (-webkit-media-controls-picture-in-picture-button)
 //     |  +-MediaControlFullscreenButtonElement
@@ -591,6 +597,8 @@ void MediaControlsImpl::InitializeControls() {
   mute_button_ = MakeGarbageCollected<MediaControlMuteButtonElement>(*this);
 
   volume_slider_ = MakeGarbageCollected<MediaControlVolumeSliderElement>(*this);
+  volume_control_container_ =
+      MakeGarbageCollected<MediaControlVolumeControlContainerElement>(*this);
   if (PreferHiddenVolumeControls(GetDocument()))
     volume_slider_->SetIsWanted(false);
 
@@ -695,14 +703,9 @@ void MediaControlsImpl::PopulatePanel() {
 
   // On modern controls, the volume slider is to the left of the mute button.
   if (IsModern()) {
-    volume_control_container_ = MediaControlElementsHelper::CreateDiv(
-        "-webkit-media-controls-volume-control-container", button_panel);
-    MediaControlElementsHelper::CreateDiv(
-        "-webkit-media-controls-volume-control-hover-background",
-        volume_control_container_);
     MaybeParserAppendChild(volume_control_container_, volume_slider_);
     volume_control_container_->ParserAppendChild(mute_button_);
-    HideVolumeControlHoverBackground();
+    button_panel->ParserAppendChild(volume_control_container_);
   } else {
     button_panel->ParserAppendChild(mute_button_);
     MaybeParserAppendChild(button_panel, volume_slider_);
@@ -1406,7 +1409,7 @@ void MediaControlsImpl::UpdateOverflowMenuWanted() const {
     element->SetDoesFit(does_fit);
 
     if (element == mute_button_.Get() && IsModern())
-      SetVolumeControlContainerIsWanted(does_fit);
+      volume_control_container_->SetIsWanted(does_fit);
 
     // The element does fit and is sticky so we should allocate space for it. If
     // we cannot fit this element we should stop allocating space for other
@@ -1438,7 +1441,7 @@ void MediaControlsImpl::UpdateOverflowMenuWanted() const {
     last_element->SetOverflowElementIsWanted(true);
 
     if (last_element == mute_button_.Get() && IsModern())
-      SetVolumeControlContainerIsWanted(false);
+      volume_control_container_->SetIsWanted(false);
   }
 
   MaybeRecordElementsDisplayed();
@@ -2308,7 +2311,7 @@ void MediaControlsImpl::StartHideMediaControlsIfNecessary() {
 
 void MediaControlsImpl::VolumeSliderWantedTimerFired(TimerBase*) {
   volume_slider_->OpenSlider();
-  ShowVolumeControlHoverBackground();
+  volume_control_container_->OpenContainer();
 }
 
 void MediaControlsImpl::OpenVolumeSliderIfNecessary() {
@@ -2316,7 +2319,7 @@ void MediaControlsImpl::OpenVolumeSliderIfNecessary() {
     if (volume_slider_->IsFocused() || mute_button_->IsFocused()) {
       // When we're focusing with the keyboard, we don't need the delay.
       volume_slider_->OpenSlider();
-      ShowVolumeControlHoverBackground();
+      volume_control_container_->OpenContainer();
     } else {
       volume_slider_wanted_timer_.StartOneShot(
           WebTestSupport::IsRunningWebTest() ? kTimeToShowVolumeSliderTest
@@ -2329,31 +2332,13 @@ void MediaControlsImpl::OpenVolumeSliderIfNecessary() {
 void MediaControlsImpl::CloseVolumeSliderIfNecessary() {
   if (ShouldCloseVolumeSlider()) {
     volume_slider_->CloseSlider();
-    HideVolumeControlHoverBackground();
+    volume_control_container_->CloseContainer();
 
     if (volume_slider_wanted_timer_.IsActive())
       volume_slider_wanted_timer_.Stop();
   }
 }
 
-void MediaControlsImpl::ShowVolumeControlHoverBackground() {
-  volume_control_container_->classList().Remove(kClosedCSSClass);
-}
-
-void MediaControlsImpl::HideVolumeControlHoverBackground() {
-  volume_control_container_->classList().Add(kClosedCSSClass);
-}
-
-void MediaControlsImpl::SetVolumeControlContainerIsWanted(
-    bool is_wanted) const {
-  if (is_wanted) {
-    volume_control_container_->RemoveInlineStyleProperty(CSSPropertyDisplay);
-  } else {
-    volume_control_container_->SetInlineStyleProperty(CSSPropertyDisplay,
-                                                      CSSValueNone);
-  }
-}
-
 bool MediaControlsImpl::ShouldOpenVolumeSlider() const {
   if (!volume_slider_ || !IsModern())
     return false;
@@ -2365,7 +2350,7 @@ bool MediaControlsImpl::ShouldCloseVolumeSlider() const {
   if (!volume_slider_ || !IsModern())
     return false;
 
-  return !(volume_slider_->IsHovered() || mute_button_->IsHovered() ||
+  return !(volume_control_container_->IsHovered() ||
            volume_slider_->IsFocused() || mute_button_->IsFocused());
 }
 

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

@@ -64,6 +64,7 @@ class MediaControlScrubbingMessageElement;
 class MediaControlTextTrackListElement;
 class MediaControlTimelineElement;
 class MediaControlToggleClosedCaptionsButtonElement;
+class MediaControlVolumeControlContainerElement;
 class MediaControlVolumeSliderElement;
 class MediaDownloadInProductHelpManager;
 class ShadowRoot;
@@ -282,9 +283,6 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement,
 
   bool ShouldOpenVolumeSlider() const;
   bool ShouldCloseVolumeSlider() const;
-  void ShowVolumeControlHoverBackground();
-  void HideVolumeControlHoverBackground();
-  void SetVolumeControlContainerIsWanted(bool) const;
 
   void ElementSizeChangedTimerFired(TimerBase*);
 
@@ -370,6 +368,7 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement,
   Member<MediaControlRemainingTimeDisplayElement> duration_display_;
   Member<MediaControlMuteButtonElement> mute_button_;
   Member<MediaControlVolumeSliderElement> volume_slider_;
+  Member<MediaControlVolumeControlContainerElement> volume_control_container_;
   Member<MediaControlToggleClosedCaptionsButtonElement>
       toggle_closed_captions_button_;
   Member<MediaControlTextTrackListElement> text_track_list_;
@@ -399,8 +398,6 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement,
   bool is_paused_for_scrubbing_ : 1;
   bool is_scrubbing_ = false;
 
-  Member<HTMLDivElement> volume_control_container_;
-
   // Watches the video element for resize and updates media controls as
   // necessary.
   Member<ResizeObserver> resize_observer_;