diff options
25 files changed, 791 insertions, 511 deletions
diff --git a/base/file_util.h b/base/file_util.h index c51e1a0..7598621 100644 --- a/base/file_util.h +++ b/base/file_util.h @@ -176,7 +176,7 @@ bool CopyFile(const std::wstring& from_path, const std::wstring& to_path); // as well. // If there are files existing under to_path, always overwrite. // Returns true if successful, false otherwise. -// Dont't use wildcards on the names, it may stop working without notice. +// Don't use wildcards on the names, it may stop working without notice. // // If you only need to copy a file use CopyFile, it's faster. bool CopyDirectory(const FilePath& from_path, const FilePath& to_path, 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 51d7983..fcfdbb0 100644 --- a/chrome/browser/extensions/extension.cc +++ b/chrome/browser/extensions/extension.cc @@ -125,23 +125,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 ece0578..0160abd 100644 --- a/chrome/browser/extensions/extension.h +++ b/chrome/browser/extensions/extension.h @@ -107,7 +107,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 @@ -165,10 +164,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. @@ -246,8 +241,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 503c099..ac79d16 100644 --- a/chrome/browser/extensions/extension_process_manager.cc +++ b/chrome/browser/extensions/extension_process_manager.cc @@ -27,6 +27,8 @@ ExtensionProcessManager::ExtensionProcessManager(Profile* profile) : browsing_instance_(new BrowsingInstance(profile)) { registrar_.Add(this, NotificationType::EXTENSIONS_LOADED, NotificationService::AllSources()); + registrar_.Add(this, NotificationType::EXTENSION_UNLOADED, + NotificationService::AllSources()); if (profile->GetExtensionsService()) CreateBackgroundHosts(this, profile->GetExtensionsService()->extensions()); @@ -60,7 +62,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 59eecdb..445f181e 100644 --- a/chrome/browser/extensions/extension_shelf.cc +++ b/chrome/browser/extensions/extension_shelf.cc @@ -145,9 +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. + // 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. @@ -160,9 +162,6 @@ ExtensionShelf::ExtensionShelf(Browser* browser) } } -ExtensionShelf::~ExtensionShelf() { -} - BrowserBubble* ExtensionShelf::GetHandle() { if (!handle_.get() && HasExtensionViews() && current_handle_view_) { ExtensionShelfHandle* handle_view = new ExtensionShelfHandle(this); @@ -266,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; @@ -301,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 23d39dd..59ef31d 100644 --- a/chrome/browser/extensions/extension_shelf.h +++ b/chrome/browser/extensions/extension_shelf.h @@ -10,6 +10,7 @@ #include "chrome/browser/extensions/extension_view.h" #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" @@ -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(); @@ -72,11 +75,12 @@ class ExtensionShelf : public views::View, // Adjust shelf handle size and position. void LayoutShelfHandle(); - NotificationRegistrar registrar_; - // 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 aa9145f..04ccd13 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 = @@ -543,10 +546,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 @@ -789,8 +790,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; @@ -869,7 +869,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( @@ -885,31 +905,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 @@ -919,7 +916,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 @@ -933,16 +931,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))) { @@ -968,21 +973,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; @@ -1013,7 +1018,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; @@ -1025,7 +1033,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..e2c6215 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,11 @@ PluginService::PluginService() hklm_watcher_.StartWatching(hklm_event_.get(), this); } #endif + + registrar_.Add(this, NotificationType::EXTENSIONS_LOADED, + NotificationService::AllSources()); + registrar_.Add(this, NotificationType::EXTENSION_UNLOADED, + NotificationService::AllSources()); } PluginService::~PluginService() { @@ -214,3 +223,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..07a6152 100644 --- a/chrome/browser/plugin_service.h +++ b/chrome/browser/plugin_service.h @@ -17,6 +17,8 @@ #include "base/singleton.h" #include "base/waitable_event_watcher.h" #include "chrome/browser/browser_process.h" +#include "chrome/common/notification_observer.h" +#include "chrome/common/notification_registrar.h" #include "webkit/glue/webplugininfo.h" #if defined(OS_WIN) @@ -36,7 +38,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 +120,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; @@ -137,6 +145,8 @@ class PluginService : base::WaitableEventWatcher::Delegate { // Need synchronization whenever we access the plugin_list singelton through // webkit_glue since this class is called on the main and IO thread. Lock lock_; + + NotificationRegistrar registrar_; #if defined(OS_WIN) // Registry keys for getting notifications when new plugins are installed. 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 c84d705..d26d47e 100644 --- a/chrome/common/notification_type.h +++ b/chrome/common/notification_type.h @@ -555,7 +555,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 diff --git a/chrome/test/data/extensions/good2.crx b/chrome/test/data/extensions/good2.crx Binary files differnew file mode 100755 index 0000000..5dadf8a --- /dev/null +++ b/chrome/test/data/extensions/good2.crx |