Extension package creation cleanup

chrome/browser/browser_init.cc

Index: chrome/browser/browser_init.cc
diff --git a/chrome/browser/browser_init.cc b/chrome/browser/browser_init.cc
index dd798155ed91ff772d198a8858fa49bd4cd8da8d..ed5a46085d4d22b884dda1e9b5ca7de9cdc8dc6d 100644
--- a/chrome/browser/browser_init.cc
+++ b/chrome/browser/browser_init.cc
@@ -27,6 +27,7 @@
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/extensions/extension_creator.h"
 #include "chrome/browser/extensions/extensions_service.h"
+#include "chrome/browser/extensions/pack_extension_job.h"
 #include "chrome/browser/first_run/first_run.h"
 #include "chrome/browser/net/predictor_api.h"
 #include "chrome/browser/net/url_fixer_upper.h"
@@ -345,20 +346,6 @@ GURL GetWelcomePageURL() {
   return GURL(welcome_url);
 }
 
-void ShowPackExtensionMessage(const std::wstring caption,
-    const std::wstring message) {
-#if defined(OS_WIN)
-  win_util::MessageBox(NULL, message, caption, MB_OK | MB_SETFOREGROUND);
-#else
-  // Just send caption & text to stdout on mac & linux.
-  std::string out_text = WideToASCII(caption);
-  out_text.append("\n\n");
-  out_text.append(WideToASCII(message));
-  out_text.append("\n");
-  printf("%s", out_text.c_str());
-#endif
-}
-
 void UrlsToTabs(const std::vector<GURL>& urls,
                 std::vector<BrowserInit::LaunchWithProfile::Tab>* tabs) {
   for (size_t i = 0; i < urls.size(); ++i) {
@@ -923,6 +910,64 @@ void BrowserInit::LaunchWithProfile::CheckDefaultBrowser(Profile* profile) {
       ChromeThread::FILE, FROM_HERE, new CheckDefaultBrowserTask());
 }
 
+class PackExtensionLogger : public PackExtensionJob::Client {
+ public:
+  PackExtensionLogger() {}
+  virtual void OnPackSuccess(const FilePath& crx_path,
+                             const FilePath& output_private_key_path);
+  virtual void OnPackFailure(const std::string& error_message);
+
+ private:
+  void ShowPackExtensionMessage(const std::wstring& caption,
+                                const std::wstring& message);
+
+  DISALLOW_COPY_AND_ASSIGN(PackExtensionLogger);
+};
+
+void PackExtensionLogger::OnPackSuccess(const FilePath& crx_path,
+                                        const FilePath& output_private_key_path) {
+  std::wstring message;
+  // TODO we probably shouldn't add instances of ToWStringHack() -- what's the
+  //      right thing to do?

[Elliot Glaysher, 2010/08/05 23:58:57]

nit: Format is "TODO(ldap): "

[Matt Perry, 2010/08/06 00:03:55]

also, for consistency, don't indent the text in the wrapping lines.

+  // TODO this code is redundant with the code in extensions_ui.cc -- would it
+  //      make sense to push the logic down into PackExtensionJob?
+  if (output_private_key_path.empty()) {
+    message = l10n_util::GetStringF(
+        IDS_EXTENSION_PACK_DIALOG_SUCCESS_BODY_UPDATE,
+        crx_path.ToWStringHack(),
+        output_private_key_path.ToWStringHack());
+  } else {
+    message = l10n_util::GetStringF(
+        IDS_EXTENSION_PACK_DIALOG_SUCCESS_BODY_NEW,
+        crx_path.ToWStringHack(),
+        output_private_key_path.ToWStringHack());
+  }
+  ShowPackExtensionMessage(L"Extension Packaging Success", message);
+}
+
+void PackExtensionLogger::OnPackFailure(const std::string& error_message) {
+  ShowPackExtensionMessage(L"Extension Packaging Error",
+                           UTF8ToWide(error_message));
+}
+
+void PackExtensionLogger::ShowPackExtensionMessage(const std::wstring& caption,
+                                                   const std::wstring& message) {
+#if defined(OS_WIN)
+  win_util::MessageBox(NULL, message, caption, MB_OK | MB_SETFOREGROUND);
+#else
+  // Just send caption & text to stdout on mac & linux.
+  std::string out_text = WideToASCII(caption);
+  out_text.append("\n\n");
+  out_text.append(WideToASCII(message));
+  out_text.append("\n");
+  printf("%s", out_text.c_str());
+#endif
+
+  // We got the notification and processed it; we don't expect any further tasks
+  // to be posted to the current thread, so we should stop blocking and exit.
+  MessageLoop::current()->Quit();

[Matt Perry, 2010/08/05 23:56:09]

add a note saying that this matches the Run() in ProcessCmdLineImpl

+}
+
 bool BrowserInit::ProcessCmdLineImpl(const CommandLine& command_line,
                                      const FilePath& cur_dir,
                                      bool process_startup,
@@ -967,44 +1012,18 @@ bool BrowserInit::ProcessCmdLineImpl(const CommandLine& command_line,
             switches::kPackExtensionKey);
       }
 
-      // Output Paths.
-      FilePath output(src_dir.DirName().Append(src_dir.BaseName().value()));
-      FilePath crx_path(output);
-      crx_path = crx_path.ReplaceExtension(chrome::kExtensionFileExtension);
-      FilePath output_private_key_path;
-      if (private_key_path.empty()) {
-        output_private_key_path = FilePath(output);
-        output_private_key_path =
-            output_private_key_path.ReplaceExtension(FILE_PATH_LITERAL("pem"));
-      }
+      // Launch a job to perform the packing on the file thread.
+      scoped_ptr<PackExtensionLogger> pack_client(new PackExtensionLogger);
+      scoped_refptr<PackExtensionJob> pack_job =
+          new PackExtensionJob(pack_client.get(), src_dir, private_key_path);
+      pack_job->Start();
+
+      // The job will post a notification task to the current thread's message
+      // loop when it is finished.  We manually run the loop here to catch this
+      // notification -- otherwise, we won't be able to report success or
+      // failure back to the user.

[Matt Perry, 2010/08/05 23:56:09]

this comment is not quite accurate. The main point of running the loop is to
block here until Quit is called (because if we didn't block, the process would
exit).

+      MessageLoop::current()->Run();

[Matt Perry, 2010/08/05 23:56:09]

add a note saying that this is matched by the Quit in PackExtensionLogger

 
-      // TODO(port): Creation & running is removed from mac & linux because
-      // ExtensionCreator depends on base/crypto/rsa_private_key and
-      // base/crypto/signature_creator, both of which only have windows
-      // implementations.
-      scoped_ptr<ExtensionCreator> creator(new ExtensionCreator());
-      if (creator->Run(src_dir, crx_path, private_key_path,
-          output_private_key_path)) {
-        std::wstring message;
-        if (private_key_path.value().empty()) {
-          message = StringPrintf(
-              L"Created the following files:\n\n"
-              L"Extension: %ls\n"
-              L"Key File: %ls\n\n"
-              L"Keep your key file in a safe place. You will need it to create "
-              L"new versions of your extension.",
-              crx_path.ToWStringHack().c_str(),
-              output_private_key_path.ToWStringHack().c_str());
-        } else {
-          message = StringPrintf(L"Created the extension:\n\n%ls",
-                                 crx_path.ToWStringHack().c_str());
-        }
-        ShowPackExtensionMessage(L"Extension Packaging Success", message);
-      } else {
-        ShowPackExtensionMessage(L"Extension Packaging Error",
-            UTF8ToWide(creator->error_message()));
-        return false;
-      }
       return false;
     }
   }