Revert "Clean up SkString reference counting a bit."

This reverts commit a910c847e9d04e183e9e610902cbd363c8488196.

Reason for revert: Compilation failure on Ubuntu14 bots

../../../../../work/skia/src/core/SkString.cpp:200:55: error: could not convert ‘{0, {0}, 0}’ from ‘<brace-enclosed initializer list>’ to ‘const SkString::Rec’
 const SkString::Rec SkString::gEmptyRec = { 0, {0}, 0 };

Original change's description:
> 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>

TBR=mtklein@google.com,bungeman@google.com,reed@google.com

Change-Id: I6ec327511e8e1c1fd7e4c1bd5839c0547d4ab609
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:7107
Reviewed-on: https://skia-review.googlesource.com/55640
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>

1 files changed, 39 insertions, 31 deletions

diff --git a/src/core/SkString.cpp b/src/core/SkString.cpp
index 23ef8f19f4..d0e3c12cbe 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;
 }
 
-sk_sp<SkString::Rec> SkString::Rec::Make(const char text[], size_t len) {
+SkString::Rec* SkString::AllocRec(const char text[], size_t len) {
     Rec* rec;
 
     if (0 == len) {
@@ -237,29 +237,15 @@ sk_sp<SkString::Rec> SkString::Rec::Make(const char text[], size_t len) {
         }
         rec->data()[len] = 0;
     }
-    return sk_sp<SkString::Rec>(rec);
+    return rec;
 }
 
-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));
+SkString::Rec* SkString::RefRec(Rec* src) {
+    if (src != &gEmptyRec) {
+        // No barrier required.
+        (void)src->fRefCnt.fetch_add(+1, std::memory_order_relaxed);
     }
-}
-
-bool SkString::Rec::unique() const {
-    return fRefCnt.load(std::memory_order_acquire) == 1;
+    return src;
 }
 
 #ifdef SK_DEBUG
@@ -269,7 +255,7 @@ void SkString::validate() const {
     SkASSERT(0 == gEmptyRec.fRefCnt.load(std::memory_order_relaxed));
     SkASSERT(0 == gEmptyRec.data()[0]);
 
-    if (fRec.get() != &gEmptyRec) {
+    if (fRec != &gEmptyRec) {
         SkASSERT(fRec->fLength > 0);
         SkASSERT(fRec->fRefCnt.load(std::memory_order_relaxed) > 0);
         SkASSERT(0 == fRec->data()[fRec->fLength]);
@@ -283,34 +269,41 @@ SkString::SkString() : fRec(const_cast<Rec*>(&gEmptyRec)) {
 }
 
 SkString::SkString(size_t len) {
-    fRec = Rec::Make(nullptr, len);
+    fRec = AllocRec(nullptr, len);
 }
 
 SkString::SkString(const char text[]) {
     size_t  len = text ? strlen(text) : 0;
 
-    fRec = Rec::Make(text, len);
+    fRec = AllocRec(text, len);
 }
 
 SkString::SkString(const char text[], size_t len) {
-    fRec = Rec::Make(text, len);
+    fRec = AllocRec(text, len);
 }
 
 SkString::SkString(const SkString& src) {
     src.validate();
 
-    fRec = src.fRec;
+    fRec = RefRec(src.fRec);
 }
 
 SkString::SkString(SkString&& src) {
     src.validate();
 
-    fRec = std::move(src.fRec);
-    src.fRec.reset(const_cast<Rec*>(&gEmptyRec));
+    fRec = src.fRec;
+    src.fRec = 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 {
@@ -357,7 +350,15 @@ SkString& SkString::operator=(const char text[]) {
 
 void SkString::reset() {
     this->validate();
-    fRec.reset(const_cast<Rec*>(&gEmptyRec));
+
+    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);
 }
 
 char* SkString::writable_str() {
@@ -365,7 +366,14 @@ char* SkString::writable_str() {
 
     if (fRec->fLength) {
         if (!fRec->unique()) {
-            fRec = Rec::Make(fRec->data(), fRec->fLength);
+            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;
         }
     }
     return fRec->data();
@@ -625,7 +633,7 @@ void SkString::swap(SkString& other) {
     this->validate();
     other.validate();
 
-    SkTSwap(fRec, other.fRec);
+    SkTSwap<Rec*>(fRec, other.fRec);
 }
 
 ///////////////////////////////////////////////////////////////////////////////