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

6 files changed, 63 insertions, 36 deletions

diff --git a/chrome/browser/extensions/convert_user_script.cc b/chrome/browser/extensions/convert_user_script.cc
index 7322c70..0b45da8 100644
--- a/chrome/browser/extensions/convert_user_script.cc
+++ b/chrome/browser/extensions/convert_user_script.cc
@@ -1,4 +1,4 @@
-// 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)) {
diff --git a/chrome/browser/extensions/convert_web_app.cc b/chrome/browser/extensions/convert_web_app.cc
index e1c8c92..d410f2a 100644
--- a/chrome/browser/extensions/convert_web_app.cc
+++ b/chrome/browser/extensions/convert_web_app.cc
@@ -1,4 +1,4 @@
-// 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)) {
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index 1593aea..f62c983 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -1,4 +1,4 @@
-// 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));
 
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.cc b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
index 6ae14a2..c43b83d 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker.cc
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
@@ -1,4 +1,4 @@
-// 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;
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.h b/chrome/browser/extensions/sandboxed_extension_unpacker.h
index e47b26c..455e3f6 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker.h
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker.h
@@ -1,4 +1,4 @@
-// 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_;
 
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc b/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc
index 6246a6f..ecc521c 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc
@@ -1,4 +1,4 @@
-// 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());