Remove "theme" in extension/app unpacker error messages, make those messages localizable, and add u…

chrome/common/extensions/unpacker.cc

Index: chrome/common/extensions/unpacker.cc
diff --git a/chrome/common/extensions/unpacker.cc b/chrome/common/extensions/unpacker.cc
index 7c1bed4b52f789920b019084d783308de4711a17..001baa5559985342a9fd0e4ff240914e9a875dc4 100644
--- a/chrome/common/extensions/unpacker.cc
+++ b/chrome/common/extensions/unpacker.cc
@@ -7,6 +7,7 @@
 #include <set>
 
 #include "base/file_util.h"
+#include "base/i18n/rtl.h"
 #include "base/json/json_file_value_serializer.h"
 #include "base/memory/scoped_handle.h"
 #include "base/scoped_temp_dir.h"
@@ -21,9 +22,11 @@
 #include "chrome/common/url_constants.h"
 #include "chrome/common/zip.h"
 #include "content/public/common/common_param_traits.h"
+#include "grit/generated_resources.h"
 #include "ipc/ipc_message_utils.h"
 #include "net/base/file_stream.h"
 #include "third_party/skia/include/core/SkBitmap.h"
+#include "ui/base/l10n/l10n_util.h"
 #include "webkit/glue/image_decoder.h"
 
 namespace errors = extension_manifest_errors;
@@ -32,14 +35,6 @@ namespace filenames = extension_filenames;
 
 namespace {
 
-// Errors
-const char* kCouldNotCreateDirectoryError =
-    "Could not create directory for unzipping: ";
-const char* kCouldNotDecodeImageError = "Could not decode theme image.";
-const char* kCouldNotUnzipExtension = "Could not unzip extension.";
-const char* kPathNamesMustBeAbsoluteOrLocalError =
-    "Path names must not be absolute or contain '..'.";
-
 // A limit to stop us passing dangerously large canvases to the browser.
 const int kMaxImageCanvas = 4096 * 4096;
 
@@ -153,23 +148,21 @@ bool Unpacker::ReadAllMessageCatalogs(const std::string& default_locale) {
 bool Unpacker::Run() {
   DVLOG(1) << "Installing extension " << extension_path_.value();
 
-  // <profile>/Extensions/INSTALL_TEMP/<version>
+  // <profile>/Extensions/CRX_INSTALL

[dharcourt, 2012/08/27 17:55:55]

Corrected comment: Temporary installation directories are now called CRX_INSTALL
instead of INSTALL_TEMP and don't contain an extension version in their path
(they never did :-).

   temp_install_dir_ =
-    extension_path_.DirName().AppendASCII(filenames::kTempExtensionName);
+      extension_path_.DirName().AppendASCII(filenames::kTempExtensionName);

[dharcourt, 2012/08/27 17:55:55]

Corrected indentation: 4 spaces for continuation lines (unless I'm mistaken?)

 
   if (!file_util::CreateDirectory(temp_install_dir_)) {
-#if defined(OS_WIN)
-    std::string dir_string = WideToUTF8(temp_install_dir_.value());
-#else
-    std::string dir_string = temp_install_dir_.value();
-#endif
-
-    SetError(kCouldNotCreateDirectoryError + dir_string);
+    SetUTF16Error(
+        l10n_util::GetStringFUTF16(
+            IDS_EXTENSION_PACKAGE_DIRECTORY_ERROR,
+            base::i18n::GetDisplayStringInLTRDirectionality(
+                temp_install_dir_.LossyDisplayName())));

[dharcourt, 2012/08/27 17:55:55]

Replacing the not-used-anywhere-else WrapPathWithLTRFormatting() with a
combination of LossyDisplayName() and GetDisplayStringInLTRDirectionality().
This also has the benefit of making the intent clearer (this is a lossy string
for display to the user, not a totally faithful representation of the path).

     return false;
   }
 
   if (!zip::Unzip(extension_path_, temp_install_dir_)) {
-    SetError(kCouldNotUnzipExtension);
+    SetUTF16Error(l10n_util::GetStringUTF16(IDS_EXTENSION_PACKAGE_UNZIP_ERROR));

[dharcourt, 2012/08/27 17:55:55]

Made error message localizable.

     return false;
   }
 
@@ -277,13 +270,21 @@ bool Unpacker::ReadMessageCatalogsFromFile(const FilePath& extension_path,
 bool Unpacker::AddDecodedImage(const FilePath& path) {
   // Make sure it's not referencing a file outside the extension's subdir.
   if (path.IsAbsolute() || PathContainsParentDirectory(path)) {
-    SetError(kPathNamesMustBeAbsoluteOrLocalError);
+    SetUTF16Error(
+        l10n_util::GetStringFUTF16(
+            IDS_EXTENSION_PACKAGE_IMAGE_PATH_ERROR,
+            base::i18n::GetDisplayStringInLTRDirectionality(
+                path.LossyDisplayName())));

[dharcourt, 2012/08/27 17:55:55]

Added path to the error message because that could be useful to CRX developers.

     return false;
   }
 
   SkBitmap image_bitmap = DecodeImage(temp_install_dir_.Append(path));
   if (image_bitmap.isNull()) {
-    SetError(kCouldNotDecodeImageError);
+    SetUTF16Error(
+        l10n_util::GetStringFUTF16(
+            IDS_EXTENSION_PACKAGE_IMAGE_ERROR,
+            base::i18n::GetDisplayStringInLTRDirectionality(
+                path.BaseName().LossyDisplayName())));

[dharcourt, 2012/08/27 17:55:55]

Added the name of the problematic image to the error message because that will
probably be useful to CRX developers.

     return false;
   }
 
@@ -328,7 +329,11 @@ bool Unpacker::ReadMessageCatalog(const FilePath& message_path) {
 }
 
 void Unpacker::SetError(const std::string &error) {
-  error_message_ = UTF8ToUTF16(error);
+  SetUTF16Error(UTF8ToUTF16(error));
+}
+
+void Unpacker::SetUTF16Error(const string16 &error) {
+  error_message_ = error;

[dharcourt, 2012/08/27 17:55:55]

Added to avoid unnecessary UTF16->UTF8->UTF16 conversions.

 }
 
 }  // namespace extensions