Revert of Clean up Test's resourcePath code. (https://codereview.chromium.org/319473003/)

Reason for revert:
Some benchmarks are written in a way that makes this change unsafe (e.g. const char* resPath = GetResourcePath().c_str(); in SkipZeroesBench) and Valgrind and ASAN caught that.  We can try again after a more careful cleanup of GetResourcePath().

Original issue's description:
> Clean up resourcePath code.
>
> 1) Make the implementation of SetResourcePath/GetResourcePath of GM and SkBenchmark match with the one in Test.
> 2) Make gResourcePath a static pointer to const char and move it inside the classes.
>
> BUG=None
> TEST=make tests && out/Debug/tests
>      make gm && out/Debug/gm
>      make bench && out/Debug/bench
> R=mtklein@google.com
>
> Committed: https://skia.googlesource.com/skia/+/52e4f413ffe2d281f9e90ff2147db08083ffcba7

R=tfarina@chromium.org
TBR=tfarina@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=None

Author: mtklein@google.com

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

diff --git a/bench/SkBenchmark.cpp b/bench/SkBenchmark.cpp
index 1f12ed339f71bd40e77ec95f21ca95823dd033be..d995415632d500fe563dbabcd860f5e2b0b8251d 100644 (file)
--- a/bench/SkBenchmark.cpp
+++ b/bench/SkBenchmark.cpp
@@ -12,7 +12,7 @@ const char* SkTriState::Name[] = { "default", "true", "false" };
 
 template BenchRegistry* BenchRegistry::gHead;
 
-const char* SkBenchmark::gResourcePath;
+SkString SkBenchmark::gResourcePath;
 
 SkBenchmark::SkBenchmark() {
     fForceAlpha = 0xFF;
@@ -55,13 +55,6 @@ void SkBenchmark::setupPaint(SkPaint* paint) {
     }
 }
 
-void SkBenchmark::SetResourcePath(const char* resourcePath) {
-    gResourcePath = resourcePath;
-}
-
-SkString SkBenchmark::GetResourcePath() {
-    return SkString(gResourcePath);
-}
 
 ///////////////////////////////////////////////////////////////////////////////
 

diff --git a/bench/SkBenchmark.h b/bench/SkBenchmark.h
index 00a816cdba76ca08316b378976023e0adf582662..bf28689236593b81cf450f29ff88fc197adc7c59 100644 (file)
--- a/bench/SkBenchmark.h
+++ b/bench/SkBenchmark.h
@@ -107,8 +107,9 @@ public:
         fClearMask = clearMask;
     }
 
-    static void SetResourcePath(const char*);
-    static SkString GetResourcePath();
+    static void SetResourcePath(const char* resPath) { gResourcePath.set(resPath); }
+
+    static SkString& GetResourcePath() { return gResourcePath; }
 
 protected:
     virtual void setupPaint(SkPaint* paint);
@@ -128,7 +129,7 @@ private:
     bool    fForceFilter;
     SkTriState::State  fDither;
     uint32_t    fOrMask, fClearMask;
-    static const char* gResourcePath;
+    static  SkString gResourcePath;
 
     typedef SkRefCnt INHERITED;
 };

diff --git a/gm/copyTo4444.cpp b/gm/copyTo4444.cpp
index 6233301fed2ecbd2000bfa5ae59ce762205f6c22..7e2c2798e450154700ee7e828e584b2ffd7c72d9 100644 (file)
--- a/gm/copyTo4444.cpp
+++ b/gm/copyTo4444.cpp
@@ -30,7 +30,8 @@ protected:
 
     virtual void onDraw(SkCanvas* canvas) {
         SkBitmap bm, bm4444;
-        SkString filename = SkOSPath::SkPathJoin(INHERITED::gResourcePath, "mandrill_512.png");
+        SkString filename = SkOSPath::SkPathJoin(
+                INHERITED::gResourcePath.c_str(), "mandrill_512.png");
         if (!SkImageDecoder::DecodeFile(filename.c_str(), &bm,
                                         SkBitmap::kARGB_8888_Config,
                                         SkImageDecoder::kDecodePixels_Mode)) {

diff --git a/gm/etc1bitmap.cpp b/gm/etc1bitmap.cpp
index ce0aa7dbad6e3018ff58b404635c88d5383b6b48..cdf8617a35627e317feb96de0ea8e09148265af0 100644 (file)
--- a/gm/etc1bitmap.cpp
+++ b/gm/etc1bitmap.cpp
@@ -93,7 +93,8 @@ protected:
 
     virtual void onDraw(SkCanvas* canvas) SK_OVERRIDE {
         SkBitmap bm;
-        SkString filename = SkOSPath::SkPathJoin(INHERITED::gResourcePath, "mandrill_128.");
+        SkString filename = SkOSPath::SkPathJoin(
+                INHERITED::gResourcePath.c_str(), "mandrill_128.");
         filename.append(this->fileExtension());
 
         SkAutoTUnref<SkData> fileData(SkData::NewFromFileName(filename.c_str()));
@@ -168,7 +169,8 @@ protected:
     virtual void onDraw(SkCanvas* canvas) SK_OVERRIDE {
 
         SkBitmap bm;
-        SkString filename = SkOSPath::SkPathJoin(INHERITED::gResourcePath, "mandrill_128.pkm");
+        SkString filename = SkOSPath::SkPathJoin(
+                INHERITED::gResourcePath.c_str(), "mandrill_128.pkm");
 
         SkAutoDataUnref fileData(SkData::NewFromFileName(filename.c_str()));
         if (NULL == fileData) {

diff --git a/gm/factory.cpp b/gm/factory.cpp
index aa643a2c5b00dc4d740cc97410cec0f295d6dc2c..bd79c9d81e5188ab7d9ac0b02bfe686d37f4507d 100644 (file)
--- a/gm/factory.cpp
+++ b/gm/factory.cpp
@@ -28,7 +28,8 @@ public:
 protected:
     virtual void onOnceBeforeDraw() SK_OVERRIDE {
         // Copyright-free file from http://openclipart.org/detail/29213/paper-plane-by-ddoo
-        SkString filename = SkOSPath::SkPathJoin(INHERITED::gResourcePath, "plane.png");
+        SkString filename = SkOSPath::SkPathJoin(INHERITED::gResourcePath.c_str(),
+                                                 "plane.png");
         SkAutoDataUnref data(SkData::NewFromFileName(filename.c_str()));
         if (NULL != data.get()) {
             // Create a cache which will boot the pixels out anytime the

diff --git a/gm/gm.cpp b/gm/gm.cpp
index 8da45c5fcb165903ca4d1a10dba756eba5ec6ca2..803874f5bc9d193c9c46a247f68397a5ba721e17 100644 (file)
--- a/gm/gm.cpp
+++ b/gm/gm.cpp
@@ -6,10 +6,9 @@
  */
 
 #include "gm.h"
-
 using namespace skiagm;
 
-const char* GM::gResourcePath;
+SkString GM::gResourcePath;
 
 GM::GM() {
     fMode = kGM_Mode;
@@ -18,7 +17,6 @@ GM::GM() {
     fHaveCalledOnceBeforeDraw = false;
     fStarterMatrix.reset();
 }
-
 GM::~GM() {}
 
 void GM::draw(SkCanvas* canvas) {
@@ -66,13 +64,5 @@ void GM::drawSizeBounds(SkCanvas* canvas, SkColor color) {
     canvas->drawRect(r, paint);
 }
 
-void GM::SetResourcePath(const char* resourcePath) {
-    gResourcePath = resourcePath;
-}
-
-SkString GM::GetResourcePath() {
-    return SkString(gResourcePath);
-}
-
 // need to explicitly declare this, or we get some weird infinite loop llist
 template GMRegistry* SkTRegistry<GM*(*)(void*)>::gHead;

diff --git a/gm/gm.h b/gm/gm.h
index a48976b20efdbe86f3b3d347b5387ff460718d87..90de96f7e80826e13f94e20dce6174f3f88ab48e 100644 (file)
--- a/gm/gm.h
+++ b/gm/gm.h
@@ -96,8 +96,13 @@ namespace skiagm {
         // GM's getISize bounds.
         void drawSizeBounds(SkCanvas*, SkColor);
 
-        static void SetResourcePath(const char*);
-        static SkString GetResourcePath();
+        static void SetResourcePath(const char* resourcePath) {
+            gResourcePath = resourcePath;
+        }
+
+        static SkString& GetResourcePath() {
+            return gResourcePath;
+        }
 
         bool isCanvasDeferred() const { return fCanvasIsDeferred; }
         void setCanvasIsDeferred(bool isDeferred) {
@@ -110,7 +115,7 @@ namespace skiagm {
         }
 
     protected:
-        static const char* gResourcePath;
+        static SkString gResourcePath;
 
         virtual void onOnceBeforeDraw() {}
         virtual void onDraw(SkCanvas*) = 0;

diff --git a/tests/Test.cpp b/tests/Test.cpp
index b904d5a76d739e184c8937b0533c6fd17f262df1..e57427a354952a7eca494826f1c160863c7b2029 100644 (file)
--- a/tests/Test.cpp
+++ b/tests/Test.cpp
@@ -1,10 +1,10 @@
+
 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
-
 #include "Test.h"
 
 #include "SkCommandLineFlags.h"
@@ -41,8 +41,6 @@ void Reporter::endTest(Test* test) {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-const char* Test::gResourcePath;
-
 Test::Test() : fReporter(NULL), fPassed(true) {}
 
 Test::~Test() {
@@ -123,9 +121,8 @@ SkString Test::GetTmpDir() {
     return SkString(tmpDir);
 }
 
-void Test::SetResourcePath(const char* resourcePath) {
-    gResourcePath = resourcePath;
-}
+static const char* gResourcePath = NULL;
+void Test::SetResourcePath(const char* resourcePath) { gResourcePath = resourcePath; }
 
 SkString Test::GetResourcePath() {
     return SkString(gResourcePath);

diff --git a/tests/Test.h b/tests/Test.h
index 90d072ef6644544feafb3e23e87a2a163e3887c1..4d2cf2b6392349ed0751eb52274d3d219d27b7d9 100644 (file)
--- a/tests/Test.h
+++ b/tests/Test.h
@@ -73,8 +73,6 @@ namespace skiatest {
         virtual void onRun(Reporter*) = 0;
 
     private:
-        static const char* gResourcePath;
-
         Reporter*   fReporter;
         SkString    fName;
         bool        fPassed;