chrome_paths: refactor and sanitize cache directory handling

chrome/common/chrome_paths_unittest.cc

Index: chrome/common/chrome_paths_unittest.cc
diff --git a/chrome/common/chrome_paths_unittest.cc b/chrome/common/chrome_paths_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a7cb242947c29521aa439df0337bd4b4f12b8382
--- /dev/null
+++ b/chrome/common/chrome_paths_unittest.cc
@@ -0,0 +1,44 @@
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/common/chrome_paths_internal.h"
+
+#include <stdlib.h>
+
+#include "base/file_path.h"
+#include "base/file_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+// Test the behavior of chrome::GetUserCacheDirectory.
+// See that function's comments for discussion of the subtleties.
+TEST(ChromePaths, UserCacheDir) {
+#if defined(OS_MACOSX)
+  // Verify that ~/Library/Application Support/foobar maps to .cache/foobar.
+  FilePath homedir = FilePath(getenv("HOME"));

[Mark Mentovai, 2010/11/18 16:54:54]

Why not file_util::GetHomeDir() as in non-Mac POSIX?

[Evan Martin, 2010/11/18 18:13:07]

We only have a Linux impl.  (If $HOME is unset you have to go to more obscure
logic.)  It's what I tried first.  :\

I'm not too happy about this, I was hoping you had a better suggestion for it. 
I'm not sure how magical ~/Library/Caches is on mac, like whether that ~ is
always getenv(HOME).

[Mark Mentovai, 2010/11/18 19:11:48]

Evan Martin wrote:
I looked at base/file_util.h and saw that GetHomeDir wasn’t decorated inside a
per-OS #if, so I figured it was safe. Let’s see when that happened…

Uh-oh!

r45380 | evan@chromium.org | 2010-04-22 19:28:05 -0400 (Thu, 22 Apr 2010)

I can give you a quick Mac implementation if you’d like. The right answer is to
call NSHomeDirectory() (and, for us, take the -fileSystemRepresentation and
convert it to a FilePath).
Although I’ve never seen a case where Caches wasn’t in ~/Library, we should be
defensive.

+  FilePath user_data_dir = homedir.Append("Library/Application Support/foobar");
+  FilePath cache_dir;
+  ASSERT_TRUE(chrome::GetUserCacheDirectory(user_data_dir, &cache_dir));
+  EXPECT_EQ(homedir.Append("Library/Caches/foobar").value(), cache_dir.value());
+
+  // Verify that a custom user data dir doesn't get a custom cache dir.

[Mark Mentovai, 2010/11/18 16:54:54]

This block is the same for Mac and non-Mac POSIX, maybe put it outside of the
#if.

+  user_data_dir = homedir.Append("some_other_dir");
+  ASSERT_TRUE(chrome::GetUserCacheDirectory(user_data_dir, &cache_dir));
+  EXPECT_EQ(user_data_dir.value(), cache_dir.value());
+
+#elif(OS_POSIX)
+  // We assume XDG_CACHE_HOME/XDG_CONFIG_HOME are at their default settings.
+  FilePath homedir = file_util::GetHomeDir();
+
+  // Verify that .config/foobar maps to .cache/foobar.
+  FilePath user_data_dir = homedir.Append(".config/foobar");
+  FilePath cache_dir;
+  ASSERT_TRUE(chrome::GetUserCacheDirectory(user_data_dir, &cache_dir));

[Mark Mentovai, 2010/11/18 16:54:54]

For that matter, maybe you can put this line outside of the #if, and the next
one too if you refactor a tiny bit to have an expected_cache_dir string inside
the #if. Then the #ifs would just contain a couple of basic variable assignments
and the meat of the test would be shared.

[Evan Martin, 2010/11/18 18:13:07]

Great idea, I've now rewritten the test to be much shorter.

+  EXPECT_EQ(homedir.Append(".cache/foobar").value(), cache_dir.value());
+
+  // Verify that a custom user data dir doesn't get a custom cache dir.
+  user_data_dir = homedir.Append("some_other_dir");
+  ASSERT_TRUE(chrome::GetUserCacheDirectory(user_data_dir, &cache_dir));
+  EXPECT_EQ(user_data_dir.value(), cache_dir.value());
+#endif
+}