Clean up Test's resourcePath code.

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

[tfarina, 2014-06-05 07:07:19]

Is this OK?

[mtklein, 2014-06-05 12:55:33]

https://codereview.chromium.org/319473003/diff/1/tests/Test.h
File tests/Test.h (right):

https://codereview.chromium.org/319473003/diff/1/tests/Test.h#newcode65
tests/Test.h:65: static void SetResourcePath(const char* resourcePath) {
I wouldn't bother with the churn?  There's not going to be any benefit to
inlining these: SetResourcePath is called once, and GetResourcePath maybe 10
times?  This is a minor feature and it's nice to get it isolated and out of the
way in its own ignorable corner of the .cc file as much as possible.  We have to
pay 2 lines in here for SetResourcePath and GetResourcePath, and I'm happy to
pay a third for the extra clarity of scoping gResourcePath under Test, but I'd
like to stop there.

https://codereview.chromium.org/319473003/diff/1/tests/Test.h#newcode68
tests/Test.h:68: static SkString& GetResourcePath() { return gResourcePath; }
SkString is fine to return by value, especially here in a part of the code that
doesn't matter performance-wise.  I don't mind returning a const SkString&, but
it's inconsistent with GetTmpDir, and also forces us to be storing an SkString.

https://codereview.chromium.org/319473003/diff/1/tests/Test.h#newcode78
tests/Test.h:78: static SkString gResourcePath;
Let's turn this back into a static const char*.  It's generally a good habit to
keep globals POD unless we're being particularly careful about their lifetimes.

[tfarina, 2014-06-06 01:53:10]

https://codereview.chromium.org/319473003/diff/1/tests/Test.h
File tests/Test.h (right):

https://codereview.chromium.org/319473003/diff/1/tests/Test.h#newcode65
tests/Test.h:65: static void SetResourcePath(const char* resourcePath) {
On 2014/06/05 12:55:33, mtklein wrote:
> I wouldn't bother with the churn?  There's not going to be any benefit to
> inlining these: SetResourcePath is called once, and GetResourcePath maybe 10
> times?  This is a minor feature and it's nice to get it isolated and out of
the
> way in its own ignorable corner of the .cc file as much as possible.  We have
to
> pay 2 lines in here for SetResourcePath and GetResourcePath, and I'm happy to
> pay a third for the extra clarity of scoping gResourcePath under Test, but I'd
> like to stop there.

Done.

https://codereview.chromium.org/319473003/diff/1/tests/Test.h#newcode68
tests/Test.h:68: static SkString& GetResourcePath() { return gResourcePath; }
On 2014/06/05 12:55:33, mtklein wrote:
> SkString is fine to return by value, especially here in a part of the code
that
> doesn't matter performance-wise.  I don't mind returning a const SkString&,
but
> it's inconsistent with GetTmpDir, and also forces us to be storing an
SkString.

Done.

https://codereview.chromium.org/319473003/diff/1/tests/Test.h#newcode78
tests/Test.h:78: static SkString gResourcePath;
On 2014/06/05 12:55:33, mtklein wrote:
> Let's turn this back into a static const char*.  It's generally a good habit
to
> keep globals POD unless we're being particularly careful about their
lifetimes.

Done.

[mtklein, 2014-06-06 02:26:20]

lgtm

[tfarina, 2014-06-08 03:28:04]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2014-06-08 03:28:53]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/tfarina@chromium.org/319473003/20001

[commit-bot: I haz the power, 2014-06-08 03:50:49]

Change committed as 52e4f413ffe2d281f9e90ff2147db08083ffcba7

[mtklein, 2014-06-08 14:02:03]

A revert of this CL has been created in
https://codereview.chromium.org/320733002/ by mtklein@google.com.

The reason for reverting is: 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()..