Modified: trunk/Source/WebCore/ChangeLog (210676 => 210677)
--- trunk/Source/WebCore/ChangeLog 2017-01-12 20:39:40 UTC (rev 210676)
+++ trunk/Source/WebCore/ChangeLog 2017-01-12 20:57:32 UTC (rev 210677)
@@ -1,3 +1,75 @@
+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 Sam Weinig <s...@webkit.org>
Update bindings test results.
Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (210676 => 210677)
--- trunk/Source/WebCore/html/HTMLMediaElement.cpp 2017-01-12 20:39:40 UTC (rev 210676)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp 2017-01-12 20:57:32 UTC (rev 210677)
@@ -569,7 +569,10 @@
m_completelyLoaded = true;
- m_player = nullptr;
+ if (m_player) {
+ m_player->invalidate();
+ m_player = nullptr;
+ }
updatePlaybackControlsManager();
}
@@ -5051,7 +5054,10 @@
document().removeMediaCanStartListener(this);
}
- m_player = nullptr;
+ if (m_player) {
+ m_player->invalidate();
+ m_player = nullptr;
+ }
updatePlaybackControlsManager();
stopPeriodicTimers();
@@ -5992,7 +5998,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: trunk/Source/WebCore/platform/graphics/MediaPlayer.cpp (210676 => 210677)
--- trunk/Source/WebCore/platform/graphics/MediaPlayer.cpp 2017-01-12 20:39:40 UTC (rev 210676)
+++ trunk/Source/WebCore/platform/graphics/MediaPlayer.cpp 2017-01-12 20:57:32 UTC (rev 210677)
@@ -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: trunk/Source/WebCore/platform/graphics/MediaPlayer.h (210676 => 210677)
--- trunk/Source/WebCore/platform/graphics/MediaPlayer.h 2017-01-12 20:39:40 UTC (rev 210676)
+++ trunk/Source/WebCore/platform/graphics/MediaPlayer.h 2017-01-12 20:57:32 UTC (rev 210677)
@@ -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;