Fix bug in setting directories for tests.

Fix bug in setting directories for tests.

make_canonical_dir_path only worked if the provided directory
did not end with a slash. Remove this function, and call
SkPathJoin instead. Update the documentation to acknowledge
that this is an acceptable use of SkPathJoin, and update its
test.

R=epoger@google.com

Committed: https://code.google.com/p/skia/source/detail?r=9458

[epoger, 2013-06-04 15:15:34]

LGTM*

https://codereview.chromium.org/16098011/diff/1/tests/skia_test.cpp
File tests/skia_test.cpp (right):

https://codereview.chromium.org/16098011/diff/1/tests/skia_test.cpp#newcode115
tests/skia_test.cpp:115: static SkString gTmpDir;
Hey, if this change fixes an existing problem, I'm in favor of committing it. 
But it seems to me that a cleaner approach would be:

1. remove global gTmpDir variable
2. rewrite Test::GetTmpDir() to just return an SkString derived from
FLAGS_tmpDir
3. adjust any callers to Test::GetTmpDir() to use SkOSPath::SkPathJoin() to
assemble their paths, rather than path.printf("%s%s", tmpDir, "data_test");

And, I suppose, the same sort of changes for gResourcePath.

[scroggo, 2013-06-04 19:50:39]

https://codereview.chromium.org/16098011/diff/1/tests/skia_test.cpp
File tests/skia_test.cpp (right):

https://codereview.chromium.org/16098011/diff/1/tests/skia_test.cpp#newcode115
tests/skia_test.cpp:115: static SkString gTmpDir;
On 2013/06/04 15:15:34, epoger wrote:
> Hey, if this change fixes an existing problem, I'm in favor of committing it. 
> But it seems to me that a cleaner approach would be:
> 
> 1. remove global gTmpDir variable

Done.

> 2. rewrite Test::GetTmpDir() to just return an SkString derived from
> FLAGS_tmpDir

Done. I am not convinced this is better, but it seems just as good to me.

> 3. adjust any callers to Test::GetTmpDir() to use SkOSPath::SkPathJoin() to
> assemble their paths, rather than path.printf("%s%s", tmpDir, "data_test");

Done.
> 
> And, I suppose, the same sort of changes for gResourcePath.

Interestingly, no one uses gResourcePath... When it was created it wasn't used,
but presumably a later change used it. I was about to delete it, but I plan on
using it with a future change.

[epoger, 2013-06-05 14:34:14]

LGTM

https://codereview.chromium.org/16098011/diff/1/tests/skia_test.cpp
File tests/skia_test.cpp (right):

https://codereview.chromium.org/16098011/diff/1/tests/skia_test.cpp#newcode115
tests/skia_test.cpp:115: static SkString gTmpDir;
On 2013/06/04 19:50:39, scroggo wrote:
> > 2. rewrite Test::GetTmpDir() to just return an SkString derived from
> > FLAGS_tmpDir
> 
> Done. I am not convinced this is better, but it seems just as good to me.

My reasoning was: why use 2 global variables (gTmpDir and FLAGS_tmpDir) when
just 1 will do?

I guess the downside is that, because FLAGS_tmpDir is a char* instead of an
SkString, we need to create the SkString every time GetTmpDir() is called.  So
this is a performance hit.

I'm happy either way.

[scroggo, 2013-06-06 15:00:00]

Committed patchset #2 manually as r9458 (presubmit successful).