Fix a crash bug on user script installation and add a test

to make sure it doesn't happen again.

Also, re-enable a test nearby. The history has been lost of
why this was failing, so it may fail again, but at least
we'll have data.

Finally, fix a Greasemonkey compat issue.

BUG=29298

Review URL: http://codereview.chromium.org/466053

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@33945 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 48 insertions, 12 deletions

diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index 9836bf6..bfd6ef4 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -36,11 +36,11 @@ void CrxInstaller::Start(const FilePath& crx_path,
                          bool allow_privilege_increase,
                          ExtensionsService* frontend,
                          ExtensionInstallUI* client) {
-  // Note: We don't keep a reference because this object manages its own
-  // lifetime.
-  CrxInstaller* installer = new CrxInstaller(crx_path, install_directory,
-                                             delete_source, frontend,
-                                             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;
@@ -49,7 +49,7 @@ void CrxInstaller::Start(const FilePath& crx_path,
       new SandboxedExtensionUnpacker(
           installer->source_file_,
           g_browser_process->resource_dispatcher_host(),
-          installer));
+          installer.get()));
 
   ChromeThread::PostTask(
       ChromeThread::FILE, FROM_HERE,
@@ -63,13 +63,15 @@ void CrxInstaller::InstallUserScript(const FilePath& source_file,
                                      bool delete_source,
                                      ExtensionsService* frontend,
                                      ExtensionInstallUI* client) {
-  CrxInstaller* installer = new CrxInstaller(source_file, install_directory,
-                                             delete_source, frontend, 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, &CrxInstaller::ConvertUserScriptOnFileThread));
+      NewRunnableMethod(installer.get(),
+                        &CrxInstaller::ConvertUserScriptOnFileThread));
 }
 
 CrxInstaller::CrxInstaller(const FilePath& source_file,
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index bccc0ac..dc8bdd6 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -12,6 +12,7 @@
 #include "base/path_service.h"
 #include "base/scoped_temp_dir.h"
 #include "base/string_util.h"
+#include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/extension_creator.h"
 #include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/extensions/external_extension_provider.h"
@@ -682,8 +683,7 @@ TEST_F(ExtensionsServiceTest, CleanupOnStartup) {
 // Test installing extensions. This test tries to install few extensions using
 // crx files. If you need to change those crx files, feel free to repackage
 // them, throw away the key used and change the id's above.
-// TODO(mad): http://crbug.com/26214
-TEST_F(ExtensionsServiceTest, DISABLED_InstallExtension) {
+TEST_F(ExtensionsServiceTest, InstallExtension) {
   InitializeEmptyExtensionsService();
 
   FilePath extensions_path;
@@ -742,6 +742,40 @@ TEST_F(ExtensionsServiceTest, DISABLED_InstallExtension) {
   // TODO(erikkay): add tests for upgrade cases.
 }
 
+// Install a user script (they get converted automatically to an extension)
+TEST_F(ExtensionsServiceTest, InstallUserScript) {
+  // The details of script conversion are tested elsewhere, this just tests
+  // integration with ExtensionsService.
+  InitializeEmptyExtensionsService();
+
+  FilePath path;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &path));
+  path = path.AppendASCII("extensions")
+             .AppendASCII("user_script_basic.user.js");
+
+  ASSERT_TRUE(file_util::PathExists(path));
+  CrxInstaller::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
+
+  loop_.RunAllPending();
+  std::vector<std::string> errors = GetErrors();
+  EXPECT_TRUE(installed_) << "Nothing was installed.";
+  ASSERT_EQ(1u, loaded_.size()) << "Nothing was loaded.";
+  EXPECT_EQ(0u, errors.size()) << "There were errors: "
+                               << JoinString(errors, ',');
+  EXPECT_TRUE(service_->GetExtensionById(loaded_[0]->id(), false)) <<
+              path.value();
+
+  installed_ = NULL;
+  loaded_.clear();
+  ExtensionErrorReporter::GetInstance()->ClearErrors();
+}
+
 // Test Packaging and installing an extension.
 TEST_F(ExtensionsServiceTest, PackExtension) {
   InitializeEmptyExtensionsService();
diff --git a/chrome/renderer/user_script_slave.cc b/chrome/renderer/user_script_slave.cc
index 68dec43..7ddbd71 100644
--- a/chrome/renderer/user_script_slave.cc
+++ b/chrome/renderer/user_script_slave.cc
@@ -189,7 +189,7 @@ bool UserScriptSlave::InjectScripts(WebFrame* frame,
 
         // We add this dumb function wrapper for standalone user script to
         // emulate what Greasemonkey does.
-        if (script->is_standalone()) {
+        if (script->is_standalone() || script->emulate_greasemonkey()) {
           content.insert(0, kUserScriptHead);
           content += kUserScriptTail;
         }