Revert of Add SkRacy (https://codereview.chromium.org/371363004/)

Reason for revert:
hidden symbol 'AnnotateBenignRaceSized' in obj/base/third_party/dynamic_annotations/libdynamic_annotations.a(obj/base/third_party/dynamic_annotations/dynamic_annotations.dynamic_annotations.o) is referenced by DSO lib/libblink_platform.so

Original issue's description:
> Add SkRacy
>
> SkRacy<T> is a zero-overhead wrapper for a T, except it also
> silences race warnings when TSAN is running.
>
> Here we apply in several classes.  In SkMatrix and SkPathRef,
> we use it to opportunistically cache some idempotent work.
>
> In SkPixelRef, we wrap the genIDs.  We think the worst that
> can happen here is we'll increment the global next-genID a
> few times instead of once when we go to get another ID.
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/d5e3e6ae1b3434ad1158f441902ff65f1eeaa3a7

R=reed@google.com, mtklein@chromium.org
TBR=mtklein@chromium.org, reed@google.com
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Author: mtklein@google.com

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

include/core/SkDynamicAnnotations.h

@@ -19,8 +19,6 @@ extern "C" {
 // TSAN provides these hooks.
 void AnnotateIgnoreReadsBegin(const char* file, int line);
 void AnnotateIgnoreReadsEnd(const char* file, int line);
-void AnnotateBenignRaceSized(const char* file, int line,
-                             const void* addr, long size, const char* desc);
 }  // extern "C"
 
 // SK_ANNOTATE_UNPROTECTED_READ can wrap any variable read to tell TSAN to ignore that it appears to
@@ -39,46 +37,10 @@ inline T SK_ANNOTATE_UNPROTECTED_READ(const volatile T& x) {
     return read;
 }
 
-// Ignore racy reads and racy writes to this pointer, indefinitely.
-// If at all possible, use the more precise SK_ANNOTATE_UNPROTECTED_READ.
-template <typename T>
-void SK_ANNOTATE_BENIGN_RACE(T* ptr) {
-    AnnotateBenignRaceSized(__FILE__, __LINE__, ptr, sizeof(*ptr), "SK_ANNOTATE_BENIGN_RACE");
-}
-
 #else  // !DYNAMIC_ANNOTATIONS_ENABLED
 
 #define SK_ANNOTATE_UNPROTECTED_READ(x) (x)
-#define SK_ANNOTATE_BENIGN_RACE(ptr)
 
 #endif
 
-// Can be used to wrap values that are intentionally racy, usually small mutable cached values, e.g.
-//   - SkMatrix type mask
-//   - SkPathRef bounds
-//   - SkPixelRef genIDs
-template <typename T>
-class SkTRacy {
-public:
-    SkTRacy() { SK_ANNOTATE_BENIGN_RACE(&fVal); }
-
-    operator const T&() const {
-        return fVal;
-    }
-
-    SkTRacy& operator=(const T& val) {
-        fVal = val;
-        return *this;
-    }
-
-    const T* get() const { return &fVal; }
-          T* get()       { return &fVal; }
-
-    const T* operator->() const { return &fVal; }
-          T* operator->()       { return &fVal; }
-
-private:
-    T fVal;
-};
-
 #endif//SkDynamicAnnotations_DEFINED

include/core/SkMatrix.h

@@ -10,7 +10,6 @@
 #ifndef SkMatrix_DEFINED
 #define SkMatrix_DEFINED
 
-#include "SkDynamicAnnotations.h"
 #include "SkRect.h"
 
 class SkString;
@@ -644,7 +643,7 @@ private:
     };
 
     SkScalar         fMat[9];
-    mutable SkTRacy<uint32_t> fTypeMask;
+    mutable uint32_t fTypeMask;
 
     uint8_t computeTypeMask() const;
     uint8_t computePerspectiveTypeMask() const;
@@ -665,7 +664,7 @@ private:
     void clearTypeMask(int mask) {
         // only allow a valid mask
         SkASSERT((mask & kAllMasks) == mask);
-        fTypeMask = fTypeMask & ~mask;
+        fTypeMask &= ~mask;
     }
 
     TypeMask getPerspectiveTypeMaskOnly() const {

include/core/SkPathRef.h

@@ -9,7 +9,6 @@
 #ifndef SkPathRef_DEFINED
 #define SkPathRef_DEFINED
 
-#include "SkDynamicAnnotations.h"
 #include "SkMatrix.h"
 #include "SkPoint.h"
 #include "SkRect.h"
@@ -293,7 +292,7 @@ private:
         SkDEBUGCODE(this->validate();)
         SkASSERT(fBoundsIsDirty);
 
-        fIsFinite = ComputePtBounds(fBounds.get(), *this);
+        fIsFinite = ComputePtBounds(&fBounds, *this);
         fBoundsIsDirty = false;
     }
 
@@ -301,7 +300,7 @@ private:
         SkASSERT(rect.fLeft <= rect.fRight && rect.fTop <= rect.fBottom);
         fBounds = rect;
         fBoundsIsDirty = false;
-        fIsFinite = fBounds->isFinite();
+        fIsFinite = fBounds.isFinite();
     }
 
     /** Makes additional room but does not change the counts or change the genID */
@@ -433,12 +432,11 @@ private:
         kMinSize = 256,
     };
 
-    mutable SkTRacy<SkRect>      fBounds;
+    mutable SkRect      fBounds;
-    mutable SkTRacy<uint8_t>     fBoundsIsDirty;
+    uint8_t             fSegmentMask;
-    mutable SkTRacy<SkBool8>     fIsFinite;    // only meaningful if bounds are valid
+    mutable uint8_t     fBoundsIsDirty;
-
+    mutable SkBool8     fIsFinite;    // only meaningful if bounds are valid
-    SkBool8  fIsOval;
+    mutable SkBool8     fIsOval;
-    uint8_t  fSegmentMask;
 
     SkPoint*            fPoints; // points to begining of the allocation
     uint8_t*            fVerbs; // points just past the end of the allocation (verbs grow backwards)

include/core/SkPixelRef.h

@@ -9,7 +9,6 @@
 #define SkPixelRef_DEFINED
 
 #include "SkBitmap.h"
-#include "SkDynamicAnnotations.h"
 #include "SkRefCnt.h"
 #include "SkString.h"
 #include "SkFlattenable.h"
@@ -350,8 +349,8 @@ private:
     LockRec         fRec;
     int             fLockCount;
 
-    mutable SkTRacy<uint32_t> fGenerationID;
+    mutable uint32_t fGenerationID;
-    mutable SkTRacy<bool>     fUniqueGenerationID;
+    mutable bool     fUniqueGenerationID;
 
     SkTDArray<GenIDChangeListener*> fGenIDChangeListeners;  // pointers are owned
 

src/core/SkPathRef.cpp

@@ -30,7 +30,9 @@ SkPathRef::Editor::Editor(SkAutoTUnref<SkPathRef>* pathRef,
 //////////////////////////////////////////////////////////////////////////////
 
 SkPathRef* SkPathRef::CreateEmptyImpl() {
-    return SkNEW(SkPathRef);
+    SkPathRef* p = SkNEW(SkPathRef);
+    p->computeBounds();   // Preemptively avoid a race to clear fBoundsIsDirty.
+    return p;
 }
 
 SkPathRef* SkPathRef::CreateEmpty() {
@@ -83,13 +85,13 @@ void SkPathRef::CreateTransformedCopy(SkAutoTUnref<SkPathRef>* dst,
     if (canXformBounds) {
         (*dst)->fBoundsIsDirty = false;
         if (src.fIsFinite) {
-            matrix.mapRect((*dst)->fBounds.get(), src.fBounds);
+            matrix.mapRect(&(*dst)->fBounds, src.fBounds);
-            if (!((*dst)->fIsFinite = (*dst)->fBounds->isFinite())) {
+            if (!((*dst)->fIsFinite = (*dst)->fBounds.isFinite())) {
-                (*dst)->fBounds->setEmpty();
+                (*dst)->fBounds.setEmpty();
             }
         } else {
             (*dst)->fIsFinite = false;
-            (*dst)->fBounds->setEmpty();
+            (*dst)->fBounds.setEmpty();
         }
     } else {
         (*dst)->fBoundsIsDirty = true;
@@ -439,14 +441,14 @@ void SkPathRef::validate() const {
     SkASSERT(this->currSize() ==
                 fFreeSpace + sizeof(SkPoint) * fPointCnt + sizeof(uint8_t) * fVerbCnt);
 
-    if (!fBoundsIsDirty && !fBounds->isEmpty()) {
+    if (!fBoundsIsDirty && !fBounds.isEmpty()) {
         bool isFinite = true;
         for (int i = 0; i < fPointCnt; ++i) {
             SkASSERT(!fPoints[i].isFinite() || (
-                     fBounds->fLeft - fPoints[i].fX   < SK_ScalarNearlyZero &&
+                     fBounds.fLeft - fPoints[i].fX   < SK_ScalarNearlyZero &&
-                     fPoints[i].fX - fBounds->fRight  < SK_ScalarNearlyZero &&
+                     fPoints[i].fX - fBounds.fRight  < SK_ScalarNearlyZero &&
-                     fBounds->fTop  - fPoints[i].fY   < SK_ScalarNearlyZero &&
+                     fBounds.fTop  - fPoints[i].fY   < SK_ScalarNearlyZero &&
-                     fPoints[i].fY - fBounds->fBottom < SK_ScalarNearlyZero));
+                     fPoints[i].fY - fBounds.fBottom < SK_ScalarNearlyZero));
             if (!fPoints[i].isFinite()) {
                 isFinite = false;
             }

tools/tsan.supp

@@ -25,3 +25,21 @@ race:is_lcd_supported
 race:RefFCI
 race:SkString
 race:SkPDF
+
+# These race benignly as used by DMQuiltTask: skia:2725.
+# Suppress while I look for a more focused way to silence this.
+race:SkPixelRef::callGenIDChangeListeners
+race:SkPixelRef::needsNewGenID
+
+# SkPathRef caches its bounding box the first time it's needed.
+# This will be fixed naturally once we create (from a single thread) a
+# bounding-box hierarchy for SkRecord-based SkPictures; all bounds will come pre-cached.
+# So just shut this up for now.
+race:SkPathRef::computeBounds
+
+# SkMatrix caches a type mask.  If we race on this, we'll just calculate the same thing a few times.
+race:SkMatrix::getType
+race:SkMatrix::rectStaysRect
+race:SkMatrix::getPerspectiveTypeMaskOnly
+
+# TODO: some sort of SkRacy<T> to handle cases like SkMatrix, SkPathRef, SkPixelRef above?