Extensions: Don't pass the selected unpacked extension path from JS to C++

chrome/browser/ui/webui/options/pack_extension_handler.cc

Index: chrome/browser/ui/webui/options/pack_extension_handler.cc
diff --git a/chrome/browser/ui/webui/options/pack_extension_handler.cc b/chrome/browser/ui/webui/options/pack_extension_handler.cc
index 3ef768c7f02b297b3dca7f9531d7dec37f33ba57..a7806deb62168ee5167c35b9a20f63a3f3188025 100644
--- a/chrome/browser/ui/webui/options/pack_extension_handler.cc
+++ b/chrome/browser/ui/webui/options/pack_extension_handler.cc
@@ -7,6 +7,8 @@
 #include "chrome/browser/extensions/extension_creator.h"
 #include "base/bind.h"
 #include "base/utf_string_conversions.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_view.h"
 #include "content/public/browser/web_ui.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -15,6 +17,11 @@ PackExtensionHandler::PackExtensionHandler() {
 }
 
 PackExtensionHandler::~PackExtensionHandler() {
+  // There may be pending file dialogs, we need to tell them that we've gone
+  // away so they don't try and call back to us.
+  if (load_extension_dialog_)
+    load_extension_dialog_->ListenerDestroyed();
+
   if (pack_job_.get())
     pack_job_->ClearClient();
 }
@@ -52,10 +59,14 @@ void PackExtensionHandler::GetLocalizedValues(
 }
 
 void PackExtensionHandler::RegisterMessages() {
-  // Setup handlers specific to this panel.
-  web_ui()->RegisterMessageCallback("pack",
+  web_ui()->RegisterMessageCallback(
+      "pack",
       base::Bind(&PackExtensionHandler::HandlePackMessage,
                  base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(
+      "packExtensionSelectFilePath",
+      base::Bind(&PackExtensionHandler::HandleSelectFilePathMessage,
+                 base::Unretained(this)));
 }
 
 void PackExtensionHandler::OnPackSuccess(const FilePath& crx_file,
@@ -84,14 +95,30 @@ void PackExtensionHandler::OnPackFailure(const std::string& error,
   }
 }
 
+void PackExtensionHandler::FileSelected(const FilePath& path, int index,
+                                        void* params) {
+  ListValue results;
+  results.Append(Value::CreateStringValue(path.value()));
+  web_ui()->CallJavascriptFunction("window.handleFilePathSelected", results);
+}
+
+void PackExtensionHandler::MultiFilesSelected(
+    const std::vector<FilePath>& files, void* params) {
+  NOTREACHED();
+}
+
 void PackExtensionHandler::HandlePackMessage(const ListValue* args) {
 
-  CHECK_EQ(3U, args->GetSize());
-  CHECK(args->GetString(0, &extension_path_));
-  CHECK(args->GetString(1, &private_key_path_));
+  DCHECK_EQ(3U, args->GetSize());
+
+  if (!args->GetString(0, &extension_path_) ||
+      !args->GetString(1, &private_key_path_))

[Evan Stade, 2012/03/20 19:56:07]

curlies

[James Hawkins, 2012/03/23 21:49:38]

The C++ style guide does not require braces for multi-line logic blocks (or I
can't find it).

[Evan Stade, 2012/03/23 21:54:09]

chrome convention, supported by style guide: "conditional or loop statements
with complex conditions or statements may be more readable with curly braces"

[James Hawkins, 2012/03/23 22:04:34]

OK, then it's an 'Optional nit'.  I've seen plenty of code to the contrary to
suggest it's not a project-wide convention.

+    NOTREACHED();
 
   double flags_double = 0.0;
-  CHECK(args->GetDouble(2, &flags_double));
+  if (!args->GetDouble(2, &flags_double))
+    NOTREACHED();
+
   int run_flags = static_cast<int>(flags_double);
 
   FilePath root_directory =
@@ -120,6 +147,48 @@ void PackExtensionHandler::HandlePackMessage(const ListValue* args) {
   pack_job_->Start();
 }
 
+void PackExtensionHandler::HandleSelectFilePathMessage(
+    const ListValue* args) {
+  DCHECK_EQ(2U, args->GetSize());
+
+  std::string select_type;
+  if (!args->GetString(0, &select_type))
+    NOTREACHED();
+
+  std::string operation;
+  if (!args->GetString(1, &operation))
+    NOTREACHED();
+
+  SelectFileDialog::Type type = SelectFileDialog::SELECT_FOLDER;
+  SelectFileDialog::FileTypeInfo info;
+  int file_type_index = 0;
+  if (select_type == "file")
+    type = SelectFileDialog::SELECT_OPEN_FILE;
+
+  string16 select_title;
+  if (operation == "load") {
+    select_title = l10n_util::GetStringUTF16(IDS_EXTENSION_LOAD_FROM_DIRECTORY);
+  } else if (operation == "pem") {
+    select_title = l10n_util::GetStringUTF16(
+        IDS_EXTENSION_PACK_DIALOG_SELECT_KEY);
+    info.extensions.push_back(std::vector<FilePath::StringType>());
+        info.extensions.front().push_back(FILE_PATH_LITERAL("pem"));
+        info.extension_description_overrides.push_back(
+            l10n_util::GetStringUTF16(
+                IDS_EXTENSION_PACK_DIALOG_KEY_FILE_TYPE_DESCRIPTION));
+        info.include_all_files = true;
+    file_type_index = 1;
+  } else {
+    NOTREACHED();
+  }
+
+  load_extension_dialog_ = SelectFileDialog::Create(this);
+  load_extension_dialog_->SelectFile(
+      type, select_title, FilePath(), &info, file_type_index,
+      FILE_PATH_LITERAL(""), web_ui()->GetWebContents(),
+      web_ui()->GetWebContents()->GetView()->GetTopLevelNativeWindow(), NULL);
+}
+
 void PackExtensionHandler::ShowAlert(const std::string& message) {
   ListValue arguments;
   arguments.Append(Value::CreateStringValue(message));