Attachment 339945 Details for Bug 185370 – Patch

[patch] Patch

bug-185370-20180509102421.patch (text/plain), 6.30 KB, created by Xabier Rodríguez Calvar on 2018-05-09 01:24:22 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Xabier Rodríguez Calvar

Created: 2018-05-09 01:24:22 PDT

Size: 6.30 KB

patch

obsolete

>Subversion Revision: 231482
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 9978f968a09a6e95f12bfc811c4711f7d5951d57..4e9175e957daf0dd81e86e3bb93a3d62a1a7017b 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2018-05-09  Xabier Rodriguez Calvar  <calvaris@igalia.com>
>+
>+        WKPageSetMediaVolume() API couldn't be used to restore the volume
>+        https://bugs.webkit.org/show_bug.cgi?id=185370
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The problem happens when the multiplier goes down to the player
>+        and comes back after the volume is set, that volume is set as the
>+        element's and following calls to that won't be able to recover
>+        it. This patch changes the volume storage in the element without
>+        taking into account the multiplier.
>+
>+        * html/HTMLMediaElement.cpp:
>+        (WebCore::HTMLMediaElement::volume const): Converts from internal
>+        not scaled volume to scaled volume.
>+        (WebCore::HTMLMediaElement::setScaledVolume): Converts from scaled
>+        volume to internally unscaled.
>+        (WebCore::HTMLMediaElement::setReadyState):
>+        (WebCore::HTMLMediaElement::setVolume):
>+        (WebCore::HTMLMediaElement::mediaPlayerVolumeChanged):
>+        (WebCore::HTMLMediaElement::updateVolume): Use volume() and
>+        setScaledVolume().
>+        * html/HTMLMediaElement.h: Added a comment to the m_volume
>+        attribute regarding the scale.
>+
> 2018-05-07  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Add FormattingContext::layoutOutOfFlowDescendants implementation
>diff --git a/Source/WebCore/html/HTMLMediaElement.cpp b/Source/WebCore/html/HTMLMediaElement.cpp
>index 96cf8b9b14a0e91fb480e2ce303d3682c03efa2b..0ffe203cdaf64d652efe2dd4fba3b8ed661634c0 100644
>--- a/Source/WebCore/html/HTMLMediaElement.cpp
>+++ b/Source/WebCore/html/HTMLMediaElement.cpp
>@@ -2503,7 +2503,7 @@ void HTMLMediaElement::setReadyState(MediaPlayer::ReadyState state)
>         if (hasEventListeners(eventNames().webkitplaybacktargetavailabilitychangedEvent))
>             enqueuePlaybackTargetAvailabilityChangedEvent();
> #endif
>-        m_initiallyMuted = m_volume < 0.05 || muted();
>+        m_initiallyMuted = volume() < 0.05 || muted();
> 
>         if (hasMediaControls())
>             mediaControls()->loadedMetadata();
>@@ -3630,24 +3630,49 @@ void HTMLMediaElement::setControls(bool b)
> 
> double HTMLMediaElement::volume() const
> {
>-    return m_volume;
>+    double volume = m_volume;
>+    Page* page = document().page();
>+    if (page)
>+        volume *= page->mediaVolume();
>+    if (m_mediaController)
>+        volume *= m_mediaController->volume();
>+#if ENABLE(MEDIA_SESSION)
>+    if (m_shouldDuck)
>+        volume *= 0.25;
>+#endif
>+    return volume;
>+}
>+
>+void HTMLMediaElement::setScaledVolume(double volume)
>+{
>+    m_volume = volume;
>+    Page* page = document().page();
>+    if (page)
>+        m_volume /= page->mediaVolume();
>+    if (m_mediaController)
>+        m_volume /= m_mediaController->volume();
>+#if ENABLE(MEDIA_SESSION)
>+    if (m_shouldDuck)
>+        m_volume /= 0.25;
>+#endif
>+    m_volume = CLAMP(m_volume, 0.0, 1.0);
> }
> 
>-ExceptionOr<void> HTMLMediaElement::setVolume(double volume)
>+ExceptionOr<void> HTMLMediaElement::setVolume(double newVolume)
> {
>-    INFO_LOG(LOGIDENTIFIER, volume);
>+    INFO_LOG(LOGIDENTIFIER, newVolume);
> 
>-    if (!(volume >= 0 && volume <= 1))
>+    if (!(newVolume >= 0 && newVolume <= 1))
>         return Exception { IndexSizeError };
> 
> #if !PLATFORM(IOS)
>-    if (m_volume == volume)
>+    if (volume() == newVolume)
>         return { };
> 
>-    if (volume && processingUserGestureForMedia())
>+    if (newVolume && processingUserGestureForMedia())
>         removeBehaviorsRestrictionsAfterFirstUserGesture(MediaElementSession::AllRestrictions & ~MediaElementSession::RequireUserGestureToControlControlsManager);
> 
>-    m_volume = volume;
>+    setScaledVolume(newVolume);
>     m_volumeInitialized = true;
>     updateVolume();
>     scheduleEvent(eventNames().volumechangeEvent);
>@@ -4812,8 +4837,8 @@ void HTMLMediaElement::mediaPlayerVolumeChanged(MediaPlayer*)
>     beginProcessingMediaPlayerCallback();
>     if (m_player) {
>         double vol = m_player->volume();
>-        if (vol != m_volume) {
>-            m_volume = vol;
>+        if (vol != volume()) {
>+            setScaledVolume(vol);
>             updateVolume();
>             scheduleEvent(eventNames().volumechangeEvent);
>         }
>@@ -5214,29 +5239,22 @@ void HTMLMediaElement::updateVolume()
> #if PLATFORM(IOS)
>     // Only the user can change audio volume so update the cached volume and post the changed event.
>     float volume = m_player->volume();
>-    if (m_volume != volume) {
>-        m_volume = volume;
>+    if (volume() != volume) {
>+        setScaledVolume(volume);
>         scheduleEvent(eventNames().volumechangeEvent);
>     }
> #else
>     // Avoid recursion when the player reports volume changes.
>     if (!processingMediaPlayerCallback()) {
>-        Page* page = document().page();
>-        double volumeMultiplier = page ? page->mediaVolume() : 1;
>         bool shouldMute = effectiveMuted();
> 
>         if (m_mediaController) {
>-            volumeMultiplier *= m_mediaController->volume();
>+            Page* page = document().page();
>             shouldMute = m_mediaController->muted() || (page && page->isAudioMuted());
>         }
> 
>-#if ENABLE(MEDIA_SESSION)
>-        if (m_shouldDuck)
>-            volumeMultiplier *= 0.25;
>-#endif
>-
>         m_player->setMuted(shouldMute);
>-        m_player->setVolume(m_volume * volumeMultiplier);
>+        m_player->setVolume(volume());
>     }
> 
> #if ENABLE(MEDIA_SESSION)
>diff --git a/Source/WebCore/html/HTMLMediaElement.h b/Source/WebCore/html/HTMLMediaElement.h
>index ba9f59a77c27bb9e738397e3baf5ae0648b00503..9dff7eadc1ab9d4daeffda4b473861d409c64adb 100644
>--- a/Source/WebCore/html/HTMLMediaElement.h
>+++ b/Source/WebCore/html/HTMLMediaElement.h
>@@ -814,6 +814,8 @@ private:
> 
>     void setPlaybackRateInternal(double);
> 
>+    void setScaledVolume(double);
>+
>     void mediaCanStart(Document&) final;
> 
>     void invalidateCachedTime() const;
>@@ -965,6 +967,7 @@ private:
>     std::unique_ptr<PendingSeek> m_pendingSeek;
>     SeekType m_pendingSeekType { NoSeek };
> 
>+    // Volume is always referenced to considering page volume to be 1.
>     double m_volume { 1 };
>     bool m_volumeInitialized { false };
>     MediaTime m_lastSeekTime;

Actions: View | Formatted Diff | Diff

Attachments on bug 185370: 339705 | 339715 | 339945