WebKit Bugzilla
Attachment 373733 Details for
Bug 199626
: Fix non thread-safe use of WeakPtr in DisplayRefreshMonitorMac::displayLinkFired()
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-199626-20190709110017.patch (text/plain), 6.08 KB, created by
Chris Dumez
on 2019-07-09 11:00:17 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Chris Dumez
Created:
2019-07-09 11:00:17 PDT
Size:
6.08 KB
patch
obsolete
>Subversion Revision: 247259 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 5192537beda30a7a91b48fe51f9afe813c439205..75838bb5d9dd1cfd712d79ec4c955e4cf579eb41 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,28 @@ >+2019-07-09 Chris Dumez <cdumez@apple.com> >+ >+ Fix non thread-safe use of WeakPtr in DisplayRefreshMonitorMac::displayLinkFired() >+ https://bugs.webkit.org/show_bug.cgi?id=199626 >+ >+ Reviewed by Ryosuke Niwa. >+ >+ Fix non thread-safe use of WeakPtr in DisplayRefreshMonitorMac::displayLinkFired(). >+ DisplayRefreshMonitorMac gets constructed / destroyed on the main thread, it is >+ not thread-safe to call makeWeakPtr() on a DisplayRefreshMonitorMac object like it >+ was done before. >+ >+ To address the issue, mark the object as ThreadSafeRefCounted and ref the object >+ in the lambda instead. >+ >+ * platform/graphics/DisplayRefreshMonitor.h: >+ (WebCore::DisplayRefreshMonitor::stop): >+ * platform/graphics/DisplayRefreshMonitorManager.cpp: >+ (WebCore::DisplayRefreshMonitorManager::unregisterClient): >+ * platform/graphics/mac/DisplayRefreshMonitorMac.cpp: >+ (WebCore::DisplayRefreshMonitorMac::~DisplayRefreshMonitorMac): >+ (WebCore::DisplayRefreshMonitorMac::stop): >+ (WebCore::DisplayRefreshMonitorMac::displayLinkFired): >+ * platform/graphics/mac/DisplayRefreshMonitorMac.h: >+ > 2019-07-09 Zalan Bujtas <zalan@apple.com> > > [LFC][IFC] Remove InlineItem references from inline runs. >diff --git a/Source/WebCore/platform/graphics/DisplayRefreshMonitor.h b/Source/WebCore/platform/graphics/DisplayRefreshMonitor.h >index 18eeaa02963690801993bfff8343fffe4d3efc3e..d3542b083139bb93b08dd76a20787cfd2b9883f8 100644 >--- a/Source/WebCore/platform/graphics/DisplayRefreshMonitor.h >+++ b/Source/WebCore/platform/graphics/DisplayRefreshMonitor.h >@@ -30,7 +30,7 @@ > #include "PlatformScreen.h" > #include <wtf/HashSet.h> > #include <wtf/Lock.h> >-#include <wtf/RefCounted.h> >+#include <wtf/ThreadSafeRefCounted.h> > #include <wtf/RefPtr.h> > > namespace WebCore { >@@ -38,7 +38,7 @@ namespace WebCore { > class DisplayAnimationClient; > class DisplayRefreshMonitorClient; > >-class DisplayRefreshMonitor : public RefCounted<DisplayRefreshMonitor> { >+class DisplayRefreshMonitor : public ThreadSafeRefCounted<DisplayRefreshMonitor> { > public: > static RefPtr<DisplayRefreshMonitor> create(DisplayRefreshMonitorClient&); > WEBCORE_EXPORT virtual ~DisplayRefreshMonitor(); >@@ -48,6 +48,9 @@ public: > // Return true if callback request was scheduled, false if it couldn't be > // (e.g., hardware refresh is not available) > virtual bool requestRefreshCallback() = 0; >+ >+ virtual void stop() { } >+ > void windowScreenDidChange(PlatformDisplayID); > > bool hasClients() const { return m_clients.size(); } >diff --git a/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp b/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp >index 8b4b69db0334ffe0143d04fda5f210ac6ef6838f..a011e5f4df1ccff9fda297dee4e756ef95411427 100644 >--- a/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp >+++ b/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp >@@ -82,8 +82,10 @@ void DisplayRefreshMonitorManager::unregisterClient(DisplayRefreshMonitorClient& > if (monitor->displayID() != clientDisplayID) > continue; > if (monitor->removeClient(client)) { >- if (!monitor->hasClients()) >+ if (!monitor->hasClients()) { > m_monitors.remove(i); >+ monitor->stop(); >+ } > } > return; > } >diff --git a/Source/WebCore/platform/graphics/mac/DisplayRefreshMonitorMac.cpp b/Source/WebCore/platform/graphics/mac/DisplayRefreshMonitorMac.cpp >index beb6c0472522d5d6a0ada5661d2fcd77f9308093..4dce8b4ba827d7478ee384cc7e25e50898b50aad 100644 >--- a/Source/WebCore/platform/graphics/mac/DisplayRefreshMonitorMac.cpp >+++ b/Source/WebCore/platform/graphics/mac/DisplayRefreshMonitorMac.cpp >@@ -40,11 +40,17 @@ DisplayRefreshMonitorMac::DisplayRefreshMonitorMac(PlatformDisplayID displayID) > > DisplayRefreshMonitorMac::~DisplayRefreshMonitorMac() > { >- if (m_displayLink) { >- CVDisplayLinkStop(m_displayLink); >- CVDisplayLinkRelease(m_displayLink); >- m_displayLink = nullptr; >- } >+ ASSERT(!m_displayLink); >+} >+ >+void DisplayRefreshMonitorMac::stop() >+{ >+ if (!m_displayLink) >+ return; >+ >+ CVDisplayLinkStop(m_displayLink); >+ CVDisplayLinkRelease(m_displayLink); >+ m_displayLink = nullptr; > } > > static CVReturn displayLinkCallback(CVDisplayLinkRef, const CVTimeStamp*, const CVTimeStamp*, CVOptionFlags, CVOptionFlags*, void* data) >@@ -89,9 +95,9 @@ void DisplayRefreshMonitorMac::displayLinkFired() > > setIsPreviousFrameDone(false); > >- RunLoop::main().dispatch([weakPtr = makeWeakPtr(*this)] { >- if (auto* monitor = weakPtr.get()) >- handleDisplayRefreshedNotificationOnMainThread(monitor); >+ RunLoop::main().dispatch([this, protectedThis = makeRef(*this)] { >+ if (m_displayLink) >+ handleDisplayRefreshedNotificationOnMainThread(this); > }); > } > >diff --git a/Source/WebCore/platform/graphics/mac/DisplayRefreshMonitorMac.h b/Source/WebCore/platform/graphics/mac/DisplayRefreshMonitorMac.h >index a454577d56ed1597ec2c9c57a808d319e78e6242..d4587fd45556ebc32d0295a42eaaba0566ea38bb 100644 >--- a/Source/WebCore/platform/graphics/mac/DisplayRefreshMonitorMac.h >+++ b/Source/WebCore/platform/graphics/mac/DisplayRefreshMonitorMac.h >@@ -34,7 +34,7 @@ typedef struct __CVDisplayLink *CVDisplayLinkRef; > > namespace WebCore { > >-class DisplayRefreshMonitorMac : public DisplayRefreshMonitor, public CanMakeWeakPtr<DisplayRefreshMonitorMac> { >+class DisplayRefreshMonitorMac : public DisplayRefreshMonitor { > public: > static Ref<DisplayRefreshMonitorMac> create(PlatformDisplayID displayID) > { >@@ -45,6 +45,7 @@ public: > > void displayLinkFired() override; > bool requestRefreshCallback() override; >+ void stop() override; > > private: > explicit DisplayRefreshMonitorMac(PlatformDisplayID);
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 199626
:
373731
|
373733
|
373736
|
373737