Title: [247380] trunk/Source/WebCore
- Revision
- 247380
- Author
- cdu...@apple.com
- Date
- 2019-07-11 19:35:07 -0700 (Thu, 11 Jul 2019)
Log Message
Drop non thread-safe usage of WeakPtr in PlaybackSessionInterfaceAVKit
https://bugs.webkit.org/show_bug.cgi?id=199698
Reviewed by Eric Carlson.
The PlaybackSessionInterfaceAVKit 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 r243337, 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_playbackSessionModel data
member. This gives the sames guarantees than WeakPtr but in a thread-safe way.
* platform/cocoa/PlaybackSessionModel.h:
(WebCore::PlaybackSessionModelClient::modelDestroyed):
* platform/ios/PlaybackSessionInterfaceAVKit.h:
* platform/ios/PlaybackSessionInterfaceAVKit.mm:
(WebCore::PlaybackSessionInterfaceAVKit::PlaybackSessionInterfaceAVKit):
(WebCore::PlaybackSessionInterfaceAVKit::~PlaybackSessionInterfaceAVKit):
(WebCore::PlaybackSessionInterfaceAVKit::playbackSessionModel const):
(WebCore::PlaybackSessionInterfaceAVKit::modelDestroyed):
* platform/ios/WebVideoFullscreenControllerAVKit.mm:
(VideoFullscreenControllerContext::~VideoFullscreenControllerContext):
(VideoFullscreenControllerContext::addClient):
(VideoFullscreenControllerContext::removeClient):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (247379 => 247380)
--- trunk/Source/WebCore/ChangeLog 2019-07-12 02:20:51 UTC (rev 247379)
+++ trunk/Source/WebCore/ChangeLog 2019-07-12 02:35:07 UTC (rev 247380)
@@ -1,3 +1,34 @@
+2019-07-11 Chris Dumez <cdu...@apple.com>
+
+ Drop non thread-safe usage of WeakPtr in PlaybackSessionInterfaceAVKit
+ https://bugs.webkit.org/show_bug.cgi?id=199698
+
+ Reviewed by Eric Carlson.
+
+ The PlaybackSessionInterfaceAVKit 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 r243337, 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_playbackSessionModel data
+ member. This gives the sames guarantees than WeakPtr but in a thread-safe way.
+
+ * platform/cocoa/PlaybackSessionModel.h:
+ (WebCore::PlaybackSessionModelClient::modelDestroyed):
+ * platform/ios/PlaybackSessionInterfaceAVKit.h:
+ * platform/ios/PlaybackSessionInterfaceAVKit.mm:
+ (WebCore::PlaybackSessionInterfaceAVKit::PlaybackSessionInterfaceAVKit):
+ (WebCore::PlaybackSessionInterfaceAVKit::~PlaybackSessionInterfaceAVKit):
+ (WebCore::PlaybackSessionInterfaceAVKit::playbackSessionModel const):
+ (WebCore::PlaybackSessionInterfaceAVKit::modelDestroyed):
+ * platform/ios/WebVideoFullscreenControllerAVKit.mm:
+ (VideoFullscreenControllerContext::~VideoFullscreenControllerContext):
+ (VideoFullscreenControllerContext::addClient):
+ (VideoFullscreenControllerContext::removeClient):
+
2019-07-11 Simon Fraser <simon.fra...@apple.com>
Fix builds where HAVE_DESIGN_SYSTEM_UI_FONTS is not defined.
Modified: trunk/Source/WebCore/platform/cocoa/PlaybackSessionModel.h (247379 => 247380)
--- trunk/Source/WebCore/platform/cocoa/PlaybackSessionModel.h 2019-07-12 02:20:51 UTC (rev 247379)
+++ trunk/Source/WebCore/platform/cocoa/PlaybackSessionModel.h 2019-07-12 02:35:07 UTC (rev 247380)
@@ -110,6 +110,7 @@
virtual void isPictureInPictureSupportedChanged(bool) { }
virtual void pictureInPictureActiveChanged(bool) { }
virtual void ensureControlsManager() { }
+ virtual void modelDestroyed() { }
};
}
Modified: trunk/Source/WebCore/platform/ios/PlaybackSessionInterfaceAVKit.h (247379 => 247380)
--- trunk/Source/WebCore/platform/ios/PlaybackSessionInterfaceAVKit.h 2019-07-12 02:20:51 UTC (rev 247379)
+++ trunk/Source/WebCore/platform/ios/PlaybackSessionInterfaceAVKit.h 2019-07-12 02:35:07 UTC (rev 247380)
@@ -77,6 +77,7 @@
WEBCORE_EXPORT void wirelessVideoPlaybackDisabledChanged(bool) override;
WEBCORE_EXPORT void mutedChanged(bool) override;
WEBCORE_EXPORT void volumeChanged(double) override;
+ WEBCORE_EXPORT void modelDestroyed() override;
WEBCORE_EXPORT virtual void invalidate();
@@ -86,7 +87,7 @@
WEBCORE_EXPORT PlaybackSessionInterfaceAVKit(PlaybackSessionModel&);
RetainPtr<WebAVPlayerController> m_playerController;
- WeakPtr<PlaybackSessionModel> m_playbackSessionModel;
+ PlaybackSessionModel* m_playbackSessionModel { nullptr };
};
}
Modified: trunk/Source/WebCore/platform/ios/PlaybackSessionInterfaceAVKit.mm (247379 => 247380)
--- trunk/Source/WebCore/platform/ios/PlaybackSessionInterfaceAVKit.mm 2019-07-12 02:20:51 UTC (rev 247379)
+++ trunk/Source/WebCore/platform/ios/PlaybackSessionInterfaceAVKit.mm 2019-07-12 02:35:07 UTC (rev 247380)
@@ -51,8 +51,9 @@
PlaybackSessionInterfaceAVKit::PlaybackSessionInterfaceAVKit(PlaybackSessionModel& model)
: m_playerController(adoptNS([[WebAVPlayerController alloc] init]))
- , m_playbackSessionModel(makeWeakPtr(model))
+ , m_playbackSessionModel(&model)
{
+ ASSERT(isUIThread());
model.addClient(*this);
[m_playerController setPlaybackSessionInterface:this];
[m_playerController setDelegate:&model];
@@ -71,6 +72,7 @@
PlaybackSessionInterfaceAVKit::~PlaybackSessionInterfaceAVKit()
{
+ ASSERT(isUIThread());
[m_playerController setPlaybackSessionInterface:nullptr];
[m_playerController setExternalPlaybackActive:false];
@@ -79,7 +81,7 @@
PlaybackSessionModel* PlaybackSessionInterfaceAVKit::playbackSessionModel() const
{
- return m_playbackSessionModel.get();
+ return m_playbackSessionModel;
}
void PlaybackSessionInterfaceAVKit::durationChanged(double duration)
@@ -219,7 +221,14 @@
m_playbackSessionModel = nullptr;
}
+void PlaybackSessionInterfaceAVKit::modelDestroyed()
+{
+ ASSERT(isUIThread());
+ invalidate();
+ ASSERT(!m_playbackSessionModel);
}
+}
+
#endif // HAVE(AVKIT)
#endif // PLATFORM(IOS_FAMILY)
Modified: trunk/Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm (247379 => 247380)
--- trunk/Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm 2019-07-12 02:20:51 UTC (rev 247379)
+++ trunk/Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm 2019-07-12 02:35:07 UTC (rev 247380)
@@ -109,6 +109,8 @@
return adoptRef(*new VideoFullscreenControllerContext);
}
+ ~VideoFullscreenControllerContext();
+
void setController(WebVideoFullscreenController* controller) { m_controller = controller; }
void setUpFullscreen(HTMLVideoElement&, UIView *, HTMLMediaElementEnums::VideoFullscreenMode);
void exitFullscreen();
@@ -216,6 +218,18 @@
RetainPtr<WebVideoFullscreenController> m_controller;
};
+VideoFullscreenControllerContext::~VideoFullscreenControllerContext()
+{
+ auto notifyClientsModelWasDestroyed = [this] {
+ while (!m_playbackClients.isEmpty())
+ (*m_playbackClients.begin())->modelDestroyed();
+ };
+ if (isUIThread())
+ notifyClientsModelWasDestroyed();
+ else
+ dispatch_sync(dispatch_get_main_queue(), WTFMove(notifyClientsModelWasDestroyed));
+}
+
#pragma mark VideoFullscreenChangeObserver
void VideoFullscreenControllerContext::requestUpdateInlineRect()
@@ -543,6 +557,7 @@
void VideoFullscreenControllerContext::addClient(VideoFullscreenModelClient& client)
{
+ ASSERT(isUIThread());
ASSERT(!m_fullscreenClients.contains(&client));
m_fullscreenClients.add(&client);
}
@@ -549,6 +564,7 @@
void VideoFullscreenControllerContext::removeClient(VideoFullscreenModelClient& client)
{
+ ASSERT(isUIThread());
ASSERT(m_fullscreenClients.contains(&client));
m_fullscreenClients.remove(&client);
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
