Introduce a ThreadSafeRefCounted parameter to ensure being destroyed on the main...
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Apr 2018 16:40:08 +0000 (16:40 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Apr 2018 16:40:08 +0000 (16:40 +0000)
https://bugs.webkit.org/show_bug.cgi?id=183988

Reviewed by Darin Adler.

Source/WebCore:

No change of behavior, TrackPrivate remains destroyed on the main thread.

* platform/graphics/TrackPrivateBase.h:
* platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.cpp:
(WebCore::AudioTrackPrivateMediaStreamCocoa::audioSamplesAvailable):
(WebCore::AudioTrackPrivateMediaStreamCocoa::render):

Source/WTF:

* wtf/ThreadSafeRefCounted.h:
(WTF::ThreadSafeRefCounted::deref const):

Tools:

* TestWebKitAPI/Tests/WTF/RefPtr.cpp:
(TestWebKitAPI::ThreadSafeRefCountedObject::create):
(TestWebKitAPI::ThreadSafeRefCountedObject::~ThreadSafeRefCountedObject):
(TestWebKitAPI::MainThreadSafeRefCountedObject::create):
(TestWebKitAPI::MainThreadSafeRefCountedObject::~MainThreadSafeRefCountedObject):
(TestWebKitAPI::TEST):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230259 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WTF/ChangeLog
Source/WTF/wtf/ThreadSafeRefCounted.h
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/TrackPrivateBase.h
Source/WebCore/platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/PlatformGTK.cmake
Tools/TestWebKitAPI/PlatformWPE.cmake
Tools/TestWebKitAPI/Tests/WTF/RefPtr.cpp

index 4474a28..90bf614 100644 (file)
@@ -1,3 +1,13 @@
+2018-04-04  Youenn Fablet  <youenn@apple.com>
+
+        Introduce a ThreadSafeRefCounted parameter to ensure being destroyed on the main thread
+        https://bugs.webkit.org/show_bug.cgi?id=183988
+
+        Reviewed by Darin Adler.
+
+        * wtf/ThreadSafeRefCounted.h:
+        (WTF::ThreadSafeRefCounted::deref const):
+
 2018-04-04  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [WTF] Remove Atomics.cpp
index 892a058..c4e809a 100644 (file)
@@ -27,6 +27,7 @@
 
 #include <atomic>
 #include <wtf/FastMalloc.h>
+#include <wtf/MainThread.h>
 #include <wtf/Noncopyable.h>
 
 namespace WTF {
@@ -63,12 +64,21 @@ private:
     mutable std::atomic<unsigned> m_refCount { 1 };
 };
 
-template<class T> class ThreadSafeRefCounted : public ThreadSafeRefCountedBase {
+enum class DestructionThread { Any, Main };
+
+template<class T, DestructionThread destructionThread = DestructionThread::Any> class ThreadSafeRefCounted : public ThreadSafeRefCountedBase {
 public:
     void deref() const
     {
-        if (derefBase())
+        if (!derefBase())
+            return;
+        if (destructionThread == DestructionThread::Any || isMainThread()) {
+            delete static_cast<const T*>(this);
+            return;
+        }
+        callOnMainThread([this] {
             delete static_cast<const T*>(this);
+        });
     }
 
 protected:
index 44ce079..53e8183 100644 (file)
@@ -1,3 +1,17 @@
+2018-04-04  Youenn Fablet  <youenn@apple.com>
+
+        Introduce a ThreadSafeRefCounted parameter to ensure being destroyed on the main thread
+        https://bugs.webkit.org/show_bug.cgi?id=183988
+
+        Reviewed by Darin Adler.
+
+        No change of behavior, TrackPrivate remains destroyed on the main thread.
+
+        * platform/graphics/TrackPrivateBase.h:
+        * platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.cpp:
+        (WebCore::AudioTrackPrivateMediaStreamCocoa::audioSamplesAvailable):
+        (WebCore::AudioTrackPrivateMediaStreamCocoa::render):
+
 2018-04-04  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Unreviewed. Fix the build with libsoup < 2.49.91 after r230251.
index 0fb6976..4766622 100644 (file)
@@ -46,7 +46,7 @@ public:
 };
 
 class TrackPrivateBase
-    : public ThreadSafeRefCounted<TrackPrivateBase>
+    : public ThreadSafeRefCounted<TrackPrivateBase, WTF::DestructionThread::Main>
 #if !RELEASE_LOG_DISABLED
     , private LoggerHelper
 #endif
index 35e9352..cbf88a4 100644 (file)
@@ -34,7 +34,6 @@
 
 #include <pal/cf/CoreMediaSoftLink.h>
 #include <pal/spi/cocoa/AudioToolboxSPI.h>
-#include <wtf/Scope.h>
 
 #if ENABLE(VIDEO_TRACK) && ENABLE(MEDIA_STREAM)
 
@@ -170,9 +169,7 @@ void AudioTrackPrivateMediaStreamCocoa::audioSamplesAvailable(const MediaTime& s
 {
     // This function is called on a background thread. The following protectedThis object ensures the object is not
     // destroyed on the main thread before this function exits.
-    auto scopeExit = WTF::makeScopeExit([protectedThis = makeRef(*this)]() mutable {
-        callOnMainThread([protectedThis = WTFMove(protectedThis)] { });
-    });
+    Ref<AudioTrackPrivateMediaStreamCocoa> protectedThis { *this };
 
     ASSERT(description.platformDescription().type == PlatformDescription::CAAudioStreamBasicType);
 
@@ -224,9 +221,7 @@ OSStatus AudioTrackPrivateMediaStreamCocoa::render(UInt32 sampleCount, AudioBuff
 {
     // This function is called on a high-priority background thread. The following protectedThis object ensures the object is not
     // destroyed on the main thread before this function exits.
-    auto scopeExit = WTF::makeScopeExit([protectedThis = makeRef(*this)]() mutable {
-        callOnMainThread([protectedThis = WTFMove(protectedThis)] { });
-    });
+    Ref<AudioTrackPrivateMediaStreamCocoa> protectedThis { *this };
 
     if (!m_isPlaying || m_muted || !m_dataSource || streamTrack().muted() || streamTrack().ended() || !streamTrack().enabled()) {
         AudioSampleBufferList::zeroABL(ioData, static_cast<size_t>(sampleCount * m_outputDescription->bytesPerFrame()));
index c18466f..b1cdd70 100644 (file)
@@ -1,3 +1,17 @@
+2018-04-04  Youenn Fablet  <youenn@apple.com>
+
+        Introduce a ThreadSafeRefCounted parameter to ensure being destroyed on the main thread
+        https://bugs.webkit.org/show_bug.cgi?id=183988
+
+        Reviewed by Darin Adler.
+
+        * TestWebKitAPI/Tests/WTF/RefPtr.cpp:
+        (TestWebKitAPI::ThreadSafeRefCountedObject::create):
+        (TestWebKitAPI::ThreadSafeRefCountedObject::~ThreadSafeRefCountedObject):
+        (TestWebKitAPI::MainThreadSafeRefCountedObject::create):
+        (TestWebKitAPI::MainThreadSafeRefCountedObject::~MainThreadSafeRefCountedObject):
+        (TestWebKitAPI::TEST):
+
 2018-04-04  Ms2ger  <Ms2ger@igalia.com>
 
         Test gardening for GTK.
index 45e02d0..3f6bb98 100644 (file)
@@ -114,6 +114,7 @@ set_tests_properties(TestWebCore PROPERTIES TIMEOUT 60)
 set_target_properties(TestWebCore PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${TESTWEBKITAPI_RUNTIME_OUTPUT_DIRECTORY}/WebCore)
 
 list(APPEND TestWTF_SOURCES
+    ${TESTWEBKITAPI_DIR}/glib/UtilitiesGLib.cpp
     ${TESTWEBKITAPI_DIR}/Tests/WTF/glib/GUniquePtr.cpp
     ${TESTWEBKITAPI_DIR}/Tests/WTF/glib/WorkQueueGLib.cpp
 )
index e9bcf35..af3f2c1 100644 (file)
@@ -45,6 +45,7 @@ set(webkit_api_harness_SOURCES
 # TestWTF
 
 list(APPEND TestWTF_SOURCES
+    ${TESTWEBKITAPI_DIR}/glib/UtilitiesGLib.cpp
     ${TESTWEBKITAPI_DIR}/Tests/WTF/glib/GUniquePtr.cpp
     ${TESTWEBKITAPI_DIR}/Tests/WTF/glib/WorkQueueGLib.cpp
 )
index a094d72..3ecab60 100644 (file)
 
 #include "config.h"
 
+#if! PLATFORM(WIN)
+#include "PlatformUtilities.h"
+#endif
 #include "RefLogger.h"
+#include <wtf/MainThread.h>
 #include <wtf/NeverDestroyed.h>
 #include <wtf/RefCounted.h>
 #include <wtf/RefPtr.h>
+#include <wtf/ThreadSafeRefCounted.h>
+#include <wtf/Threading.h>
 
 namespace TestWebKitAPI {
 
@@ -535,4 +541,49 @@ TEST(WTF_RefPtr, ReleaseNonNullBeforeDeref)
     EXPECT_STREQ("ref(a) slot=null deref(a) ", takeLogStr().c_str());
 }
 
+// FIXME: Enable these tests once Win platform supports TestWebKitAPI::Util::run
+#if! PLATFORM(WIN)
+
+static bool done;
+static bool isDestroyedInMainThread;
+struct ThreadSafeRefCountedObject : ThreadSafeRefCounted<ThreadSafeRefCountedObject> {
+    static Ref<ThreadSafeRefCountedObject> create() { return adoptRef(*new ThreadSafeRefCountedObject); }
+
+    ~ThreadSafeRefCountedObject()
+    {
+        isDestroyedInMainThread = isMainThread();
+        done = true;
+    }
+};
+
+struct MainThreadSafeRefCountedObject : ThreadSafeRefCounted<MainThreadSafeRefCountedObject, WTF::DestructionThread::Main> {
+    static Ref<MainThreadSafeRefCountedObject> create() { return adoptRef(*new MainThreadSafeRefCountedObject); }
+
+    ~MainThreadSafeRefCountedObject()
+    {
+        isDestroyedInMainThread = isMainThread();
+        done = true;
+    }
+};
+
+TEST(WTF_RefPtr, ReleaseInNonMainThread)
+{
+    done = false;
+    Thread::create("", [object = ThreadSafeRefCountedObject::create()] { });
+    TestWebKitAPI::Util::run(&done);
+
+    EXPECT_FALSE(isDestroyedInMainThread);
+}
+
+TEST(WTF_RefPtr, ReleaseInNonMainThreadDestroyInMainThread)
+{
+    done = false;
+    Thread::create("", [object = MainThreadSafeRefCountedObject::create()] { });
+    TestWebKitAPI::Util::run(&done);
+
+    EXPECT_TRUE(isDestroyedInMainThread);
+}
+
+#endif
+
 } // namespace TestWebKitAPI