Get rid of FilePath::AppendAndResolveRelative().

To resolve the problem of '..' parent references as well as symbolic links on POSIX platforms, we can simply use the file_util::AbsolutePath() function.

This has the drawback of having a different behavior on Windows and POSIX platforms, in the way that it can return a canonical path that doesn't exists when ran on Windows, but it will return an empty path (or false) when run on a POSIX platform.

So we need to add an extra PathExists() call to unify the behavior.

BUG=25681,25131 

Review URL: http://codereview.chromium.org/343003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@30334 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 24 insertions, 11 deletions

diff --git a/chrome/common/extensions/extension_resource.cc b/chrome/common/extensions/extension_resource.cc
index 7b2a833..6c1cff6 100644
--- a/chrome/common/extensions/extension_resource.cc
+++ b/chrome/common/extensions/extension_resource.cc
@@ -41,21 +41,35 @@ FilePath ExtensionResource::GetFilePath(const FilePath& extension_root,
   extension_l10n_util::GetL10nRelativePaths(relative_path,
                                             &l10n_relative_paths);
 
+  // We need to resolve the parent references in the extension_root
+  // path on its own because IsParent doesn't like parent references.
+  FilePath clean_extension_root(extension_root);
+  if (!file_util::AbsolutePath(&clean_extension_root))
+    return FilePath();
+
   // Stat l10n file(s), and return new path if it exists.
   for (size_t i = 0; i < l10n_relative_paths.size(); ++i) {
-    FilePath full_path;
-    if (extension_root.AppendAndResolveRelative(l10n_relative_paths[i],
-                                                &full_path) &&
-        extension_root.IsParent(full_path) &&
+    FilePath full_path = clean_extension_root.Append(l10n_relative_paths[i]);
+    if (file_util::AbsolutePath(&full_path) &&
+        clean_extension_root.IsParent(full_path) &&
         file_util::PathExists(full_path)) {
       return full_path;
     }
   }
 
   // Fall back to root resource.
-  FilePath full_path;
-  if (extension_root.AppendAndResolveRelative(relative_path, &full_path) &&
-      extension_root.IsParent(full_path)) {
+  FilePath full_path = clean_extension_root.Append(relative_path);
+
+  // We must resolve the absolute path of the combined path when
+  // the relative path contains references to a parent folder (i.e., '..').
+  // We also check if the path exists because the posix version of AbsolutePath
+  // will fail if the path doesn't exist, and we want the same behavior on
+  // Windows... So until the posix and Windows version of AbsolutePath are
+  // unified, we need an extra call to PathExists, unfortunately.
+  // TODO(mad): Fix this once AbsolutePath is unified.
+  if (file_util::AbsolutePath(&full_path) &&
+      file_util::PathExists(full_path) &&
+      clean_extension_root.IsParent(full_path)) {
     return full_path;
   }
 
diff --git a/chrome/common/extensions/extension_resource_unittest.cc b/chrome/common/extensions/extension_resource_unittest.cc
index f467e47..807452e 100644
--- a/chrome/common/extensions/extension_resource_unittest.cc
+++ b/chrome/common/extensions/extension_resource_unittest.cc
@@ -35,12 +35,10 @@ TEST(ExtensionResourceTest, CreateWithMissingResourceOnDisk) {
   relative_path = relative_path.AppendASCII("cira.js");
   ExtensionResource resource(root_path, relative_path);
 
+  // The path doesn't exist on disk, we will be returned an empty path.
   EXPECT_EQ(root_path.value(), resource.extension_root().value());
   EXPECT_EQ(relative_path.value(), resource.relative_path().value());
-  EXPECT_EQ(ToLower(root_path.Append(relative_path).value()),
-    ToLower(resource.GetFilePath().value()));
-
-  EXPECT_FALSE(resource.GetFilePath().empty());
+  EXPECT_TRUE(resource.GetFilePath().empty());
 }
 
 TEST(ExtensionResourceTest, CreateWithAllResourcesOnDisk) {
@@ -76,6 +74,7 @@ TEST(ExtensionResourceTest, CreateWithAllResourcesOnDisk) {
   ASSERT_FALSE(locales.empty());
   FilePath expected_path;
   expected_path = l10n_path.AppendASCII(locales[0]).AppendASCII(filename);
+  ASSERT_TRUE(file_util::AbsolutePath(&expected_path));
 
   EXPECT_EQ(ToLower(expected_path.value()), ToLower(resolved_path.value()));
   EXPECT_EQ(ToLower(temp.path().value()),