[webkit-changes] [218496] trunk

Mon, 19 Jun 2017 10:02:28 -0700

Title: [218496] trunk
Revision
218496
Author
da...@apple.com
Date
2017-06-19 10:01:14 -0700 (Mon, 19 Jun 2017)

Log Message

Fix Ref to deref before assignment, add tests for this to RefPtr, Ref, Function
https://bugs.webkit.org/show_bug.cgi?id=173526

Reviewed by Sam Weinig.

Source/WTF:

* wtf/Ref.h: Changed operator= to not be defined inside the class definition.
Added swap functions. Changed operator= implementations to use swap in the
conventional manner, the same way that RefPtr does.

Tools:

* TestWebKitAPI/CMakeLists.txt: Added Function.cpp.
* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj: Ditto.

* TestWebKitAPI/Tests/WTF/Function.cpp: Added. Contains basic tests and some
tests for assignment before destruction ones.

* TestWebKitAPI/Tests/WTF/MoveOnly.h: Added a () operator so this can be used
as a function, so it can be used in WTF::Function tests.

* TestWebKitAPI/Tests/WTF/Ref.cpp: Use EXPECT macros insead of ASSERT.
Added tests for swap and for assignment before deref.

* TestWebKitAPI/Tests/WTF/RefLogger.cpp: Stopped using inlining; no good reason
to inline everything. Also removed the unnecessary clearing of the log every time
the DerivedRefLogger constructor is called.
* TestWebKitAPI/Tests/WTF/RefLogger.h: Ditto.

* TestWebKitAPI/Tests/WTF/RefPtr.cpp: Use EXPECT macros instead of ASSERT.
Added tests for assignment before deref and similar for releaseNonNull.

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (218495 => 218496)


--- trunk/Source/WTF/ChangeLog	2017-06-19 16:47:06 UTC (rev 218495)
+++ trunk/Source/WTF/ChangeLog	2017-06-19 17:01:14 UTC (rev 218496)
@@ -1,3 +1,14 @@
+2017-06-18  Darin Adler  <da...@apple.com>
+
+        Fix Ref to deref before assignment, add tests for this to RefPtr, Ref, Function
+        https://bugs.webkit.org/show_bug.cgi?id=173526
+
+        Reviewed by Sam Weinig.
+
+        * wtf/Ref.h: Changed operator= to not be defined inside the class definition.
+        Added swap functions. Changed operator= implementations to use swap in the
+        conventional manner, the same way that RefPtr does.
+
 2017-06-18  Chris Dumez  <cdu...@apple.com>
 
         Use WTF::Function instead of std::function in WTF/

Modified: trunk/Source/WTF/wtf/Ref.h (218495 => 218496)


--- trunk/Source/WTF/wtf/Ref.h	2017-06-19 16:47:06 UTC (rev 218495)
+++ trunk/Source/WTF/wtf/Ref.h	2017-06-19 17:01:14 UTC (rev 218496)
@@ -82,38 +82,16 @@
         ASSERT(m_ptr);
     }
 
-    Ref& operator=(T& object)
-    {
-        ASSERT(m_ptr);
-        object.ref();
-        m_ptr->deref();
-        m_ptr = &object;
-        ASSERT(m_ptr);
-        return *this;
-    }
+    Ref& operator=(T&);
+    Ref& operator=(Ref&&);
+    template<typename U> Ref& operator=(Ref<U>&&);
 
     // Use copyRef() and the move assignment operators instead.
-    Ref& operator=(const Ref& reference) = delete;
-    template<typename U> Ref& operator=(const Ref<U>& reference) = delete;
+    Ref& operator=(const Ref&) = delete;
+    template<typename U> Ref& operator=(const Ref<U>&) = delete;
 
-    Ref& operator=(Ref&& reference)
-    {
-        ASSERT(m_ptr);
-        m_ptr->deref();
-        m_ptr = &reference.leakRef();
-        ASSERT(m_ptr);
-        return *this;
-    }
+    void swap(Ref&);
 
-    template<typename U> Ref& operator=(Ref<U>&& reference)
-    {
-        ASSERT(m_ptr);
-        m_ptr->deref();
-        m_ptr = &reference.leakRef();
-        ASSERT(m_ptr);
-        return *this;
-    }
-
     // Hash table deleted values, which are only constructed and never copied or destroyed.
     Ref(HashTableDeletedValueType) : m_ptr(hashTableDeletedValue()) { }
     bool isHashTableDeletedValue() const { return m_ptr == hashTableDeletedValue(); }
@@ -171,6 +149,41 @@
     T* m_ptr;
 };
 
+template<typename T> void swap(Ref<T>&, Ref<T>&);
+template<typename T> Ref<T> adoptRef(T&);
+template<typename T> Ref<T> makeRef(T&);
+
+template<typename T> inline Ref<T>& Ref<T>::operator=(T& reference)
+{
+    Ref copiedReference = reference;
+    swap(copiedReference);
+    return *this;
+}
+
+template<typename T> inline Ref<T>& Ref<T>::operator=(Ref&& reference)
+{
+    Ref movedReference = WTFMove(reference);
+    swap(movedReference);
+    return *this;
+}
+
+template<typename T> template<typename U> inline Ref<T>& Ref<T>::operator=(Ref<U>&& reference)
+{
+    Ref movedReference = WTFMove(reference);
+    swap(movedReference);
+    return *this;
+}
+
+template<typename T> inline void Ref<T>::swap(Ref& other)
+{
+    std::swap(m_ptr, other.m_ptr);
+}
+
+template<typename T> inline void swap(Ref<T>& a, Ref<T>& b)
+{
+    a.swap(b);
+}
+
 template<typename T> template<typename U> inline Ref<T> Ref<T>::replace(Ref<U>&& reference)
 {
     auto oldReference = adoptRef(*m_ptr);

Modified: trunk/Tools/ChangeLog (218495 => 218496)


--- trunk/Tools/ChangeLog	2017-06-19 16:47:06 UTC (rev 218495)
+++ trunk/Tools/ChangeLog	2017-06-19 17:01:14 UTC (rev 218496)
@@ -1,3 +1,30 @@
+2017-06-18  Darin Adler  <da...@apple.com>
+
+        Fix Ref to deref before assignment, add tests for this to RefPtr, Ref, Function
+        https://bugs.webkit.org/show_bug.cgi?id=173526
+
+        Reviewed by Sam Weinig.
+
+        * TestWebKitAPI/CMakeLists.txt: Added Function.cpp.
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj: Ditto.
+
+        * TestWebKitAPI/Tests/WTF/Function.cpp: Added. Contains basic tests and some
+        tests for assignment before destruction ones.
+
+        * TestWebKitAPI/Tests/WTF/MoveOnly.h: Added a () operator so this can be used
+        as a function, so it can be used in WTF::Function tests.
+
+        * TestWebKitAPI/Tests/WTF/Ref.cpp: Use EXPECT macros insead of ASSERT.
+        Added tests for swap and for assignment before deref.
+
+        * TestWebKitAPI/Tests/WTF/RefLogger.cpp: Stopped using inlining; no good reason
+        to inline everything. Also removed the unnecessary clearing of the log every time
+        the DerivedRefLogger constructor is called.
+        * TestWebKitAPI/Tests/WTF/RefLogger.h: Ditto.
+
+        * TestWebKitAPI/Tests/WTF/RefPtr.cpp: Use EXPECT macros instead of ASSERT.
+        Added tests for assignment before deref and similar for releaseNonNull.
+
 2017-06-19  Sam Weinig  <s...@webkit.org>
 
         [WebIDL] Properly model buffer source / typed arrays as their own IDL types

Modified: trunk/Tools/TestWebKitAPI/CMakeLists.txt (218495 => 218496)


--- trunk/Tools/TestWebKitAPI/CMakeLists.txt	2017-06-19 16:47:06 UTC (rev 218495)
+++ trunk/Tools/TestWebKitAPI/CMakeLists.txt	2017-06-19 17:01:14 UTC (rev 218496)
@@ -50,6 +50,7 @@
     ${TESTWEBKITAPI_DIR}/Tests/WTF/Deque.cpp
     ${TESTWEBKITAPI_DIR}/Tests/WTF/EnumTraits.cpp
     ${TESTWEBKITAPI_DIR}/Tests/WTF/Expected.cpp
+    ${TESTWEBKITAPI_DIR}/Tests/WTF/Function.cpp
     ${TESTWEBKITAPI_DIR}/Tests/WTF/HashCountedSet.cpp
     ${TESTWEBKITAPI_DIR}/Tests/WTF/HashMap.cpp
     ${TESTWEBKITAPI_DIR}/Tests/WTF/HashSet.cpp

Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (218495 => 218496)


--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	2017-06-19 16:47:06 UTC (rev 218495)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	2017-06-19 17:01:14 UTC (rev 218496)
@@ -489,6 +489,7 @@
 		83DE134D1EF1C50800C1B355 /* ResponsivenessTimer.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 83DE134C1EF1C4FE00C1B355 /* ResponsivenessTimer.cpp */; };
 		8E4A85371E1D1AB200F53B0F /* GridPosition.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 8E4A85361E1D1AA100F53B0F /* GridPosition.cpp */; };
 		930AD402150698D00067970F /* lots-of-text.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 930AD401150698B30067970F /* lots-of-text.html */; };
+		9310CD381EF708FB0050FFE0 /* Function.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9310CD361EF708FB0050FFE0 /* Function.cpp */; };
 		9329AA291DE3F81E003ABD07 /* TextBreakIterator.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9329AA281DE3F81E003ABD07 /* TextBreakIterator.cpp */; };
 		932AE53D1D371047005DFFAF /* focus-inputs.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 93575C551D30366E000D604D /* focus-inputs.html */; };
 		9361002914DC95A70061379D /* lots-of-iframes.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9361002814DC957B0061379D /* lots-of-iframes.html */; };
@@ -1333,6 +1334,7 @@
 		8DD76FA10486AA7600D96B5E /* TestWebKitAPI */ = {isa = PBXFileReference; explicitFileType = "compiled.mach-o.executable"; includeInIndex = 0; path = TestWebKitAPI; sourceTree = BUILT_PRODUCTS_DIR; };
 		8E4A85361E1D1AA100F53B0F /* GridPosition.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = GridPosition.cpp; sourceTree = "<group>"; };
 		930AD401150698B30067970F /* lots-of-text.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "lots-of-text.html"; sourceTree = "<group>"; };
+		9310CD361EF708FB0050FFE0 /* Function.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = Function.cpp; sourceTree = "<group>"; };
 		9329AA281DE3F81E003ABD07 /* TextBreakIterator.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = TextBreakIterator.cpp; sourceTree = "<group>"; };
 		9331407B17B4419000F083B1 /* DidNotHandleKeyDown.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DidNotHandleKeyDown.cpp; sourceTree = "<group>"; };
 		93575C551D30366E000D604D /* focus-inputs.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "focus-inputs.html"; sourceTree = "<group>"; };
@@ -2308,6 +2310,7 @@
 				E4A757D3178AEA5B00B5D7A4 /* Deque.cpp */,
 				1AF7B21D1D6CD12E008C126C /* EnumTraits.cpp */,
 				AD7C434C1DD2A5470026888B /* Expected.cpp */,
+				9310CD361EF708FB0050FFE0 /* Function.cpp */,
 				7A38D7E51C752D5F004F157D /* HashCountedSet.cpp */,
 				0BCD833414857CE400EA2003 /* HashMap.cpp */,
 				26B2DFF815BDE599004F691D /* HashSet.cpp */,
@@ -2846,6 +2849,7 @@
 				7C83DE991D0A590C00FEBCF3 /* AtomicString.cpp in Sources */,
 				1ADAD1501D77A9F600212586 /* BlockPtr.mm in Sources */,
 				7C83DE9C1D0A590C00FEBCF3 /* BloomFilter.cpp in Sources */,
+				9310CD381EF708FB0050FFE0 /* Function.cpp in Sources */,
 				7C83DEA01D0A590C00FEBCF3 /* CheckedArithmeticOperations.cpp in Sources */,
 				7C83DEC31D0A590C00FEBCF3 /* Condition.cpp in Sources */,
 				7C83DEA61D0A590C00FEBCF3 /* Counters.cpp in Sources */,

Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/Function.cpp (218495 => 218496)


--- trunk/Tools/TestWebKitAPI/Tests/WTF/Function.cpp	2017-06-19 16:47:06 UTC (rev 218495)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/Function.cpp	2017-06-19 17:01:14 UTC (rev 218496)
@@ -25,7 +25,7 @@
 
 #include "config.h"
 
-#include "Test.h"
+#include "MoveOnly.h"
 #include <wtf/Function.h>
 
 namespace TestWebKitAPI {
@@ -137,4 +137,107 @@
     EXPECT_EQ(-1, function_for_reentrancy_test());
 }
 
+TEST(WTF_Function, Basics)
+{
+    Function<unsigned()> a;
+    EXPECT_FALSE(static_cast<bool>(a));
+    EXPECT_EQ(0U, a());
+
+    a = [] {
+        return 1U;
+    };
+    EXPECT_TRUE(static_cast<bool>(a));
+    EXPECT_EQ(1U, a());
+
+    a = nullptr;
+    EXPECT_FALSE(static_cast<bool>(a));
+    EXPECT_EQ(0U, a());
+
+    a = MoveOnly { 2 };
+    EXPECT_TRUE(static_cast<bool>(a));
+    EXPECT_EQ(2U, a());
+
+    Function<unsigned()> b = WTFMove(a);
+    EXPECT_TRUE(static_cast<bool>(b));
+    EXPECT_EQ(2U, b());
+    EXPECT_FALSE(static_cast<bool>(a));
+    EXPECT_EQ(0U, a());
+
+    a = MoveOnly { 3 };
+    Function<unsigned()> c = WTFMove(a);
+    EXPECT_TRUE(static_cast<bool>(c));
+    EXPECT_EQ(3U, c());
+    EXPECT_FALSE(static_cast<bool>(a));
+    EXPECT_EQ(0U, a());
+
+    b = WTFMove(c);
+    EXPECT_TRUE(static_cast<bool>(b));
+    EXPECT_EQ(3U, b());
+    EXPECT_FALSE(static_cast<bool>(c));
+    EXPECT_EQ(0U, c());
+
+    Function<unsigned()> d = nullptr;
+    EXPECT_FALSE(static_cast<bool>(d));
+    EXPECT_EQ(0U, d());
 }
+
+struct FunctionDestructionChecker {
+    FunctionDestructionChecker(Function<unsigned()>& function)
+        : function { function }
+    {
+    }
+
+    ~FunctionDestructionChecker()
+    {
+        functionAsBool = static_cast<bool>(function);
+        functionResult = function();
+    }
+
+    unsigned operator()() const
+    {
+        return 10;
+    }
+
+    Function<unsigned()>& function;
+    static std::optional<bool> functionAsBool;
+    static std::optional<unsigned> functionResult;
+};
+
+std::optional<bool> FunctionDestructionChecker::functionAsBool;
+std::optional<unsigned> FunctionDestructionChecker::functionResult;
+
+TEST(WTF_Function, AssignBeforeDestroy)
+{
+    Function<unsigned()> a;
+
+    a = FunctionDestructionChecker(a);
+    a = [] {
+        return 1U;
+    };
+    EXPECT_TRUE(static_cast<bool>(FunctionDestructionChecker::functionAsBool));
+    EXPECT_TRUE(static_cast<bool>(FunctionDestructionChecker::functionResult));
+    EXPECT_TRUE(FunctionDestructionChecker::functionAsBool.value());
+    EXPECT_EQ(1U, FunctionDestructionChecker::functionResult.value());
+    FunctionDestructionChecker::functionAsBool = std::nullopt;
+    FunctionDestructionChecker::functionResult = std::nullopt;
+
+    a = FunctionDestructionChecker(a);
+    a = nullptr;
+    EXPECT_TRUE(static_cast<bool>(FunctionDestructionChecker::functionAsBool));
+    EXPECT_TRUE(static_cast<bool>(FunctionDestructionChecker::functionResult));
+    EXPECT_FALSE(FunctionDestructionChecker::functionAsBool.value());
+    EXPECT_EQ(0U, FunctionDestructionChecker::functionResult.value());
+    FunctionDestructionChecker::functionAsBool = std::nullopt;
+    FunctionDestructionChecker::functionResult = std::nullopt;
+
+    a = FunctionDestructionChecker(a);
+    a = MoveOnly { 2 };
+    EXPECT_TRUE(static_cast<bool>(FunctionDestructionChecker::functionAsBool));
+    EXPECT_TRUE(static_cast<bool>(FunctionDestructionChecker::functionResult));
+    EXPECT_TRUE(FunctionDestructionChecker::functionAsBool.value());
+    EXPECT_EQ(2U, FunctionDestructionChecker::functionResult.value());
+    FunctionDestructionChecker::functionAsBool = std::nullopt;
+    FunctionDestructionChecker::functionResult = std::nullopt;
+}
+
+} // namespace TestWebKitAPI

Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/MoveOnly.h (218495 => 218496)


--- trunk/Tools/TestWebKitAPI/Tests/WTF/MoveOnly.h	2017-06-19 16:47:06 UTC (rev 218495)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/MoveOnly.h	2017-06-19 17:01:14 UTC (rev 218496)
@@ -46,6 +46,11 @@
         return m_value;
     }
 
+    unsigned operator()() const
+    {
+        return m_value;
+    }
+
     MoveOnly(MoveOnly&& other)
         : m_value(other.m_value)
     {

Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/Ref.cpp (218495 => 218496)


--- trunk/Tools/TestWebKitAPI/Tests/WTF/Ref.cpp	2017-06-19 16:47:06 UTC (rev 218495)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/Ref.cpp	2017-06-19 17:01:14 UTC (rev 218496)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -26,7 +26,6 @@
 #include "config.h"
 
 #include "RefLogger.h"
-#include <wtf/Ref.h>
 #include <wtf/RefPtr.h>
 
 namespace TestWebKitAPI {
@@ -36,18 +35,18 @@
     DerivedRefLogger a("a");
 
     {
-        Ref<RefLogger> ptr(a);
-        ASSERT_EQ(&a, ptr.ptr());
-        ASSERT_EQ(&a.name, &ptr->name);
+        Ref<RefLogger> ref(a);
+        EXPECT_EQ(&a, ref.ptr());
+        EXPECT_EQ(&a.name, &ref->name);
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 
     {
-        Ref<RefLogger> ptr(adoptRef(a));
-        ASSERT_EQ(&a, ptr.ptr());
-        ASSERT_EQ(&a.name, &ptr->name);
+        Ref<RefLogger> ref(adoptRef(a));
+        EXPECT_EQ(&a, ref.ptr());
+        EXPECT_EQ(&a.name, &ref->name);
     }
-    ASSERT_STREQ("deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("deref(a) ", takeLogStr().c_str());
 }
 
 TEST(WTF_Ref, Assignment)
@@ -57,44 +56,44 @@
     DerivedRefLogger c("c");
 
     {
-        Ref<RefLogger> ptr(a);
-        ASSERT_EQ(&a, ptr.ptr());
+        Ref<RefLogger> ref(a);
+        EXPECT_EQ(&a, ref.ptr());
         log() << "| ";
-        ptr = b;
-        ASSERT_EQ(&b, ptr.ptr());
+        ref = b;
+        EXPECT_EQ(&b, ref.ptr());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) | ref(b) deref(a) | deref(b) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) | ref(b) deref(a) | deref(b) ", takeLogStr().c_str());
 
     {
-        Ref<RefLogger> ptr(a);
-        ASSERT_EQ(&a, ptr.ptr());
+        Ref<RefLogger> ref(a);
+        EXPECT_EQ(&a, ref.ptr());
         log() << "| ";
-        ptr = c;
-        ASSERT_EQ(&c, ptr.ptr());
+        ref = c;
+        EXPECT_EQ(&c, ref.ptr());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) | ref(c) deref(a) | deref(c) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) | ref(c) deref(a) | deref(c) ", takeLogStr().c_str());
 
     {
-        Ref<RefLogger> ptr(a);
-        ASSERT_EQ(&a, ptr.ptr());
+        Ref<RefLogger> ref(a);
+        EXPECT_EQ(&a, ref.ptr());
         log() << "| ";
-        ptr = adoptRef(b);
-        ASSERT_EQ(&b, ptr.ptr());
+        ref = adoptRef(b);
+        EXPECT_EQ(&b, ref.ptr());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) | deref(a) | deref(b) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) | deref(a) | deref(b) ", takeLogStr().c_str());
 
     {
-        Ref<RefLogger> ptr(a);
-        ASSERT_EQ(&a, ptr.ptr());
+        Ref<RefLogger> ref(a);
+        EXPECT_EQ(&a, ref.ptr());
         log() << "| ";
-        ptr = adoptRef(c);
-        ASSERT_EQ(&c, ptr.ptr());
+        ref = adoptRef(c);
+        EXPECT_EQ(&c, ref.ptr());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) | deref(a) | deref(c) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) | deref(a) | deref(c) ", takeLogStr().c_str());
 }
 
 static Ref<RefLogger> passWithRef(Ref<RefLogger>&& reference)
@@ -109,42 +108,168 @@
     DerivedRefLogger c("c");
 
     {
-        Ref<RefLogger> ptr(passWithRef(Ref<RefLogger>(a)));
-        ASSERT_EQ(&a, ptr.ptr());
+        Ref<RefLogger> ref(passWithRef(Ref<RefLogger>(a)));
+        EXPECT_EQ(&a, ref.ptr());
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 
     {
-        Ref<RefLogger> ptr(a);
-        ASSERT_EQ(&a, ptr.ptr());
+        Ref<RefLogger> ref(a);
+        EXPECT_EQ(&a, ref.ptr());
         log() << "| ";
-        ptr = passWithRef(b);
-        ASSERT_EQ(&b, ptr.ptr());
+        ref = passWithRef(b);
+        EXPECT_EQ(&b, ref.ptr());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) | ref(b) deref(a) | deref(b) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) | ref(b) deref(a) | deref(b) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> ptr(passWithRef(a));
-        ASSERT_EQ(&a, ptr.get());
+        EXPECT_EQ(&a, ptr.get());
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 
     {
         RefPtr<DerivedRefLogger> ptr(&a);
         RefPtr<RefLogger> ptr2(WTFMove(ptr));
-        ASSERT_EQ(nullptr, ptr.get());
-        ASSERT_EQ(&a, ptr2.get());
+        EXPECT_EQ(nullptr, ptr.get());
+        EXPECT_EQ(&a, ptr2.get());
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 
     {
         Ref<DerivedRefLogger> derivedReference(a);
         Ref<RefLogger> baseReference(passWithRef(derivedReference.copyRef()));
-        ASSERT_EQ(&a, derivedReference.ptr());
-        ASSERT_EQ(&a, baseReference.ptr());
+        EXPECT_EQ(&a, derivedReference.ptr());
+        EXPECT_EQ(&a, baseReference.ptr());
     }
-    ASSERT_STREQ("ref(a) ref(a) deref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) ref(a) deref(a) deref(a) ", takeLogStr().c_str());
 }
 
+TEST(WTF_Ref, Swap)
+{
+    RefLogger a("a");
+    RefLogger b("b");
+
+    {
+        Ref<RefLogger> p1(a);
+        Ref<RefLogger> p2(b);
+        log() << "| ";
+        EXPECT_EQ(&a, p1.ptr());
+        EXPECT_EQ(&b, p2.ptr());
+        p1.swap(p2);
+        EXPECT_EQ(&b, p1.ptr());
+        EXPECT_EQ(&a, p2.ptr());
+        log() << "| ";
+    }
+    EXPECT_STREQ("ref(a) ref(b) | | deref(a) deref(b) ", takeLogStr().c_str());
+
+    {
+        Ref<RefLogger> p1(a);
+        Ref<RefLogger> p2(b);
+        log() << "| ";
+        EXPECT_EQ(&a, p1.ptr());
+        EXPECT_EQ(&b, p2.ptr());
+        std::swap(p1, p2);
+        EXPECT_EQ(&b, p1.ptr());
+        EXPECT_EQ(&a, p2.ptr());
+        log() << "| ";
+    }
+    EXPECT_STREQ("ref(a) ref(b) | | deref(a) deref(b) ", takeLogStr().c_str());
+}
+
+struct RefCheckingRefLogger : RefLogger {
+    RefCheckingRefLogger(const char* name);
+    void ref();
+    void deref();
+    const Ref<RefCheckingRefLogger>* slotToCheck { nullptr };
+};
+
+struct DerivedRefCheckingRefLogger : RefCheckingRefLogger {
+    DerivedRefCheckingRefLogger(const char* name);
+};
+
+RefCheckingRefLogger::RefCheckingRefLogger(const char* name)
+    : RefLogger { name }
+{
+}
+
+void RefCheckingRefLogger::ref()
+{
+    if (slotToCheck)
+        log() << "slot=" << slotToCheck->get().name << " ";
+    RefLogger::ref();
+}
+
+void RefCheckingRefLogger::deref()
+{
+    if (slotToCheck)
+        log() << "slot=" << slotToCheck->get().name << " ";
+    RefLogger::deref();
+}
+
+DerivedRefCheckingRefLogger::DerivedRefCheckingRefLogger(const char* name)
+    : RefCheckingRefLogger { name }
+{
+}
+
+TEST(WTF_Ref, AssignBeforeDeref)
+{
+    DerivedRefCheckingRefLogger a("a");
+    RefCheckingRefLogger b("b");
+    DerivedRefCheckingRefLogger c("c");
+
+    {
+        Ref<RefCheckingRefLogger> ref(a);
+        EXPECT_EQ(&a, ref.ptr());
+        log() << "| ";
+        a.slotToCheck = &ref;
+        b.slotToCheck = &ref;
+        ref = b;
+        a.slotToCheck = nullptr;
+        b.slotToCheck = nullptr;
+        EXPECT_EQ(&b, ref.ptr());
+        log() << "| ";
+    }
+    EXPECT_STREQ("ref(a) | slot=a ref(b) slot=b deref(a) | deref(b) ", takeLogStr().c_str());
+
+    {
+        Ref<RefCheckingRefLogger> ref(a);
+        EXPECT_EQ(&a, ref.ptr());
+        log() << "| ";
+        a.slotToCheck = &ref;
+        c.slotToCheck = &ref;
+        ref = c;
+        a.slotToCheck = nullptr;
+        c.slotToCheck = nullptr;
+        EXPECT_EQ(&c, ref.ptr());
+        log() << "| ";
+    }
+    EXPECT_STREQ("ref(a) | slot=a ref(c) slot=c deref(a) | deref(c) ", takeLogStr().c_str());
+
+    {
+        Ref<RefCheckingRefLogger> ref(a);
+        EXPECT_EQ(&a, ref.ptr());
+        log() << "| ";
+        a.slotToCheck = &ref;
+        ref = adoptRef(b);
+        a.slotToCheck = nullptr;
+        EXPECT_EQ(&b, ref.ptr());
+        log() << "| ";
+    }
+    EXPECT_STREQ("ref(a) | slot=b deref(a) | deref(b) ", takeLogStr().c_str());
+
+    {
+        Ref<RefCheckingRefLogger> ref(a);
+        EXPECT_EQ(&a, ref.ptr());
+        log() << "| ";
+        a.slotToCheck = &ref;
+        ref = adoptRef(c);
+        a.slotToCheck = nullptr;
+        EXPECT_EQ(&c, ref.ptr());
+        log() << "| ";
+    }
+    EXPECT_STREQ("ref(a) | slot=c deref(a) | deref(c) ", takeLogStr().c_str());
+}
+
 } // namespace TestWebKitAPI

Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/RefLogger.cpp (218495 => 218496)


--- trunk/Tools/TestWebKitAPI/Tests/WTF/RefLogger.cpp	2017-06-19 16:47:06 UTC (rev 218495)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/RefLogger.cpp	2017-06-19 17:01:14 UTC (rev 218496)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -34,4 +34,31 @@
     return log;
 }
 
+std::string takeLogStr()
+{
+    std::string string = log().str();
+    log().str("");
+    return string;
 }
+
+RefLogger::RefLogger(const char* name)
+    : name { *name }
+{
+}
+
+void RefLogger::ref()
+{
+    log() << "ref(" << &name << ") ";
+}
+
+void RefLogger::deref()
+{
+    log() << "deref(" << &name << ") ";
+}
+
+DerivedRefLogger::DerivedRefLogger(const char* name)
+    : RefLogger { name }
+{
+}
+
+}

Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/RefLogger.h (218495 => 218496)


--- trunk/Tools/TestWebKitAPI/Tests/WTF/RefLogger.h	2017-06-19 16:47:06 UTC (rev 218495)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/RefLogger.h	2017-06-19 17:01:14 UTC (rev 218496)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -23,30 +23,23 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef RefLogger_h
+#pragma once
 
 namespace TestWebKitAPI {
 
 std::ostringstream& log();
 
-inline std::string takeLogStr()
-{
-    std::string string = log().str();
-    log().str("");
-    return string;
-}
+std::string takeLogStr();
 
 struct RefLogger {
-    RefLogger(const char* name) : name(*name) { }
-    void ref() { log() << "ref(" << &name << ") "; }
-    void deref() { log() << "deref(" << &name << ") "; }
+    RefLogger(const char* name);
+    void ref();
+    void deref();
     const char& name;
 };
 
 struct DerivedRefLogger : RefLogger {
-    DerivedRefLogger(const char* name) : RefLogger(name) { log().str(""); }
+    DerivedRefLogger(const char* name);
 };
 
 }
-
-#endif

Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/RefPtr.cpp (218495 => 218496)


--- trunk/Tools/TestWebKitAPI/Tests/WTF/RefPtr.cpp	2017-06-19 16:47:06 UTC (rev 218495)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/RefPtr.cpp	2017-06-19 17:01:14 UTC (rev 218496)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -37,77 +37,77 @@
     DerivedRefLogger a("a");
 
     RefPtr<RefLogger> empty;
-    ASSERT_EQ(nullptr, empty.get());
+    EXPECT_EQ(nullptr, empty.get());
 
     {
         RefPtr<RefLogger> ptr(&a);
-        ASSERT_EQ(&a, ptr.get());
-        ASSERT_EQ(&a, &*ptr);
-        ASSERT_EQ(&a.name, &ptr->name);
+        EXPECT_EQ(&a, ptr.get());
+        EXPECT_EQ(&a, &*ptr);
+        EXPECT_EQ(&a.name, &ptr->name);
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> ptr = &a;
-        ASSERT_EQ(&a, ptr.get());
+        EXPECT_EQ(&a, ptr.get());
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> p1 = &a;
         RefPtr<RefLogger> p2(p1);
-        ASSERT_EQ(&a, p1.get());
-        ASSERT_EQ(&a, p2.get());
+        EXPECT_EQ(&a, p1.get());
+        EXPECT_EQ(&a, p2.get());
     }
-    ASSERT_STREQ("ref(a) ref(a) deref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) ref(a) deref(a) deref(a) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> p1 = &a;
         RefPtr<RefLogger> p2 = p1;
-        ASSERT_EQ(&a, p1.get());
-        ASSERT_EQ(&a, p2.get());
+        EXPECT_EQ(&a, p1.get());
+        EXPECT_EQ(&a, p2.get());
     }
-    ASSERT_STREQ("ref(a) ref(a) deref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) ref(a) deref(a) deref(a) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> p1 = &a;
         RefPtr<RefLogger> p2 = WTFMove(p1);
-        ASSERT_EQ(nullptr, p1.get());
-        ASSERT_EQ(&a, p2.get());
+        EXPECT_EQ(nullptr, p1.get());
+        EXPECT_EQ(&a, p2.get());
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> p1 = &a;
         RefPtr<RefLogger> p2(WTFMove(p1));
-        ASSERT_EQ(nullptr, p1.get());
-        ASSERT_EQ(&a, p2.get());
+        EXPECT_EQ(nullptr, p1.get());
+        EXPECT_EQ(&a, p2.get());
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 
     {
         RefPtr<DerivedRefLogger> p1 = &a;
         RefPtr<RefLogger> p2 = p1;
-        ASSERT_EQ(&a, p1.get());
-        ASSERT_EQ(&a, p2.get());
+        EXPECT_EQ(&a, p1.get());
+        EXPECT_EQ(&a, p2.get());
     }
-    ASSERT_STREQ("ref(a) ref(a) deref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) ref(a) deref(a) deref(a) ", takeLogStr().c_str());
 
     {
         RefPtr<DerivedRefLogger> p1 = &a;
         RefPtr<RefLogger> p2 = WTFMove(p1);
-        ASSERT_EQ(nullptr, p1.get());
-        ASSERT_EQ(&a, p2.get());
+        EXPECT_EQ(nullptr, p1.get());
+        EXPECT_EQ(&a, p2.get());
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> ptr(&a);
-        ASSERT_EQ(&a, ptr.get());
+        EXPECT_EQ(&a, ptr.get());
         ptr = nullptr;
-        ASSERT_EQ(nullptr, ptr.get());
+        EXPECT_EQ(nullptr, ptr.get());
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 }
 
 TEST(WTF_RefPtr, AssignPassRefToRefPtr)
@@ -116,9 +116,9 @@
     {
         Ref<RefLogger> passRef(a);
         RefPtr<RefLogger> ptr = WTFMove(passRef);
-        ASSERT_EQ(&a, ptr.get());
+        EXPECT_EQ(&a, ptr.get());
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 }
 
 TEST(WTF_RefPtr, Adopt)
@@ -126,21 +126,21 @@
     DerivedRefLogger a("a");
 
     RefPtr<RefLogger> empty;
-    ASSERT_EQ(nullptr, empty.get());
+    EXPECT_EQ(nullptr, empty.get());
 
     {
         RefPtr<RefLogger> ptr(adoptRef(&a));
-        ASSERT_EQ(&a, ptr.get());
-        ASSERT_EQ(&a, &*ptr);
-        ASSERT_EQ(&a.name, &ptr->name);
+        EXPECT_EQ(&a, ptr.get());
+        EXPECT_EQ(&a, &*ptr);
+        EXPECT_EQ(&a.name, &ptr->name);
     }
-    ASSERT_STREQ("deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("deref(a) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> ptr = adoptRef(&a);
-        ASSERT_EQ(&a, ptr.get());
+        EXPECT_EQ(&a, ptr.get());
     }
-    ASSERT_STREQ("deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("deref(a) ", takeLogStr().c_str());
 }
 
 TEST(WTF_RefPtr, Assignment)
@@ -152,116 +152,116 @@
     {
         RefPtr<RefLogger> p1(&a);
         RefPtr<RefLogger> p2(&b);
-        ASSERT_EQ(&a, p1.get());
-        ASSERT_EQ(&b, p2.get());
+        EXPECT_EQ(&a, p1.get());
+        EXPECT_EQ(&b, p2.get());
         log() << "| ";
         p1 = p2;
-        ASSERT_EQ(&b, p1.get());
-        ASSERT_EQ(&b, p2.get());
+        EXPECT_EQ(&b, p1.get());
+        EXPECT_EQ(&b, p2.get());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) ref(b) | ref(b) deref(a) | deref(b) deref(b) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) ref(b) | ref(b) deref(a) | deref(b) deref(b) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> ptr(&a);
-        ASSERT_EQ(&a, ptr.get());
+        EXPECT_EQ(&a, ptr.get());
         log() << "| ";
         ptr = &b;
-        ASSERT_EQ(&b, ptr.get());
+        EXPECT_EQ(&b, ptr.get());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) | ref(b) deref(a) | deref(b) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) | ref(b) deref(a) | deref(b) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> ptr(&a);
-        ASSERT_EQ(&a, ptr.get());
+        EXPECT_EQ(&a, ptr.get());
         log() << "| ";
         ptr = adoptRef(&b);
-        ASSERT_EQ(&b, ptr.get());
+        EXPECT_EQ(&b, ptr.get());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) | deref(a) | deref(b) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) | deref(a) | deref(b) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> ptr(&a);
-        ASSERT_EQ(&a, ptr.get());
+        EXPECT_EQ(&a, ptr.get());
         ptr = nullptr;
-        ASSERT_EQ(nullptr, ptr.get());
+        EXPECT_EQ(nullptr, ptr.get());
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> p1(&a);
         RefPtr<RefLogger> p2(&b);
-        ASSERT_EQ(&a, p1.get());
-        ASSERT_EQ(&b, p2.get());
+        EXPECT_EQ(&a, p1.get());
+        EXPECT_EQ(&b, p2.get());
         log() << "| ";
         p1 = WTFMove(p2);
-        ASSERT_EQ(&b, p1.get());
-        ASSERT_EQ(nullptr, p2.get());
+        EXPECT_EQ(&b, p1.get());
+        EXPECT_EQ(nullptr, p2.get());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) ref(b) | deref(a) | deref(b) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) ref(b) | deref(a) | deref(b) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> p1(&a);
         RefPtr<DerivedRefLogger> p2(&c);
-        ASSERT_EQ(&a, p1.get());
-        ASSERT_EQ(&c, p2.get());
+        EXPECT_EQ(&a, p1.get());
+        EXPECT_EQ(&c, p2.get());
         log() << "| ";
         p1 = p2;
-        ASSERT_EQ(&c, p1.get());
-        ASSERT_EQ(&c, p2.get());
+        EXPECT_EQ(&c, p1.get());
+        EXPECT_EQ(&c, p2.get());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) ref(c) | ref(c) deref(a) | deref(c) deref(c) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) ref(c) | ref(c) deref(a) | deref(c) deref(c) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> ptr(&a);
-        ASSERT_EQ(&a, ptr.get());
+        EXPECT_EQ(&a, ptr.get());
         log() << "| ";
         ptr = &c;
-        ASSERT_EQ(&c, ptr.get());
+        EXPECT_EQ(&c, ptr.get());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) | ref(c) deref(a) | deref(c) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) | ref(c) deref(a) | deref(c) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> ptr(&a);
-        ASSERT_EQ(&a, ptr.get());
+        EXPECT_EQ(&a, ptr.get());
         log() << "| ";
         ptr = adoptRef(&c);
-        ASSERT_EQ(&c, ptr.get());
+        EXPECT_EQ(&c, ptr.get());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) | deref(a) | deref(c) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) | deref(a) | deref(c) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> p1(&a);
         RefPtr<DerivedRefLogger> p2(&c);
-        ASSERT_EQ(&a, p1.get());
-        ASSERT_EQ(&c, p2.get());
+        EXPECT_EQ(&a, p1.get());
+        EXPECT_EQ(&c, p2.get());
         log() << "| ";
         p1 = WTFMove(p2);
-        ASSERT_EQ(&c, p1.get());
-        ASSERT_EQ(nullptr, p2.get());
+        EXPECT_EQ(&c, p1.get());
+        EXPECT_EQ(nullptr, p2.get());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) ref(c) | deref(a) | deref(c) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) ref(c) | deref(a) | deref(c) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> ptr(&a);
-        ASSERT_EQ(&a, ptr.get());
+        EXPECT_EQ(&a, ptr.get());
         log() << "| ";
         ptr = ptr;
-        ASSERT_EQ(&a, ptr.get());
+        EXPECT_EQ(&a, ptr.get());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) | ref(a) deref(a) | deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) | ref(a) deref(a) | deref(a) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> ptr(&a);
-        ASSERT_EQ(&a, ptr.get());
+        EXPECT_EQ(&a, ptr.get());
 #if COMPILER(CLANG)
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wunknown-pragmas"
@@ -271,9 +271,9 @@
 #if COMPILER(CLANG)
 #pragma clang diagnostic pop
 #endif
-        ASSERT_EQ(&a, ptr.get());
+        EXPECT_EQ(&a, ptr.get());
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 }
 
 TEST(WTF_RefPtr, Swap)
@@ -284,26 +284,28 @@
     {
         RefPtr<RefLogger> p1(&a);
         RefPtr<RefLogger> p2(&b);
-        ASSERT_EQ(&a, p1.get());
-        ASSERT_EQ(&b, p2.get());
+        log() << "| ";
+        EXPECT_EQ(&a, p1.get());
+        EXPECT_EQ(&b, p2.get());
         p1.swap(p2);
-        ASSERT_EQ(&b, p1.get());
-        ASSERT_EQ(&a, p2.get());
+        EXPECT_EQ(&b, p1.get());
+        EXPECT_EQ(&a, p2.get());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) ref(b) | deref(a) deref(b) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) ref(b) | | deref(a) deref(b) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> p1(&a);
         RefPtr<RefLogger> p2(&b);
-        ASSERT_EQ(&a, p1.get());
-        ASSERT_EQ(&b, p2.get());
+        log() << "| ";
+        EXPECT_EQ(&a, p1.get());
+        EXPECT_EQ(&b, p2.get());
         std::swap(p1, p2);
-        ASSERT_EQ(&b, p1.get());
-        ASSERT_EQ(&a, p2.get());
+        EXPECT_EQ(&b, p1.get());
+        EXPECT_EQ(&a, p2.get());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) ref(b) | deref(a) deref(b) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) ref(b) | | deref(a) deref(b) ", takeLogStr().c_str());
 }
 
 TEST(WTF_RefPtr, ReleaseNonNull)
@@ -315,7 +317,7 @@
         RefPtr<RefLogger> ref = refPtr.releaseNonNull();
     }
 
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 }
 
 TEST(WTF_RefPtr, Release)
@@ -327,52 +329,52 @@
     {
         RefPtr<RefLogger> p1 = &a;
         RefPtr<RefLogger> p2 = WTFMove(p1);
-        ASSERT_EQ(nullptr, p1.get());
-        ASSERT_EQ(&a, p2.get());
+        EXPECT_EQ(nullptr, p1.get());
+        EXPECT_EQ(&a, p2.get());
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> p1 = &a;
         RefPtr<RefLogger> p2(WTFMove(p1));
-        ASSERT_EQ(nullptr, p1.get());
-        ASSERT_EQ(&a, p2.get());
+        EXPECT_EQ(nullptr, p1.get());
+        EXPECT_EQ(&a, p2.get());
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 
     {
         RefPtr<DerivedRefLogger> p1 = &a;
         RefPtr<RefLogger> p2 = WTFMove(p1);
-        ASSERT_EQ(nullptr, p1.get());
-        ASSERT_EQ(&a, p2.get());
+        EXPECT_EQ(nullptr, p1.get());
+        EXPECT_EQ(&a, p2.get());
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> p1(&a);
         RefPtr<RefLogger> p2(&b);
-        ASSERT_EQ(&a, p1.get());
-        ASSERT_EQ(&b, p2.get());
+        EXPECT_EQ(&a, p1.get());
+        EXPECT_EQ(&b, p2.get());
         log() << "| ";
         p1 = WTFMove(p2);
-        ASSERT_EQ(&b, p1.get());
-        ASSERT_EQ(nullptr, p2.get());
+        EXPECT_EQ(&b, p1.get());
+        EXPECT_EQ(nullptr, p2.get());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) ref(b) | deref(a) | deref(b) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) ref(b) | deref(a) | deref(b) ", takeLogStr().c_str());
 
     {
         RefPtr<RefLogger> p1(&a);
         RefPtr<DerivedRefLogger> p2(&c);
-        ASSERT_EQ(&a, p1.get());
-        ASSERT_EQ(&c, p2.get());
+        EXPECT_EQ(&a, p1.get());
+        EXPECT_EQ(&c, p2.get());
         log() << "| ";
         p1 = WTFMove(p2);
-        ASSERT_EQ(&c, p1.get());
-        ASSERT_EQ(nullptr, p2.get());
+        EXPECT_EQ(&c, p1.get());
+        EXPECT_EQ(nullptr, p2.get());
         log() << "| ";
     }
-    ASSERT_STREQ("ref(a) ref(c) | deref(a) | deref(c) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) ref(c) | deref(a) | deref(c) ", takeLogStr().c_str());
 }
 
 RefPtr<RefLogger> f1(RefLogger& logger)
@@ -387,15 +389,14 @@
     {
         f1(a);
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 
     {
         auto ptr = f1(a);
     }
-    ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+    EXPECT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
 }
 
-    
 struct ConstRefCounted : RefCounted<ConstRefCounted> {
     static Ref<ConstRefCounted> create() { return adoptRef(*new ConstRefCounted); }
 };
@@ -425,4 +426,113 @@
     Ref<const ConstRefCounted> i(returnRefCountedRef());
 }
 
+struct RefPtrCheckingRefLogger : RefLogger {
+    RefPtrCheckingRefLogger(const char* name);
+    void ref();
+    void deref();
+    const RefPtr<RefPtrCheckingRefLogger>* slotToCheck { nullptr };
+};
+
+RefPtrCheckingRefLogger::RefPtrCheckingRefLogger(const char* name)
+    : RefLogger { name }
+{
+}
+
+static const char* loggerName(const RefPtr<RefPtrCheckingRefLogger>& pointer)
+{
+    return pointer ? &pointer->name : "null";
+}
+
+void RefPtrCheckingRefLogger::ref()
+{
+    if (slotToCheck)
+        log() << "slot=" << loggerName(*slotToCheck) << " ";
+    RefLogger::ref();
+}
+
+void RefPtrCheckingRefLogger::deref()
+{
+    if (slotToCheck)
+        log() << "slot=" << loggerName(*slotToCheck) << " ";
+    RefLogger::deref();
+}
+
+TEST(WTF_RefPtr, AssignBeforeDeref)
+{
+    RefPtrCheckingRefLogger a("a");
+    RefPtrCheckingRefLogger b("b");
+
+    {
+        RefPtr<RefPtrCheckingRefLogger> p1(&a);
+        RefPtr<RefPtrCheckingRefLogger> p2(&b);
+        EXPECT_EQ(&a, p1.get());
+        EXPECT_EQ(&b, p2.get());
+        log() << "| ";
+        a.slotToCheck = &p1;
+        b.slotToCheck = &p1;
+        p1 = p2;
+        a.slotToCheck = nullptr;
+        b.slotToCheck = nullptr;
+        EXPECT_EQ(&b, p1.get());
+        EXPECT_EQ(&b, p2.get());
+        log() << "| ";
+    }
+    EXPECT_STREQ("ref(a) ref(b) | slot=a ref(b) slot=b deref(a) | deref(b) deref(b) ", takeLogStr().c_str());
+
+    {
+        RefPtr<RefPtrCheckingRefLogger> ptr(&a);
+        EXPECT_EQ(&a, ptr.get());
+        log() << "| ";
+        a.slotToCheck = &ptr;
+        b.slotToCheck = &ptr;
+        ptr = &b;
+        a.slotToCheck = nullptr;
+        b.slotToCheck = nullptr;
+        EXPECT_EQ(&b, ptr.get());
+        log() << "| ";
+    }
+    EXPECT_STREQ("ref(a) | slot=a ref(b) slot=b deref(a) | deref(b) ", takeLogStr().c_str());
+
+    {
+        RefPtr<RefPtrCheckingRefLogger> ptr(&a);
+        EXPECT_EQ(&a, ptr.get());
+        a.slotToCheck = &ptr;
+        ptr = nullptr;
+        a.slotToCheck = nullptr;
+        EXPECT_EQ(nullptr, ptr.get());
+    }
+    EXPECT_STREQ("ref(a) slot=null deref(a) ", takeLogStr().c_str());
+
+    {
+        RefPtr<RefPtrCheckingRefLogger> p1(&a);
+        RefPtr<RefPtrCheckingRefLogger> p2(&b);
+        EXPECT_EQ(&a, p1.get());
+        EXPECT_EQ(&b, p2.get());
+        log() << "| ";
+        a.slotToCheck = &p1;
+        b.slotToCheck = &p1;
+        p1 = WTFMove(p2);
+        a.slotToCheck = nullptr;
+        b.slotToCheck = nullptr;
+        EXPECT_EQ(&b, p1.get());
+        EXPECT_EQ(nullptr, p2.get());
+        log() << "| ";
+    }
+    EXPECT_STREQ("ref(a) ref(b) | slot=b deref(a) | deref(b) ", takeLogStr().c_str());
+}
+
+TEST(WTF_RefPtr, ReleaseNonNullBeforeDeref)
+{
+    RefPtrCheckingRefLogger a("a");
+
+    {
+        RefPtr<RefPtrCheckingRefLogger> refPtr = &a;
+        a.slotToCheck = &refPtr;
+        refPtr.releaseNonNull();
+        a.slotToCheck = nullptr;
+    }
+
+    EXPECT_STREQ("ref(a) slot=null deref(a) ", takeLogStr().c_str());
+}
+
 } // namespace TestWebKitAPI

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to