Title: [210712] branches/safari-603-branch/Source/WebCore

Diff

Modified: branches/safari-603-branch/Source/WebCore/ChangeLog (210711 => 210712)


--- branches/safari-603-branch/Source/WebCore/ChangeLog	2017-01-13 06:10:34 UTC (rev 210711)
+++ branches/safari-603-branch/Source/WebCore/ChangeLog	2017-01-13 06:10:37 UTC (rev 210712)
@@ -1,5 +1,81 @@
 2017-01-12  Matthew Hanson  <matthew_han...@apple.com>
 
+        Merge r210677. rdar://problem/21482487
+
+    2017-01-12  Jer Noble  <jer.no...@apple.com>
+
+            Protect MediaPlayer from being destroyed mid-load()
+            https://bugs.webkit.org/show_bug.cgi?id=166976
+
+            Reviewed by Eric Carlson.
+
+            It's possible for a message sent by MediaPlayer to HTMLMediaElement to cause
+            MediaPlayer to be destroyed before MediaPlayer::load() completes. We have
+            previously protected against this same problem in HTMLMediaElement::loadResource()
+            by ref'ing at the beginning of the function and deref'ing on exit. To do the
+            same in MediaPlayer, it must become RefCounted.
+
+            To keep the same semantics about m_client in MediaPlayer (always available without
+            requiring a null-check), make a new static MediaPlayerClient object which can
+            replace the real (HTMLMediaElement) client when the MediaPlayer is invalidated.
+
+            * html/HTMLMediaElement.cpp:
+            (WebCore::HTMLMediaElement::~HTMLMediaElement):
+            (WebCore::HTMLMediaElement::clearMediaPlayer):
+            (WebCore::HTMLMediaElement::createMediaPlayer):
+            * html/HTMLMediaElement.h:
+            * platform/graphics/MediaPlayer.cpp:
+            (WebCore::nullMediaPlayerClient):
+            (WebCore::MediaPlayer::create):
+            (WebCore::MediaPlayer::MediaPlayer):
+            (WebCore::MediaPlayer::invalidate):
+            (WebCore::MediaPlayer::load):
+            (WebCore::MediaPlayer::loadWithNextMediaEngine):
+            (WebCore::MediaPlayer::inMediaDocument):
+            (WebCore::MediaPlayer::fullscreenMode):
+            (WebCore::MediaPlayer::requestedRate):
+            (WebCore::MediaPlayer::currentPlaybackTargetIsWirelessChanged):
+            (WebCore::MediaPlayer::networkStateChanged):
+            (WebCore::MediaPlayer::readyStateChanged):
+            (WebCore::MediaPlayer::volumeChanged):
+            (WebCore::MediaPlayer::muteChanged):
+            (WebCore::MediaPlayer::timeChanged):
+            (WebCore::MediaPlayer::sizeChanged):
+            (WebCore::MediaPlayer::repaint):
+            (WebCore::MediaPlayer::durationChanged):
+            (WebCore::MediaPlayer::rateChanged):
+            (WebCore::MediaPlayer::playbackStateChanged):
+            (WebCore::MediaPlayer::firstVideoFrameAvailable):
+            (WebCore::MediaPlayer::characteristicChanged):
+            (WebCore::MediaPlayer::cachedKeyForKeyId):
+            (WebCore::MediaPlayer::keyNeeded):
+            (WebCore::MediaPlayer::mediaKeysStorageDirectory):
+            (WebCore::MediaPlayer::referrer):
+            (WebCore::MediaPlayer::userAgent):
+            (WebCore::MediaPlayer::graphicsDeviceAdapter):
+            (WebCore::MediaPlayer::cachedResourceLoader):
+            (WebCore::MediaPlayer::createResourceLoader):
+            (WebCore::MediaPlayer::addAudioTrack):
+            (WebCore::MediaPlayer::removeAudioTrack):
+            (WebCore::MediaPlayer::addTextTrack):
+            (WebCore::MediaPlayer::removeTextTrack):
+            (WebCore::MediaPlayer::addVideoTrack):
+            (WebCore::MediaPlayer::removeVideoTrack):
+            (WebCore::MediaPlayer::outOfBandTrackSources):
+            (WebCore::MediaPlayer::shouldWaitForResponseToAuthenticationChallenge):
+            (WebCore::MediaPlayer::handlePlaybackCommand):
+            (WebCore::MediaPlayer::sourceApplicationIdentifier):
+            (WebCore::MediaPlayer::preferredAudioCharacteristics):
+            (WebCore::MediaPlayer::doesHaveAttribute):
+            (WebCore::MediaPlayer::mediaPlayerNetworkInterfaceName):
+            (WebCore::MediaPlayer::getRawCookies):
+            (WebCore::MediaPlayer::shouldDisableSleep):
+            * platform/graphics/MediaPlayer.h:
+            (WebCore::MediaPlayer::platformVolumeConfigurationRequired):
+            (WebCore::MediaPlayer::client):
+
+2017-01-12  Matthew Hanson  <matthew_han...@apple.com>
+
         Merge r210663. rdar://problem/29916484
 
     2017-01-12  Said Abou-Hallawa  <sabouhall...@apple.com>

Modified: branches/safari-603-branch/Source/WebCore/html/HTMLMediaElement.cpp (210711 => 210712)


--- branches/safari-603-branch/Source/WebCore/html/HTMLMediaElement.cpp	2017-01-13 06:10:34 UTC (rev 210711)
+++ branches/safari-603-branch/Source/WebCore/html/HTMLMediaElement.cpp	2017-01-13 06:10:37 UTC (rev 210712)
@@ -569,7 +569,10 @@
 
     m_completelyLoaded = true;
 
-    m_player = nullptr;
+    if (m_player) {
+        m_player->invalidate();
+        m_player = nullptr;
+    }
     updatePlaybackControlsManager();
 }
 
@@ -5049,7 +5052,10 @@
         document().removeMediaCanStartListener(this);
     }
 
-    m_player = nullptr;
+    if (m_player) {
+        m_player->invalidate();
+        m_player = nullptr;
+    }
     updatePlaybackControlsManager();
 
     stopPeriodicTimers();
@@ -5990,7 +5996,7 @@
 #if ENABLE(VIDEO_TRACK)
     forgetResourceSpecificTracks();
 #endif
-    m_player = std::make_unique<MediaPlayer>(static_cast<MediaPlayerClient&>(*this));
+    m_player = MediaPlayer::create(*this);
     scheduleUpdatePlaybackControlsManager();
 
 #if ENABLE(WEB_AUDIO)

Modified: branches/safari-603-branch/Source/WebCore/html/HTMLMediaElement.h (210711 => 210712)


--- branches/safari-603-branch/Source/WebCore/html/HTMLMediaElement.h	2017-01-13 06:10:34 UTC (rev 210711)
+++ branches/safari-603-branch/Source/WebCore/html/HTMLMediaElement.h	2017-01-13 06:10:37 UTC (rev 210712)
@@ -888,7 +888,7 @@
     MediaPlayerEnums::VideoGravity m_videoFullscreenGravity { MediaPlayer::VideoGravityResizeAspect };
 #endif
 
-    std::unique_ptr<MediaPlayer> m_player;
+    RefPtr<MediaPlayer> m_player;
 
     MediaPlayerEnums::Preload m_preload { MediaPlayer::Auto };
 

Modified: branches/safari-603-branch/Source/WebCore/platform/graphics/MediaPlayer.cpp (210711 => 210712)


--- branches/safari-603-branch/Source/WebCore/platform/graphics/MediaPlayer.cpp	2017-01-13 06:10:34 UTC (rev 210711)
+++ branches/safari-603-branch/Source/WebCore/platform/graphics/MediaPlayer.cpp	2017-01-13 06:10:37 UTC (rev 210712)
@@ -161,6 +161,12 @@
     bool hasSingleSecurityOrigin() const override { return true; }
 };
 
+static MediaPlayerClient& nullMediaPlayerClient()
+{
+    static NeverDestroyed<MediaPlayerClient> client;
+    return client.get();
+}
+
 // engine support
 
 struct MediaPlayerFactory {
@@ -332,8 +338,13 @@
 
 // media player
 
+Ref<MediaPlayer> MediaPlayer::create(MediaPlayerClient& client)
+{
+    return adoptRef(*new MediaPlayer(client));
+}
+
 MediaPlayer::MediaPlayer(MediaPlayerClient& client)
-    : m_client(client)
+    : m_client(&client)
     , m_reloadTimer(*this, &MediaPlayer::reloadTimerFired)
     , m_private(std::make_unique<NullMediaPlayerPrivate>(this))
     , m_currentMediaEngine(0)
@@ -353,10 +364,18 @@
     ASSERT(!m_initializingMediaEngine);
 }
 
+void MediaPlayer::invalidate()
+{
+    m_client = &nullMediaPlayerClient();
+}
+
 bool MediaPlayer::load(const URL& url, const ContentType& contentType, const String& keySystem)
 {
     ASSERT(!m_reloadTimer.isActive());
 
+    // Protect against MediaPlayer being destroyed during a MediaPlayerClient callback.
+    Ref<MediaPlayer> protectedThis(*this);
+
     m_contentMIMEType = contentType.type().convertToASCIILowercase();
     m_contentTypeCodecs = contentType.parameter(codecs());
     m_url = url;
@@ -474,7 +493,7 @@
     } else if (m_currentMediaEngine != engine) {
         m_currentMediaEngine = engine;
         m_private = engine->constructor(this);
-        m_client.mediaPlayerEngineUpdated(this);
+        client().mediaPlayerEngineUpdated(this);
         m_private->setPrivateBrowsingMode(m_privateBrowsing);
         m_private->setPreload(m_preload);
         m_private->setPreservesPitch(preservesPitch());
@@ -496,8 +515,8 @@
         m_private->load(m_url.string());
     } else {
         m_private = std::make_unique<NullMediaPlayerPrivate>(this);
-        m_client.mediaPlayerEngineUpdated(this);
-        m_client.mediaPlayerResourceNotSupported(this);
+        client().mediaPlayerEngineUpdated(this);
+        client().mediaPlayerResourceNotSupported(this);
     }
 
     m_initializingMediaEngine = false;
@@ -648,7 +667,7 @@
 
 bool MediaPlayer::inMediaDocument() const
 {
-    return m_visible && m_client.mediaPlayerIsInMediaDocument();
+    return m_visible && client().mediaPlayerIsInMediaDocument();
 }
 
 PlatformMedia MediaPlayer::platformMedia() const
@@ -684,7 +703,7 @@
 
 MediaPlayer::VideoFullscreenMode MediaPlayer::fullscreenMode() const
 {
-    return m_client.mediaPlayerFullscreenMode();
+    return client().mediaPlayerFullscreenMode();
 }
 #endif
 
@@ -765,7 +784,7 @@
 
 double MediaPlayer::requestedRate() const
 {
-    return m_client.mediaPlayerRequestedPlaybackRate();
+    return client().mediaPlayerRequestedPlaybackRate();
 }
 
 bool MediaPlayer::preservesPitch() const
@@ -937,7 +956,7 @@
 
 void MediaPlayer::currentPlaybackTargetIsWirelessChanged()
 {
-    m_client.mediaPlayerCurrentPlaybackTargetIsWirelessChanged(this);
+    client().mediaPlayerCurrentPlaybackTargetIsWirelessChanged(this);
 }
 
 bool MediaPlayer::canPlayToWirelessPlaybackTarget() const
@@ -1101,18 +1120,18 @@
     // If more than one media engine is installed and this one failed before finding metadata,
     // let the next engine try.
     if (m_private->networkState() >= FormatError && m_private->readyState() < HaveMetadata) {
-        m_client.mediaPlayerEngineFailedToLoad();
+        client().mediaPlayerEngineFailedToLoad();
         if (installedMediaEngines().size() > 1 && (m_contentMIMEType.isEmpty() || nextBestMediaEngine(m_currentMediaEngine))) {
             m_reloadTimer.startOneShot(0);
             return;
         }
     }
-    m_client.mediaPlayerNetworkStateChanged(this);
+    client().mediaPlayerNetworkStateChanged(this);
 }
 
 void MediaPlayer::readyStateChanged()
 {
-    m_client.mediaPlayerReadyStateChanged(this);
+    client().mediaPlayerReadyStateChanged(this);
 }
 
 void MediaPlayer::volumeChanged(double newVolume)
@@ -1123,53 +1142,53 @@
 #else
     m_volume = newVolume;
 #endif
-    m_client.mediaPlayerVolumeChanged(this);
+    client().mediaPlayerVolumeChanged(this);
 }
 
 void MediaPlayer::muteChanged(bool newMuted)
 {
     m_muted = newMuted;
-    m_client.mediaPlayerMuteChanged(this);
+    client().mediaPlayerMuteChanged(this);
 }
 
 void MediaPlayer::timeChanged()
 {
-    m_client.mediaPlayerTimeChanged(this);
+    client().mediaPlayerTimeChanged(this);
 }
 
 void MediaPlayer::sizeChanged()
 {
-    m_client.mediaPlayerSizeChanged(this);
+    client().mediaPlayerSizeChanged(this);
 }
 
 void MediaPlayer::repaint()
 {
-    m_client.mediaPlayerRepaint(this);
+    client().mediaPlayerRepaint(this);
 }
 
 void MediaPlayer::durationChanged()
 {
-    m_client.mediaPlayerDurationChanged(this);
+    client().mediaPlayerDurationChanged(this);
 }
 
 void MediaPlayer::rateChanged()
 {
-    m_client.mediaPlayerRateChanged(this);
+    client().mediaPlayerRateChanged(this);
 }
 
 void MediaPlayer::playbackStateChanged()
 {
-    m_client.mediaPlayerPlaybackStateChanged(this);
+    client().mediaPlayerPlaybackStateChanged(this);
 }
 
 void MediaPlayer::firstVideoFrameAvailable()
 {
-    m_client.mediaPlayerFirstVideoFrameAvailable(this);
+    client().mediaPlayerFirstVideoFrameAvailable(this);
 }
 
 void MediaPlayer::characteristicChanged()
 {
-    m_client.mediaPlayerCharacteristicChanged(this);
+    client().mediaPlayerCharacteristicChanged(this);
 }
 
 #if ENABLE(WEB_AUDIO)
@@ -1182,28 +1201,28 @@
 #if ENABLE(LEGACY_ENCRYPTED_MEDIA)
 RefPtr<ArrayBuffer> MediaPlayer::cachedKeyForKeyId(const String& keyId) const
 {
-    return m_client.mediaPlayerCachedKeyForKeyId(keyId);
+    return client().mediaPlayerCachedKeyForKeyId(keyId);
 }
 
 bool MediaPlayer::keyNeeded(Uint8Array* initData)
 {
-    return m_client.mediaPlayerKeyNeeded(this, initData);
+    return client().mediaPlayerKeyNeeded(this, initData);
 }
 
 String MediaPlayer::mediaKeysStorageDirectory() const
 {
-    return m_client.mediaPlayerMediaKeysStorageDirectory();
+    return client().mediaPlayerMediaKeysStorageDirectory();
 }
 #endif
 
 String MediaPlayer::referrer() const
 {
-    return m_client.mediaPlayerReferrer();
+    return client().mediaPlayerReferrer();
 }
 
 String MediaPlayer::userAgent() const
 {
-    return m_client.mediaPlayerUserAgent();
+    return client().mediaPlayerUserAgent();
 }
 
 String MediaPlayer::engineDescription() const
@@ -1225,18 +1244,18 @@
 #if PLATFORM(WIN) && USE(AVFOUNDATION)
 GraphicsDeviceAdapter* MediaPlayer::graphicsDeviceAdapter() const
 {
-    return m_client.mediaPlayerGraphicsDeviceAdapter(this);
+    return client().mediaPlayerGraphicsDeviceAdapter(this);
 }
 #endif
 
 CachedResourceLoader* MediaPlayer::cachedResourceLoader()
 {
-    return m_client.mediaPlayerCachedResourceLoader();
+    return client().mediaPlayerCachedResourceLoader();
 }
 
 PassRefPtr<PlatformMediaResourceLoader> MediaPlayer::createResourceLoader()
 {
-    return m_client.mediaPlayerCreateResourceLoader();
+    return client().mediaPlayerCreateResourceLoader();
 }
 
 #if ENABLE(VIDEO_TRACK)
@@ -1243,32 +1262,32 @@
 
 void MediaPlayer::addAudioTrack(AudioTrackPrivate& track)
 {
-    m_client.mediaPlayerDidAddAudioTrack(track);
+    client().mediaPlayerDidAddAudioTrack(track);
 }
 
 void MediaPlayer::removeAudioTrack(AudioTrackPrivate& track)
 {
-    m_client.mediaPlayerDidRemoveAudioTrack(track);
+    client().mediaPlayerDidRemoveAudioTrack(track);
 }
 
 void MediaPlayer::addTextTrack(InbandTextTrackPrivate& track)
 {
-    m_client.mediaPlayerDidAddTextTrack(track);
+    client().mediaPlayerDidAddTextTrack(track);
 }
 
 void MediaPlayer::removeTextTrack(InbandTextTrackPrivate& track)
 {
-    m_client.mediaPlayerDidRemoveTextTrack(track);
+    client().mediaPlayerDidRemoveTextTrack(track);
 }
 
 void MediaPlayer::addVideoTrack(VideoTrackPrivate& track)
 {
-    m_client.mediaPlayerDidAddVideoTrack(track);
+    client().mediaPlayerDidAddVideoTrack(track);
 }
 
 void MediaPlayer::removeVideoTrack(VideoTrackPrivate& track)
 {
-    m_client.mediaPlayerDidRemoveVideoTrack(track);
+    client().mediaPlayerDidRemoveVideoTrack(track);
 }
 
 bool MediaPlayer::requiresTextTrackRepresentation() const
@@ -1301,7 +1320,7 @@
 
 Vector<RefPtr<PlatformTextTrack>> MediaPlayer::outOfBandTrackSources()
 {
-    return m_client.outOfBandTrackSources();
+    return client().outOfBandTrackSources();
 }
 
 #endif
@@ -1391,22 +1410,22 @@
 
 bool MediaPlayer::shouldWaitForResponseToAuthenticationChallenge(const AuthenticationChallenge& challenge)
 {
-    return m_client.mediaPlayerShouldWaitForResponseToAuthenticationChallenge(challenge);
+    return client().mediaPlayerShouldWaitForResponseToAuthenticationChallenge(challenge);
 }
 
 void MediaPlayer::handlePlaybackCommand(PlatformMediaSession::RemoteControlCommandType command)
 {
-    m_client.mediaPlayerHandlePlaybackCommand(command);
+    client().mediaPlayerHandlePlaybackCommand(command);
 }
 
 String MediaPlayer::sourceApplicationIdentifier() const
 {
-    return m_client.mediaPlayerSourceApplicationIdentifier();
+    return client().mediaPlayerSourceApplicationIdentifier();
 }
 
 Vector<String> MediaPlayer::preferredAudioCharacteristics() const
 {
-    return m_client.mediaPlayerPreferredAudioCharacteristics();
+    return client().mediaPlayerPreferredAudioCharacteristics();
 }
 
 void MediaPlayerFactorySupport::callRegisterMediaEngine(MediaEngineRegister registerMediaEngine)
@@ -1416,18 +1435,18 @@
 
 bool MediaPlayer::doesHaveAttribute(const AtomicString& attribute, AtomicString* value) const
 {
-    return m_client.doesHaveAttribute(attribute, value);
+    return client().doesHaveAttribute(attribute, value);
 }
 
 #if PLATFORM(IOS)
 String MediaPlayer::mediaPlayerNetworkInterfaceName() const
 {
-    return m_client.mediaPlayerNetworkInterfaceName();
+    return client().mediaPlayerNetworkInterfaceName();
 }
 
 bool MediaPlayer::getRawCookies(const URL& url, Vector<Cookie>& cookies) const
 {
-    return m_client.mediaPlayerGetRawCookies(url, cookies);
+    return client().mediaPlayerGetRawCookies(url, cookies);
 }
 #endif
 
@@ -1439,7 +1458,7 @@
 
 bool MediaPlayer::shouldDisableSleep() const
 {
-    return m_client.mediaPlayerShouldDisableSleep();
+    return client().mediaPlayerShouldDisableSleep();
 }
 
 }

Modified: branches/safari-603-branch/Source/WebCore/platform/graphics/MediaPlayer.h (210711 => 210712)


--- branches/safari-603-branch/Source/WebCore/platform/graphics/MediaPlayer.h	2017-01-13 06:10:34 UTC (rev 210711)
+++ branches/safari-603-branch/Source/WebCore/platform/graphics/MediaPlayer.h	2017-01-13 06:10:37 UTC (rev 210712)
@@ -48,6 +48,8 @@
 #include <wtf/HashSet.h>
 #include <wtf/MediaTime.h>
 #include <wtf/Noncopyable.h>
+#include <wtf/Ref.h>
+#include <wtf/RefCounted.h>
 #include <wtf/text/StringHash.h>
 
 #if ENABLE(AVF_CAPTIONS)
@@ -281,12 +283,14 @@
     virtual String mediaPlayerDocumentHost() const { return String(); }
 };
 
-class MediaPlayer : public MediaPlayerEnums {
+class MediaPlayer : public MediaPlayerEnums, public RefCounted<MediaPlayer> {
     WTF_MAKE_NONCOPYABLE(MediaPlayer); WTF_MAKE_FAST_ALLOCATED;
 public:
-    explicit MediaPlayer(MediaPlayerClient&);
+    static Ref<MediaPlayer> create(MediaPlayerClient&);
     virtual ~MediaPlayer();
 
+    void invalidate();
+
     // Media engine support.
     enum SupportsType { IsNotSupported, IsSupported, MayBeSupported };
     static MediaPlayer::SupportsType supportsType(const MediaEngineSupportParameters&, const MediaPlayerSupportsTypeClient*);
@@ -386,7 +390,7 @@
 
     double volume() const;
     void setVolume(double);
-    bool platformVolumeConfigurationRequired() const { return m_client.mediaPlayerPlatformVolumeConfigurationRequired(); }
+    bool platformVolumeConfigurationRequired() const { return client().mediaPlayerPlatformVolumeConfigurationRequired(); }
 
     bool muted() const;
     void setMuted(bool);
@@ -444,7 +448,7 @@
 
     void repaint();
 
-    MediaPlayerClient& client() const { return m_client; }
+    MediaPlayerClient& client() const { return *m_client; }
 
     bool hasAvailableVideoFrame() const;
     void prepareForRendering();
@@ -582,6 +586,8 @@
     bool shouldDisableSleep() const;
 
 private:
+    MediaPlayer(MediaPlayerClient&);
+
     const MediaPlayerFactory* nextBestMediaEngine(const MediaPlayerFactory*) const;
     void loadWithNextMediaEngine(const MediaPlayerFactory*);
     void reloadTimerFired();
@@ -588,7 +594,7 @@
 
     static void initializeMediaEngines();
 
-    MediaPlayerClient& m_client;
+    MediaPlayerClient* m_client;
     Timer m_reloadTimer;
     std::unique_ptr<MediaPlayerPrivateInterface> m_private;
     const MediaPlayerFactory* m_currentMediaEngine;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to