- Revision
- 247273
- Author
- cdu...@apple.com
- Date
- 2019-07-09 13:03:08 -0700 (Tue, 09 Jul 2019)
Log Message
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:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (247272 => 247273)
--- trunk/Source/WebCore/ChangeLog 2019-07-09 19:56:24 UTC (rev 247272)
+++ trunk/Source/WebCore/ChangeLog 2019-07-09 20:03:08 UTC (rev 247273)
@@ -1,3 +1,28 @@
+2019-07-09 Chris Dumez <cdu...@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 Sihui Liu <sihui_...@apple.com>
Only allow fetching and removing session credentials from WebsiteDataStore
Modified: trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitor.h (247272 => 247273)
--- trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitor.h 2019-07-09 19:56:24 UTC (rev 247272)
+++ trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitor.h 2019-07-09 20:03:08 UTC (rev 247273)
@@ -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 @@
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 @@
// 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(); }
Modified: trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp (247272 => 247273)
--- trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp 2019-07-09 19:56:24 UTC (rev 247272)
+++ trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp 2019-07-09 20:03:08 UTC (rev 247273)
@@ -45,7 +45,8 @@
DisplayRefreshMonitor* DisplayRefreshMonitorManager::createMonitorForClient(DisplayRefreshMonitorClient& client)
{
PlatformDisplayID clientDisplayID = client.displayID();
- for (const RefPtr<DisplayRefreshMonitor>& monitor : m_monitors) {
+ for (auto& monitorWrapper : m_monitors) {
+ auto& monitor = monitorWrapper.monitor;
if (monitor->displayID() != clientDisplayID)
continue;
monitor->addClient(client);
@@ -59,7 +60,7 @@
LOG(RequestAnimationFrame, "DisplayRefreshMonitorManager::createMonitorForClient() - created monitor %p", monitor.get());
monitor->addClient(client);
DisplayRefreshMonitor* result = monitor.get();
- m_monitors.append(WTFMove(monitor));
+ m_monitors.append({ WTFMove(monitor) });
return result;
}
@@ -78,7 +79,7 @@
PlatformDisplayID clientDisplayID = client.displayID();
for (size_t i = 0; i < m_monitors.size(); ++i) {
- RefPtr<DisplayRefreshMonitor> monitor = m_monitors[i];
+ RefPtr<DisplayRefreshMonitor> monitor = m_monitors[i].monitor;
if (monitor->displayID() != clientDisplayID)
continue;
if (monitor->removeClient(client)) {
@@ -108,7 +109,7 @@
return;
LOG(RequestAnimationFrame, "DisplayRefreshMonitorManager::displayDidRefresh() - destroying monitor %p", &monitor);
- size_t monitorIndex = m_monitors.find(&monitor);
+ size_t monitorIndex = m_monitors.findMatching([&](auto& monitorWrapper) { return monitorWrapper.monitor == &monitor; });
if (monitorIndex != notFound)
m_monitors.remove(monitorIndex);
}
@@ -127,7 +128,8 @@
void DisplayRefreshMonitorManager::displayWasUpdated(PlatformDisplayID displayID)
{
- for (const auto& monitor : m_monitors) {
+ for (const auto& monitorWrapper : m_monitors) {
+ auto& monitor = monitorWrapper.monitor;
if (displayID == monitor->displayID() && monitor->hasRequestedRefreshCallback())
monitor->displayLinkFired();
}
Modified: trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.h (247272 => 247273)
--- trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.h 2019-07-09 19:56:24 UTC (rev 247272)
+++ trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.h 2019-07-09 20:03:08 UTC (rev 247273)
@@ -57,7 +57,18 @@
DisplayRefreshMonitor* createMonitorForClient(DisplayRefreshMonitorClient&);
- Vector<RefPtr<DisplayRefreshMonitor>> m_monitors;
+ struct DisplayRefreshMonitorWrapper {
+ DisplayRefreshMonitorWrapper(DisplayRefreshMonitorWrapper&&) = default;
+ ~DisplayRefreshMonitorWrapper()
+ {
+ if (monitor)
+ monitor->stop();
+ }
+
+ RefPtr<DisplayRefreshMonitor> monitor;
+ };
+
+ Vector<DisplayRefreshMonitorWrapper> m_monitors;
};
}
Modified: trunk/Source/WebCore/platform/graphics/mac/DisplayRefreshMonitorMac.cpp (247272 => 247273)
--- trunk/Source/WebCore/platform/graphics/mac/DisplayRefreshMonitorMac.cpp 2019-07-09 19:56:24 UTC (rev 247272)
+++ trunk/Source/WebCore/platform/graphics/mac/DisplayRefreshMonitorMac.cpp 2019-07-09 20:03:08 UTC (rev 247273)
@@ -40,13 +40,19 @@
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)
{
DisplayRefreshMonitorMac* monitor = static_cast<DisplayRefreshMonitorMac*>(data);
@@ -89,9 +95,9 @@
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);
});
}
Modified: trunk/Source/WebCore/platform/graphics/mac/DisplayRefreshMonitorMac.h (247272 => 247273)
--- trunk/Source/WebCore/platform/graphics/mac/DisplayRefreshMonitorMac.h 2019-07-09 19:56:24 UTC (rev 247272)
+++ trunk/Source/WebCore/platform/graphics/mac/DisplayRefreshMonitorMac.h 2019-07-09 20:03:08 UTC (rev 247273)
@@ -34,7 +34,7 @@
namespace WebCore {
-class DisplayRefreshMonitorMac : public DisplayRefreshMonitor, public CanMakeWeakPtr<DisplayRefreshMonitorMac> {
+class DisplayRefreshMonitorMac : public DisplayRefreshMonitor {
public:
static Ref<DisplayRefreshMonitorMac> create(PlatformDisplayID displayID)
{
@@ -45,6 +45,7 @@
void displayLinkFired() override;
bool requestRefreshCallback() override;
+ void stop() override;
private:
explicit DisplayRefreshMonitorMac(PlatformDisplayID);
