Mechanical refactor of CrxInstaller signatures. This will make
it easier to add additional configuration features to
CrxInstaller.
Review URL: http://codereview.chromium.org/875003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@41503 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index e083b60e..998b252 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -2452,14 +2452,12 @@
reply_message);
const FilePath& install_dir = service->install_directory();
- CrxInstaller::Start(crx_path,
- install_dir,
- Extension::INTERNAL,
- "", // no expected id
- false, // please do not delete crx file
- true, // privilege increase allowed
- service,
- NULL); // silent isntall, no UI
+ scoped_refptr<CrxInstaller> installer(
+ new CrxInstaller(install_dir,
+ service,
+ NULL)); // silent install, no UI
+ installer->set_allow_privilege_increase(true);
+ installer->InstallCrx(crx_path);
} else {
AutomationMsg_InstallExtension::WriteReplyParams(
reply_message, AUTOMATION_MSG_EXTENSION_INSTALL_FAILED);
diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index 423f037..e9ead6c 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -1438,28 +1438,23 @@
ExtensionsService* service = profile_->GetExtensionsService();
if (service) {
NotificationService* nservice = NotificationService::current();
- GURL nonconst_download_url = download_url;
- nservice->Notify(NotificationType::EXTENSION_READY_FOR_INSTALL,
- Source<DownloadManager>(this),
- Details<GURL>(&nonconst_download_url));
+ GURL nonconst_download_url = download_url;
+ nservice->Notify(NotificationType::EXTENSION_READY_FOR_INSTALL,
+ Source<DownloadManager>(this),
+ Details<GURL>(&nonconst_download_url));
+
+ scoped_refptr<CrxInstaller> installer(
+ new CrxInstaller(service->install_directory(),
+ service,
+ new ExtensionInstallUI(profile_)));
+ installer->set_delete_source(true);
+
if (UserScript::HasUserScriptFileExtension(download_url)) {
- CrxInstaller::InstallUserScript(
- full_path,
- download_url,
- service->install_directory(),
- true, // please delete crx on completion
- service,
- new ExtensionInstallUI(profile_));
+ installer->InstallUserScript(full_path, download_url);
} else {
- CrxInstaller::Start(
- full_path,
- service->install_directory(),
- Extension::INTERNAL,
- "", // no expected id
- true, // please delete crx on completion
- true, // privilege increase allowed
- service,
- new ExtensionInstallUI(profile_));
+ installer->set_allow_privilege_increase(true);
+ installer->set_original_url(download_url);
+ installer->InstallCrx(full_path);
}
} else {
TabContents* contents = NULL;
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index e4c5a93..71397f4 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -32,61 +32,12 @@
}
}
-void CrxInstaller::Start(const FilePath& crx_path,
- const FilePath& install_directory,
- Extension::Location install_source,
- const std::string& expected_id,
- bool delete_source,
- bool allow_privilege_increase,
- ExtensionsService* frontend,
- ExtensionInstallUI* client) {
- // Note: This object manages its own lifetime.
- scoped_refptr<CrxInstaller> installer(
- new CrxInstaller(crx_path, install_directory, delete_source, frontend,
- client));
-
- installer->install_source_ = install_source;
- installer->expected_id_ = expected_id;
- installer->allow_privilege_increase_ = allow_privilege_increase;
-
- scoped_refptr<SandboxedExtensionUnpacker> unpacker(
- new SandboxedExtensionUnpacker(
- installer->source_file_,
- g_browser_process->resource_dispatcher_host(),
- installer.get()));
-
- ChromeThread::PostTask(
- ChromeThread::FILE, FROM_HERE,
- NewRunnableMethod(
- unpacker.get(), &SandboxedExtensionUnpacker::Start));
-}
-
-void CrxInstaller::InstallUserScript(const FilePath& source_file,
- const GURL& original_url,
- const FilePath& install_directory,
- bool delete_source,
- ExtensionsService* frontend,
- ExtensionInstallUI* client) {
- scoped_refptr<CrxInstaller> installer(
- new CrxInstaller(source_file, install_directory, delete_source, frontend,
- client));
- installer->original_url_ = original_url;
-
- ChromeThread::PostTask(
- ChromeThread::FILE, FROM_HERE,
- NewRunnableMethod(installer.get(),
- &CrxInstaller::ConvertUserScriptOnFileThread));
-}
-
-CrxInstaller::CrxInstaller(const FilePath& source_file,
- const FilePath& install_directory,
- bool delete_source,
+CrxInstaller::CrxInstaller(const FilePath& install_directory,
ExtensionsService* frontend,
ExtensionInstallUI* client)
- : source_file_(source_file),
- install_directory_(install_directory),
+ : install_directory_(install_directory),
install_source_(Extension::INTERNAL),
- delete_source_(delete_source),
+ delete_source_(false),
allow_privilege_increase_(false),
create_app_shortcut_(false),
frontend_(frontend),
@@ -111,6 +62,33 @@
}
}
+void CrxInstaller::InstallCrx(const FilePath& source_file) {
+ source_file_ = source_file;
+
+ scoped_refptr<SandboxedExtensionUnpacker> unpacker(
+ new SandboxedExtensionUnpacker(
+ source_file,
+ g_browser_process->resource_dispatcher_host(),
+ this));
+
+ ChromeThread::PostTask(
+ ChromeThread::FILE, FROM_HERE,
+ NewRunnableMethod(
+ unpacker.get(), &SandboxedExtensionUnpacker::Start));
+}
+
+void CrxInstaller::InstallUserScript(const FilePath& source_file,
+ const GURL& original_url) {
+ DCHECK(!original_url.is_empty());
+
+ source_file_ = source_file;
+ original_url_ = original_url;
+
+ ChromeThread::PostTask(
+ ChromeThread::FILE, FROM_HERE,
+ NewRunnableMethod(this, &CrxInstaller::ConvertUserScriptOnFileThread));
+}
+
void CrxInstaller::ConvertUserScriptOnFileThread() {
std::string error;
Extension* extension = ConvertUserScriptToExtension(source_file_,
diff --git a/chrome/browser/extensions/crx_installer.h b/chrome/browser/extensions/crx_installer.h
index 0b19fef9..e2d4c023 100644
--- a/chrome/browser/extensions/crx_installer.h
+++ b/chrome/browser/extensions/crx_installer.h
@@ -33,54 +33,57 @@
// Additionally, we hold a reference to our own client so that it lives at least
// long enough to receive the result of unpacking.
//
-// TODO(aa): Pull out a frontend interface for testing?
+// IMPORTANT: Callers should keep a reference to a CrxInstaller while they are
+// working with it, eg:
+//
+// scoped_refptr<CrxInstaller> installer(new CrxInstaller(...));
+// installer->set_foo();
+// installer->set_bar();
+// installer->InstallCrx(...);
class CrxInstaller
: public SandboxedExtensionUnpackerClient,
public ExtensionInstallUI::Delegate {
public:
- // Starts the installation of the crx file in |source_file| into
- // |install_directory|.
- //
- // Other params:
- // install_source: The source of the install (external, --load-extension, etc
- // expected_id: Optional. If the caller knows what the ID of this extension
- // should be after unpacking, it can be specified here as a
- // sanity check.
- // delete_crx: Whether the crx should be deleted on completion.
- // frontend: The ExtensionsService to report the successfully installed
- // extension to.
- // client: Optional. If specified, will be used to confirm installation and
- // also notified of success/fail. Note that we hold a reference to
- // this, so it can outlive its creator (eg the UI).
- static void Start(const FilePath& source_file,
- const FilePath& install_directory,
- Extension::Location install_source,
- const std::string& expected_id,
- bool delete_source,
- bool allow_privilege_increase,
- ExtensionsService* frontend,
- ExtensionInstallUI* client);
+ // Constructor. Extensions will be unpacked to |install_directory|.
+ // Extension objects will be sent to |frontend|, and any UI will be shown
+ // via |client|. For silent install, pass NULL for |client|.
+ CrxInstaller(const FilePath& install_directory,
+ ExtensionsService* frontend,
+ ExtensionInstallUI* client);
- // Starts the installation of the user script file in |source_file| into
- // |install_directory|. The script will be converted to an extension.
- // See Start() for argument descriptions.
- static void InstallUserScript(const FilePath& source_file,
- const GURL& original_url,
- const FilePath& install_directory,
- bool delete_source,
- ExtensionsService* frontend,
- ExtensionInstallUI* client);
+ // Install the crx in |source_file|. Note that this will most likely
+ // complete asynchronously.
+ void InstallCrx(const FilePath& source_file);
+
+ // Install the user script in |source_file|. Note that this will most likely
+ // complete asynchronously.
+ void InstallUserScript(const FilePath& source_file,
+ const GURL& original_url);
// ExtensionInstallUI::Delegate
virtual void InstallUIProceed(bool create_app_shortcut);
virtual void InstallUIAbort();
+ const GURL& original_url() { return original_url_; }
+ void set_original_url(const GURL& val) { original_url_ = val; }
+
+ Extension::Location install_source() { return install_source_; }
+ void set_install_source(Extension::Location source) {
+ install_source_ = source;
+ }
+
+ const std::string& expected_id() { return expected_id_; }
+ void set_expected_id(const std::string& val) { expected_id_ = val; }
+
+ bool delete_source() { return delete_source_; }
+ void set_delete_source(bool val) { delete_source_ = val; }
+
+ bool allow_privilege_increase() { return allow_privilege_increase_; }
+ void set_allow_privilege_increase(bool val) {
+ allow_privilege_increase_ = val;
+ }
+
private:
- CrxInstaller(const FilePath& source_file,
- const FilePath& install_directory,
- bool delete_source,
- ExtensionsService* frontend,
- ExtensionInstallUI* client);
~CrxInstaller();
// Converts the source user script to an extension.
@@ -111,7 +114,7 @@
// The file we're installing.
FilePath source_file_;
- // The URL the file was downloaded from. Only used for user scripts.
+ // The URL the file was downloaded from.
GURL original_url_;
// The directory extensions are installed to.
@@ -119,7 +122,7 @@
// The location the installation came from (bundled with Chromium, registry,
// manual install, etc). This metadata is saved with the installation if
- // successful.
+ // successful. Defaults to INTERNAL.
Extension::Location install_source_;
// For updates and external installs we have an ID we're expecting the
@@ -131,11 +134,16 @@
// allowed.
bool extensions_enabled_;
- // Whether we're supposed to delete the source file on destruction.
+ // Whether we're supposed to delete the source file on destruction. Defaults
+ // to false.
bool delete_source_;
// Whether privileges should be allowed to silently increaes from any
- // previously installed version of the extension.
+ // previously installed version of the extension. This is used for things
+ // like external extensions, where extensions come with third-party software
+ // or are distributed by the network administrator. There is no UI shown
+ // for these extensions, so there shouldn't be UI for privilege increase,
+ // either. Defaults to false.
bool allow_privilege_increase_;
// Whether to create an app shortcut after successful installation. This is
diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc
index 9f7cdcf..7041fab 100644
--- a/chrome/browser/extensions/extension_browsertest.cc
+++ b/chrome/browser/extensions/extension_browsertest.cc
@@ -128,15 +128,14 @@
NotificationService::AllSources());
registrar.Add(this, NotificationType::EXTENSION_INSTALL_ERROR,
NotificationService::AllSources());
- CrxInstaller::Start(
- path,
- service->install_directory(),
- Extension::INTERNAL,
- id,
- false, // do not delete after install
- id == "" ? true : false, // only allow privilege increase for installs
- service,
- should_cancel ? new MockAbortExtensionInstallUI() : NULL);
+
+ scoped_refptr<CrxInstaller> installer(
+ new CrxInstaller(
+ service->install_directory(),
+ service,
+ should_cancel ? new MockAbortExtensionInstallUI() : NULL));
+ installer->set_expected_id(id);
+ installer->InstallCrx(path);
MessageLoop::current()->PostDelayedTask(
FROM_HERE, new MessageLoop::QuitTask, kTimeoutMs);
diff --git a/chrome/browser/extensions/extension_browsertest.h b/chrome/browser/extensions/extension_browsertest.h
index 8a34903..51f5c597d 100644
--- a/chrome/browser/extensions/extension_browsertest.h
+++ b/chrome/browser/extensions/extension_browsertest.h
@@ -36,8 +36,8 @@
return InstallOrUpdateExtension("", path, false, expected_change);
}
- // Same as above but passes an id to CrxInstaller::Start and does not allow
- // a privilege increase.
+ // Same as above but passes an id to CrxInstaller and does not allow a
+ // privilege increase.
bool UpdateExtension(const std::string& id, const FilePath& path,
int expected_change) {
return InstallOrUpdateExtension(id, path, false, expected_change);
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index fc2dadff..8870a06 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -193,12 +193,12 @@
}
void ExtensionsService::InstallExtension(const FilePath& extension_path) {
- CrxInstaller::Start(extension_path, install_directory_, Extension::INTERNAL,
- "", // no expected id
- false, // don't delete crx when complete
- true, // allow privilege increase
- this,
- NULL); // no client (silent install)
+ scoped_refptr<CrxInstaller> installer(
+ new CrxInstaller(install_directory_,
+ this, // frontend
+ NULL)); // no client (silent install)
+ installer->set_allow_privilege_increase(true);
+ installer->InstallCrx(extension_path);
}
void ExtensionsService::UpdateExtension(const std::string& id,
@@ -209,12 +209,13 @@
return;
}
- CrxInstaller::Start(extension_path, install_directory_, Extension::INTERNAL,
- id,
- true, // delete crx when complete
- false, // do not allow upgrade of privileges
- this,
- NULL); // no client (silent install)
+ scoped_refptr<CrxInstaller> installer(
+ new CrxInstaller(install_directory_,
+ this, // frontend
+ NULL)); // no client (silent install)
+ installer->set_expected_id(id);
+ installer->set_delete_source(true);
+ installer->InstallCrx(extension_path);
}
void ExtensionsService::ReloadExtension(const std::string& extension_id) {
@@ -905,11 +906,14 @@
}
}
- CrxInstaller::Start(path, install_directory_, location, id,
- false, // don't delete crx when complete
- true, // allow privilege increase
- this,
- NULL); // no client (silent install)
+ scoped_refptr<CrxInstaller> installer(
+ new CrxInstaller(install_directory_,
+ this, // frontend
+ NULL)); // no client (silent install)
+ installer->set_install_source(location);
+ installer->set_expected_id(id);
+ installer->set_allow_privilege_increase(true);
+ installer->InstallCrx(path);
}
void ExtensionsService::ReportExtensionLoadError(
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index e866d26..e5cf7b4 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -798,13 +798,13 @@
.AppendASCII("user_script_basic.user.js");
ASSERT_TRUE(file_util::PathExists(path));
- CrxInstaller::InstallUserScript(
+ scoped_refptr<CrxInstaller> installer(
+ new CrxInstaller(service_->install_directory(),
+ service_,
+ NULL)); // silent install
+ installer->InstallUserScript(
path,
- GURL("http://www.aaronboodman.com/scripts/user_script_basic.user.js"),
- service_->install_directory(),
- false, // do not delete source
- service_,
- NULL); // install UI
+ GURL("http://www.aaronboodman.com/scripts/user_script_basic.user.js"));
loop_.RunAllPending();
std::vector<std::string> errors = GetErrors();
