Clean up SkString reference counting a bit.

BUG=skia:7107

Change-Id: I47072bf31b902c79dbb850179ff6d35940de3e63
Reviewed-on: https://skia-review.googlesource.com/54720
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>

1 files changed, 31 insertions, 39 deletions

diff --git a/src/core/SkString.cpp b/src/core/SkString.cpp
index d0e3c12cbe..23ef8f19f4 100644
--- a/src/core/SkString.cpp
+++ b/src/core/SkString.cpp
@@ -220,7 +220,7 @@ static size_t check_add32(size_t base, size_t extra) {
     return extra;
 }
 
-SkString::Rec* SkString::AllocRec(const char text[], size_t len) {
+sk_sp<SkString::Rec> SkString::Rec::Make(const char text[], size_t len) {
     Rec* rec;
 
     if (0 == len) {
@@ -237,15 +237,29 @@ SkString::Rec* SkString::AllocRec(const char text[], size_t len) {
         }
         rec->data()[len] = 0;
     }
-    return rec;
+    return sk_sp<SkString::Rec>(rec);
 }
 
-SkString::Rec* SkString::RefRec(Rec* src) {
-    if (src != &gEmptyRec) {
-        // No barrier required.
-        (void)src->fRefCnt.fetch_add(+1, std::memory_order_relaxed);
+void SkString::Rec::ref() const {
+    if (this == &SkString::gEmptyRec) {
+        return;
+    }
+    // No barrier required.
+    (void)this->fRefCnt.fetch_add(+1, std::memory_order_relaxed);
+}
+
+void SkString::Rec::unref() const {
+    if (this == &SkString::gEmptyRec) {
+        return;
+    }
+    SkASSERT(this->fRefCnt.load(std::memory_order_relaxed) > 0);
+    if (1 == this->fRefCnt.fetch_add(-1, std::memory_order_acq_rel)) {
+        sk_free(const_cast<SkString::Rec*>(this));
     }
-    return src;
+}
+
+bool SkString::Rec::unique() const {
+    return fRefCnt.load(std::memory_order_acquire) == 1;
 }
 
 #ifdef SK_DEBUG
@@ -255,7 +269,7 @@ void SkString::validate() const {
     SkASSERT(0 == gEmptyRec.fRefCnt.load(std::memory_order_relaxed));
     SkASSERT(0 == gEmptyRec.data()[0]);
 
-    if (fRec != &gEmptyRec) {
+    if (fRec.get() != &gEmptyRec) {
         SkASSERT(fRec->fLength > 0);
         SkASSERT(fRec->fRefCnt.load(std::memory_order_relaxed) > 0);
         SkASSERT(0 == fRec->data()[fRec->fLength]);
@@ -269,41 +283,34 @@ SkString::SkString() : fRec(const_cast<Rec*>(&gEmptyRec)) {
 }
 
 SkString::SkString(size_t len) {
-    fRec = AllocRec(nullptr, len);
+    fRec = Rec::Make(nullptr, len);
 }
 
 SkString::SkString(const char text[]) {
     size_t  len = text ? strlen(text) : 0;
 
-    fRec = AllocRec(text, len);
+    fRec = Rec::Make(text, len);
 }
 
 SkString::SkString(const char text[], size_t len) {
-    fRec = AllocRec(text, len);
+    fRec = Rec::Make(text, len);
 }
 
 SkString::SkString(const SkString& src) {
     src.validate();
 
-    fRec = RefRec(src.fRec);
+    fRec = src.fRec;
 }
 
 SkString::SkString(SkString&& src) {
     src.validate();
 
-    fRec = src.fRec;
-    src.fRec = const_cast<Rec*>(&gEmptyRec);
+    fRec = std::move(src.fRec);
+    src.fRec.reset(const_cast<Rec*>(&gEmptyRec));
 }
 
 SkString::~SkString() {
     this->validate();
-
-    if (fRec->fLength) {
-        SkASSERT(fRec->fRefCnt.load(std::memory_order_relaxed) > 0);
-        if (1 == fRec->fRefCnt.fetch_add(-1, std::memory_order_acq_rel)) {
-            sk_free(fRec);
-        }
-    }
 }
 
 bool SkString::equals(const SkString& src) const {
@@ -350,15 +357,7 @@ SkString& SkString::operator=(const char text[]) {
 
 void SkString::reset() {
     this->validate();
-
-    if (fRec->fLength) {
-        SkASSERT(fRec->fRefCnt.load(std::memory_order_relaxed) > 0);
-        if (1 == fRec->fRefCnt.fetch_add(-1, std::memory_order_acq_rel)) {
-            sk_free(fRec);
-        }
-    }
-
-    fRec = const_cast<Rec*>(&gEmptyRec);
+    fRec.reset(const_cast<Rec*>(&gEmptyRec));
 }
 
 char* SkString::writable_str() {
@@ -366,14 +365,7 @@ char* SkString::writable_str() {
 
     if (fRec->fLength) {
         if (!fRec->unique()) {
-            Rec* rec = AllocRec(fRec->data(), fRec->fLength);
-            if (1 == fRec->fRefCnt.fetch_add(-1, std::memory_order_acq_rel)) {
-                // In this case after our check of fRecCnt > 1, we suddenly
-                // did become the only owner, so now we have two copies of the
-                // data (fRec and rec), so we need to delete one of them.
-                sk_free(fRec);
-            }
-            fRec = rec;
+            fRec = Rec::Make(fRec->data(), fRec->fLength);
         }
     }
     return fRec->data();
@@ -633,7 +625,7 @@ void SkString::swap(SkString& other) {
     this->validate();
     other.validate();
 
-    SkTSwap<Rec*>(fRec, other.fRec);
+    SkTSwap(fRec, other.fRec);
 }
 
 ///////////////////////////////////////////////////////////////////////////////