Drop non thread-safe usage of WeakPtr in VideoFullscreenInterfaceAVKit
https://bugs.webkit.org/show_bug.cgi?id=199775
Reviewed by Eric Carlson.
The VideoFullscreenInterfaceAVKit constructor was making a weakPtr on the UI Thread
of an WebThread object. The WeakPtr would then be used as a data member throughout
the class on the UIThread. This is not thread-safe.
This patch switches to using a raw pointer instead of a WeakPtr. This is a partial
rollout of r243298, which turned the raw pointer into a WeakPtr for hardening
purposes. For extra safety, this patch updates the VideoFullscreenControllerContext
so that it notifies its clients (i.e. PlaybackSessionInterfaceAVKit) that it is
getting destroyed, so that they can null-out their m_videoFullscreenModel &
m_fullscreenChangeObserver data members. This gives the sames guarantees as WeakPtr
but in a thread-safe way.
This is very similar to the fix that was done for PlaybackSessionInterfaceAVKit in
r247380.
* platform/cocoa/VideoFullscreenModel.h:
(WebCore::VideoFullscreenModelClient::modelDestroyed):
* platform/ios/VideoFullscreenInterfaceAVKit.h:
* platform/ios/VideoFullscreenInterfaceAVKit.mm:
(VideoFullscreenInterfaceAVKit::setVideoFullscreenModel):
(VideoFullscreenInterfaceAVKit::setVideoFullscreenChangeObserver):
(VideoFullscreenInterfaceAVKit::modelDestroyed):
* platform/ios/WebVideoFullscreenControllerAVKit.mm:
(VideoFullscreenControllerContext::~VideoFullscreenControllerContext):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@247417 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 52ca927..9e5d9f3 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,35 @@
+2019-07-13 Chris Dumez <cdumez@apple.com>
+
+ Drop non thread-safe usage of WeakPtr in VideoFullscreenInterfaceAVKit
+ https://bugs.webkit.org/show_bug.cgi?id=199775
+
+ Reviewed by Eric Carlson.
+
+ The VideoFullscreenInterfaceAVKit constructor was making a weakPtr on the UI Thread
+ of an WebThread object. The WeakPtr would then be used as a data member throughout
+ the class on the UIThread. This is not thread-safe.
+
+ This patch switches to using a raw pointer instead of a WeakPtr. This is a partial
+ rollout of r243298, which turned the raw pointer into a WeakPtr for hardening
+ purposes. For extra safety, this patch updates the VideoFullscreenControllerContext
+ so that it notifies its clients (i.e. PlaybackSessionInterfaceAVKit) that it is
+ getting destroyed, so that they can null-out their m_videoFullscreenModel &
+ m_fullscreenChangeObserver data members. This gives the sames guarantees as WeakPtr
+ but in a thread-safe way.
+
+ This is very similar to the fix that was done for PlaybackSessionInterfaceAVKit in
+ r247380.
+
+ * platform/cocoa/VideoFullscreenModel.h:
+ (WebCore::VideoFullscreenModelClient::modelDestroyed):
+ * platform/ios/VideoFullscreenInterfaceAVKit.h:
+ * platform/ios/VideoFullscreenInterfaceAVKit.mm:
+ (VideoFullscreenInterfaceAVKit::setVideoFullscreenModel):
+ (VideoFullscreenInterfaceAVKit::setVideoFullscreenChangeObserver):
+ (VideoFullscreenInterfaceAVKit::modelDestroyed):
+ * platform/ios/WebVideoFullscreenControllerAVKit.mm:
+ (VideoFullscreenControllerContext::~VideoFullscreenControllerContext):
+
2019-07-13 Zalan Bujtas <zalan@apple.com>
Cannot bring up custom media controls at all on v.youku.com
diff --git a/Source/WebCore/platform/cocoa/VideoFullscreenModel.h b/Source/WebCore/platform/cocoa/VideoFullscreenModel.h
index 7b2bfa6..3f457d3 100644
--- a/Source/WebCore/platform/cocoa/VideoFullscreenModel.h
+++ b/Source/WebCore/platform/cocoa/VideoFullscreenModel.h
@@ -83,6 +83,7 @@
virtual void failedToEnterPictureInPicture() { }
virtual void willExitPictureInPicture() { }
virtual void didExitPictureInPicture() { }
+ virtual void modelDestroyed() { }
};
WEBCORE_EXPORT bool supportsPictureInPicture();
diff --git a/Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.h b/Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.h
index 5d494f8..e7485cb 100644
--- a/Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.h
+++ b/Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.h
@@ -74,6 +74,7 @@
// VideoFullscreenModelClient
WEBCORE_EXPORT void hasVideoChanged(bool) final;
WEBCORE_EXPORT void videoDimensionsChanged(const FloatSize&) final;
+ WEBCORE_EXPORT void modelDestroyed() final;
// PlaybackSessionModelClient
WEBCORE_EXPORT void externalPlaybackChanged(bool enabled, PlaybackSessionModel::ExternalPlaybackTargetType, const String& localizedDeviceName) final;
@@ -127,7 +128,7 @@
Mode m_currentMode;
Mode m_targetMode;
- VideoFullscreenModel* videoFullscreenModel() const { return m_videoFullscreenModel.get(); }
+ VideoFullscreenModel* videoFullscreenModel() const { return m_videoFullscreenModel; }
bool shouldExitFullscreenWithReason(ExitFullScreenReason);
HTMLMediaElementEnums::VideoFullscreenMode mode() const { return m_currentMode.mode(); }
bool allowsPictureInPicturePlayback() const { return m_allowsPictureInPicturePlayback; }
@@ -171,8 +172,8 @@
Ref<PlaybackSessionInterfaceAVKit> m_playbackSessionInterface;
RetainPtr<WebAVPlayerViewControllerDelegate> m_playerViewControllerDelegate;
RetainPtr<WebAVPlayerViewController> m_playerViewController;
- WeakPtr<VideoFullscreenModel> m_videoFullscreenModel;
- WeakPtr<VideoFullscreenChangeObserver> m_fullscreenChangeObserver;
+ VideoFullscreenModel* m_videoFullscreenModel { nullptr };
+ VideoFullscreenChangeObserver* m_fullscreenChangeObserver { nullptr };
// These are only used when fullscreen is presented in a separate window.
RetainPtr<UIWindow> m_window;
diff --git a/Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.mm b/Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.mm
index 5739561..5a7465d 100644
--- a/Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.mm
+++ b/Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.mm
@@ -760,7 +760,7 @@
if (m_videoFullscreenModel)
m_videoFullscreenModel->removeClient(*this);
- m_videoFullscreenModel = makeWeakPtr(model);
+ m_videoFullscreenModel = model;
if (m_videoFullscreenModel) {
m_videoFullscreenModel->addClient(*this);
@@ -779,7 +779,7 @@
void VideoFullscreenInterfaceAVKit::setVideoFullscreenChangeObserver(VideoFullscreenChangeObserver* observer)
{
- m_fullscreenChangeObserver = makeWeakPtr(observer);
+ m_fullscreenChangeObserver = observer;
}
void VideoFullscreenInterfaceAVKit::hasVideoChanged(bool hasVideo)
@@ -930,6 +930,12 @@
cleanupFullscreen();
}
+void VideoFullscreenInterfaceAVKit::modelDestroyed()
+{
+ ASSERT(isUIThread());
+ invalidate();
+}
+
void VideoFullscreenInterfaceAVKit::requestHideAndExitFullscreen()
{
if (m_currentMode.hasPictureInPicture())
diff --git a/Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm b/Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm
index d894c94..436a970 100644
--- a/Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm
+++ b/Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm
@@ -223,6 +223,8 @@
auto notifyClientsModelWasDestroyed = [this] {
while (!m_playbackClients.isEmpty())
(*m_playbackClients.begin())->modelDestroyed();
+ while (!m_fullscreenClients.isEmpty())
+ (*m_fullscreenClients.begin())->modelDestroyed();
};
if (isUIThread())
notifyClientsModelWasDestroyed();
