Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(210)

Issue 380703002: Revert of Remove ability for Release code to call getRefCnt() or getWeakRefCnt(). (Closed)

Created:
6 years, 5 months ago by mtklein
Modified:
6 years, 5 months ago
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@master
Project:
skia
Visibility:
Public.

Description

Revert of Remove ability for Release code to call getRefCnt() or getWeakRefCnt(). (https://codereview.chromium.org/378643003/) Reason for revert: External unit tests call getRefCnt(). Original issue's description: > Remove ability for Release code to call getRefCnt() or getWeakRefCnt(). > > These getRefCnt() methods are not thread safe, so Skia code should not > be calling them. unique() is fine. > > SkDEBUG code (SkASSERTs) can still call getRefCnt() / getWeakRefCnt(). > > This adds tools/RefCntIs.{h,cpp}, which lets tests make their assertions in > both debug and release modes. > > BUG=skia:2726 > > Committed: https://skia.googlesource.com/skia/+/4ae94ffce5ecf1b71cb5e295b68bf4ec9e697443 TBR=senorblanco@chromium.org,reed@google.com,mtklein@chromium.org NOTREECHECKS=true NOTRY=true BUG=skia:2726

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+72 lines, -122 lines) Patch
M gyp/tests.gypi View 1 chunk +0 lines, -1 line 0 comments Download
M gyp/tools.gyp View 1 chunk +0 lines, -11 lines 0 comments Download
M include/core/SkImageFilter.h View 1 chunk +0 lines, -1 line 0 comments Download
M include/core/SkRefCnt.h View 2 chunks +0 lines, -5 lines 0 comments Download
M include/core/SkWeakRefCnt.h View 2 chunks +3 lines, -4 lines 0 comments Download
M src/core/SkImageFilter.cpp View 2 chunks +2 lines, -8 lines 0 comments Download
M tests/ClipCacheTest.cpp View 4 chunks +5 lines, -6 lines 0 comments Download
M tests/ImageFilterTest.cpp View 1 chunk +1 line, -1 line 0 comments Download
M tests/MetaDataTest.cpp View 2 chunks +5 lines, -6 lines 0 comments Download
M tests/RefCntTest.cpp View 5 chunks +5 lines, -6 lines 0 comments Download
M tests/RefDictTest.cpp View 2 chunks +15 lines, -16 lines 0 comments Download
M tests/SurfaceTest.cpp View 2 chunks +3 lines, -4 lines 0 comments Download
M tests/UtilsTest.cpp View 5 chunks +30 lines, -31 lines 0 comments Download
D tools/RefCntIs.h View 1 chunk +0 lines, -13 lines 0 comments Download
D tools/RefCntIs.cpp View 1 chunk +0 lines, -9 lines 0 comments Download
M tools/tsan.supp View 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
mtklein
Created Revert of Remove ability for Release code to call getRefCnt() or getWeakRefCnt().
6 years, 5 months ago (2014-07-09 12:58:01 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/mtklein@google.com/380703002/1
6 years, 5 months ago (2014-07-09 12:59:06 UTC) #2
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 5 months ago (2014-07-09 12:59:11 UTC) #3
commit-bot: I haz the power
6 years, 5 months ago (2014-07-09 12:59:11 UTC) #4
Failed to apply patch for tools/tsan.supp:
While running git apply --index -p1;
  error: patch failed: tools/tsan.supp:31
  error: tools/tsan.supp: patch does not apply

Patch:       tools/tsan.supp
Index: tools/tsan.supp
diff --git a/tools/tsan.supp b/tools/tsan.supp
index
6c2b0909fc0abdc221c6dad153f8e3abf338595d..179adc9b4afd1f4f20d2cc40ad47d9bd48961536
100644
--- a/tools/tsan.supp
+++ b/tools/tsan.supp
@@ -31,6 +31,9 @@
 race:SkPixelRef::callGenIDChangeListeners
 race:SkPixelRef::needsNewGenID
 
+# This calls SkRefCnt::getRefCnt(), which is not thread safe.  skia:2726
+race:SkImageFilter::filterImage
+
 # 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.

Powered by Google App Engine
This is Rietveld 408576698