Fail gracefully if profile Temp dir can not be accessed.

chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc

Index: chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc b/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc
index 6246a6fd45ab57b25af4e9875c4baa971165707c..4139de2c330930fa9b65f541370c2e4b68fc7afd 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc
@@ -1,12 +1,15 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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 <fstream>
+
 #include "base/file_util.h"
 #include "base/path_service.h"
 #include "base/ref_counted.h"
 #include "base/scoped_temp_dir.h"
 #include "base/string_util.h"
+#include "base/utf_string_conversions.h"
 #include "chrome/browser/extensions/sandboxed_extension_unpacker.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension.h"
@@ -22,12 +25,32 @@ namespace keys = extension_manifest_keys;
 using testing::_;
 using testing::Invoke;
 
+namespace {
+
+// Create a non-empty file.
+void CreateTextFile(const FilePath& filename) {
+  std::ofstream file;
+  file.open(WideToUTF8(filename.ToWStringHack()).c_str());

[Erik does not do reviews, 2011/01/14 22:34:44]

This looks problematic - potential filename incompatibility on different OSs. 
Instead, just use file_util::WriteFile.

[Sam Kerner (Chrome), 2011/01/18 19:26:45]

Done.

+  ASSERT_TRUE(file.is_open());
+  file << "I am a text file.";
+  file.close();
+}
+
+// Clear a scoped_temp_dir if it has any contents.
+void ClearScopedTempDir(ScopedTempDir& scoped_temp_dir) {
+  if (scoped_temp_dir.IsValid()) {

[Erik does not do reviews, 2011/01/14 22:34:44]

nit: remove {}

[Sam Kerner (Chrome), 2011/01/18 19:26:45]

Done.

+    EXPECT_TRUE(scoped_temp_dir.Delete());
+  }
+}
+
 void OnUnpackSuccess(const FilePath& temp_dir,
                      const FilePath& extension_root,
                      const Extension* extension) {
   // Don't delete temp_dir here, we need to do some post op checking.
 }
 
+}
+
 class MockSandboxedExtensionUnpackerClient
     : public SandboxedExtensionUnpackerClient {
  public:
@@ -78,7 +101,7 @@ class SandboxedExtensionUnpackerTest : public testing::Test {
     // a temp folder to play in.
     ASSERT_TRUE(PathService::Get(base::DIR_TEMP, &install_dir_));
     install_dir_ =
-      install_dir_.AppendASCII("sandboxed_extension_unpacker_test");
+       install_dir_.AppendASCII("sandboxed_extension_unpacker_test");
     file_util::Delete(install_dir_, true);
     file_util::CreateDirectory(install_dir_);
 
@@ -89,18 +112,19 @@ class SandboxedExtensionUnpackerTest : public testing::Test {
 
     unpacker_.reset(new ExtensionUnpacker(crx_path));
 
-
     // Build a temp area where the extension will be unpacked.
     ASSERT_TRUE(PathService::Get(base::DIR_TEMP, &temp_dir_));
     temp_dir_ = temp_dir_.AppendASCII("sandboxed_extension_unpacker_test_Temp");
-    file_util::CreateDirectory(temp_dir_);
+    ASSERT_TRUE(file_util::CreateDirectory(temp_dir_));
+
+    //PathService::Override(chrome::DIR_USER_DATA_TEMP, temp_dir_);

[Erik does not do reviews, 2011/01/14 22:34:44]

remove unused code

[Sam Kerner (Chrome), 2011/01/18 19:26:45]

Done.

 
     sandboxed_unpacker_ =
-      new SandboxedExtensionUnpacker(crx_path, temp_dir_, NULL, client_);
+        new SandboxedExtensionUnpacker(crx_path, NULL, client_);
+
     // Hack since SandboxedExtensionUnpacker gets its background thread id from
     // the Start call, but we don't call it here.
     sandboxed_unpacker_->thread_identifier_ = BrowserThread::FILE;
-    EXPECT_TRUE(PrepareUnpackerEnv());
   }
 
   bool PrepareUnpackerEnv() {
@@ -159,17 +183,17 @@ class SandboxedExtensionUnpackerTest : public testing::Test {
 
 TEST_F(SandboxedExtensionUnpackerTest, NoCatalogsSuccess) {
   EXPECT_CALL(*client_, OnUnpackSuccess(_, _, _));
+  EXPECT_CALL(*client_, OnUnpackFailure(_)).Times(0);
 
   SetupUnpacker("no_l10n.crx");
+  PrepareUnpackerEnv();

[Erik does not do reviews, 2011/01/14 22:34:44]

shouldn't this have EXPECT or ASSERT on it? (and other callsites)

[Sam Kerner (Chrome), 2011/01/18 19:26:45]

Yes.  Done.

   ASSERT_TRUE(unpacker_->Run());
   ASSERT_TRUE(unpacker_->DumpImagesToFile());
   ASSERT_TRUE(unpacker_->DumpMessageCatalogsToFile());
-
   // Check that there is no _locales folder.
   FilePath install_path =
     GetInstallPath().Append(Extension::kLocaleFolder);
   EXPECT_FALSE(file_util::PathExists(install_path));
-
   OnUnpackSucceeded();
 
   // Check that there still is no _locales folder.
@@ -180,8 +204,10 @@ TEST_F(SandboxedExtensionUnpackerTest, NoCatalogsSuccess) {
 
 TEST_F(SandboxedExtensionUnpackerTest, WithCatalogsSuccess) {
   EXPECT_CALL(*client_, OnUnpackSuccess(_, _, _));
+  EXPECT_CALL(*client_, OnUnpackFailure(_)).Times(0);
 
   SetupUnpacker("good_l10n.crx");
+  PrepareUnpackerEnv();
   ASSERT_TRUE(unpacker_->Run());
   ASSERT_TRUE(unpacker_->DumpImagesToFile());
   ASSERT_TRUE(unpacker_->DumpMessageCatalogsToFile());
@@ -209,3 +235,88 @@ TEST_F(SandboxedExtensionUnpackerTest, WithCatalogsSuccess) {
 
   ASSERT_TRUE(TempFilesRemoved());
 }
+
+TEST_F(SandboxedExtensionUnpackerTest, CreateTempDirectory) {

[Erik does not do reviews, 2011/01/14 22:34:44]

If we ditch the multiple dir support for now, is this test still necessary?

[Sam Kerner (Chrome), 2011/01/18 19:26:45]

No, but it could be used to test error handling of the code that sets up the
temp directory.

+  // To be sure overriding path keys does not change the paths seen
+  // by existing code, use some path keys that do not exist outside
+  // this test.
+  enum {
+    PATH_START = chrome::PATH_END+1,
+    TEST_DIR_WORKS,
+    TEST_DIR_CANT_GET,
+    TEST_DIR_CANT_WRITE,
+    PATH_END
+  };
+
+  // Create a fake temp path.
+  ScopedTempDir fake_user_temp_dir;
+  ASSERT_TRUE(fake_user_temp_dir.CreateUniqueTempDir());
+
+  // |dir_works| will work for unpacking.
+  FilePath dir_works(fake_user_temp_dir.path());
+
+  // Needed because path service code runs AbsolutePath() on all values
+  // it returns.  This changes the path on the mac trybots, and we need
+  // to compare this path to the result from PathService::Get() below.
+  ASSERT_TRUE(file_util::AbsolutePath(&dir_works));
+
+  // To make directory creation fail, use a base path which is a file.
+  FilePath dir_cant_write(fake_user_temp_dir.path().AppendASCII("cant_write"));
+  CreateTextFile(dir_cant_write);
+
+  // Set up test paths:
+  PathService::Override(TEST_DIR_WORKS, dir_works);
+  PathService::Override(TEST_DIR_CANT_GET, FilePath());
+  PathService::Override(TEST_DIR_CANT_WRITE, dir_cant_write);
+
+  // Test that our path keys give paths with the desired properties:
+  FilePath scratch;
+  ASSERT_TRUE(PathService::Get(TEST_DIR_WORKS, &scratch));
+  ASSERT_EQ(dir_works.value(), scratch.value());
+
+  ASSERT_FALSE(PathService::Get(TEST_DIR_CANT_GET, &scratch));
+
+  ASSERT_TRUE(PathService::Get(TEST_DIR_CANT_WRITE, &scratch));
+  ASSERT_EQ(dir_cant_write.value(), scratch.value());
+  ASSERT_FALSE(file_util::CreateDirectory(
+      dir_cant_write.AppendASCII("cant_make_dir_in_a_file")));
+
+  SetupUnpacker("good_l10n.crx");
+
+  // Test that a valid, writable path is read and used.
+  const int kWorkingDir[] = {
+    TEST_DIR_WORKS
+  };
+  // Clear |temp_dir_|, so that we can test that it was set.
+  ClearScopedTempDir(sandboxed_unpacker_->temp_dir_);
+  EXPECT_CALL(*client_, OnUnpackFailure(_)).Times(0);
+  ASSERT_TRUE(sandboxed_unpacker_->CreateTempDirectory(
+      kWorkingDir, arraysize(kWorkingDir)));
+  ASSERT_EQ(dir_works.value(),
+            sandboxed_unpacker_->temp_dir_.path().DirName().value());
+
+  // Test that two unusable paths both fail.
+  const int kFailingDir[] = {
+    TEST_DIR_CANT_GET,
+    TEST_DIR_CANT_WRITE
+  };
+  ClearScopedTempDir(sandboxed_unpacker_->temp_dir_);
+  EXPECT_CALL(*client_, OnUnpackFailure(_)).Times(1);
+  ASSERT_FALSE(sandboxed_unpacker_->CreateTempDirectory(
+      kFailingDir, arraysize(kFailingDir)));
+
+  // If at first you don't succeed, try try again.
+  const int kFallback[] = {
+    TEST_DIR_CANT_WRITE,
+    TEST_DIR_CANT_GET,
+    TEST_DIR_WORKS
+  };
+  ClearScopedTempDir(sandboxed_unpacker_->temp_dir_);
+  EXPECT_CALL(*client_, OnUnpackFailure(_)).Times(0);
+  ASSERT_TRUE(
+      sandboxed_unpacker_->CreateTempDirectory(
+          kFallback, arraysize(kFallback)));
+  ASSERT_EQ(
+      dir_works.value(),
+      sandboxed_unpacker_->temp_dir_.path().DirName().value());
+}