Attachment 373733 Details for Bug 199626 – Patch

[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:

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);

Actions: View | Formatted Diff | Diff

Attachments on bug 199626: 373731 | 373733 | 373736 | 373737