[Extensions] Make chrome://extensions use developerPrivate for packing crxs

Make the chrome://extensions page use chrome.developerPrivate API to
select a file path for a crx and to perform the pack job itself.
Also add tests for the api function, and convert it to a
UIThreadExtensionFunction.

BUG=461039

Review URL: https://codereview.chromium.org/958803004

Cr-Commit-Position: refs/heads/master@{#318754}

chrome/browser/extensions/api/developer_private/developer_private_api.cc

@@ -96,6 +96,10 @@ const char kCannotModifyPolicyExtensionError[] =
     "Cannot modify the extension by policy.";
 const char kRequiresUserGestureError[] =
     "This action requires a user gesture.";
+const char kCouldNotShowSelectFileDialogError[] =
+    "Could not show a file chooser.";
+const char kFileSelectionCanceled[] =
+    "File selection was canceled.";
 
 const char kUnpackedAppsFolder[] = "apps_target";
 
@@ -867,67 +871,59 @@ bool DeveloperPrivateInspectFunction::RunSync() {
 
 DeveloperPrivateInspectFunction::~DeveloperPrivateInspectFunction() {}
 
-bool DeveloperPrivateLoadUnpackedFunction::RunAsync() {
-  base::string16 select_title =
-      l10n_util::GetStringUTF16(IDS_EXTENSION_LOAD_FROM_DIRECTORY);
-
-  // Balanced in FileSelected / FileSelectionCanceled.
-  AddRef();
-  bool result = ShowPicker(
+ExtensionFunction::ResponseAction DeveloperPrivateLoadUnpackedFunction::Run() {
+  if (!ShowPicker(
            ui::SelectFileDialog::SELECT_FOLDER,
-      DeveloperPrivateAPI::Get(GetProfile())->GetLastUnpackedDirectory(),
-      select_title,
+           l10n_util::GetStringUTF16(IDS_EXTENSION_LOAD_FROM_DIRECTORY),
            ui::SelectFileDialog::FileTypeInfo(),
-      0);
-  return result;
+           0 /* file_type_index */)) {
+    return RespondNow(Error(kCouldNotShowSelectFileDialogError));
+  }
+
+  AddRef();  // Balanced in FileSelected / FileSelectionCanceled.
+  return RespondLater();
 }
 
 void DeveloperPrivateLoadUnpackedFunction::FileSelected(
     const base::FilePath& path) {
-  ExtensionService* service = GetExtensionService(GetProfile());
-  UnpackedInstaller::Create(service)->Load(path);
-  DeveloperPrivateAPI::Get(GetProfile())->SetLastUnpackedDirectory(path);
-  SendResponse(true);
-  Release();
+  UnpackedInstaller::Create(GetExtensionService(browser_context()))->Load(path);
+  DeveloperPrivateAPI::Get(browser_context())->SetLastUnpackedDirectory(path);
+  // TODO(devlin): Shouldn't we wait until the extension is loaded?
+  Respond(NoArguments());
+  Release();  // Balanced in Run().
 }
 
 void DeveloperPrivateLoadUnpackedFunction::FileSelectionCanceled() {
-  SendResponse(false);
-  Release();
+  // This isn't really an error, but we should keep it like this for
+  // backward compatability.
+  Respond(Error(kFileSelectionCanceled));
+  Release();  // Balanced in Run().
 }
 
 bool DeveloperPrivateChooseEntryFunction::ShowPicker(
     ui::SelectFileDialog::Type picker_type,
-    const base::FilePath& last_directory,
     const base::string16& select_title,
     const ui::SelectFileDialog::FileTypeInfo& info,
     int file_type_index) {
-  AppWindowRegistry* registry = AppWindowRegistry::Get(GetProfile());
-  DCHECK(registry);
-  AppWindow* app_window =
-      registry->GetAppWindowForRenderViewHost(render_view_host());
-  if (!app_window) {
+  content::WebContents* web_contents = GetSenderWebContents();
+  if (!web_contents)
     return false;
-  }
 
   // The entry picker will hold a reference to this function instance,
   // and subsequent sending of the function response) until the user has
   // selected a file or cancelled the picker. At that point, the picker will
   // delete itself.
   new EntryPicker(this,
-                  app_window->web_contents(),
+                  web_contents,
                   picker_type,
-                  last_directory,
+                  DeveloperPrivateAPI::Get(browser_context())->
+                      GetLastUnpackedDirectory(),
                   select_title,
                   info,
                   file_type_index);
   return true;
 }
 
-bool DeveloperPrivateChooseEntryFunction::RunAsync() {
-  return false;
-}
-
 DeveloperPrivateChooseEntryFunction::~DeveloperPrivateChooseEntryFunction() {}
 
 void DeveloperPrivatePackDirectoryFunction::OnPackSuccess(
@@ -937,9 +933,8 @@ void DeveloperPrivatePackDirectoryFunction::OnPackSuccess(
   response.message = base::UTF16ToUTF8(
       PackExtensionJob::StandardSuccessMessage(crx_file, pem_file));
   response.status = developer::PACK_STATUS_SUCCESS;
-  results_ = developer::PackDirectory::Results::Create(response);
-  SendResponse(true);
-  Release();
+  Respond(OneArgument(response.ToValue().release()));
+  Release();  // Balanced in Run().
 }
 
 void DeveloperPrivatePackDirectoryFunction::OnPackFailure(
@@ -955,15 +950,14 @@ void DeveloperPrivatePackDirectoryFunction::OnPackFailure(
   } else {
     response.status = developer::PACK_STATUS_ERROR;
   }
-  results_ = developer::PackDirectory::Results::Create(response);
-  SendResponse(true);
-  Release();
+  Respond(OneArgument(response.ToValue().release()));
+  Release();  // Balanced in Run().
 }
 
-bool DeveloperPrivatePackDirectoryFunction::RunAsync() {
+ExtensionFunction::ResponseAction DeveloperPrivatePackDirectoryFunction::Run() {
   scoped_ptr<PackDirectory::Params> params(
       PackDirectory::Params::Create(*args_));
-  EXTENSION_FUNCTION_VALIDATE(params.get());
+  EXTENSION_FUNCTION_VALIDATE(params);
 
   int flags = params->flags ? *params->flags : 0;
   item_path_str_ = params->path;
@@ -972,7 +966,6 @@ bool DeveloperPrivatePackDirectoryFunction::RunAsync() {
 
   base::FilePath root_directory =
       base::FilePath::FromUTF8Unsafe(item_path_str_);
-
   base::FilePath key_file = base::FilePath::FromUTF8Unsafe(key_path_str_);
 
   developer::PackDirectoryResponse response;
@@ -985,33 +978,29 @@ bool DeveloperPrivatePackDirectoryFunction::RunAsync() {
           IDS_EXTENSION_PACK_DIALOG_ERROR_ROOT_INVALID);
 
     response.status = developer::PACK_STATUS_ERROR;
-    results_ = developer::PackDirectory::Results::Create(response);
-    SendResponse(true);
-    return true;
+    return RespondNow(OneArgument(response.ToValue().release()));
   }
 
   if (!key_path_str_.empty() && key_file.empty()) {
     response.message = l10n_util::GetStringUTF8(
         IDS_EXTENSION_PACK_DIALOG_ERROR_KEY_INVALID);
     response.status = developer::PACK_STATUS_ERROR;
-    results_ = developer::PackDirectory::Results::Create(response);
-    SendResponse(true);
-    return true;
+    return RespondNow(OneArgument(response.ToValue().release()));
   }
 
-  // Balanced in OnPackSuccess / OnPackFailure.
-  AddRef();
+  AddRef();  // Balanced in OnPackSuccess / OnPackFailure.
 
+  // TODO(devlin): Why is PackExtensionJob ref-counted?
   pack_job_ = new PackExtensionJob(this, root_directory, key_file, flags);
   pack_job_->Start();
-  return true;
+  return RespondLater();
 }
 
-DeveloperPrivatePackDirectoryFunction::DeveloperPrivatePackDirectoryFunction()
-{}
+DeveloperPrivatePackDirectoryFunction::DeveloperPrivatePackDirectoryFunction() {
+}
 
-DeveloperPrivatePackDirectoryFunction::~DeveloperPrivatePackDirectoryFunction()
-{}
+DeveloperPrivatePackDirectoryFunction::
+~DeveloperPrivatePackDirectoryFunction() {}
 
 DeveloperPrivateLoadUnpackedFunction::~DeveloperPrivateLoadUnpackedFunction() {}
 
@@ -1246,16 +1235,16 @@ DeveloperPrivateLoadDirectoryFunction::DeveloperPrivateLoadDirectoryFunction()
 DeveloperPrivateLoadDirectoryFunction::~DeveloperPrivateLoadDirectoryFunction()
     {}
 
-bool DeveloperPrivateChoosePathFunction::RunAsync() {
+ExtensionFunction::ResponseAction DeveloperPrivateChoosePathFunction::Run() {
   scoped_ptr<developer::ChoosePath::Params> params(
       developer::ChoosePath::Params::Create(*args_));
-  EXTENSION_FUNCTION_VALIDATE(params.get() != NULL);
+  EXTENSION_FUNCTION_VALIDATE(params);
 
   ui::SelectFileDialog::Type type = ui::SelectFileDialog::SELECT_FOLDER;
   ui::SelectFileDialog::FileTypeInfo info;
-  if (params->select_type == developer::SELECT_TYPE_FILE) {
+
+  if (params->select_type == developer::SELECT_TYPE_FILE)
     type = ui::SelectFileDialog::SELECT_OPEN_FILE;
-  }
   base::string16 select_title;
 
   int file_type_index = 0;
@@ -1264,8 +1253,8 @@ bool DeveloperPrivateChoosePathFunction::RunAsync() {
   } else if (params->file_type == developer::FILE_TYPE_PEM) {
     select_title = l10n_util::GetStringUTF16(
         IDS_EXTENSION_PACK_DIALOG_SELECT_KEY);
-    info.extensions.push_back(std::vector<base::FilePath::StringType>());
-    info.extensions.front().push_back(FILE_PATH_LITERAL("pem"));
+    info.extensions.push_back(std::vector<base::FilePath::StringType>(
+        1, FILE_PATH_LITERAL("pem")));
     info.extension_description_overrides.push_back(
         l10n_util::GetStringUTF16(
             IDS_EXTENSION_PACK_DIALOG_KEY_FILE_TYPE_DESCRIPTION));
@@ -1275,26 +1264,28 @@ bool DeveloperPrivateChoosePathFunction::RunAsync() {
     NOTREACHED();
   }
 
-  // Balanced by FileSelected / FileSelectionCanceled.
-  AddRef();
-  bool result = ShowPicker(
+  if (!ShowPicker(
            type,
-      DeveloperPrivateAPI::Get(GetProfile())->GetLastUnpackedDirectory(),
            select_title,
            info,
-      file_type_index);
-  return result;
+           file_type_index)) {
+    return RespondNow(Error(kCouldNotShowSelectFileDialogError));
+  }
+
+  AddRef();  // Balanced by FileSelected / FileSelectionCanceled.
+  return RespondLater();
 }
 
 void DeveloperPrivateChoosePathFunction::FileSelected(
     const base::FilePath& path) {
-  SetResult(new base::StringValue(base::UTF16ToUTF8(path.LossyDisplayName())));
-  SendResponse(true);
+  Respond(OneArgument(new base::StringValue(path.LossyDisplayName())));
   Release();
 }
 
 void DeveloperPrivateChoosePathFunction::FileSelectionCanceled() {
-  SendResponse(false);
+  // This isn't really an error, but we should keep it like this for
+  // backward compatability.
+  Respond(Error(kFileSelectionCanceled));
   Release();
 }
 

chrome/browser/extensions/api/developer_private/developer_private_api.h

@@ -296,20 +296,14 @@ class DeveloperPrivateShowPermissionsDialogFunction
   std::string extension_id_;
 };
 
-class DeveloperPrivateChooseEntryFunction : public ChromeAsyncExtensionFunction,
+class DeveloperPrivateChooseEntryFunction : public UIThreadExtensionFunction,
                                             public EntryPickerClient {
  protected:
   ~DeveloperPrivateChooseEntryFunction() override;
-  bool RunAsync() override;
   bool ShowPicker(ui::SelectFileDialog::Type picker_type,
-                  const base::FilePath& last_directory,
                   const base::string16& select_title,
                   const ui::SelectFileDialog::FileTypeInfo& info,
                   int file_type_index);
-
-  // EntryPickerClient functions.
-  void FileSelected(const base::FilePath& path) override = 0;
-  void FileSelectionCanceled() override = 0;
 };
 
 
@@ -321,9 +315,9 @@ class DeveloperPrivateLoadUnpackedFunction
 
  protected:
   ~DeveloperPrivateLoadUnpackedFunction() override;
-  bool RunAsync() override;
+  ResponseAction Run() override;
 
-  // EntryPickerCLient implementation.
+  // EntryPickerClient:
   void FileSelected(const base::FilePath& path) override;
   void FileSelectionCanceled() override;
 };
@@ -336,15 +330,15 @@ class DeveloperPrivateChoosePathFunction
 
  protected:
   ~DeveloperPrivateChoosePathFunction() override;
-  bool RunAsync() override;
+  ResponseAction Run() override;
 
-  // EntryPickerClient functions.
+  // EntryPickerClient:
   void FileSelected(const base::FilePath& path) override;
   void FileSelectionCanceled() override;
 };
 
 class DeveloperPrivatePackDirectoryFunction
-    : public ChromeAsyncExtensionFunction,
+    : public DeveloperPrivateAPIFunction,
       public PackExtensionJob::Client {
 
  public:
@@ -361,7 +355,7 @@ class DeveloperPrivatePackDirectoryFunction
 
  protected:
   ~DeveloperPrivatePackDirectoryFunction() override;
-  bool RunAsync() override;
+  ResponseAction Run() override;
 
  private:
   scoped_refptr<PackExtensionJob> pack_job_;

chrome/browser/extensions/api/developer_private/developer_private_api_unittest.cc

@@ -12,6 +12,7 @@
 #include "chrome/browser/extensions/unpacked_installer.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/host_desktop.h"
+#include "chrome/common/extensions/api/developer_private.h"
 #include "chrome/test/base/test_browser_window.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
@@ -43,6 +44,11 @@ class DeveloperPrivateApiUnitTest : public ExtensionServiceTestBase {
   void TestExtensionPrefSetting(
       bool (*has_pref)(const std::string&, content::BrowserContext*));
 
+  testing::AssertionResult TestPackExtensionFunction(
+      const base::ListValue& args,
+      api::developer_private::PackStatus expected_status,
+      int expected_flags);
+
   Browser* browser() { return browser_.get(); }
 
  private:
@@ -134,6 +140,37 @@ void DeveloperPrivateApiUnitTest::TestExtensionPrefSetting(
   EXPECT_FALSE(has_pref(extension_id, profile()));
 }
 
+testing::AssertionResult DeveloperPrivateApiUnitTest::TestPackExtensionFunction(
+    const base::ListValue& args,
+    api::developer_private::PackStatus expected_status,
+    int expected_flags) {
+  scoped_refptr<UIThreadExtensionFunction> function(
+      new api::DeveloperPrivatePackDirectoryFunction());
+  if (!RunFunction(function, args))
+    return testing::AssertionFailure() << "Could not run function.";
+
+  // Extract the result. We don't have to test this here, since it's verified as
+  // part of the general extension api system.
+  const base::Value* response_value = nullptr;
+  CHECK(function->GetResultList()->Get(0u, &response_value));
+  scoped_ptr<api::developer_private::PackDirectoryResponse> response =
+      api::developer_private::PackDirectoryResponse::FromValue(*response_value);
+  CHECK(response);
+
+  if (response->status != expected_status) {
+    return testing::AssertionFailure() << "Expected status: " <<
+        expected_status << ", found status: " << response->status <<
+        ", message: " << response->message;
+  }
+
+  if (response->override_flags != expected_flags) {
+    return testing::AssertionFailure() << "Expected flags: " <<
+        expected_flags << ", found flags: " << response->override_flags;
+  }
+
+  return testing::AssertionSuccess();
+}
+
 void DeveloperPrivateApiUnitTest::SetUp() {
   ExtensionServiceTestBase::SetUp();
   InitializeEmptyExtensionService();
@@ -183,4 +220,48 @@ TEST_F(DeveloperPrivateApiUnitTest, DeveloperPrivateAllowFileAccess) {
       &util::AllowFileAccess);
 }
 
+// Test developerPrivate.packDirectory.
+TEST_F(DeveloperPrivateApiUnitTest, DeveloperPrivatePackFunction) {
+  ResetThreadBundle(content::TestBrowserThreadBundle::DEFAULT);
+
+  base::FilePath root_path = data_dir().AppendASCII("good_unpacked");
+  base::FilePath crx_path = data_dir().AppendASCII("good_unpacked.crx");
+  base::FilePath pem_path = data_dir().AppendASCII("good_unpacked.pem");
+
+  // First, test a directory that should pack properly.
+  base::ListValue pack_args;
+  pack_args.AppendString(root_path.AsUTF8Unsafe());
+  EXPECT_TRUE(TestPackExtensionFunction(
+      pack_args, api::developer_private::PACK_STATUS_SUCCESS, 0));
+
+  // Should have created crx file and pem file.
+  EXPECT_TRUE(base::PathExists(crx_path));
+  EXPECT_TRUE(base::PathExists(pem_path));
+
+  // Deliberately don't cleanup the files, and append the pem path.
+  pack_args.AppendString(pem_path.AsUTF8Unsafe());
+
+  // Try to pack again - we should get a warning abot overwriting the crx.
+  EXPECT_TRUE(TestPackExtensionFunction(
+      pack_args,
+      api::developer_private::PACK_STATUS_WARNING,
+      ExtensionCreator::kOverwriteCRX));
+
+  // Try to pack again, with the overwrite flag; this should succeed.
+  pack_args.AppendInteger(ExtensionCreator::kOverwriteCRX);
+  EXPECT_TRUE(TestPackExtensionFunction(
+      pack_args, api::developer_private::PACK_STATUS_SUCCESS, 0));
+
+  // Try to pack a final time when omitting (an existing) pem file. We should
+  // get an error.
+  base::DeleteFile(crx_path, false);
+  EXPECT_TRUE(pack_args.Remove(1u, nullptr));  // Remove the pem key argument.
+  EXPECT_TRUE(pack_args.Remove(1u, nullptr));  // Remove the flags argument.
+  EXPECT_TRUE(TestPackExtensionFunction(
+      pack_args, api::developer_private::PACK_STATUS_ERROR, 0));
+
+  base::DeleteFile(crx_path, false);
+  base::DeleteFile(pem_path, false);
+}
+
 }  // namespace extensions

chrome/browser/extensions/api/developer_private/entry_picker.cc

@@ -79,6 +79,13 @@ void EntryPicker::FileSelectionCanceled(void* params) {
   delete this;
 }
 
+void EntryPicker::MultiFilesSelected(const std::vector<base::FilePath>& files,
+                                     void* params) {
+  NOTREACHED();
+  client_->FileSelectionCanceled();
+  delete this;
+}
+
 // static
 void EntryPicker::SkipPickerAndAlwaysSelectPathForTest(
     base::FilePath* path) {

chrome/browser/extensions/api/developer_private/entry_picker.h

@@ -42,12 +42,13 @@ class EntryPicker : public ui::SelectFileDialog::Listener {
   ~EntryPicker() override;
 
  private:
-  // ui::SelectFileDialog::Listener implementation.
+  // ui::SelectFileDialog::Listener:
   void FileSelected(const base::FilePath& path,
                     int index,
                     void* params) override;
-
   void FileSelectionCanceled(void* params) override;
+  void MultiFilesSelected(const std::vector<base::FilePath>& files,
+                          void* params) override;
 
   scoped_refptr<ui::SelectFileDialog> select_file_dialog_;
   EntryPickerClient* client_;

chrome/browser/resources/extensions/extensions.js

@@ -343,28 +343,6 @@ cr.define('extensions', function() {
     ExtensionList.decorate($('extension-settings-list'));
   };
 
-  // Indicate that warning |message| has occured for pack of |crx_path| and
-  // |pem_path| files.  Ask if user wants override the warning.  Send
-  // |overrideFlags| to repeated 'pack' call to accomplish the override.
-  ExtensionSettings.askToOverrideWarning =
-      function(message, crx_path, pem_path, overrideFlags) {
-    var closeAlert = function() {
-      ExtensionSettings.showOverlay(null);
-    };
-
-    alertOverlay.setValues(
-        loadTimeData.getString('packExtensionWarningTitle'),
-        message,
-        loadTimeData.getString('packExtensionProceedAnyway'),
-        loadTimeData.getString('cancel'),
-        function() {
-          chrome.send('pack', [crx_path, pem_path, overrideFlags]);
-          closeAlert();
-        },
-        closeAlert);
-    ExtensionSettings.showOverlay($('alertOverlay'));
-  };
-
   /**
    * Returns the current overlay or null if one does not exist.
    * @return {Element} The overlay element.

chrome/browser/resources/extensions/pack_extension_overlay.js

@@ -48,24 +48,24 @@ cr.define('extensions', function() {
     handleCommit_: function(e) {
       var extensionPath = $('extension-root-dir').value;
       var privateKeyPath = $('extension-private-key').value;
-      chrome.send('pack', [extensionPath, privateKeyPath, 0]);
+      chrome.developerPrivate.packDirectory(
+          extensionPath, privateKeyPath, 0, this.onPackResponse_.bind(this));
     },
 
     /**
      * Utility function which asks the C++ to show a platform-specific file
-     * select dialog, and fire |callback| with the |filePath| that resulted.
-     * |selectType| can be either 'file' or 'folder'. |operation| can be 'load'
-     * or 'pem' which are signals to the C++ to do some operation-specific
-     * configuration.
+     * select dialog, and set the value property of |node| to the selected path.
+     * @param {SelectType} selectType The type of selection to use.
+     * @param {FileType} fileType The type of file to select.
+     * @param {HTMLInputElement} node The node to set the value of.
      * @private
      */
-    showFileDialog_: function(selectType, operation, callback) {
-      window.handleFilePathSelected = function(filePath) {
-        callback(filePath);
-        window.handleFilePathSelected = function() {};
-      };
-
-      chrome.send('packExtensionSelectFilePath', [selectType, operation]);
+    showFileDialog_: function(selectType, fileType, node) {
+      chrome.developerPrivate.choosePath(selectType, fileType, function(path) {
+        // Last error is set if the user canceled the dialog.
+        if (!chrome.runtime.lastError && path)
+          node.value = path;
+      });
     },
 
     /**
@@ -74,9 +74,10 @@ cr.define('extensions', function() {
      * @private
      */
     handleBrowseExtensionDir_: function(e) {
-      this.showFileDialog_('folder', 'load', function(filePath) {
-        $('extension-root-dir').value = filePath;
-      });
+      this.showFileDialog_(
+          'FOLDER',
+          'LOAD',
+          /** @type {HTMLInputElement} */ ($('extension-root-dir')));
     },
 
     /**
@@ -85,44 +86,78 @@ cr.define('extensions', function() {
      * @private
      */
     handleBrowsePrivateKey_: function(e) {
-      this.showFileDialog_('file', 'pem', function(filePath) {
-        $('extension-private-key').value = filePath;
-      });
+      this.showFileDialog_(
+          'FILE',
+          'PEM',
+          /** @type {HTMLInputElement} */ ($('extension-private-key')));
     },
-  };
 
     /**
-   * Wrap up the pack process by showing the success |message| and closing
-   * the overlay.
-   * @param {string} message The message to show to the user.
+     * Handles a response from a packDirectory call.
+     * @param {PackDirectoryResponse} response The response of the pack call.
+     * @private
      */
-  PackExtensionOverlay.showSuccessMessage = function(message) {
-    alertOverlay.setValues(
-        loadTimeData.getString('packExtensionOverlay'),
-        message,
-        loadTimeData.getString('ok'),
-        '',
-        function() {
+    onPackResponse_: function(response) {
+      /** @type {string} */
+      var alertTitle;
+      /** @type {string} */
+      var alertOk;
+      /** @type {string} */
+      var alertCancel;
+      /** @type {function()} */
+      var alertOkCallback;
+      /** @type {function()} */
+      var alertCancelCallback;
+
+      var closeAlert = function() {
         extensions.ExtensionSettings.showOverlay(null);
-        });
-    extensions.ExtensionSettings.showOverlay($('alertOverlay'));
       };
 
-  /**
-   * Post an alert overlay showing |message|, and upon acknowledgement, close
-   * the alert overlay and return to showing the PackExtensionOverlay.
-   * @param {string} message The error message.
-   */
-  PackExtensionOverlay.showError = function(message) {
-    alertOverlay.setValues(
-        loadTimeData.getString('packExtensionErrorTitle'),
-        message,
-        loadTimeData.getString('ok'),
-        '',
-        function() {
-          extensions.ExtensionSettings.showOverlay($('pack-extension-overlay'));
-        });
+      // TODO(devlin): Once we expose enums on extension APIs, we should use
+      // those objects, instead of a string.
+      switch (response.status) {
+        case 'SUCCESS':
+          alertTitle = loadTimeData.getString('packExtensionOverlay');
+          alertOk = loadTimeData.getString('ok');
+          alertOkCallback = closeAlert;
+          // No 'Cancel' option.
+          break;
+        case 'WARNING':
+          alertTitle = loadTimeData.getString('packExtensionWarningTitle');
+          alertOk = loadTimeData.getString('packExtensionProceedAnyway');
+          alertCancel = loadTimeData.getString('cancel');
+          alertOkCallback = function() {
+            chrome.developerPrivate.packDirectory(
+                response.item_path,
+                response.pem_path,
+                response.override_flags,
+                this.onPackResponse_.bind(this));
+            closeAlert();
+          }.bind(this);
+          alertCancelCallback = closeAlert;
+          break;
+        case 'ERROR':
+          alertTitle = loadTimeData.getString('packExtensionErrorTitle');
+          alertOk = loadTimeData.getString('ok');
+          alertOkCallback = function() {
+            extensions.ExtensionSettings.showOverlay(
+                $('pack-extension-overlay'));
+          };
+          // No 'Cancel' option.
+          break;
+        default:
+          assertNotReached();
+          return;
+      }
+
+      alertOverlay.setValues(alertTitle,
+                             response.message,
+                             alertOk,
+                             alertCancel,
+                             alertOkCallback,
+                             alertCancelCallback);
       extensions.ExtensionSettings.showOverlay($('alertOverlay'));
+    },
   };
 
   // Export

chrome/browser/ui/webui/extensions/extension_settings_handler.cc

@@ -636,6 +636,30 @@ void ExtensionSettingsHandler::GetLocalizedValues(
   // uber extensions.
   source->AddString("extensionUninstall",
       l10n_util::GetStringUTF16(IDS_EXTENSIONS_UNINSTALL));
+
+  // Pack Extension Overlay:
+  source->AddString("packExtensionOverlay",
+      l10n_util::GetStringUTF16(IDS_EXTENSION_PACK_DIALOG_TITLE));
+  source->AddString("packExtensionHeading",
+      l10n_util::GetStringUTF16(IDS_EXTENSION_PACK_DIALOG_HEADING));
+  source->AddString("packExtensionCommit",
+      l10n_util::GetStringUTF16(IDS_EXTENSION_PACK_BUTTON));
+  source->AddString("ok", l10n_util::GetStringUTF16(IDS_OK));
+  source->AddString("cancel", l10n_util::GetStringUTF16(IDS_CANCEL));
+  source->AddString("packExtensionRootDir",
+      l10n_util::GetStringUTF16(
+          IDS_EXTENSION_PACK_DIALOG_ROOT_DIRECTORY_LABEL));
+  source->AddString("packExtensionPrivateKey",
+      l10n_util::GetStringUTF16(IDS_EXTENSION_PACK_DIALOG_PRIVATE_KEY_LABEL));
+  source->AddString("packExtensionBrowseButton",
+      l10n_util::GetStringUTF16(IDS_EXTENSION_PACK_DIALOG_BROWSE));
+  source->AddString("packExtensionProceedAnyway",
+      l10n_util::GetStringUTF16(IDS_EXTENSION_PROCEED_ANYWAY));
+  source->AddString("packExtensionWarningTitle",
+      l10n_util::GetStringUTF16(IDS_EXTENSION_PACK_WARNING_TITLE));
+  source->AddString("packExtensionErrorTitle",
+      l10n_util::GetStringUTF16(IDS_EXTENSION_PACK_ERROR_TITLE));
+
 }
 
 void ExtensionSettingsHandler::RenderViewDeleted(

chrome/browser/ui/webui/extensions/extensions_ui.cc

@@ -10,7 +10,6 @@
 #include "chrome/browser/ui/webui/extensions/extension_loader_handler.h"
 #include "chrome/browser/ui/webui/extensions/extension_settings_handler.h"
 #include "chrome/browser/ui/webui/extensions/install_extension_handler.h"
-#include "chrome/browser/ui/webui/extensions/pack_extension_handler.h"
 #include "chrome/browser/ui/webui/metrics_handler.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/web_ui.h"
@@ -51,10 +50,6 @@ ExtensionsUI::ExtensionsUI(content::WebUI* web_ui) : WebUIController(web_ui) {
   handler->GetLocalizedValues(source);
   web_ui->AddMessageHandler(handler);
 
-  PackExtensionHandler* pack_handler = new PackExtensionHandler();
-  pack_handler->GetLocalizedValues(source);
-  web_ui->AddMessageHandler(pack_handler);
-
   CommandHandler* commands_handler = new CommandHandler(profile);
   commands_handler->GetLocalizedValues(source);
   web_ui->AddMessageHandler(commands_handler);

chrome/browser/ui/webui/extensions/pack_extension_handler.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/browser/ui/webui/extensions/pack_extension_handler.h"
-
-#include "base/bind.h"
-#include "base/strings/utf_string_conversions.h"
-#include "chrome/browser/extensions/extension_creator.h"
-#include "chrome/browser/ui/chrome_select_file_policy.h"
-#include "chrome/grit/generated_resources.h"
-#include "content/public/browser/web_contents.h"
-#include "content/public/browser/web_ui.h"
-#include "content/public/browser/web_ui_data_source.h"
-#include "ui/base/l10n/l10n_util.h"
-
-namespace extensions {
-
-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_.get())
-    load_extension_dialog_->ListenerDestroyed();
-
-  if (pack_job_.get())
-    pack_job_->ClearClient();
-}
-
-void PackExtensionHandler::GetLocalizedValues(
-    content::WebUIDataSource* source) {
-  source->AddString("packExtensionOverlay",
-      l10n_util::GetStringUTF16(IDS_EXTENSION_PACK_DIALOG_TITLE));
-  source->AddString("packExtensionHeading",
-      l10n_util::GetStringUTF16(IDS_EXTENSION_PACK_DIALOG_HEADING));
-  source->AddString("packExtensionCommit",
-      l10n_util::GetStringUTF16(IDS_EXTENSION_PACK_BUTTON));
-  source->AddString("ok", l10n_util::GetStringUTF16(IDS_OK));
-  source->AddString("cancel", l10n_util::GetStringUTF16(IDS_CANCEL));
-  source->AddString("packExtensionRootDir",
-      l10n_util::GetStringUTF16(
-          IDS_EXTENSION_PACK_DIALOG_ROOT_DIRECTORY_LABEL));
-  source->AddString("packExtensionPrivateKey",
-      l10n_util::GetStringUTF16(IDS_EXTENSION_PACK_DIALOG_PRIVATE_KEY_LABEL));
-  source->AddString("packExtensionBrowseButton",
-      l10n_util::GetStringUTF16(IDS_EXTENSION_PACK_DIALOG_BROWSE));
-  source->AddString("packExtensionProceedAnyway",
-      l10n_util::GetStringUTF16(IDS_EXTENSION_PROCEED_ANYWAY));
-  source->AddString("packExtensionWarningTitle",
-      l10n_util::GetStringUTF16(IDS_EXTENSION_PACK_WARNING_TITLE));
-  source->AddString("packExtensionErrorTitle",
-      l10n_util::GetStringUTF16(IDS_EXTENSION_PACK_ERROR_TITLE));
-}
-
-void PackExtensionHandler::RegisterMessages() {
-  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 base::FilePath& crx_file,
-                                         const base::FilePath& pem_file) {
-  base::ListValue arguments;
-  arguments.Append(new base::StringValue(base::UTF16ToUTF8(
-      PackExtensionJob::StandardSuccessMessage(crx_file, pem_file))));
-  web_ui()->CallJavascriptFunction(
-      "extensions.PackExtensionOverlay.showSuccessMessage", arguments);
-}
-
-void PackExtensionHandler::OnPackFailure(const std::string& error,
-                                         ExtensionCreator::ErrorType type) {
-  if (type == ExtensionCreator::kCRXExists) {
-    base::StringValue error_str(error);
-    base::StringValue extension_path_str(extension_path_.value());
-    base::StringValue key_path_str(private_key_path_.value());
-    base::FundamentalValue overwrite_flag(ExtensionCreator::kOverwriteCRX);
-
-    web_ui()->CallJavascriptFunction(
-        "extensions.ExtensionSettings.askToOverrideWarning",
-        error_str, extension_path_str, key_path_str, overwrite_flag);
-  } else {
-    ShowAlert(error);
-  }
-}
-
-void PackExtensionHandler::FileSelected(const base::FilePath& path, int index,
-                                        void* params) {
-  base::ListValue results;
-  results.Append(new base::StringValue(path.value()));
-  web_ui()->CallJavascriptFunction("window.handleFilePathSelected", results);
-}
-
-void PackExtensionHandler::MultiFilesSelected(
-    const std::vector<base::FilePath>& files, void* params) {
-  NOTREACHED();
-}
-
-void PackExtensionHandler::HandlePackMessage(const base::ListValue* args) {
-  DCHECK_EQ(3U, args->GetSize());
-
-  double flags_double = 0.0;
-  base::FilePath::StringType extension_path_str;
-  base::FilePath::StringType private_key_path_str;
-  if (!args->GetString(0, &extension_path_str) ||
-      !args->GetString(1, &private_key_path_str) ||
-      !args->GetDouble(2, &flags_double)) {
-    NOTREACHED();
-    return;
-  }
-
-  extension_path_ = base::FilePath(extension_path_str);
-  private_key_path_ = base::FilePath(private_key_path_str);
-
-  int run_flags = static_cast<int>(flags_double);
-
-  base::FilePath root_directory = extension_path_;
-  base::FilePath key_file = private_key_path_;
-  last_used_path_ = extension_path_;
-
-  if (root_directory.empty()) {
-    if (extension_path_.empty()) {
-      ShowAlert(l10n_util::GetStringUTF8(
-          IDS_EXTENSION_PACK_DIALOG_ERROR_ROOT_REQUIRED));
-    } else {
-      ShowAlert(l10n_util::GetStringUTF8(
-          IDS_EXTENSION_PACK_DIALOG_ERROR_ROOT_INVALID));
-    }
-
-    return;
-  }
-
-  if (!private_key_path_.empty() && key_file.empty()) {
-    ShowAlert(l10n_util::GetStringUTF8(
-        IDS_EXTENSION_PACK_DIALOG_ERROR_KEY_INVALID));
-    return;
-  }
-
-  pack_job_ = new PackExtensionJob(this, root_directory, key_file, run_flags);
-  pack_job_->Start();
-}
-
-void PackExtensionHandler::HandleSelectFilePathMessage(
-    const base::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();
-
-  ui::SelectFileDialog::Type type = ui::SelectFileDialog::SELECT_FOLDER;
-  ui::SelectFileDialog::FileTypeInfo info;
-  int file_type_index = 0;
-  base::FilePath path_to_use = last_used_path_;
-  if (select_type == "file") {
-    type = ui::SelectFileDialog::SELECT_OPEN_FILE;
-    path_to_use = base::FilePath();
-  }
-
-  base::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<base::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_ = ui::SelectFileDialog::Create(
-      this, new ChromeSelectFilePolicy(web_ui()->GetWebContents()));
-  load_extension_dialog_->SelectFile(
-      type,
-      select_title,
-      path_to_use,
-      &info,
-      file_type_index,
-      base::FilePath::StringType(),
-      web_ui()->GetWebContents()->GetTopLevelNativeWindow(),
-      NULL);
-}
-
-void PackExtensionHandler::ShowAlert(const std::string& message) {
-  base::ListValue arguments;
-  arguments.Append(new base::StringValue(message));
-  web_ui()->CallJavascriptFunction(
-      "extensions.PackExtensionOverlay.showError", arguments);
-}
-
-}  // namespace extensions

chrome/browser/ui/webui/extensions/pack_extension_handler.h

-// 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.
-
-#ifndef CHROME_BROWSER_UI_WEBUI_EXTENSIONS_PACK_EXTENSION_HANDLER_H_
-#define CHROME_BROWSER_UI_WEBUI_EXTENSIONS_PACK_EXTENSION_HANDLER_H_
-
-#include <string>
-
-#include "base/files/file_path.h"
-#include "chrome/browser/browsing_data/browsing_data_remover.h"
-#include "chrome/browser/extensions/pack_extension_job.h"
-#include "chrome/browser/plugins/plugin_data_remover_helper.h"
-#include "content/public/browser/web_ui_message_handler.h"
-#include "ui/shell_dialogs/select_file_dialog.h"
-
-namespace content {
-class WebUIDataSource;
-}
-
-namespace extensions {
-
-// Clear browser data handler page UI handler.
-class PackExtensionHandler : public content::WebUIMessageHandler,
-                             public ui::SelectFileDialog::Listener,
-                             public PackExtensionJob::Client {
- public:
-  PackExtensionHandler();
-  ~PackExtensionHandler() override;
-
-  void GetLocalizedValues(content::WebUIDataSource* source);
-
-  // WebUIMessageHandler implementation.
-  void RegisterMessages() override;
-
-  // ExtensionPackJob::Client implementation.
-  void OnPackSuccess(const base::FilePath& crx_file,
-                     const base::FilePath& key_file) override;
-
-  void OnPackFailure(const std::string& error,
-                     ExtensionCreator::ErrorType) override;
-
- private:
-  // SelectFileDialog::Listener implementation.
-  void FileSelected(const base::FilePath& path,
-                    int index,
-                    void* params) override;
-  void MultiFilesSelected(const std::vector<base::FilePath>& files,
-                          void* params) override;
-  void FileSelectionCanceled(void* params) override {}
-
-  // JavaScript callback to start packing an extension.
-  void HandlePackMessage(const base::ListValue* args);
-
-  // JavaScript callback to show a file browse dialog.
-  // |args[0]| must be a string that specifies the file dialog type: file or
-  // folder.
-  // |args[1]| must be a string that specifies the operation to perform: load
-  // or pem.
-  void HandleSelectFilePathMessage(const base::ListValue* args);
-
-  // A function to ask the page to show an alert.
-  void ShowAlert(const std::string& message);
-
-  // Used to package the extension.
-  scoped_refptr<PackExtensionJob> pack_job_;
-
-  // Returned by the SelectFileDialog machinery. Used to initiate the selection
-  // dialog.
-  scoped_refptr<ui::SelectFileDialog> load_extension_dialog_;
-
-  // Path to root directory of extension.
-  base::FilePath extension_path_;
-
-  // Path to private key file, or null if none specified.
-  base::FilePath private_key_path_;
-
-  // Path to the last used folder to load an extension.
-  base::FilePath last_used_path_;
-
-  DISALLOW_COPY_AND_ASSIGN(PackExtensionHandler);
-};
-
-}  // namespace extensions
-
-#endif  // CHROME_BROWSER_UI_WEBUI_EXTENSIONS_PACK_EXTENSION_HANDLER_H_

chrome/chrome_browser_ui.gypi

@@ -2504,8 +2504,6 @@
       'browser/ui/webui/extensions/extension_error_ui_util.h',
       'browser/ui/webui/extensions/extension_icon_source.cc',
       'browser/ui/webui/extensions/extension_icon_source.h',
-      'browser/ui/webui/extensions/pack_extension_handler.cc',
-      'browser/ui/webui/extensions/pack_extension_handler.h',
       'browser/ui/webui/voice_search_ui.cc',
       'browser/ui/webui/voice_search_ui.h',
     ],