Extension package creation cleanup

chrome/browser/browser_init.cc

 // Copyright (c) 2010 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/browser/browser_init.h"
 
 #include <algorithm>   // For max().
 
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
 #include "base/environment.h"
 #include "base/event_recorder.h"
 #include "base/file_path.h"
 #include "base/histogram.h"
 #include "base/path_service.h"
 #include "base/scoped_ptr.h"
 #include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/automation/automation_provider.h"
 #include "chrome/browser/automation/automation_provider_list.h"
 #include "chrome/browser/automation/chrome_frame_automation_provider.h"
 #include "chrome/browser/browser_list.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_window.h"
 #include "chrome/browser/child_process_security_policy.h"
 #include "chrome/browser/chrome_thread.h"
 #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"
 #include "chrome/browser/notifications/desktop_notification_service.h"
 #include "chrome/browser/pref_service.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/renderer_host/render_process_host.h"
 #include "chrome/browser/search_engines/template_url_model.h"
 #include "chrome/browser/session_startup_pref.h"
 #include "chrome/browser/sessions/session_restore.h"
@@ skipping 298 matching lines @@
   UMA_HISTOGRAM_COUNTS_100("Launch.Modes", bucket);
 }
 
 static bool in_startup = false;
 
 GURL GetWelcomePageURL() {
   std::string welcome_url = l10n_util::GetStringUTF8(IDS_WELCOME_PAGE_URL);
   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) {
     BrowserInit::LaunchWithProfile::Tab tab;
     tab.is_pinned = false;
     tab.url = urls[i];
     tabs->push_back(tab);
   }
 }
 
@@ skipping 544 matching lines @@
   // - the user said "don't ask me again" on the infobar earlier.
   // - this is the first launch after the first run flow.
   if (!profile->GetPrefs()->GetBoolean(prefs::kCheckDefaultBrowser) ||
       FirstRun::IsChromeFirstRun()) {
     return;
   }
   ChromeThread::PostTask(
       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,
                                      Profile* profile,
                                      int* return_code,
                                      BrowserInit* browser_init) {
   DCHECK(profile);
   if (process_startup) {
     if (command_line.HasSwitch(switches::kDisablePromptOnRepost))
       NavigationController::DisablePromptOnRepost();
@@ skipping 24 matching lines @@
     if (command_line.HasSwitch(switches::kPackExtension)) {
       // Input Paths.
       FilePath src_dir = command_line.GetSwitchValuePath(
           switches::kPackExtension);
       FilePath private_key_path;
       if (command_line.HasSwitch(switches::kPackExtensionKey)) {
         private_key_path = command_line.GetSwitchValuePath(
             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();
 
-      // 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;
-      }
+      // 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

+
       return false;
     }
   }
 
   bool silent_launch = false;
 
   if (command_line.HasSwitch(switches::kAutomationClientChannelID)) {
     std::string automation_channel_id = command_line.GetSwitchValueASCII(
         switches::kAutomationClientChannelID);
     // If there are any extra parameters, we expect each one to generate a
@@ skipping 42 matching lines @@
   scoped_refptr<AutomationProviderClass> automation =
       new AutomationProviderClass(profile);
   automation->ConnectToChannel(channel_id);
   automation->SetExpectedTabCount(expected_tabs);
 
   AutomationProviderList* list =
       g_browser_process->InitAutomationProviderList();
   DCHECK(list);
   list->AddProvider(automation);
 }