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

chrome/common/extensions/unpacker.cc

 // Copyright (c) 2012 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 "chrome/common/extensions/unpacker.h"
 
 #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"
 #include "base/string_util.h"
 #include "base/threading/thread.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_manifest_constants.h"
 #include "chrome/common/extensions/extension_file_util.h"
 #include "chrome/common/extensions/extension_l10n_util.h"
 #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;
 namespace keys = extension_manifest_keys;
 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;
 
 SkBitmap DecodeImage(const FilePath& path) {
   // Read the file from disk.
   std::string file_contents;
   if (!file_util::PathExists(path) ||
       !file_util::ReadFileToString(path, &file_contents)) {
     return SkBitmap();
   }
@@ skipping 93 matching lines @@
     if (!ReadMessageCatalog(messages_path))
       return false;
   }
 
   return true;
 }
 
 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;
   }
 
   // Parse the manifest.
   parsed_manifest_.reset(ReadManifest());
   if (!parsed_manifest_.get())
     return false;  // Error was already reported.
 
   std::string error;
   scoped_refptr<Extension> extension(Extension::Create(
@@ skipping 87 matching lines @@
     return false;
 
   IPC::Message pickle(file_str.data(), file_str.size());
   PickleIterator iter(pickle);
   return IPC::ReadParam(&pickle, &iter, catalogs);
 }
 
 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;
   }
 
   decoded_images_.push_back(MakeTuple(image_bitmap, path));
   return true;
 }
 
 bool Unpacker::ReadMessageCatalog(const FilePath& message_path) {
   std::string error;
   JSONFileValueSerializer serializer(message_path);
@@ skipping 24 matching lines @@
   if (dir_name.empty()) {
     NOTREACHED();
     return false;
   }
   parsed_catalogs_->Set(dir_name, root.release());
 
   return true;
 }
 
 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