diff options
Diffstat (limited to 'chrome/browser/extensions')
12 files changed, 489 insertions, 543 deletions
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc new file mode 100644 index 0000000..6b6c732 --- /dev/null +++ b/chrome/browser/extensions/crx_installer.cc @@ -0,0 +1,194 @@ +// Copyright (c) 2009 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/extensions/crx_installer.h" + +#include "app/l10n_util.h" +#include "base/file_util.h" +#include "base/scoped_temp_dir.h" +#include "base/string_util.h" +#include "base/task.h" +#include "chrome/browser/browser_process.h" +#include "chrome/browser/extensions/extension_file_util.h" +#include "chrome/common/extensions/extension_error_reporter.h" +#include "grit/chromium_strings.h" + +#if defined(OS_WIN) +#include "app/win_util.h" +#elif defined(OS_MACOSX) +#include "base/scoped_cftyperef.h" +#include "base/sys_string_conversions.h" +#include <CoreFoundation/CFUserNotification.h> +#endif + +CrxInstaller::CrxInstaller(const FilePath& crx_path, + const FilePath& install_directory, + Extension::Location install_source, + const std::string& expected_id, + bool extensions_enabled, + bool is_from_gallery, + bool show_prompts, + bool delete_crx, + MessageLoop* file_loop, + ExtensionsService* frontend) + : crx_path_(crx_path), + install_directory_(install_directory), + install_source_(install_source), + expected_id_(expected_id), + extensions_enabled_(extensions_enabled), + is_from_gallery_(is_from_gallery), + show_prompts_(show_prompts), + delete_crx_(delete_crx), + file_loop_(file_loop), + ui_loop_(MessageLoop::current()) { + + // Note: this is a refptr so that we keep the frontend alive long enough to + // get our response. + frontend_ = frontend; + unpacker_ = new SandboxedExtensionUnpacker( + crx_path, g_browser_process->resource_dispatcher_host(), this); + + file_loop->PostTask(FROM_HERE, NewRunnableMethod(unpacker_, + &SandboxedExtensionUnpacker::Start)); +} + +void CrxInstaller::OnUnpackFailure(const std::string& error_message) { + ReportFailureFromFileThread(error_message); +} + +void CrxInstaller::OnUnpackSuccess(const FilePath& temp_dir, + const FilePath& extension_dir, + Extension* extension) { + // Note: We take ownership of |extension| and |temp_dir|. + extension_.reset(extension); + temp_dir_ = temp_dir; + + // temp_dir_deleter is stack allocated instead of a member of CrxInstaller, so + // that delete always happens on the file thread. + ScopedTempDir temp_dir_deleter; + temp_dir_deleter.Set(temp_dir); + + // The unpack dir we don't have to delete explicity since it is a child of + // the temp dir. + unpacked_extension_root_ = extension_dir; + DCHECK(file_util::ContainsPath(temp_dir_, unpacked_extension_root_)); + + // If we were supposed to delete the source file, we can do that now. + if (delete_crx_) + file_util::Delete(crx_path_, false); // non-recursive + + // Determine whether to allow installation. We always allow themes and + // external installs. + if (!extensions_enabled_ && !extension->IsTheme() && + !Extension::IsExternalLocation(install_source_)) { + ReportFailureFromFileThread("Extensions are not enabled."); + return; + } + + // Make sure the expected id matches. + // TODO(aa): Also support expected version? + if (!expected_id_.empty() && expected_id_ != extension->id()) { + ReportFailureFromFileThread( + StringPrintf("ID in new extension manifest (%s) does not match " + "expected id (%s)", + extension->id().c_str(), + expected_id_.c_str())); + return; + } + + // Show the confirm UI if necessary. + // NOTE: We also special case themes to not have a dialog, because we show + // a special infobar UI for them instead. + if (show_prompts_ && !extension->IsTheme()) { + if (!ConfirmInstall()) + return; // error reported by ConfirmInstall() + } + + CompleteInstall(); +} + +bool CrxInstaller::ConfirmInstall() { +#if defined(OS_WIN) + if (win_util::MessageBox(GetForegroundWindow(), + L"Are you sure you want to install this extension?\n\n" + L"You should only install extensions from sources you trust.", + l10n_util::GetString(IDS_PRODUCT_NAME).c_str(), + MB_OKCANCEL) != IDOK) { + ReportFailureFromFileThread("User did not allow extension to be " + "installed."); + return false; + } +#elif defined(OS_MACOSX) + // Using CoreFoundation to do this dialog is unimaginably lame but will do + // until the UI is redone. + scoped_cftyperef<CFStringRef> product_name( + base::SysWideToCFStringRef(l10n_util::GetString(IDS_PRODUCT_NAME))); + CFOptionFlags response; + if (kCFUserNotificationAlternateResponse == CFUserNotificationDisplayAlert( + 0, kCFUserNotificationCautionAlertLevel, NULL, NULL, NULL, + product_name, + CFSTR("Are you sure you want to install this extension?\n\n" + "This is a temporary message and it will be removed when " + "extensions UI is finalized."), + NULL, CFSTR("Cancel"), NULL, &response)) { + ReportFailureFromFileThread("User did not allow extension to be " + "installed."); + return false; + } +#endif // OS_* + + return true; +} + +void CrxInstaller::CompleteInstall() { + FilePath version_dir; + Extension::InstallType install_type = Extension::INSTALL_ERROR; + std::string error_msg; + if (!extension_file_util::InstallExtension(unpacked_extension_root_, + install_directory_, + extension_->id(), + extension_->VersionString(), + &version_dir, + &install_type, &error_msg)) { + ReportFailureFromFileThread(error_msg); + return; + } + + if (install_type == Extension::DOWNGRADE) { + ReportFailureFromFileThread("Attempted to downgrade extension."); + return; + } + + extension_->set_path(version_dir); + extension_->set_location(install_source_); + + if (install_type == Extension::REINSTALL) { + // We use this as a signal to switch themes. + ReportOverinstallFromFileThread(); + return; + } + + ReportSuccessFromFileThread(); +} + +void CrxInstaller::ReportFailureFromFileThread(const std::string& error) { + ui_loop_->PostTask(FROM_HERE, NewRunnableMethod(this, + &CrxInstaller::ReportFailureFromUIThread, error)); +} + +void CrxInstaller::ReportFailureFromUIThread(const std::string& error) { + ExtensionErrorReporter::GetInstance()->ReportError(error, show_prompts_); +} + +void CrxInstaller::ReportOverinstallFromFileThread() { + ui_loop_->PostTask(FROM_HERE, NewRunnableMethod(frontend_.get(), + &ExtensionsService::OnExtensionOverinstallAttempted, extension_->id())); +} + +void CrxInstaller::ReportSuccessFromFileThread() { + // Tell the frontend about the installation and hand off ownership of + // extension_ to it. + ui_loop_->PostTask(FROM_HERE, NewRunnableMethod(frontend_.get(), + &ExtensionsService::OnExtensionInstalled, extension_.release())); +} diff --git a/chrome/browser/extensions/crx_installer.h b/chrome/browser/extensions/crx_installer.h new file mode 100644 index 0000000..9e5ff7c --- /dev/null +++ b/chrome/browser/extensions/crx_installer.h @@ -0,0 +1,141 @@ +// Copyright (c) 2009 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_EXTENSIONS_CRX_INSTALLER_H_ +#define CHROME_BROWSER_EXTENSIONS_CRX_INSTALLER_H_ + +#include <string> + +#include "base/file_path.h" +#include "base/message_loop.h" +#include "base/ref_counted.h" +#include "chrome/browser/extensions/extensions_service.h" +#include "chrome/browser/extensions/sandboxed_extension_unpacker.h" +#include "chrome/common/extensions/extension.h" + +// This class installs a crx file into a profile. +// +// Installing a CRX is a multi-step process, including unpacking the crx, +// validating it, prompting the user, and installing. Since many of these +// steps must occur on the file thread, this class contains a copy of all data +// necessary to do its job. (This also minimizes external dependencies for +// easier testing). +// +// +// Lifetime management: +// +// This class is ref-counted by each call it makes to itself on another thread, +// and by UtilityProcessHost. +// +// Additionally, we hold a reference to our own client so that it lives at least +// long enough to receive the result of unpacking. +// +// +// NOTE: This class is rather huge at the moment because it is handling all +// types of installation (external, autoupdate, and manual). In the future, +// manual installation will probably pulled out of it. +// +// TODO(aa): Pull out the manual installation bits. +// TODO(aa): Pull out a frontend interface for testing? +class CrxInstaller : public SandboxedExtensionUnpackerClient { + public: + CrxInstaller(const FilePath& crx_path, + const FilePath& install_directory, + Extension::Location install_source, + const std::string& expected_id, + bool extensions_enabled, + bool is_from_gallery, + bool show_prompts, + bool delete_crx, + MessageLoop* file_loop, + ExtensionsService* frontend); + ~CrxInstaller() { + // This is only here for debugging purposes, as a convenient place to set + // breakpoints. + } + + private: + // SandboxedExtensionUnpackerClient + virtual void OnUnpackFailure(const std::string& error_message); + virtual void OnUnpackSuccess(const FilePath& temp_dir, + const FilePath& extension_dir, + Extension* extension); + + // Confirm with the user that it is OK to install this extension. + // + // Note that this runs on the file thread. It happens to be OK to do this on + // Windows and Mac, and although ugly, we leave it because this is all getting + // pulled out soon, anyway. + // + // TODO(aa): Pull this up, closer to the UI layer. + bool ConfirmInstall(); + + // Runs on File thread. Install the unpacked extension into the profile and + // notify the frontend. + void CompleteInstall(); + + // Result reporting. + void ReportFailureFromFileThread(const std::string& error); + void ReportFailureFromUIThread(const std::string& error); + void ReportOverinstallFromFileThread(); + void ReportSuccessFromFileThread(); + + // The crx file we're installing. + FilePath crx_path_; + + // The directory extensions are installed to. + FilePath install_directory_; + + // The location the installation came from (bundled with Chromium, registry, + // manual install, etc). This metadata is saved with the installation if + // successful. + Extension::Location install_source_; + + // For updates and external installs we have an ID we're expecting the + // extension to contain. + std::string expected_id_; + + // Whether extension installation is set. We can't just check this before + // trying to install because themes are special-cased to always be allowed. + bool extensions_enabled_; + + // Whether this installation was initiated from the gallery. We trust it more + // and have special UI if it was. + bool is_from_gallery_; + + // Whether we shoud should show prompts. This is sometimes false for testing + // and autoupdate. + bool show_prompts_; + + // Whether we're supposed to delete the source crx file on destruction. + bool delete_crx_; + + // The message loop to use for file IO. + MessageLoop* file_loop_; + + // The message loop the UI is running on. + MessageLoop* ui_loop_; + + // The extension we're installing. We own this and either pass it off to + // ExtensionsService on success, or delete it on failure. + scoped_ptr<Extension> extension_; + + // The temp directory extension resources were unpacked to. We own this and + // must delete it when we are done with it. + FilePath temp_dir_; + + // The frontend we will report results back to. + scoped_refptr<ExtensionsService> frontend_; + + // The root of the unpacked extension directory. This is a subdirectory of + // temp_dir_, so we don't have to delete it explicitly. + FilePath unpacked_extension_root_; + + // The unpacker we will use to unpack the extension. + SandboxedExtensionUnpacker* unpacker_; + + DISALLOW_COPY_AND_ASSIGN(CrxInstaller); +}; + +#endif // CHROME_BROWSER_EXTENSIONS_CRX_INSTALLER_H_ diff --git a/chrome/browser/extensions/extension_updater.cc b/chrome/browser/extensions/extension_updater.cc index 4d5ef16f..e41071b 100644 --- a/chrome/browser/extensions/extension_updater.cc +++ b/chrome/browser/extensions/extension_updater.cc @@ -209,14 +209,7 @@ void ExtensionUpdater::OnCRXFetchComplete(const GURL& url, void ExtensionUpdater::OnCRXFileWritten(const std::string& id, const FilePath& path) { - // TODO(asargent) - Instead of calling InstallExtension here, we should have - // an UpdateExtension method in ExtensionsService and rely on it to check - // that the extension is still installed, and still an older version than - // what we're handing it. - // (http://crbug.com/12764). - ExtensionInstallCallback* callback = - NewCallback(this, &ExtensionUpdater::OnExtensionInstallFinished); - service_->UpdateExtension(id, path, false, callback); + service_->UpdateExtension(id, path); } void ExtensionUpdater::OnExtensionInstallFinished(const FilePath& path, diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc index 263ebfd..3db5fed 100644 --- a/chrome/browser/extensions/extension_updater_unittest.cc +++ b/chrome/browser/extensions/extension_updater_unittest.cc @@ -96,9 +96,7 @@ class MockService : public ExtensionUpdateService { } virtual void UpdateExtension(const std::string& id, - const FilePath& extension_path, - bool alert_on_error, - ExtensionInstallCallback* callback) { + const FilePath& extension_path) { EXPECT_TRUE(false); } @@ -162,33 +160,10 @@ class ServiceForManifestTests : public MockService { class ServiceForDownloadTests : public MockService { public: - explicit ServiceForDownloadTests() : callback_(NULL) {} - virtual ~ServiceForDownloadTests() { - // expect that cleanup happened via FireInstallCallback - EXPECT_EQ(NULL, callback_); - EXPECT_TRUE(install_path_.empty()); - } - virtual void UpdateExtension(const std::string& id, - const FilePath& extension_path, - bool alert_on_error, - ExtensionInstallCallback* callback) { - // Since this mock only has support for one oustanding update, ensure - // that the callback doesn't need to be run. - EXPECT_TRUE(install_path_.empty()); - EXPECT_EQ(NULL, callback_); - + const FilePath& extension_path) { extension_id_ = id; install_path_ = extension_path; - callback_ = callback; - } - - void FireInstallCallback() { - EXPECT_TRUE(callback_ != NULL); - callback_->Run(install_path_, static_cast<Extension*>(NULL)); - delete callback_; - callback_ = NULL; - install_path_ = FilePath(); } const std::string& extension_id() { return extension_id_; } @@ -197,7 +172,6 @@ class ServiceForDownloadTests : public MockService { private: std::string extension_id_; FilePath install_path_; - ExtensionInstallCallback* callback_; }; static const int kUpdateFrequencySecs = 15; @@ -418,13 +392,6 @@ class ExtensionUpdaterTest : public testing::Test { EXPECT_TRUE(file_util::ReadFileToString(tmpfile_path, &file_contents)); EXPECT_TRUE(extension_data == file_contents); - service.FireInstallCallback(); - - message_loop.RunAllPending(); - - // Make sure the temp file is cleaned up - EXPECT_FALSE(file_util::PathExists(tmpfile_path)); - URLFetcher::set_factory(NULL); } @@ -456,17 +423,11 @@ class ExtensionUpdaterTest : public testing::Test { extension_data1); message_loop.RunAllPending(); - // Expect that the service was asked to do an install with the right data, - // and fire the callback indicating the install finished. + // Expect that the service was asked to do an install with the right data. FilePath tmpfile_path = service.install_path(); EXPECT_FALSE(tmpfile_path.empty()); EXPECT_EQ(id1, service.extension_id()); - service.FireInstallCallback(); - - // Make sure the tempfile got cleaned up. message_loop.RunAllPending(); - EXPECT_FALSE(tmpfile_path.empty()); - EXPECT_FALSE(file_util::PathExists(tmpfile_path)); // Make sure the second fetch finished and asked the service to do an // update. @@ -485,8 +446,6 @@ class ExtensionUpdaterTest : public testing::Test { EXPECT_TRUE(file_util::ReadFileToString(service.install_path(), &file_contents)); EXPECT_TRUE(extension_data2 == file_contents); - service.FireInstallCallback(); - message_loop.RunAllPending(); } }; diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc index 0733380..adc554f 100644 --- a/chrome/browser/extensions/extensions_service.cc +++ b/chrome/browser/extensions/extensions_service.cc @@ -4,18 +4,13 @@ #include "chrome/browser/extensions/extensions_service.h" -#include "app/l10n_util.h" #include "base/command_line.h" #include "base/file_util.h" #include "base/string_util.h" #include "base/values.h" -#include "chrome/browser/browser.h" -#include "chrome/browser/browser_list.h" -#include "chrome/browser/browser_process.h" -#include "chrome/browser/chrome_thread.h" +#include "chrome/browser/extensions/crx_installer.h" #include "chrome/browser/extensions/extension_browser_event_router.h" #include "chrome/browser/extensions/extension_file_util.h" -#include "chrome/browser/extensions/extension_process_manager.h" #include "chrome/browser/extensions/extension_updater.h" #include "chrome/browser/extensions/external_extension_provider.h" #include "chrome/browser/extensions/external_pref_extension_provider.h" @@ -29,16 +24,9 @@ #include "chrome/common/pref_names.h" #include "chrome/common/pref_service.h" #include "chrome/common/url_constants.h" -#include "grit/chromium_strings.h" -#include "grit/generated_resources.h" #if defined(OS_WIN) -#include "app/win_util.h" #include "chrome/browser/extensions/external_registry_extension_provider_win.h" -#elif defined(OS_MACOSX) -#include "base/scoped_cftyperef.h" -#include "base/sys_string_conversions.h" -#include <CoreFoundation/CFUserNotification.h> #endif // ExtensionsService. @@ -62,61 +50,6 @@ bool ExtensionsService::IsDownloadFromGallery(const GURL& download_url, } } -// This class hosts a SandboxedExtensionUnpacker task and routes the results -// back to ExtensionsService. The unpack process is started immediately on -// construction of this object. -class ExtensionsServiceBackend::UnpackerClient - : public SandboxedExtensionUnpackerClient { - public: - UnpackerClient(ExtensionsServiceBackend* backend, - const FilePath& extension_path, - const std::string& expected_id, - bool silent, bool from_gallery) - : backend_(backend), extension_path_(extension_path), - expected_id_(expected_id), silent_(silent), from_gallery_(from_gallery) { - unpacker_ = new SandboxedExtensionUnpacker(extension_path, - backend->resource_dispatcher_host_, this); - unpacker_->Start(); - } - - private: - // SandboxedExtensionUnpackerClient - virtual void OnUnpackSuccess(const FilePath& temp_dir, - const FilePath& extension_dir, - Extension* extension) { - backend_->OnExtensionUnpacked(extension_path_, extension_dir, extension, - expected_id_, silent_, from_gallery_); - file_util::Delete(temp_dir, true); - delete this; - } - - virtual void OnUnpackFailure(const std::string& error_message) { - backend_->ReportExtensionInstallError(extension_path_, error_message); - delete this; - } - - scoped_refptr<SandboxedExtensionUnpacker> unpacker_; - - scoped_refptr<ExtensionsServiceBackend> backend_; - - // The path to the crx file that we're installing. - FilePath extension_path_; - - // The path to the copy of the crx file in the temporary directory where we're - // unpacking it. - FilePath temp_extension_path_; - - // The ID we expect this extension to have, if any. - std::string expected_id_; - - // True if the install should be done with no confirmation dialog. - bool silent_; - - // True if the install is from the gallery (and therefore should not get an - // alert UI if it turns out to also be a theme). - bool from_gallery_; -}; - ExtensionsService::ExtensionsService(Profile* profile, const CommandLine* command_line, PrefService* prefs, @@ -147,9 +80,7 @@ ExtensionsService::ExtensionsService(Profile* profile, updater_ = new ExtensionUpdater(this, update_frequency, backend_loop_); } - backend_ = new ExtensionsServiceBackend( - install_directory_, g_browser_process->resource_dispatcher_host(), - frontend_loop, extensions_enabled()); + backend_ = new ExtensionsServiceBackend(install_directory_, frontend_loop); } ExtensionsService::~ExtensionsService() { @@ -159,12 +90,6 @@ ExtensionsService::~ExtensionsService() { } } -void ExtensionsService::SetExtensionsEnabled(bool enabled) { - extensions_enabled_ = true; - backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(), - &ExtensionsServiceBackend::set_extensions_enabled, enabled)); -} - void ExtensionsService::Init() { DCHECK(!ready_); DCHECK_EQ(extensions_.size(), 0u); @@ -189,41 +114,32 @@ void ExtensionsService::InstallExtension(const FilePath& extension_path) { void ExtensionsService::InstallExtension(const FilePath& extension_path, const GURL& download_url, const GURL& referrer_url) { - backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(), - &ExtensionsServiceBackend::InstallExtension, extension_path, - IsDownloadFromGallery(download_url, referrer_url), - scoped_refptr<ExtensionsService>(this))); - + new CrxInstaller(extension_path, install_directory_, Extension::INTERNAL, + "", // no expected id + extensions_enabled_, + IsDownloadFromGallery(download_url, referrer_url), + show_extensions_prompts(), + false, // don't delete crx when complete + backend_loop_, + this); } void ExtensionsService::UpdateExtension(const std::string& id, - const FilePath& extension_path, - bool alert_on_error, - ExtensionInstallCallback* callback) { - if (callback) { - if (install_callbacks_.find(extension_path) != install_callbacks_.end()) { - // We can't have multiple outstanding install requests for the same - // path, so immediately indicate error via the callback here. - LOG(WARNING) << "Dropping update request for '" << - extension_path.value() << "' (already in progress)'"; - callback->Run(extension_path, static_cast<Extension*>(NULL)); - delete callback; - return; - } - install_callbacks_[extension_path] = - linked_ptr<ExtensionInstallCallback>(callback); - } + const FilePath& extension_path) { - if (!GetExtensionById(id)) { + if (!GetExtensionById(id)) { LOG(WARNING) << "Will not update extension " << id << " because it is not " - << "installed"; - FireInstallCallback(extension_path, NULL); - return; + << "installed"; + return; } - backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(), - &ExtensionsServiceBackend::UpdateExtension, id, extension_path, - alert_on_error, scoped_refptr<ExtensionsService>(this))); + new CrxInstaller(extension_path, install_directory_, Extension::INTERNAL, + id, extensions_enabled_, + false, // not from gallery + show_extensions_prompts(), + true, // delete crx when complete + backend_loop_, + this); } void ExtensionsService::ReloadExtension(const std::string& extension_id) { @@ -269,6 +185,8 @@ void ExtensionsService::LoadAllExtensions() { void ExtensionsService::CheckForExternalUpdates() { // This installs or updates externally provided extensions. + // TODO(aa): Why pass this list into the provider, why not just filter it + // later? std::set<std::string> killed_extensions; extension_prefs_->GetKilledExtensionIds(&killed_extensions); backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(), @@ -388,9 +306,7 @@ void ExtensionsService::OnExtensionsLoaded(ExtensionList* new_extensions) { } } -void ExtensionsService::OnExtensionInstalled(const FilePath& path, - Extension* extension, Extension::InstallType install_type) { - FireInstallCallback(path, extension); +void ExtensionsService::OnExtensionInstalled(Extension* extension) { extension_prefs_->OnExtensionInstalled(extension); // If the extension is a theme, tell the profile (and therefore ThemeProvider) @@ -407,25 +323,15 @@ void ExtensionsService::OnExtensionInstalled(const FilePath& path, Source<ExtensionsService>(this), Details<Extension>(extension)); } -} - -void ExtensionsService::OnExtenionInstallError(const FilePath& path) { - FireInstallCallback(path, NULL); + // Also load the extension. + ExtensionList* list = new ExtensionList; + list->push_back(extension); + OnExtensionsLoaded(list); } -void ExtensionsService::FireInstallCallback(const FilePath& path, - Extension* extension) { - CallbackMap::iterator iter = install_callbacks_.find(path); - if (iter != install_callbacks_.end()) { - iter->second->Run(path, extension); - install_callbacks_.erase(iter); - } -} -void ExtensionsService::OnExtensionOverinstallAttempted(const std::string& id, - const FilePath& path) { - FireInstallCallback(path, NULL); +void ExtensionsService::OnExtensionOverinstallAttempted(const std::string& id) { Extension* extension = GetExtensionById(id); if (extension && extension->IsTheme()) { ShowThemePreviewInfobar(extension); @@ -480,17 +386,50 @@ bool ExtensionsService::ShowThemePreviewInfobar(Extension* extension) { return true; } +void ExtensionsService::OnExternalExtensionFound(const std::string& id, + const std::string& version, + const FilePath& path, + Extension::Location location) { + // Before even bothering to unpack, check and see if we already have this + // version. This is important because these extensions are going to get + // installed on every startup. + Extension* existing = GetExtensionById(id); + if (existing) { + switch (existing->version()->CompareTo( + *Version::GetVersionFromString(version))) { + case -1: // existing version is older, we should upgrade + break; + case 0: // existing version is same, do nothing + return; + case 1: // existing version is newer, uh-oh + LOG(WARNING) << "Found external version of extension " << id + << "that is older than current version. Current version " + << "is: " << existing->VersionString() << ". New version " + << "is: " << version << ". Keeping current version."; + return; + } + } + + new CrxInstaller(path, install_directory_, location, id, extensions_enabled_, + false, // not from gallery + show_extensions_prompts(), + false, // don't delete crx when complete + backend_loop_, + this); +} + + // ExtensionsServicesBackend ExtensionsServiceBackend::ExtensionsServiceBackend( - const FilePath& install_directory, ResourceDispatcherHost* rdh, - MessageLoop* frontend_loop, bool extensions_enabled) + const FilePath& install_directory, MessageLoop* frontend_loop) : frontend_(NULL), install_directory_(install_directory), - resource_dispatcher_host_(rdh), alert_on_error_(false), - frontend_loop_(frontend_loop), - extensions_enabled_(extensions_enabled) { + frontend_loop_(frontend_loop) { + // TODO(aa): This ends up doing blocking IO on the UI thread because it reads + // pref data in the ctor and that is called on the UI thread. Would be better + // to re-read data each time we list external extensions, anyway. external_extension_providers_[Extension::EXTERNAL_PREF] = linked_ptr<ExternalExtensionProvider>( new ExternalPrefExtensionProvider()); @@ -597,182 +536,6 @@ void ExtensionsServiceBackend::ReportExtensionsLoaded( frontend_, &ExtensionsService::OnExtensionsLoaded, extensions)); } -void ExtensionsServiceBackend::InstallExtension( - const FilePath& extension_path, bool from_gallery, - scoped_refptr<ExtensionsService> frontend) { - LOG(INFO) << "Installing extension " << extension_path.value(); - - frontend_ = frontend; - alert_on_error_ = true; - - InstallOrUpdateExtension(extension_path, from_gallery, std::string(), false); -} - -void ExtensionsServiceBackend::UpdateExtension(const std::string& id, - const FilePath& extension_path, bool alert_on_error, - scoped_refptr<ExtensionsService> frontend) { - LOG(INFO) << "Updating extension " << id << " " << extension_path.value(); - - frontend_ = frontend; - alert_on_error_ = alert_on_error; - - InstallOrUpdateExtension(extension_path, false, id, true); -} - -void ExtensionsServiceBackend::InstallOrUpdateExtension( - const FilePath& extension_path, bool from_gallery, - const std::string& expected_id, bool silent) { - // NOTE: We don't need to keep a reference to this, it deletes itself when it - // is done. - new UnpackerClient(this, extension_path, expected_id, silent, from_gallery); -} - -void ExtensionsServiceBackend::OnExtensionUnpacked( - const FilePath& crx_path, const FilePath& unpacked_path, - Extension* extension, const std::string expected_id, bool silent, - bool from_gallery) { - // Take ownership of the extension object. - scoped_ptr<Extension> extension_deleter(extension); - - Extension::Location location = Extension::INTERNAL; - LookupExternalExtension(extension->id(), NULL, &location); - extension->set_location(location); - - bool allow_install = false; - if (extensions_enabled_) - allow_install = true; - - // Always allow themes. - if (extension->IsTheme()) - allow_install = true; - - // Always allow externally installed extensions (partners use this). - if (Extension::IsExternalLocation(location)) - allow_install = true; - - if (!allow_install) { - ReportExtensionInstallError(crx_path, "Extensions are not enabled."); - return; - } - - // TODO(extensions): Make better extensions UI. http://crbug.com/12116 - - // We also skip the dialog for a few special cases: - // - themes (because we show the preview infobar for them) - // - externally registered extensions - // - during tests (!frontend->show_extension_prompts()) - // - autoupdate (silent). - bool show_dialog = true; - if (extension->IsTheme()) - show_dialog = false; - - if (Extension::IsExternalLocation(location)) - show_dialog = false; - - if (silent || !frontend_->show_extensions_prompts()) - show_dialog = false; - - if (show_dialog) { -#if defined(OS_WIN) - if (win_util::MessageBox(GetForegroundWindow(), - L"Are you sure you want to install this extension?\n\n" - L"You should only install extensions from sources you trust.", - l10n_util::GetString(IDS_PRODUCT_NAME).c_str(), - MB_OKCANCEL) != IDOK) { - return; - } -#elif defined(OS_MACOSX) - // Using CoreFoundation to do this dialog is unimaginably lame but will do - // until the UI is redone. - scoped_cftyperef<CFStringRef> product_name( - base::SysWideToCFStringRef(l10n_util::GetString(IDS_PRODUCT_NAME))); - CFOptionFlags response; - CFUserNotificationDisplayAlert( - 0, kCFUserNotificationCautionAlertLevel, NULL, NULL, NULL, - product_name, - CFSTR("Are you sure you want to install this extension?\n\n" - "This is a temporary message and it will be removed when " - "extensions UI is finalized."), - NULL, CFSTR("Cancel"), NULL, &response); - - if (response == kCFUserNotificationAlternateResponse) { - ReportExtensionInstallError(crx_path, - "User did not allow extension to be installed."); - return; - } -#endif // OS_* - } - - // If an expected id was provided, make sure it matches. - if (!expected_id.empty() && expected_id != extension->id()) { - ReportExtensionInstallError(crx_path, - StringPrintf("ID in new extension manifest (%s) does not match " - "expected id (%s)", extension->id().c_str(), - expected_id.c_str())); - return; - } - - FilePath version_dir; - Extension::InstallType install_type = Extension::INSTALL_ERROR; - std::string error_msg; - if (!extension_file_util::InstallExtension(unpacked_path, install_directory_, - extension->id(), - extension->VersionString(), - &version_dir, - &install_type, &error_msg)) { - ReportExtensionInstallError(crx_path, error_msg); - return; - } - - if (install_type == Extension::DOWNGRADE) { - ReportExtensionInstallError(crx_path, "Attempted to downgrade extension."); - return; - } - - extension->set_path(version_dir); - - if (install_type == Extension::REINSTALL) { - // The client may use this as a signal (to switch themes, for instance). - ReportExtensionOverinstallAttempted(extension->id(), crx_path); - return; - } - - frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod( - frontend_, &ExtensionsService::OnExtensionInstalled, crx_path, - extension, install_type)); - - // Only one extension, but ReportExtensionsLoaded can handle multiple, - // so we need to construct a list. - ExtensionList* extensions = new ExtensionList; - - // Hand off ownership of the extension to the frontend. - extensions->push_back(extension_deleter.release()); - ReportExtensionsLoaded(extensions); -} - -void ExtensionsServiceBackend::ReportExtensionInstallError( - const FilePath& extension_path, const std::string &error) { - - // TODO(erikkay): note that this isn't guaranteed to work properly on Linux. - std::string path_str = WideToASCII(extension_path.ToWStringHack()); - std::string message = - StringPrintf("Could not install extension from '%s'. %s", - path_str.c_str(), error.c_str()); - ExtensionErrorReporter::GetInstance()->ReportError(message, alert_on_error_); - - frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod( - frontend_, - &ExtensionsService::OnExtenionInstallError, - extension_path)); -} - -void ExtensionsServiceBackend::ReportExtensionOverinstallAttempted( - const std::string& id, const FilePath& path) { - frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod( - frontend_, &ExtensionsService::OnExtensionOverinstallAttempted, id, - path)); -} - bool ExtensionsServiceBackend::LookupExternalExtension( const std::string& id, Version** version, Extension::Location* location) { scoped_ptr<Version> extension_version; @@ -845,9 +608,9 @@ void ExtensionsServiceBackend::SetProviderForTesting( } void ExtensionsServiceBackend::OnExternalExtensionFound( - const std::string& id, const Version* version, const FilePath& path) { - InstallOrUpdateExtension(path, - false, // not from gallery - id, // expected id - true); // silent + const std::string& id, const Version* version, const FilePath& path, + Extension::Location location) { + frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod(frontend_, + &ExtensionsService::OnExternalExtensionFound, id, version->GetString(), + path, location)); } diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h index a62ebee..67f7c4a 100644 --- a/chrome/browser/extensions/extensions_service.h +++ b/chrome/browser/extensions/extensions_service.h @@ -37,20 +37,13 @@ class SiteInstance; typedef std::vector<Extension*> ExtensionList; -// A callback for when installs finish. If the Extension* parameter is -// null then the install failed. -typedef Callback2<const FilePath&, Extension*>::Type ExtensionInstallCallback; - // This is an interface class to encapsulate the dependencies that // ExtensionUpdater has on ExtensionsService. This allows easy mocking. class ExtensionUpdateService { public: virtual ~ExtensionUpdateService() {} virtual const ExtensionList* extensions() const = 0; - virtual void UpdateExtension(const std::string& id, - const FilePath& path, - bool alert_on_error, - ExtensionInstallCallback* callback) = 0; + virtual void UpdateExtension(const std::string& id, const FilePath& path) = 0; virtual Extension* GetExtensionById(const std::string& id) = 0; }; @@ -112,15 +105,9 @@ class ExtensionsService const GURL& referrer_url); // Updates a currently-installed extension with the contents from - // |extension_path|. The |alert_on_error| parameter controls whether the - // user will be notified in the event of failure. If |callback| is non-null, - // it will be called back when the update is finished (in success or failure). - // This is useful to know when the service is done using |extension_path|. - // Also, this takes ownership of |callback| if it's non-null. + // |extension_path|. virtual void UpdateExtension(const std::string& id, - const FilePath& extension_path, - bool alert_on_error, - ExtensionInstallCallback* callback); + const FilePath& extension_path); // Reloads the specified extension. void ReloadExtension(const std::string& extension_id); @@ -169,7 +156,26 @@ class ExtensionsService void SetProviderForTesting(Extension::Location location, ExternalExtensionProvider* test_provider); - void SetExtensionsEnabled(bool enabled); + // Called by the backend when the initial extension load has completed. + void OnLoadedInstalledExtensions(); + + // Called by the backend when extensions have been loaded. + void OnExtensionsLoaded(ExtensionList* extensions); + + // Called by the backend when an extension has been installed. + void OnExtensionInstalled(Extension* extension); + + // Called by the backend when an attempt was made to reinstall the same + // version of an existing extension. + void OnExtensionOverinstallAttempted(const std::string& id); + + // Called by the backend when an external extension is found. + void OnExternalExtensionFound(const std::string& id, + const std::string& version, + const FilePath& path, + Extension::Location location); + + void set_extensions_enabled(bool enabled) { extensions_enabled_ = enabled; } bool extensions_enabled() { return extensions_enabled_; } void set_show_extensions_prompts(bool enabled) { @@ -189,31 +195,6 @@ class ExtensionsService bool is_ready() { return ready_; } private: - // For OnExtensionLoaded, OnExtensionInstalled, and - // OnExtensionVersionReinstalled. - friend class ExtensionsServiceBackend; - - // Called by the backend when the initial extension load has completed. - void OnLoadedInstalledExtensions(); - - // Called by the backend when extensions have been loaded. - void OnExtensionsLoaded(ExtensionList* extensions); - - // Called by the backend when an extension has been installed. - void OnExtensionInstalled(const FilePath& path, Extension* extension, - Extension::InstallType install_type); - - // Calls and then removes any registered install callback for |path|. - void FireInstallCallback(const FilePath& path, Extension* extension); - - // Called by the backend when there was an error installing an extension. - void OnExtenionInstallError(const FilePath& path); - - // Called by the backend when an attempt was made to reinstall the same - // version of an existing extension. - void OnExtensionOverinstallAttempted(const std::string& id, - const FilePath& path); - // Show a confirm installation infobar on the currently active tab. // TODO(aa): This should be moved up into the UI and attached to the tab it // actually occured in. This requires some modularization of @@ -244,10 +225,6 @@ class ExtensionsService // The backend that will do IO on behalf of this instance. scoped_refptr<ExtensionsServiceBackend> backend_; - // Stores data we'll need to do callbacks as installs complete. - typedef std::map<FilePath, linked_ptr<ExtensionInstallCallback> > CallbackMap; - CallbackMap install_callbacks_; - // Is the service ready to go? bool ready_; @@ -267,14 +244,10 @@ class ExtensionsServiceBackend // |extension_prefs| contains a dictionary value that points to the extension // preferences. ExtensionsServiceBackend(const FilePath& install_directory, - ResourceDispatcherHost* rdh, - MessageLoop* frontend_loop, - bool extensions_enabled); + MessageLoop* frontend_loop); virtual ~ExtensionsServiceBackend(); - void set_extensions_enabled(bool enabled) { extensions_enabled_ = enabled; } - // Loads the installed extensions. // Errors are reported through ExtensionErrorReporter. On completion, // OnExtensionsLoaded() is called with any successfully loaded extensions. @@ -291,19 +264,6 @@ class ExtensionsServiceBackend void LoadSingleExtension(const FilePath &path, scoped_refptr<ExtensionsService> frontend); - // Install the extension file at |extension_path|. Errors are reported through - // ExtensionErrorReporter. OnExtensionInstalled is called in the frontend on - // success. - void InstallExtension(const FilePath& extension_path, bool from_gallery, - scoped_refptr<ExtensionsService> frontend); - - // Similar to InstallExtension, but |extension_path| must be an updated - // version of an installed extension with id of |id|. - void UpdateExtension(const std::string& id, - const FilePath& extension_path, - bool alert_on_error, - scoped_refptr<ExtensionsService> frontend); - // Check externally updated extensions for updates and install if necessary. // Errors are reported through ExtensionErrorReporter. Succcess is not // reported. @@ -321,24 +281,13 @@ class ExtensionsServiceBackend // ExternalExtensionProvider::Visitor implementation. virtual void OnExternalExtensionFound(const std::string& id, const Version* version, - const FilePath& path); + const FilePath& path, + Extension::Location location); private: - class UnpackerClient; - friend class UnpackerClient; - // Loads a single installed extension. void LoadInstalledExtension(const std::string& id, const FilePath& path, Extension::Location location); - // Install a crx file at |extension_path|. If |expected_id| is not empty, it's - // verified against the extension's manifest before installation. If the - // extension is already installed, install the new version only if its version - // number is greater than the current installed version. If |silent| is true, - // the confirmation dialog will not pop up. - void InstallOrUpdateExtension(const FilePath& extension_path, - bool from_gallery, - const std::string& expected_id, bool silent); - // Finish installing the extension in |crx_path| after it has been unpacked to // |unpacked_path|. If |expected_id| is not empty, it's verified against the // extension's manifest before installation. If |silent| is true, there will @@ -350,9 +299,7 @@ class ExtensionsServiceBackend const FilePath& crx_path, const FilePath& unpacked_path, Extension* extension, - const std::string expected_id, - bool silent, - bool from_gallery); + const std::string expected_id); // Notify the frontend that there was an error loading an extension. void ReportExtensionLoadError(const FilePath& extension_path, @@ -365,11 +312,6 @@ class ExtensionsServiceBackend void ReportExtensionInstallError(const FilePath& extension_path, const std::string& error); - // Notify the frontend that an attempt was made (but not carried out) to - // install the same version of an existing extension. - void ReportExtensionOverinstallAttempted(const std::string& id, - const FilePath& path); - // Lookup an external extension by |id| by going through all registered // external extension providers until we find a provider that contains an // extension that matches. If |version| is not NULL, the extension version @@ -393,19 +335,12 @@ class ExtensionsServiceBackend // The top-level extensions directory being installed to. FilePath install_directory_; - // We only need a pointer to this to pass along to other interfaces. - ResourceDispatcherHost* resource_dispatcher_host_; - // Whether errors result in noisy alerts. bool alert_on_error_; // The message loop to use to call the frontend. MessageLoop* frontend_loop_; - // Whether non-theme extensions are enabled (themes and externally registered - // extensions are always enabled). - bool extensions_enabled_; - // A map of all external extension providers. typedef std::map<Extension::Location, linked_ptr<ExternalExtensionProvider> > ProviderMap; diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc index 3292e12..33d81d2 100644 --- a/chrome/browser/extensions/extensions_service_unittest.cc +++ b/chrome/browser/extensions/extensions_service_unittest.cc @@ -99,7 +99,7 @@ class MockExtensionProvider : public ExternalExtensionProvider { version.reset(Version::GetVersionFromString(i->second.first)); visitor->OnExternalExtensionFound( - i->first, version.get(), i->second.second); + i->first, version.get(), i->second.second, location_); } } @@ -160,7 +160,8 @@ class MockProviderVisitor : public ExternalExtensionProvider::Visitor { virtual void OnExternalExtensionFound(const std::string& id, const Version* version, - const FilePath& path) { + const FilePath& path, + Extension::Location unused) { ++ids_found_; DictionaryValue* pref; // This tests is to make sure that the provider only notifies us of the @@ -217,7 +218,7 @@ class ExtensionsServiceTest &loop_, &loop_, false); - service_->SetExtensionsEnabled(true); + service_->set_extensions_enabled(true); service_->set_show_extensions_prompts(false); // When we start up, we want to make sure there is no external provider, @@ -308,8 +309,8 @@ class ExtensionsServiceTest } } - void SetExtensionsEnabled(bool enabled) { - service_->SetExtensionsEnabled(enabled); + void set_extensions_enabled(bool enabled) { + service_->set_extensions_enabled(enabled); } void SetMockExternalProvider(Extension::Location location, @@ -318,28 +319,6 @@ class ExtensionsServiceTest } protected: - // A class to record whether a ExtensionInstallCallback has fired, and - // to remember the args it was called with. - class CallbackRecorder { - public: - CallbackRecorder() : was_called_(false), path_(NULL), extension_(NULL) {} - - void CallbackFunc(const FilePath& path, Extension* extension) { - was_called_ = true; - path_.reset(new FilePath(path)); - extension_ = extension; - } - - bool was_called() { return was_called_; } - const FilePath* path() { return path_.get(); } - Extension* extension() { return extension_; } - - private: - bool was_called_; - scoped_ptr<FilePath> path_; - Extension* extension_; - }; - void InstallExtension(const FilePath& path, bool should_succeed) { ASSERT_TRUE(file_util::PathExists(path)); @@ -372,41 +351,32 @@ class ExtensionsServiceTest ExtensionErrorReporter::GetInstance()->ClearErrors(); } - void UpdateExtension(const std::string& id, const FilePath& path, - bool should_succeed, bool use_callback, - bool expect_report_on_failure) { - ASSERT_TRUE(file_util::PathExists(path)); + void UpdateExtension(const std::string& id, const FilePath& in_path, + bool should_succeed, bool expect_report_on_failure) { + ASSERT_TRUE(file_util::PathExists(in_path)); - CallbackRecorder callback_recorder; - ExtensionInstallCallback* callback = NULL; - if (use_callback) { - callback = NewCallback(&callback_recorder, - &CallbackRecorder::CallbackFunc); - } + // We need to copy this to a temporary location because Update() will delete + // it. + FilePath temp_dir; + ASSERT_TRUE(PathService::Get(base::DIR_TEMP, &temp_dir)); + FilePath path = temp_dir.Append(in_path.BaseName()); + ASSERT_TRUE(file_util::CopyFile(in_path, path)); - service_->UpdateExtension(id, path, false, callback); + service_->UpdateExtension(id, path); loop_.RunAllPending(); std::vector<std::string> errors = GetErrors(); - if (use_callback) { - EXPECT_TRUE(callback_recorder.was_called()); - EXPECT_TRUE(path == *callback_recorder.path()); - } - if (should_succeed) { EXPECT_EQ(0u, errors.size()) << path.value(); EXPECT_EQ(1u, service_->extensions()->size()); - if (use_callback) { - EXPECT_EQ(service_->extensions()->at(0), callback_recorder.extension()); - } } else { if (expect_report_on_failure) { EXPECT_EQ(1u, errors.size()) << path.value(); } - if (use_callback) { - EXPECT_EQ(NULL, callback_recorder.extension()); - } } + + // Update() should delete the temporary input file. + EXPECT_FALSE(file_util::PathExists(path)); } void ValidatePrefKeyCount(size_t count) { @@ -674,10 +644,10 @@ TEST_F(ExtensionsServiceTest, InstallExtension) { extensions_path = extensions_path.AppendASCII("extensions"); // Extensions not enabled. - SetExtensionsEnabled(false); + set_extensions_enabled(false); FilePath path = extensions_path.AppendASCII("good.crx"); InstallExtension(path, false); - SetExtensionsEnabled(true); + set_extensions_enabled(true); ValidatePrefKeyCount(0); @@ -799,7 +769,7 @@ TEST_F(ExtensionsServiceTest, InstallTheme) { // A theme when extensions are disabled. Themes can be installed, even when // extensions are disabled. - SetExtensionsEnabled(false); + set_extensions_enabled(false); path = extensions_path.AppendASCII("theme2.crx"); InstallExtension(path, true); ValidatePrefKeyCount(++pref_count); @@ -808,7 +778,7 @@ TEST_F(ExtensionsServiceTest, InstallTheme) { // A theme with extension elements. Themes cannot have extension elements so // this test should fail. - SetExtensionsEnabled(true); + set_extensions_enabled(true); path = extensions_path.AppendASCII("theme_with_extension.crx"); InstallExtension(path, false); ValidatePrefKeyCount(pref_count); @@ -923,26 +893,7 @@ TEST_F(ExtensionsServiceTest, UpdateExtension) { ASSERT_EQ(good_crx, good->id()); path = extensions_path.AppendASCII("good2.crx"); - UpdateExtension(good_crx, path, true, true, true); - ASSERT_EQ("1.0.0.1", loaded_[0]->version()->GetString()); -} - -// Test doing an update without passing a completion callback -TEST_F(ExtensionsServiceTest, UpdateWithoutCallback) { - InitializeEmptyExtensionsService(); - FilePath extensions_path; - ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); - extensions_path = extensions_path.AppendASCII("extensions"); - - FilePath path = extensions_path.AppendASCII("good.crx"); - - InstallExtension(path, true); - Extension* good = service_->extensions()->at(0); - ASSERT_EQ("1.0.0.0", good->VersionString()); - ASSERT_EQ(good_crx, good->id()); - - path = extensions_path.AppendASCII("good2.crx"); - UpdateExtension(good_crx, path, true, false, true); + UpdateExtension(good_crx, path, true, true); ASSERT_EQ("1.0.0.1", loaded_[0]->version()->GetString()); } @@ -954,7 +905,7 @@ TEST_F(ExtensionsServiceTest, UpdateNotInstalledExtension) { extensions_path = extensions_path.AppendASCII("extensions"); FilePath path = extensions_path.AppendASCII("good.crx"); - service_->UpdateExtension(good_crx, path, false, NULL); + service_->UpdateExtension(good_crx, path); loop_.RunAllPending(); ASSERT_EQ(0u, service_->extensions()->size()); @@ -978,7 +929,7 @@ TEST_F(ExtensionsServiceTest, UpdateWillNotDowngrade) { // Change path from good2.crx -> good.crx path = extensions_path.AppendASCII("good.crx"); - UpdateExtension(good_crx, path, false, true, true); + UpdateExtension(good_crx, path, false, true); ASSERT_EQ("1.0.0.1", service_->extensions()->at(0)->VersionString()); } @@ -994,7 +945,7 @@ TEST_F(ExtensionsServiceTest, UpdateToSameVersionIsNoop) { InstallExtension(path, true); Extension* good = service_->extensions()->at(0); ASSERT_EQ(good_crx, good->id()); - UpdateExtension(good_crx, path, false, true, false); + UpdateExtension(good_crx, path, false, false); } // Tests uninstalling normal extensions @@ -1133,7 +1084,7 @@ TEST_F(ExtensionsServiceTest, GenerateID) { TEST_F(ExtensionsServiceTest, ExternalInstallRegistry) { // This should all work, even when normal extension installation is disabled. InitializeEmptyExtensionsService(); - SetExtensionsEnabled(false); + set_extensions_enabled(false); // Verify that starting with no providers loads no extensions. service_->Init(); loop_.RunAllPending(); @@ -1338,7 +1289,7 @@ TEST_F(ExtensionsServiceTest, ExternalInstallPref) { // It should still work if extensions are disabled (disableness doesn't // apply to externally registered extensions). - SetExtensionsEnabled(false); + set_extensions_enabled(false); pref_provider->UpdateOrAddExtension(good_crx, "1.0", source_path); service_->CheckForExternalUpdates(); diff --git a/chrome/browser/extensions/external_extension_provider.h b/chrome/browser/extensions/external_extension_provider.h index d67a918..f431bfc 100644 --- a/chrome/browser/extensions/external_extension_provider.h +++ b/chrome/browser/extensions/external_extension_provider.h @@ -25,7 +25,8 @@ class ExternalExtensionProvider { public: virtual void OnExternalExtensionFound(const std::string& id, const Version* version, - const FilePath& path) = 0; + const FilePath& path, + Extension::Location location) = 0; }; virtual ~ExternalExtensionProvider() {} diff --git a/chrome/browser/extensions/external_pref_extension_provider.cc b/chrome/browser/extensions/external_pref_extension_provider.cc index f2ea35a..7b371ed 100644 --- a/chrome/browser/extensions/external_pref_extension_provider.cc +++ b/chrome/browser/extensions/external_pref_extension_provider.cc @@ -75,8 +75,8 @@ void ExternalPrefExtensionProvider::VisitRegisteredExtension( scoped_ptr<Version> version; version.reset(Version::GetVersionFromString(external_version)); - visitor->OnExternalExtensionFound( - WideToASCII(extension_id), version.get(), path); + visitor->OnExternalExtensionFound(WideToASCII(extension_id), version.get(), + path, Extension::EXTERNAL_PREF); } } diff --git a/chrome/browser/extensions/external_registry_extension_provider_win.cc b/chrome/browser/extensions/external_registry_extension_provider_win.cc index 3809eb4..d8057a0 100644 --- a/chrome/browser/extensions/external_registry_extension_provider_win.cc +++ b/chrome/browser/extensions/external_registry_extension_provider_win.cc @@ -51,7 +51,8 @@ void ExternalRegistryExtensionProvider::VisitRegisteredExtension( scoped_ptr<Version> version; version.reset(Version::GetVersionFromString(extension_version)); FilePath path = FilePath::FromWStringHack(extension_path); - visitor->OnExternalExtensionFound(id, version.get(), path); + visitor->OnExternalExtensionFound(id, version.get(), path, + Extension::EXTERNAL_REGISTRY); } else { // TODO(erikkay): find a way to get this into about:extensions LOG(WARNING) << "Missing value " << kRegistryExtensionVersion << diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.cc b/chrome/browser/extensions/sandboxed_extension_unpacker.cc index 28b3b2b..8048ff2 100644 --- a/chrome/browser/extensions/sandboxed_extension_unpacker.cc +++ b/chrome/browser/extensions/sandboxed_extension_unpacker.cc @@ -30,8 +30,6 @@ SandboxedExtensionUnpacker::SandboxedExtensionUnpacker( SandboxedExtensionUnpackerClient* client) : crx_path_(crx_path), client_loop_(MessageLoop::current()), rdh_(rdh), client_(client), got_response_(false) { - - AddRef(); } void SandboxedExtensionUnpacker::Start() { @@ -269,12 +267,10 @@ bool SandboxedExtensionUnpacker::ValidateSignature() { void SandboxedExtensionUnpacker::ReportFailure(const std::string& error) { client_->OnUnpackFailure(error); - Release(); } void SandboxedExtensionUnpacker::ReportSuccess() { // Client takes ownership of temporary directory and extension. client_->OnUnpackSuccess(temp_dir_.Take(), extension_root_, extension_.release()); - Release(); } diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.h b/chrome/browser/extensions/sandboxed_extension_unpacker.h index 4023ea9..02b34fb 100644 --- a/chrome/browser/extensions/sandboxed_extension_unpacker.h +++ b/chrome/browser/extensions/sandboxed_extension_unpacker.h @@ -17,8 +17,12 @@ class Extension; class MessageLoop; class ResourceDispatcherHost; -class SandboxedExtensionUnpackerClient { +class SandboxedExtensionUnpackerClient + : public base::RefCountedThreadSafe<SandboxedExtensionUnpackerClient> { public: + virtual ~SandboxedExtensionUnpackerClient(){ + } + // temp_dir - A temporary directoy containing the results of the extension // unpacking. The client is responsible for deleting this directory. // @@ -42,9 +46,17 @@ class SandboxedExtensionUnpackerClient { // such, it should not be used when the output is not intended to be given back // to the author. // +// +// Lifetime management: +// +// This class is ref-counted by each call it makes to itself on another thread, +// and by UtilityProcessHost. +// +// Additionally, we hold a reference to our own client so that it lives at least +// long enough to receive the result of unpacking. +// +// // NOTE: This class should only be used on the file thread. - - class SandboxedExtensionUnpacker : public UtilityProcessHost::Client { public: // The size of the magic character sequence at the beginning of each crx @@ -114,7 +126,7 @@ class SandboxedExtensionUnpacker : public UtilityProcessHost::Client { FilePath crx_path_; MessageLoop* client_loop_; ResourceDispatcherHost* rdh_; - SandboxedExtensionUnpackerClient* client_; + scoped_refptr<SandboxedExtensionUnpackerClient> client_; ScopedTempDir temp_dir_; FilePath extension_root_; scoped_ptr<Extension> extension_; |