diff options
author | aa@chromium.org <aa@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-05-20 20:47:25 +0000 |
---|---|---|
committer | aa@chromium.org <aa@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-05-20 20:47:25 +0000 |
commit | 6d7c8059b0b3c9e5f079bb8f0569b0de6a115330 (patch) | |
tree | 93b51fcbae522fdd93b0d8031922ab131a9eb2ac /chrome | |
parent | b674dc73f0b635aa2f7be660153e5b05f6a9b593 (diff) | |
download | chromium_src-6d7c8059b0b3c9e5f079bb8f0569b0de6a115330.zip chromium_src-6d7c8059b0b3c9e5f079bb8f0569b0de6a115330.tar.gz chromium_src-6d7c8059b0b3c9e5f079bb8f0569b0de6a115330.tar.bz2 |
Hook up more of extension uninstall.
Also removed all external dependencies from ExtensionsService.
It now only sends out notifications, which other services
consume. This should allow us to unit test the
ExtensionsService frontend, but I haven't added that yet.
Review URL: http://codereview.chromium.org/113493
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@16529 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
23 files changed, 792 insertions, 515 deletions
diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc index e452b67..3a02939 100644 --- a/chrome/browser/browser.cc +++ b/chrome/browser/browser.cc @@ -19,6 +19,7 @@ #include "chrome/browser/debugger/debugger_host.h" #include "chrome/browser/debugger/devtools_manager.h" #include "chrome/browser/download/download_manager.h" +#include "chrome/browser/extensions/extension.h" #include "chrome/browser/find_bar.h" #include "chrome/browser/find_bar_controller.h" #include "chrome/browser/location_bar.h" @@ -188,6 +189,10 @@ Browser::Browser(Type type, Profile* profile) this, NotificationType::SSL_VISIBLE_STATE_CHANGED, NotificationService::AllSources()); + NotificationService::current()->AddObserver( + this, + NotificationType::EXTENSION_UNLOADED, + NotificationService::AllSources()); InitCommandState(); BrowserList::AddBrowser(this); @@ -233,6 +238,10 @@ Browser::~Browser() { this, NotificationType::SSL_VISIBLE_STATE_CHANGED, NotificationService::AllSources()); + NotificationService::current()->RemoveObserver( + this, + NotificationType::EXTENSION_UNLOADED, + NotificationService::AllSources()); if (profile_->IsOffTheRecord() && !BrowserList::IsOffTheRecordSessionActive()) { @@ -2045,6 +2054,20 @@ void Browser::Observe(NotificationType type, UpdateToolbar(false); break; + case NotificationType::EXTENSION_UNLOADED: { + // Close any tabs from the unloaded extension. + Extension* extension = Details<Extension>(details).ptr(); + for (int i = 0; i < tabstrip_model_.count(); i++) { + TabContents* tc = tabstrip_model_.GetTabContentsAt(i); + if (tc->GetURL().SchemeIs(chrome::kExtensionScheme) && + tc->GetURL().host() == extension->id()) { + CloseTabContents(tc); + return; + } + } + break; + } + default: NOTREACHED() << "Got a notification we didn't register for."; } diff --git a/chrome/browser/browser_resources.grd b/chrome/browser/browser_resources.grd index 69733b7..97dc62f 100644 --- a/chrome/browser/browser_resources.grd +++ b/chrome/browser/browser_resources.grd @@ -1,6 +1,6 @@ <?xml version="1.0" encoding="UTF-8"?> <!-- This comment is only here because changes to resources are not picked up -without changes to the corresponding grd file. --> +without changes to the corresponding grd file. --> <grit latest_public_release="0" current_release="1"> <outputs> <output filename="grit/browser_resources.h" type="rc_header"> diff --git a/chrome/browser/extensions/extension.cc b/chrome/browser/extensions/extension.cc index da1e841..609bd6a 100644 --- a/chrome/browser/extensions/extension.cc +++ b/chrome/browser/extensions/extension.cc @@ -127,23 +127,6 @@ const char* Extension::kInvalidThemeTintsError = const size_t Extension::kIdSize = 20; // SHA1 (160 bits) == 20 bytes -Extension::Extension(const Extension& rhs) - : path_(rhs.path_), - extension_url_(rhs.extension_url_), - id_(rhs.id_), - version_(new Version(*rhs.version_)), - name_(rhs.name_), - description_(rhs.description_), - content_scripts_(rhs.content_scripts_), - page_actions_(rhs.page_actions_), - plugins_dir_(rhs.plugins_dir_), - zip_hash_(rhs.zip_hash_), - is_theme_(rhs.is_theme_) { - theme_images_.reset(rhs.GetThemeImages()); - theme_colors_.reset(rhs.GetThemeColors()); - theme_tints_.reset(rhs.GetThemeTints()); -} - Extension::~Extension() { for (PageActionMap::iterator i = page_actions_.begin(); i != page_actions_.end(); ++i) diff --git a/chrome/browser/extensions/extension.h b/chrome/browser/extensions/extension.h index f944142..d4964077 100644 --- a/chrome/browser/extensions/extension.h +++ b/chrome/browser/extensions/extension.h @@ -108,7 +108,6 @@ class Extension { Extension() : location_(INVALID) {} explicit Extension(const FilePath& path); - explicit Extension(const Extension& other); virtual ~Extension(); // Returns an absolute url to a resource inside of an extension. The @@ -166,10 +165,6 @@ class Extension { DictionaryValue* GetThemeTints() const { return theme_tints_.get(); } bool IsTheme() { return is_theme_; } - // It doesn't really make sense to 'uninstall' extensions loaded from - // --load-extension or external locations. - const bool is_uninstallable() const { return location_ == INTERNAL; } - private: // Helper method that loads a UserScript object from a // dictionary in the content_script list of the manifest. @@ -247,8 +242,7 @@ class Extension { // The sites this extension has permission to talk to (using XHR, etc). std::vector<URLPattern> permissions_; - // We implement copy, but not assign. - void operator=(const Extension&); + DISALLOW_COPY_AND_ASSIGN(Extension); }; #endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_H_ diff --git a/chrome/browser/extensions/extension_process_manager.cc b/chrome/browser/extensions/extension_process_manager.cc index 79b2e0f..369150c 100644 --- a/chrome/browser/extensions/extension_process_manager.cc +++ b/chrome/browser/extensions/extension_process_manager.cc @@ -30,6 +30,9 @@ ExtensionProcessManager::ExtensionProcessManager(Profile* profile) NotificationService::current()->AddObserver(this, NotificationType::EXTENSIONS_LOADED, NotificationService::AllSources()); + NotificationService::current()->AddObserver(this, + NotificationType::EXTENSION_UNLOADED, + NotificationService::AllSources()); if (profile->GetExtensionsService()) { CreateBackgroundHosts(this, profile->GetExtensionsService()->extensions()); @@ -65,7 +68,28 @@ SiteInstance* ExtensionProcessManager::GetSiteInstanceForURL(const GURL& url) { void ExtensionProcessManager::Observe(NotificationType type, const NotificationSource& source, const NotificationDetails& details) { - DCHECK(type == NotificationType::EXTENSIONS_LOADED); - const ExtensionList* extensions = Details<ExtensionList>(details).ptr(); - CreateBackgroundHosts(this, extensions); + switch (type.value) { + case NotificationType::EXTENSIONS_LOADED: { + const ExtensionList* extensions = Details<ExtensionList>(details).ptr(); + CreateBackgroundHosts(this, extensions); + break; + } + + case NotificationType::EXTENSION_UNLOADED: { + Extension* extension = Details<Extension>(details).ptr(); + for (ExtensionHostList::iterator iter = background_hosts_.begin(); + iter != background_hosts_.end(); ++iter) { + ExtensionHost* host = *iter; + if (host->extension()->id() == extension->id()) { + background_hosts_.erase(iter); + delete host; + break; + } + } + break; + } + + default: + NOTREACHED(); + } } diff --git a/chrome/browser/extensions/extension_shelf.cc b/chrome/browser/extensions/extension_shelf.cc index 17623b6..445f181e 100644 --- a/chrome/browser/extensions/extension_shelf.cc +++ b/chrome/browser/extensions/extension_shelf.cc @@ -145,11 +145,11 @@ ExtensionShelf::ExtensionShelf(Browser* browser) handle_visible_(false), current_handle_view_(NULL), ALLOW_THIS_IN_INITIALIZER_LIST(timer_factory_(this)) { - // Watch extensions loaded notification. - NotificationService* ns = NotificationService::current(); - Source<Profile> ns_source(browser->profile()->GetOriginalProfile()); - ns->AddObserver(this, NotificationType::EXTENSIONS_LOADED, - NotificationService::AllSources()); + // Watch extensions loaded and unloaded notifications. + registrar_.Add(this, NotificationType::EXTENSIONS_LOADED, + NotificationService::AllSources()); + registrar_.Add(this, NotificationType::EXTENSION_UNLOADED, + NotificationService::AllSources()); // Add any already-loaded extensions now, since we missed the notification for // those. @@ -162,12 +162,6 @@ ExtensionShelf::ExtensionShelf(Browser* browser) } } -ExtensionShelf::~ExtensionShelf() { - NotificationService* ns = NotificationService::current(); - ns->RemoveObserver(this, NotificationType::EXTENSIONS_LOADED, - NotificationService::AllSources()); -} - BrowserBubble* ExtensionShelf::GetHandle() { if (!handle_.get() && HasExtensionViews() && current_handle_view_) { ExtensionShelfHandle* handle_view = new ExtensionShelfHandle(this); @@ -271,6 +265,11 @@ void ExtensionShelf::Observe(NotificationType type, AddExtensionViews(extensions); break; } + case NotificationType::EXTENSION_UNLOADED: { + Extension* extension = Details<Extension>(details).ptr(); + RemoveExtensionViews(extension); + break; + } default: DCHECK(false) << "Unhandled notification of type: " << type.value; break; @@ -306,6 +305,28 @@ bool ExtensionShelf::AddExtensionViews(const ExtensionList* extensions) { return added_toolstrip; } +bool ExtensionShelf::RemoveExtensionViews(Extension* extension) { + if (!HasExtensionViews()) + return false; + + bool removed_toolstrip = false; + int count = GetChildViewCount(); + for (int i = count - 1; i >= 0; --i) { + ExtensionView* view = static_cast<ExtensionView*>(GetChildViewAt(i)); + if (view->host()->extension()->id() == extension->id()) { + RemoveChildView(view); + delete view; + removed_toolstrip = true; + } + } + + if (removed_toolstrip) { + SchedulePaint(); + PreferredSizeChanged(); + } + return removed_toolstrip; +} + bool ExtensionShelf::HasExtensionViews() { return GetChildViewCount() > 0; } diff --git a/chrome/browser/extensions/extension_shelf.h b/chrome/browser/extensions/extension_shelf.h index 4bc88eb..59ef31d 100644 --- a/chrome/browser/extensions/extension_shelf.h +++ b/chrome/browser/extensions/extension_shelf.h @@ -11,6 +11,7 @@ #include "chrome/browser/extensions/extensions_service.h" #include "chrome/browser/views/browser_bubble.h" #include "chrome/common/notification_observer.h" +#include "chrome/common/notification_registrar.h" #include "views/view.h" class Browser; @@ -27,7 +28,6 @@ class ExtensionShelf : public views::View, public BrowserBubble::Delegate { public: explicit ExtensionShelf(Browser* browser); - virtual ~ExtensionShelf(); // Return the current active ExtensionShelfHandle (if any). BrowserBubble* GetHandle(); @@ -63,6 +63,9 @@ class ExtensionShelf : public views::View, // Inits the background bitmap. void InitBackground(gfx::Canvas* canvas, const SkRect& subset); + // Removes any toolstrips associated with an extension. + bool RemoveExtensionViews(Extension* extension); + // Show / Hide the shelf handle. void ShowShelfHandle(); void DoShowShelfHandle(); @@ -75,6 +78,9 @@ class ExtensionShelf : public views::View, // Which browser window this shelf is in. Browser* browser_; + // Manages our notification registrations. + NotificationRegistrar registrar_; + // Background bitmap to draw under extension views. SkBitmap background_; diff --git a/chrome/browser/extensions/extension_unittest.cc b/chrome/browser/extensions/extension_unittest.cc index df4c64f..0607186 100644 --- a/chrome/browser/extensions/extension_unittest.cc +++ b/chrome/browser/extensions/extension_unittest.cc @@ -254,20 +254,3 @@ TEST(ExtensionTest, GetResourceURLAndPath) { EXPECT_EQ(FilePath().value(), Extension::GetResourcePath(extension.path(), "../baz.js").value()); } - -TEST(ExtensionTest, Location) { - Extension extension; - EXPECT_EQ(Extension::INVALID, extension.location()); - - extension.set_location(Extension::INTERNAL); - EXPECT_EQ(Extension::INTERNAL, extension.location()); - EXPECT_TRUE(extension.is_uninstallable()); - - extension.set_location(Extension::EXTERNAL); - EXPECT_EQ(Extension::EXTERNAL, extension.location()); - EXPECT_FALSE(extension.is_uninstallable()); - - extension.set_location(Extension::LOAD); - EXPECT_EQ(Extension::LOAD, extension.location()); - EXPECT_FALSE(extension.is_uninstallable()); -} diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc index 23c37fe..db14670 100644 --- a/chrome/browser/extensions/extensions_service.cc +++ b/chrome/browser/extensions/extensions_service.cc @@ -21,13 +21,12 @@ #include "chrome/browser/extensions/extension_browser_event_router.h" #include "chrome/browser/extensions/extension_error_reporter.h" #include "chrome/browser/extensions/extension_process_manager.h" -#include "chrome/browser/extensions/user_script_master.h" -#include "chrome/browser/plugin_service.h" #include "chrome/browser/profile.h" #include "chrome/browser/utility_process_host.h" #include "chrome/common/extensions/extension_unpacker.h" #include "chrome/common/json_value_serializer.h" #include "chrome/common/notification_service.h" +#include "chrome/common/pref_service.h" #include "chrome/common/unzip.h" #include "chrome/common/url_constants.h" @@ -55,10 +54,17 @@ struct ExtensionHeader { const size_t kZipHashBytes = 32; // SHA-256 const size_t kZipHashHexBytes = kZipHashBytes * 2; // Hex string is 2x size. -#if defined(OS_WIN) +// A preference that keeps track of external extensions the user has +// uninstalled. +const wchar_t kUninstalledExternalPref[] = + L"extensions.uninstalled_external_ids"; // Registry key where registry defined extension installers live. -const wchar_t kRegistryExtensions[] = L"Software\\Google\\Chrome\\Extensions"; +// TODO(port): Assuming this becomes a similar key into the appropriate +// platform system. +const char kRegistryExtensions[] = "Software\\Google\\Chrome\\Extensions"; + +#if defined(OS_WIN) // Registry value of of that key that defines the path to the .crx file. const wchar_t kRegistryExtensionPath[] = L"path"; @@ -178,12 +184,16 @@ class ExtensionsServiceBackend::UnpackerClient }; ExtensionsService::ExtensionsService(Profile* profile, - UserScriptMaster* user_script_master) - : message_loop_(MessageLoop::current()), + MessageLoop* frontend_loop, + MessageLoop* backend_loop, + const std::string& registry_path) + : prefs_(profile->GetPrefs()), + backend_loop_(backend_loop), install_directory_(profile->GetPath().AppendASCII(kInstallDirectoryName)), backend_(new ExtensionsServiceBackend( - install_directory_, g_browser_process->resource_dispatcher_host())), - user_script_master_(user_script_master) { + install_directory_, g_browser_process->resource_dispatcher_host(), + frontend_loop, registry_path)) { + prefs_->RegisterListPref(kUninstalledExternalPref); } ExtensionsService::~ExtensionsService() { @@ -198,39 +208,42 @@ bool ExtensionsService::Init() { ExtensionBrowserEventRouter::GetInstance()->Init(); #if defined(OS_WIN) - // TODO(port): ExtensionsServiceBackend::CheckForExternalUpdates depends on - // the Windows registry. + + std::set<std::string> uninstalled_external_ids; + const ListValue* list = prefs_->GetList(kUninstalledExternalPref); + if (list) { + for (size_t i = 0; i < list->GetSize(); ++i) { + std::string val; + bool ok = list->GetString(i, &val); + DCHECK(ok); + DCHECK(uninstalled_external_ids.find(val) == + uninstalled_external_ids.end()); + uninstalled_external_ids.insert(val); + } + } + // TODO(erikkay): Should we monitor the registry during run as well? - g_browser_process->file_thread()->message_loop()->PostTask(FROM_HERE, - NewRunnableMethod(backend_.get(), - &ExtensionsServiceBackend::CheckForExternalUpdates, - scoped_refptr<ExtensionsServiceFrontendInterface>(this))); + backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(), + &ExtensionsServiceBackend::CheckForExternalUpdates, + uninstalled_external_ids, scoped_refptr<ExtensionsService>(this))); +#else + + // TODO(port) + #endif - // TODO(aa): This message loop should probably come from a backend - // interface, similar to how the message loop for the frontend comes - // from the frontend interface. - g_browser_process->file_thread()->message_loop()->PostTask(FROM_HERE, - NewRunnableMethod(backend_.get(), - &ExtensionsServiceBackend::LoadExtensionsFromInstallDirectory, - scoped_refptr<ExtensionsServiceFrontendInterface>(this))); + backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(), + &ExtensionsServiceBackend::LoadExtensionsFromInstallDirectory, + scoped_refptr<ExtensionsService>(this))); return true; } -MessageLoop* ExtensionsService::GetMessageLoop() { - return message_loop_; -} - void ExtensionsService::InstallExtension(const FilePath& extension_path) { - // TODO(aa): This message loop should probably come from a backend - // interface, similar to how the message loop for the frontend comes - // from the frontend interface. - g_browser_process->file_thread()->message_loop()->PostTask(FROM_HERE, - NewRunnableMethod(backend_.get(), - &ExtensionsServiceBackend::InstallExtension, - extension_path, - scoped_refptr<ExtensionsServiceFrontendInterface>(this))); + backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(), + &ExtensionsServiceBackend::InstallExtension, + extension_path, + scoped_refptr<ExtensionsService>(this))); } void ExtensionsService::UninstallExtension(const std::string& extension_id) { @@ -244,70 +257,48 @@ void ExtensionsService::UninstallExtension(const std::string& extension_id) { } } + // Callers should not send us nonexistant extensions. + CHECK(extension); + // Remove the extension from our list. extensions_.erase(iter); - // Callers should check existence and that the extension is internal before - // calling us. - DCHECK(extension); - DCHECK(extension->is_uninstallable()); - // Tell other services the extension is gone. NotificationService::current()->Notify(NotificationType::EXTENSION_UNLOADED, NotificationService::AllSources(), Details<Extension>(extension)); - // Tell the file thread to start deleting. - g_browser_process->file_thread()->message_loop()->PostTask(FROM_HERE, - NewRunnableMethod(backend_.get(), - &ExtensionsServiceBackend::UninstallExtension, extension_id)); + // For external extensions, we save a preference reminding ourself not to try + // and install the extension anymore. + if (extension->location() == Extension::EXTERNAL) { + ListValue* list = prefs_->GetMutableList(kUninstalledExternalPref); + list->Append(Value::CreateStringValue(extension->id())); + prefs_->ScheduleSavePersistentPrefs(); + } + + // Tell the backend to start deleting installed extensions on the file thread. + if (extension->location() == Extension::INTERNAL || + extension->location() == Extension::EXTERNAL) { + backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(), + &ExtensionsServiceBackend::UninstallExtension, extension_id)); + } delete extension; } void ExtensionsService::LoadExtension(const FilePath& extension_path) { - g_browser_process->file_thread()->message_loop()->PostTask(FROM_HERE, - NewRunnableMethod(backend_.get(), - &ExtensionsServiceBackend::LoadSingleExtension, - extension_path, - scoped_refptr<ExtensionsServiceFrontendInterface>(this))); + backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(), + &ExtensionsServiceBackend::LoadSingleExtension, + extension_path, scoped_refptr<ExtensionsService>(this))); } void ExtensionsService::OnExtensionsLoaded(ExtensionList* new_extensions) { extensions_.insert(extensions_.end(), new_extensions->begin(), new_extensions->end()); - - // TODO: Fix race here. A page could need a user script on startup, before - // the user script is loaded. We need to freeze the renderer in that case. - // TODO(mpcomplete): We also need to force a renderer to refresh its cache of - // the plugin list when we inject user scripts, since it could have a stale - // version by the time extensions are loaded. - - for (ExtensionList::iterator extension = extensions_.begin(); - extension != extensions_.end(); ++extension) { - // Tell NPAPI about any plugins in the loaded extensions. - if (!(*extension)->plugins_dir().empty()) { - PluginService::GetInstance()->AddExtraPluginDir( - (*extension)->plugins_dir()); - } - - // Tell UserScriptMaster about any scripts in the loaded extensions. - const UserScriptList& scripts = (*extension)->content_scripts(); - for (UserScriptList::const_iterator script = scripts.begin(); - script != scripts.end(); ++script) { - user_script_master_->AddLoneScript(*script); - } - } - - // Since user scripts may have changed, tell UserScriptMaster to kick off - // a scan. - user_script_master_->StartScan(); - NotificationService::current()->Notify( NotificationType::EXTENSIONS_LOADED, NotificationService::AllSources(), Details<ExtensionList>(new_extensions)); - delete new_extensions; } @@ -350,8 +341,21 @@ Extension* ExtensionsService::GetExtensionByID(std::string id) { // ExtensionsServicesBackend +ExtensionsServiceBackend::ExtensionsServiceBackend( + const FilePath& install_directory, ResourceDispatcherHost* rdh, + MessageLoop* frontend_loop, const std::string& registry_path) + : install_directory_(install_directory), + resource_dispatcher_host_(rdh), + frontend_loop_(frontend_loop), + registry_path_(registry_path) { + // Default the registry path if unspecified. + if (registry_path_.empty()) { + registry_path_ = kRegistryExtensions; + } +} + void ExtensionsServiceBackend::LoadExtensionsFromInstallDirectory( - scoped_refptr<ExtensionsServiceFrontendInterface> frontend) { + scoped_refptr<ExtensionsService> frontend) { frontend_ = frontend; alert_on_error_ = false; @@ -392,7 +396,25 @@ void ExtensionsServiceBackend::LoadExtensionsFromInstallDirectory( extension_path = enumerator.Next()) { std::string extension_id = WideToASCII( extension_path.BaseName().ToWStringHack()); - if (CheckExternalUninstall(extension_path, extension_id)) { + + // If there is no Current Version file, just delete the directory and move + // on. This can legitimately happen when an uninstall does not complete, for + // example, when a plugin is in use at uninstall time. + FilePath current_version_path = extension_path.AppendASCII( + ExtensionsService::kCurrentVersionFileName); + if (!file_util::PathExists(current_version_path)) { + LOG(INFO) << "Deleting incomplete install for directory " + << WideToASCII(extension_path.ToWStringHack()) << "."; + file_util::Delete(extension_path, true); // recursive; + continue; + } + + std::string current_version; + if (!ReadCurrentVersion(extension_path, ¤t_version)) + continue; + + FilePath version_path = extension_path.AppendASCII(current_version); + if (CheckExternalUninstall(version_path, extension_id)) { // TODO(erikkay): Possibly defer this operation to avoid slowing initial // load of extensions. UninstallExtension(extension_id); @@ -402,7 +424,7 @@ void ExtensionsServiceBackend::LoadExtensionsFromInstallDirectory( continue; } - Extension* extension = LoadExtensionCurrentVersion(extension_path); + Extension* extension = LoadExtension(version_path, true); // require id if (extension) extensions->push_back(extension); } @@ -412,8 +434,7 @@ void ExtensionsServiceBackend::LoadExtensionsFromInstallDirectory( } void ExtensionsServiceBackend::LoadSingleExtension( - const FilePath& path_in, - scoped_refptr<ExtensionsServiceFrontendInterface> frontend) { + const FilePath& path_in, scoped_refptr<ExtensionsService> frontend) { frontend_ = frontend; // Explicit UI loads are always noisy. @@ -436,24 +457,6 @@ void ExtensionsServiceBackend::LoadSingleExtension( } } -Extension* ExtensionsServiceBackend::LoadExtensionCurrentVersion( - const FilePath& extension_path) { - std::string version_str; - if (!ReadCurrentVersion(extension_path, &version_str)) { - ReportExtensionLoadError(extension_path, - StringPrintf("Could not read '%s' file.", - ExtensionsService::kCurrentVersionFileName)); - return NULL; - } - - LOG(INFO) << " " << - WideToASCII(extension_path.BaseName().ToWStringHack()) << - " version: " << version_str; - - return LoadExtension(extension_path.AppendASCII(version_str), - true); // require id -} - Extension* ExtensionsServiceBackend::LoadExtension( const FilePath& extension_path, bool require_id) { FilePath manifest_path = @@ -529,10 +532,8 @@ void ExtensionsServiceBackend::ReportExtensionLoadError( void ExtensionsServiceBackend::ReportExtensionsLoaded( ExtensionList* extensions) { - frontend_->GetMessageLoop()->PostTask(FROM_HERE, NewRunnableMethod( - frontend_, - &ExtensionsServiceFrontendInterface::OnExtensionsLoaded, - extensions)); + frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod( + frontend_, &ExtensionsService::OnExtensionsLoaded, extensions)); } // The extension file format is a header, followed by the manifest, followed @@ -775,8 +776,7 @@ bool ExtensionsServiceBackend::SetCurrentVersion(const FilePath& dest_dir, } void ExtensionsServiceBackend::InstallExtension( - const FilePath& extension_path, - scoped_refptr<ExtensionsServiceFrontendInterface> frontend) { + const FilePath& extension_path, scoped_refptr<ExtensionsService> frontend) { LOG(INFO) << "Installing extension " << extension_path.value(); frontend_ = frontend; @@ -855,7 +855,27 @@ void ExtensionsServiceBackend::OnExtensionUnpacked( file_util::WriteFile(marker, NULL, 0); } - ReportExtensionInstalled(dest_dir, was_update); + // Load the extension immediately and then report installation success. We + // don't load extensions for external installs because external installation + // occurs before the normal startup so we just let startup pick them up. We + // don't notify installation because there is no UI or external install so + // there is nobody to notify. + if (!from_external) { + Extension* extension = LoadExtension(version_dir, true); // require id + CHECK(extension); + + frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod( + frontend_, &ExtensionsService::OnExtensionInstalled, extension, + was_update)); + + // Only one extension, but ReportExtensionsLoaded can handle multiple, + // so we need to construct a list. + scoped_ptr<ExtensionList> extensions(new ExtensionList); + extensions->push_back(extension); + LOG(INFO) << "Done."; + // Hand off ownership of the loaded extensions to the frontend. + ReportExtensionsLoaded(extensions.release()); + } } void ExtensionsServiceBackend::ReportExtensionInstallError( @@ -871,31 +891,8 @@ void ExtensionsServiceBackend::ReportExtensionInstallError( void ExtensionsServiceBackend::ReportExtensionVersionReinstalled( const std::string& id) { - frontend_->GetMessageLoop()->PostTask(FROM_HERE, NewRunnableMethod( - frontend_, - &ExtensionsServiceFrontendInterface::OnExtensionVersionReinstalled, - id)); -} - -void ExtensionsServiceBackend::ReportExtensionInstalled( - const FilePath& path, bool update) { - // After it's installed, load it right away with the same settings. - Extension* extension = LoadExtensionCurrentVersion(path); - CHECK(extension); - - frontend_->GetMessageLoop()->PostTask(FROM_HERE, NewRunnableMethod( - frontend_, - &ExtensionsServiceFrontendInterface::OnExtensionInstalled, - extension, - update)); - - // Only one extension, but ReportExtensionsLoaded can handle multiple, - // so we need to construct a list. - scoped_ptr<ExtensionList> extensions(new ExtensionList); - extensions->push_back(extension); - LOG(INFO) << "Done."; - // Hand off ownership of the loaded extensions to the frontend. - ReportExtensionsLoaded(extensions.release()); + frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod( + frontend_, &ExtensionsService::OnExtensionVersionReinstalled, id)); } // Some extensions will autoupdate themselves externally from Chrome. These @@ -905,7 +902,8 @@ void ExtensionsServiceBackend::ReportExtensionInstalled( // check that location for a .crx file, which it will then install locally if // a new version is available. void ExtensionsServiceBackend::CheckForExternalUpdates( - scoped_refptr<ExtensionsServiceFrontendInterface> frontend) { + std::set<std::string> ids_to_ignore, + scoped_refptr<ExtensionsService> frontend) { // Note that this installation is intentionally silent (since it didn't // go through the front-end). Extensions that are registered in this @@ -919,16 +917,23 @@ void ExtensionsServiceBackend::CheckForExternalUpdates( // TODO(port): Pull this out into an interface. That will also allow us to // test the behavior of external extensions. HKEY reg_root = HKEY_LOCAL_MACHINE; - RegistryKeyIterator iterator(reg_root, kRegistryExtensions); + RegistryKeyIterator iterator(reg_root, ASCIIToWide(registry_path_).c_str()); while (iterator.Valid()) { + // Fold + std::string id = StringToLowerASCII(WideToASCII(iterator.Name())); + if (ids_to_ignore.find(id) != ids_to_ignore.end()) { + LOG(INFO) << "Skipping uninstalled external extension " << id; + ++iterator; + continue; + } + RegKey key; - std::wstring key_path = kRegistryExtensions; + std::wstring key_path = ASCIIToWide(registry_path_); key_path.append(L"\\"); key_path.append(iterator.Name()); if (key.Open(reg_root, key_path.c_str())) { std::wstring extension_path; if (key.ReadValue(kRegistryExtensionPath, &extension_path)) { - std::string id = WideToASCII(iterator.Name()); std::wstring extension_version; if (key.ReadValue(kRegistryExtensionVersion, &extension_version)) { if (ShouldInstall(id, WideToASCII(extension_version))) { @@ -954,21 +959,21 @@ void ExtensionsServiceBackend::CheckForExternalUpdates( #endif } -bool ExtensionsServiceBackend::CheckExternalUninstall(const FilePath& path, +bool ExtensionsServiceBackend::CheckExternalUninstall(const FilePath& version_path, const std::string& id) { - FilePath external_file = path.AppendASCII(kExternalInstallFile); + FilePath external_file = version_path.AppendASCII(kExternalInstallFile); if (file_util::PathExists(external_file)) { #if defined(OS_WIN) HKEY reg_root = HKEY_LOCAL_MACHINE; RegKey key; - std::wstring key_path = kRegistryExtensions; + std::wstring key_path = ASCIIToWide(registry_path_); key_path.append(L"\\"); key_path.append(ASCIIToWide(id)); // If the key doesn't exist, then we should uninstall. return !key.Open(reg_root, key_path.c_str()); #else - NOTREACHED(); + // TODO(port) #endif } return false; @@ -999,7 +1004,10 @@ void ExtensionsServiceBackend::UninstallExtension( } } - // Ok, now try and delete the entire rest of the directory. + // Ok, now try and delete the entire rest of the directory. One major place + // this can fail is if the extension contains a plugin (stupid plugins). It's + // not a big deal though, because we'll notice next time we startup that the + // Current Version file is gone and finish the delete then. if (!file_util::Delete(extension_directory, true)) { LOG(WARNING) << "Could not delete directory for extension " << extension_id; @@ -1011,7 +1019,7 @@ bool ExtensionsServiceBackend::ShouldInstall(const std::string& id, FilePath dir(install_directory_.AppendASCII(id.c_str())); std::string current_version; if (ReadCurrentVersion(dir, ¤t_version)) { - return !CheckCurrentVersion(version, current_version, dir); + return CheckCurrentVersion(version, current_version, dir); } return true; } diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h index b601b73..47abcbd 100644 --- a/chrome/browser/extensions/extensions_service.h +++ b/chrome/browser/extensions/extensions_service.h @@ -5,6 +5,7 @@ #ifndef CHROME_BROWSER_EXTENSIONS_EXTENSIONS_SERVICE_H_ #define CHROME_BROWSER_EXTENSIONS_EXTENSIONS_SERVICE_H_ +#include <set> #include <string> #include <vector> @@ -18,43 +19,21 @@ class Browser; class Extension; class ExtensionsServiceBackend; class GURL; +class PrefService; class Profile; class ResourceDispatcherHost; class SiteInstance; class UserScriptMaster; typedef std::vector<Extension*> ExtensionList; -// Interface for the frontend to implement. Typically, this will be -// ExtensionsService, but it can also be a test harness. -class ExtensionsServiceFrontendInterface - : public base::RefCountedThreadSafe<ExtensionsServiceFrontendInterface> { - public: - virtual ~ExtensionsServiceFrontendInterface() {} - - // The message loop to invoke the frontend's methods on. - virtual MessageLoop* GetMessageLoop() = 0; - - // Called when extensions are loaded by the backend. The frontend takes - // ownership of the list. - virtual void OnExtensionsLoaded(ExtensionList* extensions) = 0; - - // Called with results from InstallExtension(). - // |is_update| is true if the installation was an update to an existing - // installed extension rather than a new installation. - virtual void OnExtensionInstalled(Extension* extension, bool is_update) = 0; - - // Called when an existing extension is installed by the user. We may wish to - // notify the user about the prior existence of the extension, or take some - // action using the fact that the user chose to reinstall the extension as a - // signal (for example, setting the default theme to the extension). - virtual void OnExtensionVersionReinstalled(const std::string& id) = 0; -}; - - // Manages installed and running Chromium extensions. -class ExtensionsService : public ExtensionsServiceFrontendInterface { +class ExtensionsService + : public base::RefCountedThreadSafe<ExtensionsService> { public: - ExtensionsService(Profile* profile, UserScriptMaster* user_script_master); + ExtensionsService(Profile* profile, + MessageLoop* frontend_loop, + MessageLoop* backend_loop, + const std::string& registry_path); ~ExtensionsService(); // Gets the list of currently installed extensions. @@ -79,24 +58,34 @@ class ExtensionsService : public ExtensionsServiceFrontendInterface { void LoadExtension(const FilePath& extension_path); // Lookup an extension by |id|. - virtual Extension* GetExtensionByID(std::string id); - - // ExtensionsServiceFrontendInterface - virtual MessageLoop* GetMessageLoop(); - virtual void OnExtensionsLoaded(ExtensionList* extensions); - virtual void OnExtensionInstalled(Extension* extension, bool is_update); - virtual void OnExtensionVersionReinstalled(const std::string& id); + Extension* GetExtensionByID(std::string id); // The name of the file that the current active version number is stored in. static const char* kCurrentVersionFileName; private: + // For OnExtensionLoaded, OnExtensionInstalled, and + // OnExtensionVersionReinstalled. + friend class ExtensionsServiceBackend; + + // Called by the backend when extensions have been loaded. + void OnExtensionsLoaded(ExtensionList* extensions); + + // Called by the backend when an extensoin hsa been installed. + void OnExtensionInstalled(Extension* extension, bool is_update); + + // Called by the backend when an extension has been reinstalled. + void OnExtensionVersionReinstalled(const std::string& id); + // The name of the directory inside the profile where extensions are // installed to. static const char* kInstallDirectoryName; - // The message loop for the thread the ExtensionsService is running on. - MessageLoop* message_loop_; + // Preferences for the owning profile. + PrefService* prefs_; + + // The message loop to use with the backend. + MessageLoop* backend_loop_; // The current list of installed extensions. ExtensionList extensions_; @@ -107,29 +96,28 @@ class ExtensionsService : public ExtensionsServiceFrontendInterface { // The backend that will do IO on behalf of this instance. scoped_refptr<ExtensionsServiceBackend> backend_; - // The user script master for this profile. - scoped_refptr<UserScriptMaster> user_script_master_; - DISALLOW_COPY_AND_ASSIGN(ExtensionsService); }; // Implements IO for the ExtensionsService. -// TODO(aa): Extract an interface out of this for testing the frontend, once the -// frontend has significant logic to test. +// TODO(aa): This can probably move into the .cc file. class ExtensionsServiceBackend : public base::RefCountedThreadSafe<ExtensionsServiceBackend> { public: - explicit ExtensionsServiceBackend(const FilePath& install_directory, - ResourceDispatcherHost* rdh) - : install_directory_(install_directory), - resource_dispatcher_host_(rdh) {} + // |rdh| can be NULL in the case of test environment. + // |registry_path| can be NULL *except* in the case of the test environment, + // where it is specified to a temp location. + ExtensionsServiceBackend(const FilePath& install_directory, + ResourceDispatcherHost* rdh, + MessageLoop* frontend_loop, + const std::string& registry_path); // Loads extensions from the install directory. The extensions are assumed to // be unpacked in directories that are direct children of the specified path. // Errors are reported through ExtensionErrorReporter. On completion, // OnExtensionsLoaded() is called with any successfully loaded extensions. void LoadExtensionsFromInstallDirectory( - scoped_refptr<ExtensionsServiceFrontendInterface> frontend); + scoped_refptr<ExtensionsService> frontend); // Loads a single extension from |path| where |path| is the top directory of // a specific extension where its manifest file lives. @@ -138,21 +126,20 @@ class ExtensionsServiceBackend // extensions. // TODO(erikkay): It might be useful to be able to load a packed extension // (presumably into memory) without installing it. - void LoadSingleExtension( - const FilePath &path, - scoped_refptr<ExtensionsServiceFrontendInterface> frontend); + void LoadSingleExtension(const FilePath &path, + scoped_refptr<ExtensionsService> frontend); // Install the extension file at |extension_path|. Errors are reported through - // ExtensionErrorReporter. ReportExtensionInstalled is called on success. - void InstallExtension( - const FilePath& extension_path, - scoped_refptr<ExtensionsServiceFrontendInterface> frontend); + // ExtensionErrorReporter. OnExtensionInstalled is called in the frontend on + // success. + void InstallExtension(const FilePath& extension_path, + scoped_refptr<ExtensionsService> frontend); // Check externally updated extensions for updates and install if necessary. - // Errors are reported through ExtensionErrorReporter. - // ReportExtensionInstalled is called on success. - void CheckForExternalUpdates( - scoped_refptr<ExtensionsServiceFrontendInterface> frontend); + // Errors are reported through ExtensionErrorReporter. Succcess is not + // reported. + void CheckForExternalUpdates(std::set<std::string> ids_to_ignore, + scoped_refptr<ExtensionsService> frontend); // Deletes all versions of the extension from the filesystem. Note that only // extensions whose location() == INTERNAL can be uninstalled. Attempting to @@ -203,10 +190,6 @@ class ExtensionsServiceBackend // Notify the frontend that the extension had already been installed. void ReportExtensionVersionReinstalled(const std::string& id); - // Notify the frontend that extensions were installed. - // |is_update| is true if this was an update to an existing extension. - void ReportExtensionInstalled(const FilePath& path, bool is_update); - // Read the manifest from the front of the extension file. // Caller takes ownership of return value. DictionaryValue* ReadManifest(const FilePath& extension_path); @@ -228,10 +211,11 @@ class ExtensionsServiceBackend bool SetCurrentVersion(const FilePath& dest_dir, std::string version); - // For the extension at |path| with |id|, check to see if it's an + // For the extension in |version_path| with |id|, check to see if it's an // externally managed extension. If so return true if it should be // uninstalled. - bool CheckExternalUninstall(const FilePath& path, const std::string& id); + bool CheckExternalUninstall(const FilePath& version_path, + const std::string& id); // Should an extension of |id| and |version| be installed? // Returns true if no extension of type |id| is installed or if |version| @@ -244,7 +228,7 @@ class ExtensionsServiceBackend // This is a naked pointer which is set by each entry point. // The entry point is responsible for ensuring lifetime. - ExtensionsServiceFrontendInterface* frontend_; + ExtensionsService* frontend_; // The top-level extensions directory being installed to. FilePath install_directory_; @@ -255,6 +239,14 @@ class ExtensionsServiceBackend // Whether errors result in noisy alerts. bool alert_on_error_; + // The message loop to use to call the frontend. + MessageLoop* frontend_loop_; + + // The path to look for externally registered extensions in. This is a + // registry key on windows, but it could be a similar string for the + // appropriate system on other platforms in the future. + std::string registry_path_; + DISALLOW_COPY_AND_ASSIGN(ExtensionsServiceBackend); }; diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc index ea01c25..8471fbe 100644 --- a/chrome/browser/extensions/extensions_service_unittest.cc +++ b/chrome/browser/extensions/extensions_service_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2006-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. @@ -11,15 +11,24 @@ #include "base/message_loop.h" #include "base/path_service.h" #include "base/string_util.h" +#include "base/time.h" #include "chrome/browser/extensions/extension.h" #include "chrome/browser/extensions/extension_error_reporter.h" #include "chrome/browser/extensions/extensions_service.h" #include "chrome/common/extensions/url_pattern.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/json_value_serializer.h" +#include "chrome/common/notification_registrar.h" +#include "chrome/common/notification_service.h" +#include "chrome/common/notification_type.h" +#include "chrome/test/testing_profile.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" +#if defined(OS_WIN) +#include "base/registry.h" +#endif + namespace { struct ExtensionsOrder { @@ -49,69 +58,79 @@ static std::vector<std::string> GetErrors() { } // namespace -// A mock implementation of ExtensionsServiceFrontendInterface for testing the -// backend. -class ExtensionsServiceTestFrontend - : public ExtensionsServiceFrontendInterface { +class ExtensionsServiceTest + : public testing::Test, public NotificationObserver { public: - - ~ExtensionsServiceTestFrontend() { - for (ExtensionList::iterator iter = extensions_.begin(); - iter != extensions_.end(); ++iter) { - delete *iter; - } - } - - ExtensionList* extensions() { - return &extensions_; + ExtensionsServiceTest() : installed_(NULL) { + registrar_.Add(this, NotificationType::EXTENSIONS_LOADED, + NotificationService::AllSources()); + registrar_.Add(this, NotificationType::EXTENSION_UNLOADED, + NotificationService::AllSources()); + registrar_.Add(this, NotificationType::EXTENSION_INSTALLED, + NotificationService::AllSources()); + + // Create a temporary area in the registry to test external extensions. + registry_path_ = "Software\\Google\\Chrome\\ExtensionsServiceTest_"; + registry_path_ += IntToString( + static_cast<int>(base::Time::Now().ToDoubleT())); + + profile_.reset(new TestingProfile()); + service_ = new ExtensionsService(profile_.get(), &loop_, &loop_, + registry_path_); } - void ClearInstalledReinstalled() { - installed_ = NULL; - reinstalled_id_ = std::string(); + static void SetUpTestCase() { + ExtensionErrorReporter::Init(false); // no noisy errors } - Extension* installed() { - return installed_; + virtual void SetUp() { + ExtensionErrorReporter::GetInstance()->ClearErrors(); } - std::string reinstalled_id() { - return reinstalled_id_; - } + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + switch (type.value) { + case NotificationType::EXTENSIONS_LOADED: { + ExtensionList* list = Details<ExtensionList>(details).ptr(); + for (ExtensionList::iterator iter = list->begin(); iter != list->end(); + ++iter) { + loaded_.push_back(*iter); + } + // The tests rely on the errors being in a certain order, which can vary + // depending on how filesystem iteration works. + std::stable_sort(loaded_.begin(), loaded_.end(), ExtensionsOrder()); + break; + } - // ExtensionsServiceFrontendInterface - virtual MessageLoop* GetMessageLoop() { - return &message_loop_; - } + case NotificationType::EXTENSION_UNLOADED: + unloaded_id_ = Details<Extension>(details).ptr()->id(); + break; - virtual void OnExtensionsLoaded(ExtensionList* new_extensions) { - extensions_.insert(extensions_.end(), new_extensions->begin(), - new_extensions->end()); - delete new_extensions; - // In the tests we rely on extensions being in particular order, which is - // not always the case (and is not guaranteed by used APIs). - std::stable_sort(extensions_.begin(), extensions_.end(), ExtensionsOrder()); - } + case NotificationType::EXTENSION_INSTALLED: + installed_ = Details<Extension>(details).ptr(); + break; - virtual void OnExtensionInstalled(Extension* extension, bool is_update) { - installed_ = extension; - } + // TODO(glen): Add tests for themes. + // See: http://code.google.com/p/chromium/issues/detail?id=12231 - virtual void OnExtensionVersionReinstalled(const std::string& id) { - reinstalled_id_ = id; + default: + DCHECK(false); + } } void TestInstallExtension(const FilePath& path, - ExtensionsServiceBackend* backend, bool should_succeed) { ASSERT_TRUE(file_util::PathExists(path)); - backend->InstallExtension(path, - scoped_refptr<ExtensionsServiceFrontendInterface>(this)); - message_loop_.RunAllPending(); + service_->InstallExtension(path); + loop_.RunAllPending(); std::vector<std::string> errors = GetErrors(); if (should_succeed) { EXPECT_TRUE(installed_) << path.value(); + EXPECT_EQ(1u, loaded_.size()) << path.value(); EXPECT_EQ(0u, errors.size()) << path.value(); + EXPECT_EQ(1u, service_->extensions()->size()) << path.value(); + EXPECT_TRUE(service_->GetExtensionByID(loaded_[0]->id())) << path.value(); for (std::vector<std::string>::iterator err = errors.begin(); err != errors.end(); ++err) { LOG(ERROR) << *err; @@ -122,61 +141,55 @@ class ExtensionsServiceTestFrontend } installed_ = NULL; + loaded_.clear(); ExtensionErrorReporter::GetInstance()->ClearErrors(); } - private: - MessageLoop message_loop_; - ExtensionList extensions_; + protected: + scoped_ptr<TestingProfile> profile_; + scoped_refptr<ExtensionsService> service_; + MessageLoop loop_; + std::vector<Extension*> loaded_; + std::string unloaded_id_; Extension* installed_; - std::string reinstalled_id_; -}; + std::string registry_path_; -// make the test a PlatformTest to setup autorelease pools properly on mac -class ExtensionsServiceTest : public testing::Test { - public: - static void SetUpTestCase() { - ExtensionErrorReporter::Init(false); // no noisy errors - } - - virtual void SetUp() { - ExtensionErrorReporter::GetInstance()->ClearErrors(); - } + private: + NotificationRegistrar registrar_; }; // Test loading good extensions from the profile directory. TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectorySuccess) { - FilePath extensions_path; - ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); - extensions_path = extensions_path.AppendASCII("extensions"); - extensions_path = extensions_path.AppendASCII("good"); + // Copy the test extensions into the test profile. + FilePath source_path; + ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &source_path)); + source_path = source_path.AppendASCII("extensions"); + source_path = source_path.AppendASCII("good"); - scoped_refptr<ExtensionsServiceBackend> backend( - new ExtensionsServiceBackend(extensions_path, NULL)); - scoped_refptr<ExtensionsServiceTestFrontend> frontend( - new ExtensionsServiceTestFrontend); + FilePath dest_path = profile_->GetPath().AppendASCII("Extensions"); + file_util::CopyDirectory(source_path, dest_path, true); // recursive - std::vector<Extension*> extensions; - backend->LoadExtensionsFromInstallDirectory( - scoped_refptr<ExtensionsServiceFrontendInterface>(frontend.get())); - frontend->GetMessageLoop()->RunAllPending(); + ASSERT_TRUE(service_->Init()); + loop_.RunAllPending(); std::vector<std::string> errors = GetErrors(); for (std::vector<std::string>::iterator err = errors.begin(); err != errors.end(); ++err) { LOG(ERROR) << *err; } - ASSERT_EQ(3u, frontend->extensions()->size()); + ASSERT_EQ(3u, loaded_.size()); EXPECT_EQ(std::string("00123456789abcdef0123456789abcdef0123456"), - frontend->extensions()->at(0)->id()); + loaded_[0]->id()); EXPECT_EQ(std::string("My extension 1"), - frontend->extensions()->at(0)->name()); + loaded_[0]->name()); EXPECT_EQ(std::string("The first extension that I made."), - frontend->extensions()->at(0)->description()); - EXPECT_EQ(Extension::INTERNAL, frontend->extensions()->at(0)->location()); + loaded_[0]->description()); + EXPECT_EQ(Extension::INTERNAL, loaded_[0]->location()); + EXPECT_TRUE(service_->GetExtensionByID(loaded_[0]->id())); + EXPECT_EQ(3u, service_->extensions()->size()); - Extension* extension = frontend->extensions()->at(0); + Extension* extension = loaded_[0]; const UserScriptList& scripts = extension->content_scripts(); const std::vector<std::string>& toolstrips = extension->toolstrips(); ASSERT_EQ(2u, scripts.size()); @@ -204,47 +217,39 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectorySuccess) { EXPECT_EQ("toolstrip2.html", toolstrips[1]); EXPECT_EQ(std::string("10123456789abcdef0123456789abcdef0123456"), - frontend->extensions()->at(1)->id()); - EXPECT_EQ(std::string("My extension 2"), - frontend->extensions()->at(1)->name()); - EXPECT_EQ(std::string(""), - frontend->extensions()->at(1)->description()); - EXPECT_EQ(frontend->extensions()->at(1)->path().AppendASCII("npapi").value(), - frontend->extensions()->at(1)->plugins_dir().value()); - EXPECT_EQ(frontend->extensions()->at(1)->GetResourceURL("background.html"), - frontend->extensions()->at(1)->background_url()); - EXPECT_EQ(0u, frontend->extensions()->at(1)->content_scripts().size()); - EXPECT_EQ(Extension::INTERNAL, frontend->extensions()->at(1)->location()); + loaded_[1]->id()); + EXPECT_EQ(std::string("My extension 2"), loaded_[1]->name()); + EXPECT_EQ(std::string(""), loaded_[1]->description()); + EXPECT_EQ(loaded_[1]->path().AppendASCII("npapi").value(), + loaded_[1]->plugins_dir().value()); + EXPECT_EQ(loaded_[1]->GetResourceURL("background.html"), + loaded_[1]->background_url()); + EXPECT_EQ(0u, loaded_[1]->content_scripts().size()); + EXPECT_EQ(Extension::INTERNAL, loaded_[1]->location()); EXPECT_EQ(std::string("20123456789abcdef0123456789abcdef0123456"), - frontend->extensions()->at(2)->id()); - EXPECT_EQ(std::string("My extension 3"), - frontend->extensions()->at(2)->name()); - EXPECT_EQ(std::string(""), - frontend->extensions()->at(2)->description()); - EXPECT_EQ(0u, frontend->extensions()->at(2)->content_scripts().size()); - EXPECT_EQ(Extension::EXTERNAL, frontend->extensions()->at(2)->location()); + loaded_[2]->id()); + EXPECT_EQ(std::string("My extension 3"), loaded_[2]->name()); + EXPECT_EQ(std::string(""), loaded_[2]->description()); + EXPECT_EQ(0u, loaded_[2]->content_scripts().size()); + EXPECT_EQ(Extension::INTERNAL, loaded_[2]->location()); }; // Test loading bad extensions from the profile directory. TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectoryFail) { - FilePath extensions_path; - ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); - extensions_path = extensions_path.AppendASCII("extensions"); - extensions_path = extensions_path.AppendASCII("bad"); + FilePath source_path; + ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &source_path)); + source_path = source_path.AppendASCII("extensions"); + source_path = source_path.AppendASCII("bad"); - scoped_refptr<ExtensionsServiceBackend> backend( - new ExtensionsServiceBackend(extensions_path, NULL)); - scoped_refptr<ExtensionsServiceTestFrontend> frontend( - new ExtensionsServiceTestFrontend); + FilePath dest_path = profile_->GetPath().AppendASCII("Extensions"); + file_util::CopyDirectory(source_path, dest_path, true); // recursive - std::vector<Extension*> extensions; - backend->LoadExtensionsFromInstallDirectory( - scoped_refptr<ExtensionsServiceFrontendInterface>(frontend.get())); - frontend->GetMessageLoop()->RunAllPending(); + ASSERT_TRUE(service_->Init()); + loop_.RunAllPending(); - EXPECT_EQ(4u, GetErrors().size()); - EXPECT_EQ(0u, frontend->extensions()->size()); + EXPECT_EQ(3u, GetErrors().size()); + EXPECT_EQ(0u, loaded_.size()); EXPECT_TRUE(MatchPattern(GetErrors()[0], std::string("Could not load extension from '*'. * ") + @@ -257,206 +262,275 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectoryFail) { EXPECT_TRUE(MatchPattern(GetErrors()[2], std::string("Could not load extension from '*'. ") + Extension::kInvalidManifestError)) << GetErrors()[2]; - - EXPECT_TRUE(MatchPattern(GetErrors()[3], - "Could not load extension from '*'. Could not read '*' file.")) << - GetErrors()[3]; }; +// Test that partially deleted extensions are cleaned up during startup +// Test loading bad extensions from the profile directory. +TEST_F(ExtensionsServiceTest, CleanupOnStartup) { + FilePath source_path; + ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &source_path)); + source_path = source_path.AppendASCII("extensions"); + source_path = source_path.AppendASCII("good"); + + FilePath dest_path = profile_->GetPath().AppendASCII("Extensions"); + file_util::CopyDirectory(source_path, dest_path, true); // recursive + + // Simulate that one of them got partially deleted by deling the + // Current Version file. + dest_path = dest_path.AppendASCII("extension1") + .AppendASCII(ExtensionsService::kCurrentVersionFileName); + ASSERT_TRUE(file_util::Delete(dest_path, false)); // not recursive + + service_->Init(); + loop_.RunAllPending(); + + // We should have only gotten two extensions now. + EXPECT_EQ(2u, loaded_.size()); + + // And extension1 dir should now be toast. + dest_path = dest_path.DirName(); + ASSERT_FALSE(file_util::PathExists(dest_path)); +} + // Test installing extensions. TEST_F(ExtensionsServiceTest, InstallExtension) { FilePath extensions_path; ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); extensions_path = extensions_path.AppendASCII("extensions"); - FilePath install_dir; - file_util::CreateNewTempDirectory(FILE_PATH_LITERAL("ext_test"), - &install_dir); - scoped_refptr<ExtensionsServiceBackend> backend( - new ExtensionsServiceBackend(install_dir, NULL)); - scoped_refptr<ExtensionsServiceTestFrontend> frontend( - new ExtensionsServiceTestFrontend); - - FilePath path = extensions_path.AppendASCII("good.crx"); - // A simple extension that should install without error. - frontend->TestInstallExtension(path, backend, true); + FilePath path = extensions_path.AppendASCII("good.crx"); + TestInstallExtension(path, true); // TODO(erikkay): verify the contents of the installed extension. // 0-length extension file. path = extensions_path.AppendASCII("not_an_extension.crx"); - frontend->TestInstallExtension(path, backend, false); + TestInstallExtension(path, false); // Bad magic number. path = extensions_path.AppendASCII("bad_magic.crx"); - frontend->TestInstallExtension(path, backend, false); + TestInstallExtension(path, false); // Poorly formed JSON. path = extensions_path.AppendASCII("bad_json.crx"); - frontend->TestInstallExtension(path, backend, false); + TestInstallExtension(path, false); // Incorrect zip hash. path = extensions_path.AppendASCII("bad_hash.crx"); - frontend->TestInstallExtension(path, backend, false); + TestInstallExtension(path, false); // TODO(erikkay): add more tests for many of the failure cases. // TODO(erikkay): add tests for upgrade cases. } -// Tests uninstalling extensions -TEST_F(ExtensionsServiceTest, UninstallExtension) { +// Test that when an extension version is reinstalled, nothing happens. +TEST_F(ExtensionsServiceTest, Reinstall) { FilePath extensions_path; ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); extensions_path = extensions_path.AppendASCII("extensions"); - FilePath install_path; - file_util::CreateNewTempDirectory(FILE_PATH_LITERAL("ext_test"), - &install_path); - scoped_refptr<ExtensionsServiceBackend> backend( - new ExtensionsServiceBackend(install_path, NULL)); - scoped_refptr<ExtensionsServiceTestFrontend> frontend( - new ExtensionsServiceTestFrontend); - + // A simple extension that should install without error. FilePath path = extensions_path.AppendASCII("good.crx"); + service_->InstallExtension(path); + loop_.RunAllPending(); + + ASSERT_TRUE(installed_); + ASSERT_EQ(1u, loaded_.size()); + ASSERT_EQ(0u, GetErrors().size()); + + installed_ = NULL; + loaded_.clear(); + ExtensionErrorReporter::GetInstance()->ClearErrors(); + + // Reinstall the same version, nothing should happen. + service_->InstallExtension(path); + loop_.RunAllPending(); + + ASSERT_FALSE(installed_); + ASSERT_EQ(0u, loaded_.size()); + ASSERT_EQ(0u, GetErrors().size()); +} + +// Tests uninstalling normal extensions +TEST_F(ExtensionsServiceTest, UninstallExtension) { + FilePath extensions_path; + ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); + extensions_path = extensions_path.AppendASCII("extensions"); // A simple extension that should install without error. - frontend->TestInstallExtension(path, backend, true); + FilePath path = extensions_path.AppendASCII("good.crx"); + TestInstallExtension(path, true); // The directory should be there now. + FilePath install_path = profile_->GetPath().AppendASCII("Extensions"); const char* extension_id = "00123456789abcdef0123456789abcdef0123456"; FilePath extension_path = install_path.AppendASCII(extension_id); EXPECT_TRUE(file_util::PathExists(extension_path)); - // Uninstall it, directory should be gone. - backend->UninstallExtension(extension_id); + // Uninstall it. + service_->UninstallExtension(extension_id); + + // We should get an unload notification. + ASSERT_TRUE(unloaded_id_.length()); + EXPECT_EQ(extension_id, unloaded_id_); + + // The extension should not be in the service anymore. + ASSERT_FALSE(service_->GetExtensionByID(extension_id)); + loop_.RunAllPending(); + + // The directory should be gone. EXPECT_FALSE(file_util::PathExists(extension_path)); // Try uinstalling one that doesn't have a Current Version file for some // reason. - frontend->TestInstallExtension(path, backend, true); + unloaded_id_.clear(); + TestInstallExtension(path, true); FilePath current_version_file = extension_path.AppendASCII(ExtensionsService::kCurrentVersionFileName); EXPECT_TRUE(file_util::Delete(current_version_file, true)); - backend->UninstallExtension(extension_id); + service_->UninstallExtension(extension_id); + loop_.RunAllPending(); EXPECT_FALSE(file_util::PathExists(extension_path)); - - // Try uninstalling one that doesn't even exist. We shouldn't crash. - backend->UninstallExtension(extension_id); -} - -TEST_F(ExtensionsServiceTest, ReinstallExtension) { - // In this test, we install two extensions, verify that they both install - // correctly, then install the first extension again and verify that it was - // not installed, and that VersionReinstalled was called instead. - FilePath extensions_path; - ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); - extensions_path = extensions_path.AppendASCII("extensions"); - - FilePath install_dir; - file_util::CreateNewTempDirectory(FILE_PATH_LITERAL("ext_test"), - &install_dir); - scoped_refptr<ExtensionsServiceBackend> backend( - new ExtensionsServiceBackend(install_dir, NULL)); - scoped_refptr<ExtensionsServiceTestFrontend> frontend( - new ExtensionsServiceTestFrontend); - - FilePath path = extensions_path.AppendASCII("good.crx"); - FilePath path2 = extensions_path.AppendASCII("theme.crx"); - - // Verify that our extensions are valid. - ASSERT_TRUE(file_util::PathExists(path)); - ASSERT_TRUE(file_util::PathExists(path2)); - - frontend->ClearInstalledReinstalled(); - // Install an extension. - backend->InstallExtension(path, - scoped_refptr<ExtensionsServiceFrontendInterface>(frontend.get())); - frontend->GetMessageLoop()->RunAllPending(); - std::vector<std::string> errors = GetErrors(); - - // Verify that it was installed. - EXPECT_TRUE(frontend->installed()) << path.value(); - EXPECT_EQ(0u, errors.size()) << path.value(); - - // Install our second extension. - frontend->ClearInstalledReinstalled(); - backend->InstallExtension(path2, - scoped_refptr<ExtensionsServiceFrontendInterface>(frontend.get())); - frontend->GetMessageLoop()->RunAllPending(); - errors = GetErrors(); - - // Verify that it was installed without reinstall getting called. - EXPECT_TRUE(frontend->installed()) << path2.value(); - EXPECT_TRUE(frontend->reinstalled_id().empty()); - EXPECT_EQ(0u, errors.size()) << path.value(); - - // Install the first extension again. - frontend->ClearInstalledReinstalled(); - backend->InstallExtension(path, - scoped_refptr<ExtensionsServiceFrontendInterface>(frontend.get())); - frontend->GetMessageLoop()->RunAllPending(); - errors = GetErrors(); - - // Verify that reinstall was called and installed was not. - EXPECT_FALSE(frontend->installed()) << path.value(); - EXPECT_FALSE(frontend->reinstalled_id().empty()) << path.value(); - EXPECT_EQ(0u, errors.size()) << path.value(); } +// Tests loading single extensions (like --load-extension) TEST_F(ExtensionsServiceTest, LoadExtension) { FilePath extensions_path; ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); extensions_path = extensions_path.AppendASCII("extensions"); - scoped_refptr<ExtensionsServiceBackend> backend( - new ExtensionsServiceBackend(extensions_path, NULL)); - scoped_refptr<ExtensionsServiceTestFrontend> frontend( - new ExtensionsServiceTestFrontend); - FilePath ext1 = extensions_path.AppendASCII("good").AppendASCII("extension1") .AppendASCII("1"); - backend->LoadSingleExtension(ext1, - scoped_refptr<ExtensionsServiceFrontendInterface>(frontend.get())); - frontend->GetMessageLoop()->RunAllPending(); + service_->LoadExtension(ext1); + loop_.RunAllPending(); EXPECT_EQ(0u, GetErrors().size()); - ASSERT_EQ(1u, frontend->extensions()->size()); + ASSERT_EQ(1u, loaded_.size()); + ASSERT_EQ(Extension::LOAD, loaded_[0]->location()); FilePath no_manifest = extensions_path.AppendASCII("bad") .AppendASCII("no_manifest").AppendASCII("1"); - backend->LoadSingleExtension(no_manifest, - scoped_refptr<ExtensionsServiceFrontendInterface>(frontend.get())); - frontend->GetMessageLoop()->RunAllPending(); + service_->LoadExtension(no_manifest); + loop_.RunAllPending(); EXPECT_EQ(1u, GetErrors().size()); - ASSERT_EQ(1u, frontend->extensions()->size()); - ASSERT_EQ(Extension::LOAD, frontend->extensions()->at(0)->location()); + ASSERT_EQ(1u, loaded_.size()); + + // Test uninstall + std::string id = loaded_[0]->id(); + EXPECT_FALSE(unloaded_id_.length()); + service_->UninstallExtension(id); + loop_.RunAllPending(); + EXPECT_EQ(id, unloaded_id_); } +// Tests that we generate IDs when they are not specified in the manifest for +// --load-extension. TEST_F(ExtensionsServiceTest, GenerateID) { FilePath extensions_path; ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); extensions_path = extensions_path.AppendASCII("extensions"); - scoped_refptr<ExtensionsServiceBackend> backend( - new ExtensionsServiceBackend(extensions_path, NULL)); - scoped_refptr<ExtensionsServiceTestFrontend> frontend( - new ExtensionsServiceTestFrontend); - FilePath no_id_ext = extensions_path.AppendASCII("no_id"); - backend->LoadSingleExtension(no_id_ext, - scoped_refptr<ExtensionsServiceFrontendInterface>(frontend.get())); - frontend->GetMessageLoop()->RunAllPending(); + service_->LoadExtension(no_id_ext); + loop_.RunAllPending(); EXPECT_EQ(0u, GetErrors().size()); - ASSERT_EQ(1u, frontend->extensions()->size()); - std::string id1 = frontend->extensions()->at(0)->id(); + ASSERT_EQ(1u, loaded_.size()); + std::string id1 = loaded_[0]->id(); ASSERT_EQ("0000000000000000000000000000000000000000", id1); ASSERT_EQ("chrome-extension://0000000000000000000000000000000000000000/", - frontend->extensions()->at(0)->url().spec()); + loaded_[0]->url().spec()); - backend->LoadSingleExtension(no_id_ext, - scoped_refptr<ExtensionsServiceFrontendInterface>(frontend.get())); - frontend->GetMessageLoop()->RunAllPending(); - std::string id2 = frontend->extensions()->at(1)->id(); + service_->LoadExtension(no_id_ext); + loop_.RunAllPending(); + std::string id2 = loaded_[1]->id(); ASSERT_EQ("0000000000000000000000000000000000000001", id2); ASSERT_EQ("chrome-extension://0000000000000000000000000000000000000001/", - frontend->extensions()->at(1)->url().spec()); + loaded_[1]->url().spec()); } + +// Tests the external installation feature +#if defined(OS_WIN) + +TEST_F(ExtensionsServiceTest, ExternalInstall) { + // Register a test extension externally. + FilePath source_path; + ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &source_path)); + source_path = source_path.AppendASCII("extensions").AppendASCII("good.crx"); + + RegKey key; + std::wstring reg_path = ASCIIToWide(registry_path_); + reg_path += L"\\00123456789ABCDEF0123456789ABCDEF0123456"; + ASSERT_TRUE(key.Create(HKEY_LOCAL_MACHINE, reg_path.c_str(), KEY_WRITE)); + ASSERT_TRUE(key.WriteValue(L"path", source_path.ToWStringHack().c_str())); + ASSERT_TRUE(key.WriteValue(L"version", L"1.0.0.0")); + + // Start up the service, it should find our externally registered extension + // and install it. + service_->Init(); + loop_.RunAllPending(); + + ASSERT_EQ(0u, GetErrors().size()); + ASSERT_EQ(1u, loaded_.size()); + ASSERT_EQ(Extension::EXTERNAL, loaded_[0]->location()); + ASSERT_EQ("1.0.0.0", loaded_[0]->version()->GetString()); + + // Reinit the service without changing anything. The extension should be + // loaded again. + loaded_.clear(); + service_->Init(); + loop_.RunAllPending(); + ASSERT_EQ(0u, GetErrors().size()); + ASSERT_EQ(1u, loaded_.size()); + + // Now update the extension with a new version. We should get upgraded. + source_path = source_path.DirName().AppendASCII("good2.crx"); + ASSERT_TRUE(key.WriteValue(L"path", source_path.ToWStringHack().c_str())); + ASSERT_TRUE(key.WriteValue(L"version", L"1.0.0.1")); + + loaded_.clear(); + service_->Init(); + loop_.RunAllPending(); + ASSERT_EQ(0u, GetErrors().size()); + ASSERT_EQ(1u, loaded_.size()); + ASSERT_EQ("1.0.0.1", loaded_[0]->version()->GetString()); + + // Uninstall the extension and reinit. Nothing should happen because the + // preference should prevent us from reinstalling. + std::string id = loaded_[0]->id(); + service_->UninstallExtension(id); + loop_.RunAllPending(); + + // The extension should also be gone from the install directory. + FilePath install_path = profile_->GetPath().AppendASCII("Extensions") + .AppendASCII(id); + ASSERT_FALSE(file_util::PathExists(install_path)); + + loaded_.clear(); + service_->Init(); + loop_.RunAllPending(); + ASSERT_EQ(0u, loaded_.size()); + + // Now clear the preference, reinstall, then remove the reg key. The extension + // should be uninstalled. + profile_->GetPrefs()->GetMutableList(L"extensions.uninstalled_external_ids") + ->Clear(); + profile_->GetPrefs()->ScheduleSavePersistentPrefs(); + + loaded_.clear(); + service_->Init(); + loop_.RunAllPending(); + ASSERT_EQ(1u, loaded_.size()); + + RegKey parent_key; + key.Open(HKEY_LOCAL_MACHINE, ASCIIToWide(registry_path_).c_str(), KEY_WRITE); + key.DeleteKey(ASCIIToWide(id).c_str()); + loaded_.clear(); + service_->Init(); + loop_.RunAllPending(); + ASSERT_EQ(0u, loaded_.size()); +} + +#else + +// TODO(port) + +#endif diff --git a/chrome/browser/extensions/extensions_ui.cc b/chrome/browser/extensions/extensions_ui.cc index e150840..8dafb8e 100644 --- a/chrome/browser/extensions/extensions_ui.cc +++ b/chrome/browser/extensions/extensions_ui.cc @@ -68,6 +68,8 @@ ExtensionsDOMHandler::ExtensionsDOMHandler(DOMUI* dom_ui, NewCallback(this, &ExtensionsDOMHandler::HandleRequestExtensionsData)); dom_ui_->RegisterMessageCallback("inspect", NewCallback(this, &ExtensionsDOMHandler::HandleInspectMessage)); + dom_ui_->RegisterMessageCallback("uninstall", + NewCallback(this, &ExtensionsDOMHandler::HandleUninstallMessage)); } void ExtensionsDOMHandler::HandleRequestExtensionsData(const Value* value) { @@ -118,6 +120,15 @@ void ExtensionsDOMHandler::HandleInspectMessage(const Value* value) { g_browser_process->devtools_manager()->OpenDevToolsWindow(host); } +void ExtensionsDOMHandler::HandleUninstallMessage(const Value* value) { + CHECK(value->IsType(Value::TYPE_LIST)); + const ListValue* list = static_cast<const ListValue*>(value); + CHECK(list->GetSize() == 1); + std::string extension_id; + CHECK(list->GetString(0, &extension_id)); + extensions_service_->UninstallExtension(extension_id); +} + static void CreateScriptFileDetailValue( const FilePath& extension_path, const UserScript::FileList& scripts, const wchar_t* key, DictionaryValue* script_data) { diff --git a/chrome/browser/extensions/extensions_ui.h b/chrome/browser/extensions/extensions_ui.h index 953cb31e..417fd00 100644 --- a/chrome/browser/extensions/extensions_ui.h +++ b/chrome/browser/extensions/extensions_ui.h @@ -69,6 +69,9 @@ class ExtensionsDOMHandler : public DOMMessageHandler { // Callback for "inspect" message. void HandleInspectMessage(const Value* value); + // Callback for "uninstall" message. + void HandleUninstallMessage(const Value* value); + // Helper that lists the current active html pages for an extension. std::vector<ExtensionPage> GetActivePagesForExtension( const std::string& extension_id); diff --git a/chrome/browser/extensions/user_script_master.cc b/chrome/browser/extensions/user_script_master.cc index 8b5e4d9..e1ed5db 100644 --- a/chrome/browser/extensions/user_script_master.cc +++ b/chrome/browser/extensions/user_script_master.cc @@ -14,6 +14,8 @@ #include "base/string_util.h" #include "base/thread.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/extensions/extension.h" +#include "chrome/browser/extensions/extensions_service.h" #include "chrome/common/notification_service.h" #include "chrome/common/url_constants.h" #include "net/base/net_util.h" @@ -257,6 +259,11 @@ UserScriptMaster::UserScriptMaster(MessageLoop* worker_loop, pending_scan_(false) { if (!user_script_dir_.value().empty()) AddWatchedPath(script_dir); + + registrar_.Add(this, NotificationType::EXTENSIONS_LOADED, + NotificationService::AllSources()); + registrar_.Add(this, NotificationType::EXTENSION_UNLOADED, + NotificationService::AllSources()); } UserScriptMaster::~UserScriptMaster() { @@ -313,6 +320,55 @@ void UserScriptMaster::OnDirectoryChanged(const FilePath& path) { StartScan(); } +void UserScriptMaster::Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + switch (type.value) { + case NotificationType::EXTENSIONS_LOADED: { + // TODO(aa): Fix race here. A page could need a content script on startup, + // before the extension has loaded. We need to freeze the renderer in + // that case. + // See: http://code.google.com/p/chromium/issues/detail?id=11547. + + // Add any content scripts inside the extension. + ExtensionList* extensions = Details<ExtensionList>(details).ptr(); + for (ExtensionList::iterator extension_iterator = extensions->begin(); + extension_iterator != extensions->end(); ++extension_iterator) { + Extension* extension = *extension_iterator; + const UserScriptList& scripts = extension->content_scripts(); + for (UserScriptList::const_iterator iter = scripts.begin(); + iter != scripts.end(); ++iter) { + lone_scripts_.push_back(*iter); + } + } + StartScan(); + break; + } + + case NotificationType::EXTENSION_UNLOADED: { + // Remove any content scripts. + Extension* extension = Details<Extension>(details).ptr(); + UserScriptList new_lone_scripts; + for (UserScriptList::iterator iter = lone_scripts_.begin(); + iter != lone_scripts_.end(); ++iter) { + if (iter->extension_id() != extension->id()) { + new_lone_scripts.push_back(*iter); + } + } + lone_scripts_ = new_lone_scripts; + StartScan(); + + // TODO(aa): Do we want to do something smarter for the scripts that have + // already been injected? + + break; + } + + default: + DCHECK(false); + } +} + void UserScriptMaster::StartScan() { if (!script_reloader_) script_reloader_ = new ScriptReloader(this); diff --git a/chrome/browser/extensions/user_script_master.h b/chrome/browser/extensions/user_script_master.h index 6a9a425..4367c7e 100644 --- a/chrome/browser/extensions/user_script_master.h +++ b/chrome/browser/extensions/user_script_master.h @@ -14,6 +14,7 @@ #include "base/scoped_ptr.h" #include "base/shared_memory.h" #include "chrome/common/extensions/user_script.h" +#include "chrome/common/notification_registrar.h" #include "googleurl/src/gurl.h" #include "testing/gtest/include/gtest/gtest_prod.h" @@ -22,7 +23,8 @@ class StringPiece; // Manages a segment of shared memory that contains the user scripts the user // has installed. Lives on the UI thread. class UserScriptMaster : public base::RefCounted<UserScriptMaster>, - public DirectoryWatcher::Delegate { + public DirectoryWatcher::Delegate, + public NotificationObserver { public: // For testability, the constructor takes the MessageLoop to run the // script-reloading worker on as well as the path the scripts live in. @@ -30,11 +32,6 @@ class UserScriptMaster : public base::RefCounted<UserScriptMaster>, UserScriptMaster(MessageLoop* worker, const FilePath& script_dir); ~UserScriptMaster(); - // Add a single user script that exists outside the script directory. - void AddLoneScript(const UserScript& script) { - lone_scripts_.push_back(script); - } - // Add a watched directory. All scripts will be reloaded when any file in // this directory changes. void AddWatchedPath(const FilePath& path); @@ -126,6 +123,14 @@ class UserScriptMaster : public base::RefCounted<UserScriptMaster>, // DirectoryWatcher::Delegate implementation. virtual void OnDirectoryChanged(const FilePath& path); + // NotificationObserver implementation. + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details); + + // Manages our notification registrations. + NotificationRegistrar registrar_; + // The directories containing user scripts. FilePath user_script_dir_; diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc index 3ea9cc1..ae75216 100644 --- a/chrome/browser/net/chrome_url_request_context.cc +++ b/chrome/browser/net/chrome_url_request_context.cc @@ -312,6 +312,9 @@ ChromeURLRequestContext::ChromeURLRequestContext(Profile* profile) NotificationService::current()->AddObserver( this, NotificationType::EXTENSIONS_LOADED, NotificationService::AllSources()); + NotificationService::current()->AddObserver( + this, NotificationType::EXTENSION_UNLOADED, + NotificationService::AllSources()); } // NotificationObserver implementation. @@ -349,6 +352,11 @@ void ChromeURLRequestContext::Observe(NotificationType type, g_browser_process->io_thread()->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(this, &ChromeURLRequestContext::OnNewExtensions, new_paths)); + } else if (NotificationType::EXTENSION_UNLOADED == type) { + Extension* extension = Details<Extension>(details).ptr(); + g_browser_process->io_thread()->message_loop()->PostTask(FROM_HERE, + NewRunnableMethod(this, &ChromeURLRequestContext::OnUnloadedExtension, + extension->id())); } else { NOTREACHED(); } @@ -363,6 +371,9 @@ void ChromeURLRequestContext::CleanupOnUIThread() { NotificationService::current()->RemoveObserver( this, NotificationType::EXTENSIONS_LOADED, NotificationService::AllSources()); + NotificationService::current()->RemoveObserver( + this, NotificationType::EXTENSION_UNLOADED, + NotificationService::AllSources()); } FilePath ChromeURLRequestContext::GetPathForExtension(const std::string& id) { @@ -399,6 +410,13 @@ void ChromeURLRequestContext::OnNewExtensions(ExtensionPaths* new_paths) { delete new_paths; } +void ChromeURLRequestContext::OnUnloadedExtension( + const std::string& extension_id) { + ExtensionPaths::iterator iter = extension_paths_.find(extension_id); + DCHECK(iter != extension_paths_.end()); + extension_paths_.erase(iter); +} + ChromeURLRequestContext::~ChromeURLRequestContext() { DCHECK(NULL == prefs_); diff --git a/chrome/browser/net/chrome_url_request_context.h b/chrome/browser/net/chrome_url_request_context.h index 55af609..d5024c1 100644 --- a/chrome/browser/net/chrome_url_request_context.h +++ b/chrome/browser/net/chrome_url_request_context.h @@ -94,6 +94,9 @@ class ChromeURLRequestContext : public URLRequestContext, // Callback for when new extensions are loaded. void OnNewExtensions(ExtensionPaths* new_paths); + // Callback for when an extension is unloaded. + void OnUnloadedExtension(const std::string& id); + // Destructor. virtual ~ChromeURLRequestContext(); diff --git a/chrome/browser/plugin_service.cc b/chrome/browser/plugin_service.cc index eb8ec2a..a5c81b2 100644 --- a/chrome/browser/plugin_service.cc +++ b/chrome/browser/plugin_service.cc @@ -12,11 +12,15 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/chrome_plugin_host.h" #include "chrome/browser/chrome_thread.h" +#include "chrome/browser/extensions/extension.h" +#include "chrome/browser/extensions/extensions_service.h" #include "chrome/browser/plugin_process_host.h" #include "chrome/browser/renderer_host/render_process_host.h" #include "chrome/common/chrome_plugin_lib.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/logging_chrome.h" +#include "chrome/common/notification_type.h" +#include "chrome/common/notification_service.h" #include "chrome/common/render_messages.h" #include "webkit/glue/plugins/plugin_constants_win.h" #include "webkit/glue/plugins/plugin_list.h" @@ -53,6 +57,13 @@ PluginService::PluginService() hklm_watcher_.StartWatching(hklm_event_.get(), this); } #endif + + NotificationService::current()->AddObserver( + this, NotificationType::EXTENSIONS_LOADED, + NotificationService::AllSources()); + NotificationService::current()->AddObserver( + this, NotificationType::EXTENSION_UNLOADED, + NotificationService::AllSources()); } PluginService::~PluginService() { @@ -214,3 +225,32 @@ void PluginService::OnWaitableEventSignaled(base::WaitableEvent* waitable_event) } #endif } + +void PluginService::Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + switch (type.value) { + case NotificationType::EXTENSIONS_LOADED: { + // TODO(mpcomplete): We also need to force a renderer to refresh its + // cache of the plugin list when we inject user scripts, since it could + // have a stale version by the time extensions are loaded. + // See: http://code.google.com/p/chromium/issues/detail?id=12306 + ExtensionList* extensions = Details<ExtensionList>(details).ptr(); + for (ExtensionList::iterator extension = extensions->begin(); + extension != extensions->end(); ++extension) + if (!(*extension)->plugins_dir().empty()) + AddExtraPluginDir((*extension)->plugins_dir()); + break; + } + + case NotificationType::EXTENSION_UNLOADED: { + // TODO(aa): Implement this. Also, will it be possible to delete the + // extension folder if this isn't unloaded? + // See: http://code.google.com/p/chromium/issues/detail?id=12306 + break; + } + + default: + DCHECK(false); + } +} diff --git a/chrome/browser/plugin_service.h b/chrome/browser/plugin_service.h index 883953c..676df28 100644 --- a/chrome/browser/plugin_service.h +++ b/chrome/browser/plugin_service.h @@ -17,6 +17,7 @@ #include "base/singleton.h" #include "base/waitable_event_watcher.h" #include "chrome/browser/browser_process.h" +#include "chrome/common/notification_observer.h" #include "webkit/glue/webplugininfo.h" #if defined(OS_WIN) @@ -36,7 +37,9 @@ class ResourceMessageFilter; // This can be called on the main thread and IO thread. However it must // be created on the main thread. -class PluginService : base::WaitableEventWatcher::Delegate { +class PluginService + : public base::WaitableEventWatcher::Delegate, + public NotificationObserver { public: // Returns the PluginService singleton. static PluginService* GetInstance(); @@ -116,7 +119,11 @@ class PluginService : base::WaitableEventWatcher::Delegate { ~PluginService(); // base::WaitableEventWatcher::Delegate implementation. - void OnWaitableEventSignaled(base::WaitableEvent* waitable_event); + virtual void OnWaitableEventSignaled(base::WaitableEvent* waitable_event); + + // NotificationObserver implementation + virtual void Observe(NotificationType type, const NotificationSource& source, + const NotificationDetails& details); // mapping between plugin path and PluginProcessHost typedef base::hash_map<FilePath, PluginProcessHost*> PluginMap; diff --git a/chrome/browser/profile.cc b/chrome/browser/profile.cc index cdb7c00..2ca2342 100644 --- a/chrome/browser/profile.cc +++ b/chrome/browser/profile.cc @@ -477,7 +477,10 @@ void ProfileImpl::InitExtensions() { ExtensionErrorReporter::Init(true); // allow noisy errors. user_script_master_ = new UserScriptMaster( g_browser_process->file_thread()->message_loop(), script_dir); - extensions_service_ = new ExtensionsService(this, user_script_master_.get()); + extensions_service_ = new ExtensionsService( + this, MessageLoop::current(), + g_browser_process->file_thread()->message_loop(), + std::string()); // Use default registry path // If we have extensions, the extension service will kick off the first scan // after extensions are loaded. Otherwise, we need to do that now. diff --git a/chrome/browser/resources/extensions_ui.html b/chrome/browser/resources/extensions_ui.html index 8273c59..c689c82 100644 --- a/chrome/browser/resources/extensions_ui.html +++ b/chrome/browser/resources/extensions_ui.html @@ -117,6 +117,25 @@ function sendInspectMessage(viewData) { String(viewData.renderProcessId), String(viewData.renderViewId) ]); } + +/** + * Handles an 'uninstall' button getting clicked. + */ +function handleUninstallExtension(node) { + // Tell the C++ ExtensionDOMHandler to uninstall an extension. + chrome.send('uninstall', [node.extensionId]); + + // Find the div above us with class 'extension' and remove it. + while (node) { + if (node.className == 'extension') { + node.parentNode.removeChild(node); + return; + } + node = node.parentNode; + } + + throw new Error("Couldn't find containing extension element."); +} </script> <style> body { @@ -142,10 +161,13 @@ div.extension-name { font-size: large; font-weight: bold; margin-top: 2em; - margin-bottom: 1em; text-align: left; } +div.extension-uninstall { + margin-bottom: 1em; +} + dl { margin: 0px 0px 3px 0px; } @@ -189,10 +211,6 @@ th.desc { width: 50%; } -th.enabled { - width: 10%; -} - #error-box { background-color:#D8D8D8; margin-top: 8px; @@ -225,8 +243,13 @@ th.enabled { <div jsdisplay="extensions.length > 0"> <div class="extension" jsselect="extensions"> - <div class="extension-name" jscontent="name"> - sExtension Name</div> + <div class="extension-name" jscontent="name">Extension Name</div> + <div class="extension-uninstall"> + <button + jsvalues=".extensionId:id" + onclick="handleUninstallExtension(this)" + >Uninstall</button> + </div> <dl> <dd>Id: <span jscontent="id">0000000000000000000000000000000000000000</span></dd> <dd> diff --git a/chrome/common/notification_type.h b/chrome/common/notification_type.h index 340f5d1..0622979 100644 --- a/chrome/common/notification_type.h +++ b/chrome/common/notification_type.h @@ -553,7 +553,7 @@ class NotificationType { // Sent when a new theme is installed. The details are an Extension. THEME_INSTALLED, - // Sent when new extensions are installed. The details are a FilePath. + // Sent when new extensions are installed. The details are an Extension. EXTENSION_INSTALLED, // Sent when an extension is unloaded. This happens when an extension is diff --git a/chrome/test/data/extensions/good/extension3/1.0/EXTERNAL_INSTALL b/chrome/test/data/extensions/good/extension3/1.0/EXTERNAL_INSTALL deleted file mode 100644 index e69de29..0000000 --- a/chrome/test/data/extensions/good/extension3/1.0/EXTERNAL_INSTALL +++ /dev/null |