Add element_type, swap, operators, fix reset on sk_sp.

The 'element_type' typedef is to play nice with std::pointer_traits.

The full complement of operators and swap to match unique_ptr so that
sk_sp can be properly compared to nullptr and used with standard
containers.

Update to 'reset' so that calling 'unref' is the last operation.

This also adds tests for these changes, and sets the fPtr to nullptr
in debug for easier bug finding.

Review URL: https://codereview.chromium.org/1773453002

1 files changed, 74 insertions, 0 deletions

diff --git a/tests/RefCntTest.cpp b/tests/RefCntTest.cpp
index d3cda7f38d..2932913c3d 100644
--- a/tests/RefCntTest.cpp
+++ b/tests/RefCntTest.cpp
@@ -275,6 +275,80 @@ DEF_TEST(sk_sp, reporter) {
     check(reporter, 1, 1, 1, 0);
     paint.set(nullptr);
     check(reporter, 1, 2, 1, 1);
+
+    {
+        sk_sp<SkRefCnt> empty;
+        sk_sp<SkRefCnt> notEmpty = sk_make_sp<SkRefCnt>();
+        REPORTER_ASSERT(reporter, empty == sk_sp<SkRefCnt>());
+
+        REPORTER_ASSERT(reporter, notEmpty != empty);
+        REPORTER_ASSERT(reporter, empty != notEmpty);
+
+        REPORTER_ASSERT(reporter, nullptr == empty);
+        REPORTER_ASSERT(reporter, empty == nullptr);
+        REPORTER_ASSERT(reporter, empty == empty);
+
+        REPORTER_ASSERT(reporter, nullptr <= empty);
+        REPORTER_ASSERT(reporter, empty <= nullptr);
+        REPORTER_ASSERT(reporter, empty <= empty);
+
+        REPORTER_ASSERT(reporter, nullptr >= empty);
+        REPORTER_ASSERT(reporter, empty >= nullptr);
+        REPORTER_ASSERT(reporter, empty >= empty);
+    }
+
+    {
+        sk_sp<SkRefCnt> a = sk_make_sp<SkRefCnt>();
+        sk_sp<SkRefCnt> b = sk_make_sp<SkRefCnt>();
+        REPORTER_ASSERT(reporter, a != b);
+        REPORTER_ASSERT(reporter, (a < b) != (b < a));
+        REPORTER_ASSERT(reporter, (b > a) != (a > b));
+        REPORTER_ASSERT(reporter, (a <= b) != (b <= a));
+        REPORTER_ASSERT(reporter, (b >= a) != (a >= b));
+
+        REPORTER_ASSERT(reporter, a == a);
+        REPORTER_ASSERT(reporter, a <= a);
+        REPORTER_ASSERT(reporter, a >= a);
+    }
+
+    // http://wg21.cmeerw.net/lwg/issue998
+    {
+        class foo : public SkRefCnt {
+        public:
+            foo() : bar(this) {}
+            void reset() { bar.reset(); }
+        private:
+            sk_sp<foo> bar;
+        };
+        // The following should properly delete the object and not cause undefined behavior.
+        // This is an ugly example, but the same issue can arise in more subtle ways.
+        (new foo)->reset();
+    }
+
+    // https://crrev.com/0d4ef2583a6f19c3e61be04d36eb1a60b133832c
+    {
+        struct StructB;
+        struct StructA : public SkRefCnt {
+            sk_sp<StructB> b;
+        };
+
+        struct StructB : public SkRefCnt {
+            sk_sp<StructA> a;
+            ~StructB() override {}; // Some clang versions don't emit this implicitly.
+        };
+
+        // Create a reference cycle.
+        StructA* a = new StructA;
+        a->b.reset(new StructB);
+        a->b->a.reset(a);
+
+        // Break the cycle by calling reset(). This will cause |a| (and hence, |a.b|)
+        // to be deleted before the call to reset() returns. This tests that the
+        // implementation of sk_sp::reset() doesn't access |this| after it
+        // deletes the underlying pointer. This behaviour is consistent with the
+        // definition of unique_ptr::reset in C++11.
+        a->b.reset();
+    }
 }
 
 namespace {