Fail gracefully if profile Temp dir can not be accessed.

chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc

-// 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"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/extension_unpacker.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 
 namespace errors = extension_manifest_errors;
 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:
   virtual ~MockSandboxedExtensionUnpackerClient() {}
 
   MOCK_METHOD3(OnUnpackSuccess,
                void(const FilePath& temp_dir,
                     const FilePath& extension_root,
                     const Extension* extension));
 
@@ skipping 30 matching lines @@
     ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &original_path));
     original_path = original_path.AppendASCII("extensions")
         .AppendASCII("unpacker")
         .AppendASCII(crx_name);
     ASSERT_TRUE(file_util::PathExists(original_path)) << original_path.value();
 
     // Try bots won't let us write into DIR_TEST_DATA, so we have to create
     // 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_);
 
     FilePath crx_path = install_dir_.AppendASCII(crx_name);
     ASSERT_TRUE(file_util::CopyFile(original_path, crx_path)) <<
         "Original path: " << original_path.value() <<
         ", Crx path: " << crx_path.value();
 
     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() {
     sandboxed_unpacker_->extension_root_ =
       install_dir_.AppendASCII(extension_filenames::kTempExtensionName);
 
     if (!sandboxed_unpacker_->temp_dir_.Set(install_dir_))
       return false;
     sandboxed_unpacker_->public_key_ =
       "ocnapchkplbmjmpfehjocmjnipfmogkh";
@@ skipping 38 matching lines @@
   FilePath temp_dir_;
   MockSandboxedExtensionUnpackerClient* client_;
   scoped_ptr<ExtensionUnpacker> unpacker_;
   scoped_refptr<SandboxedExtensionUnpacker> sandboxed_unpacker_;
   MessageLoop loop_;
   scoped_ptr<BrowserThread> file_thread_;
 };
 
 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.
   EXPECT_FALSE(file_util::PathExists(install_path));
 
   ASSERT_TRUE(TempFilesRemoved());
 }
 
 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());
 
   // Set timestamp on _locales/en_US/messages.json into the past.
   FilePath messages_file;
   messages_file = GetInstallPath().Append(Extension::kLocaleFolder)
       .AppendASCII("en_US")
       .Append(Extension::kMessagesFilename);
   base::PlatformFileInfo old_info;
   EXPECT_TRUE(file_util::GetFileInfo(messages_file, &old_info));
   base::Time old_time =
       old_info.last_modified - base::TimeDelta::FromSeconds(2);
   EXPECT_TRUE(file_util::SetLastModifiedTime(messages_file, old_time));
   // Refresh old_info, just to be sure.
   EXPECT_TRUE(file_util::GetFileInfo(messages_file, &old_info));
 
   OnUnpackSucceeded();
 
   // Check that there is newer _locales/en_US/messages.json file.
   base::PlatformFileInfo new_info;
   EXPECT_TRUE(file_util::GetFileInfo(messages_file, &new_info));
 
   EXPECT_TRUE(new_info.last_modified > old_info.last_modified);
 
   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());
+}