Fail gracefully if profile Temp dir can not be accessed.

BUG=60634, 67627
TEST=Manually interfere with Temp directory creation, see that expected failures happen.

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

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

base/file_util_win.cc

-// Copyright (c) 2010 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.
 
@@ -667,9 +667,10 @@ bool CreateDirectory(const FilePath& full_path) {
   if (!::CreateDirectory(full_path_str, NULL)) {
     DWORD error_code = ::GetLastError();
     if (error_code == ERROR_ALREADY_EXISTS && DirectoryExists(full_path)) {
-      // This error code doesn't indicate whether we were racing with someone
-      // creating the same directory, or a file with the same path, therefore
-      // we check.
+      // This error code ERROR_ALREADY_EXISTS doesn't indicate whether we
+      // were racing with someone creating the same directory, or a file
+      // with the same path.  If DirectoryExists() returns true, we lost the
+      // race to create the same directory.
       return true;
     } else {
       LOG(WARNING) << "Failed to create directory " << full_path_str

chrome/browser/extensions/convert_user_script.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.
 
@@ -18,6 +18,7 @@
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
+#include "chrome/common/extensions/extension_file_util.h"
 #include "chrome/common/extensions/user_script.h"
 #include "chrome/common/json_value_serializer.h"
 #include "googleurl/src/gurl.h"
@@ -45,8 +46,11 @@ scoped_refptr<Extension> ConvertUserScriptToExtension(
     return NULL;
   }
 
-  FilePath user_data_temp_dir;
-  CHECK(PathService::Get(chrome::DIR_USER_DATA_TEMP, &user_data_temp_dir));
+  FilePath user_data_temp_dir = extension_file_util::GetUserDataTempDir();
+  if (user_data_temp_dir.empty()) {
+    *error = "Could not get path to profile temporary directory.";
+    return NULL;
+  }
 
   ScopedTempDir temp_dir;
   if (!temp_dir.CreateUniqueTempDirUnderPath(user_data_temp_dir)) {

chrome/browser/extensions/convert_web_app.cc

-// Copyright (c) 2010 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.
 
@@ -22,6 +22,7 @@
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
+#include "chrome/common/extensions/extension_file_util.h"
 #include "chrome/common/json_value_serializer.h"
 #include "chrome/common/web_apps.h"
 #include "gfx/codec/png_codec.h"
@@ -84,8 +85,11 @@ std::string ConvertTimeToExtensionVersion(const Time& create_time) {
 scoped_refptr<Extension> ConvertWebAppToExtension(
     const WebApplicationInfo& web_app,
     const Time& create_time) {
-  FilePath user_data_temp_dir;
-  CHECK(PathService::Get(chrome::DIR_USER_DATA_TEMP, &user_data_temp_dir));
+  FilePath user_data_temp_dir = extension_file_util::GetUserDataTempDir();
+  if (user_data_temp_dir.empty()) {
+    LOG(ERROR) << "Could not get path to profile temporary directory.";
+    return NULL;
+  }
 
   ScopedTempDir temp_dir;
   if (!temp_dir.CreateUniqueTempDirUnderPath(user_data_temp_dir)) {

chrome/browser/extensions/crx_installer.cc

-// Copyright (c) 2010 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.
 
@@ -117,18 +117,9 @@ CrxInstaller::~CrxInstaller() {
 void CrxInstaller::InstallCrx(const FilePath& source_file) {
   source_file_ = source_file;
 
-  FilePath user_data_temp_dir;
-  {
-    // We shouldn't be doing disk IO on the UI thread.
-    //   http://code.google.com/p/chromium/issues/detail?id=60634
-    base::ThreadRestrictions::ScopedAllowIO allow_io;
-    CHECK(PathService::Get(chrome::DIR_USER_DATA_TEMP, &user_data_temp_dir));
-  }
-
   scoped_refptr<SandboxedExtensionUnpacker> unpacker(
       new SandboxedExtensionUnpacker(
           source_file,
-          user_data_temp_dir,
           g_browser_process->resource_dispatcher_host(),
           this));
 

chrome/browser/extensions/sandboxed_extension_unpacker.cc

-// Copyright (c) 2010 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.
 
@@ -12,12 +12,15 @@
 #include "base/file_util.h"
 #include "base/file_util_proxy.h"
 #include "base/message_loop.h"
+#include "base/metrics/histogram.h"
+#include "base/path_service.h"
 #include "base/scoped_handle.h"
 #include "base/task.h"
 #include "base/utf_string_conversions.h"  // TODO(viettrungluu): delete me.
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/renderer_host/resource_dispatcher_host.h"
+#include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
@@ -33,28 +36,44 @@ const char SandboxedExtensionUnpacker::kExtensionHeaderMagic[] = "Cr24";
 
 SandboxedExtensionUnpacker::SandboxedExtensionUnpacker(
     const FilePath& crx_path,
-    const FilePath& temp_path,
     ResourceDispatcherHost* rdh,
     SandboxedExtensionUnpackerClient* client)
-    : crx_path_(crx_path), temp_path_(temp_path),
+    : crx_path_(crx_path),
       thread_identifier_(BrowserThread::ID_COUNT),
       rdh_(rdh), client_(client), got_response_(false) {
 }
 
-void SandboxedExtensionUnpacker::Start() {
-  // We assume that we are started on the thread that the client wants us to do
-  // file IO on.
+bool SandboxedExtensionUnpacker::CreateTempDirectory() {
   CHECK(BrowserThread::GetCurrentThreadIdentifier(&thread_identifier_));
 
-  // Create a temporary directory to work in.
-  if (!temp_dir_.CreateUniqueTempDirUnderPath(temp_path_)) {
-    // Could not create temporary directory.
+  FilePath user_data_temp_dir = extension_file_util::GetUserDataTempDir();
+  if (user_data_temp_dir.empty()) {
+    // TODO(skerner): This should have its own string.
+    // Using an existing string so that the change can be merged.
     ReportFailure(l10n_util::GetStringFUTF8(
         IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
         ASCIIToUTF16("COULD_NOT_CREATE_TEMP_DIRECTORY")));
-    return;
+    return false;
+  }
+
+  if (!temp_dir_.CreateUniqueTempDirUnderPath(user_data_temp_dir)) {
+    ReportFailure(l10n_util::GetStringFUTF8(
+        IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
+        ASCIIToUTF16("COULD_NOT_CREATE_TEMP_DIRECTORY")));
+    return false;
   }
 
+  return true;
+}
+
+void SandboxedExtensionUnpacker::Start() {
+  // We assume that we are started on the thread that the client wants us to do
+  // file IO on.
+  CHECK(BrowserThread::GetCurrentThreadIdentifier(&thread_identifier_));
+
+  if (!CreateTempDirectory())
+    return;  // ReportFailure() already called.
+
   // Initialize the path that will eventually contain the unpacked extension.
   extension_root_ = temp_dir_.path().AppendASCII(
       extension_filenames::kTempExtensionName);
@@ -311,10 +330,13 @@ bool SandboxedExtensionUnpacker::ValidateSignature() {
 }
 
 void SandboxedExtensionUnpacker::ReportFailure(const std::string& error) {
+  UMA_HISTOGRAM_COUNTS("Extensions.SandboxUnpackFailure", 1);
   client_->OnUnpackFailure(error);
 }
 
 void SandboxedExtensionUnpacker::ReportSuccess() {
+  UMA_HISTOGRAM_COUNTS("Extensions.SandboxUnpackSuccess", 1);
+
   // Client takes ownership of temporary directory and extension.
   client_->OnUnpackSuccess(temp_dir_.Take(), extension_root_, extension_);
   extension_ = NULL;

chrome/browser/extensions/sandboxed_extension_unpacker.h

-// Copyright (c) 2010 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.
 
@@ -93,7 +93,6 @@ class SandboxedExtensionUnpacker : public UtilityProcessHost::Client {
   // |client| with the result. If |rdh| is provided, unpacking is done in a
   // sandboxed subprocess. Otherwise, it is done in-process.
   SandboxedExtensionUnpacker(const FilePath& crx_path,
-                             const FilePath& temp_path,
                              ResourceDispatcherHost* rdh,
                              SandboxedExtensionUnpackerClient* cilent);
 
@@ -107,6 +106,10 @@ class SandboxedExtensionUnpacker : public UtilityProcessHost::Client {
 
   virtual ~SandboxedExtensionUnpacker();
 
+  // Set |temp_dir_| as a temporary directory to unpack the extension in.
+  // Return true on success.
+  virtual bool CreateTempDirectory();
+
   // Validates the signature of the extension and extract the key to
   // |public_key_|. Returns true if the signature validates, false otherwise.
   //
@@ -141,9 +144,6 @@ class SandboxedExtensionUnpacker : public UtilityProcessHost::Client {
   // The path to the CRX to unpack.
   FilePath crx_path_;
 
-  // A path to a temp dir to unpack in.
-  FilePath temp_path_;
-
   // Our client's thread. This is the thread we respond on.
   BrowserThread::ID thread_identifier_;
 

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.
 
@@ -22,12 +22,16 @@ namespace keys = extension_manifest_keys;
 using testing::_;
 using testing::Invoke;
 
+namespace {
+
 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.
 }
 
+}  // namespace
+
 class MockSandboxedExtensionUnpackerClient
     : public SandboxedExtensionUnpackerClient {
  public:
@@ -78,7 +82,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,14 +93,14 @@ 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_));
 
     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;
@@ -159,6 +163,7 @@ class SandboxedExtensionUnpackerTest : public testing::Test {
 
 TEST_F(SandboxedExtensionUnpackerTest, NoCatalogsSuccess) {
   EXPECT_CALL(*client_, OnUnpackSuccess(_, _, _));
+  EXPECT_CALL(*client_, OnUnpackFailure(_)).Times(0);
 
   SetupUnpacker("no_l10n.crx");
   ASSERT_TRUE(unpacker_->Run());
@@ -180,6 +185,7 @@ TEST_F(SandboxedExtensionUnpackerTest, NoCatalogsSuccess) {
 
 TEST_F(SandboxedExtensionUnpackerTest, WithCatalogsSuccess) {
   EXPECT_CALL(*client_, OnUnpackSuccess(_, _, _));
+  EXPECT_CALL(*client_, OnUnpackFailure(_)).Times(0);
 
   SetupUnpacker("good_l10n.crx");
   ASSERT_TRUE(unpacker_->Run());

chrome/common/chrome_paths.cc

-// Copyright (c) 2006-2010 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.
 
@@ -182,7 +182,6 @@ bool PathProvider(int key, FilePath* result) {
       if (!PathService::Get(chrome::DIR_USER_DATA, &cur))
         return false;
       cur = cur.Append(FILE_PATH_LITERAL("Temp"));
-      create_dir = true;
       break;
     case chrome::DIR_INTERNAL_PLUGINS:
       if (!GetInternalPluginsDirectory(&cur))

chrome/common/chrome_paths.h

-// Copyright (c) 2006-2010 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.
 
@@ -35,6 +35,9 @@ enum {
                                 // this when a temporary file or directory will
                                 // be moved into the profile, to avoid issues
                                 // moving across volumes.  See crbug.com/13044 .
+                                // Getting this path does not create it.  Users
+                                // should check that the path exists before
+                                // using it.
   DIR_INTERNAL_PLUGINS,         // Directory where internal plugins reside.
 #if !defined(OS_MACOSX) && defined(OS_POSIX)
   DIR_POLICY_FILES,             // Directory for system-wide read-only

chrome/common/extensions/extension_file_util.cc

-// Copyright (c) 2010 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.
 
@@ -10,8 +10,12 @@
 #include "app/l10n_util.h"
 #include "base/file_util.h"
 #include "base/logging.h"
+#include "base/metrics/histogram.h"
+#include "base/path_service.h"
 #include "base/scoped_temp_dir.h"
+#include "base/threading/thread_restrictions.h"
 #include "base/utf_string_conversions.h"
+#include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_action.h"
 #include "chrome/common/extensions/extension_l10n_util.h"
@@ -517,4 +521,70 @@ FilePath ExtensionURLToRelativeFilePath(const GURL& url) {
   return path;
 }
 
+FilePath GetUserDataTempDir() {
+  // We do file IO in this function, but only when the current profile's
+  // Temp directory has never been used before, or in a rare error case.
+  // Developers are not likely to see these situations often, so do an
+  // explicit thread check.
+  base::ThreadRestrictions::AssertIOAllowed();
+
+  // Getting chrome::DIR_USER_DATA_TEMP is failing.  Use histogram to see why.
+  // TODO(skerner): Fix the problem, and remove this code.  crbug.com/70056
+  enum DirectoryCreationResult {
+    SUCCESS = 0,
+
+    CANT_GET_PARENT_PATH,
+    CANT_GET_UDT_PATH,
+    NOT_A_DIRECTORY,
+    CANT_CREATE_DIR,
+    CANT_WRITE_TO_PATH,
+
+    UNSET,
+    NUM_DIRECTORY_CREATION_RESULTS
+  };
+
+  // All paths should set |result|.
+  DirectoryCreationResult result = UNSET;
+
+  FilePath temp_path;
+  if (!PathService::Get(chrome::DIR_USER_DATA_TEMP, &temp_path)) {
+    FilePath parent_path;
+    if (!PathService::Get(chrome::DIR_USER_DATA, &parent_path))
+      result = CANT_GET_PARENT_PATH;
+    else
+      result = CANT_GET_UDT_PATH;
+
+  } else if (file_util::PathExists(temp_path)) {
+
+    // Path exists.  Check that it is a directory we can write to.
+    if (!file_util::DirectoryExists(temp_path)) {
+      result = NOT_A_DIRECTORY;
+
+    } else if (!file_util::PathIsWritable(temp_path)) {
+      result = CANT_WRITE_TO_PATH;
+
+    } else {
+      // Temp is a writable directory.
+      result = SUCCESS;
+    }
+
+  } else if (!file_util::CreateDirectory(temp_path)) {
+    // Path doesn't exist, and we failed to create it.
+    result = CANT_CREATE_DIR;
+
+  } else {
+    // Successfully created the Temp directory.
+    result = SUCCESS;
+  }
+
+  UMA_HISTOGRAM_ENUMERATION("Extensions.GetUserDataTempDir",
+                            result,
+                            NUM_DIRECTORY_CREATION_RESULTS);
+
+  if (result == SUCCESS)
+    return temp_path;
+
+  return FilePath();
+}
+
 }  // namespace extension_file_util

chrome/common/extensions/extension_file_util.h

-// Copyright (c) 2010 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.
 
@@ -76,6 +76,12 @@ bool CheckForIllegalFilenames(const FilePath& extension_path,
 // Get a relative file path from a chrome-extension:// URL.
 FilePath ExtensionURLToRelativeFilePath(const GURL& url);
 
+// Get a path to a temp directory for unpacking an extension.
+// This is essentially PathService::Get(chrome::DIR_USER_DATA_TEMP, ...),
+// with a histogram that allows us to understand why it is failing.
+// Return an empty file path on failure.
+FilePath GetUserDataTempDir();
+
 }  // namespace extension_file_util
 
 #endif  // CHROME_COMMON_EXTENSIONS_EXTENSION_FILE_UTIL_H_