diff options
author | akalin@chromium.org <akalin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-06-25 01:36:40 +0000 |
---|---|---|
committer | akalin@chromium.org <akalin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-06-25 01:36:40 +0000 |
commit | f3113e238e6a127ca42099c9413cc0c32db1120c (patch) | |
tree | e67dda2e3999acc08abaf508f9814f3d2e67b471 /chrome/browser | |
parent | a6ae341e9c13a377e7defbde0c41d29bd6a6184e (diff) | |
download | chromium_src-f3113e238e6a127ca42099c9413cc0c32db1120c.zip chromium_src-f3113e238e6a127ca42099c9413cc0c32db1120c.tar.gz chromium_src-f3113e238e6a127ca42099c9413cc0c32db1120c.tar.bz2 |
Reworked ExtensionsService::AddPendingExtension().
Made it take options for enabled/incognito enabled state to apply
on install.
BUG=46515
TEST=unittests, manual (newly-synced disabled extensions should stay
disabled)
Review URL: http://codereview.chromium.org/2819023
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@50804 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
-rw-r--r-- | chrome/browser/extensions/extension_browsertest.cc | 3 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_prefs.cc | 13 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_prefs.h | 4 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_prefs_unittest.cc | 23 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_updater.cc | 41 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_updater_unittest.cc | 34 | ||||
-rw-r--r-- | chrome/browser/extensions/extensions_service.cc | 95 | ||||
-rw-r--r-- | chrome/browser/extensions/extensions_service.h | 18 | ||||
-rw-r--r-- | chrome/browser/extensions/extensions_service_unittest.cc | 114 | ||||
-rw-r--r-- | chrome/browser/extensions/test_extension_prefs.cc | 4 | ||||
-rw-r--r-- | chrome/browser/sync/glue/extension_model_associator.cc | 16 | ||||
-rw-r--r-- | chrome/browser/sync/glue/theme_util.cc | 9 |
12 files changed, 232 insertions, 142 deletions
diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc index ce0f37d..d3494da 100644 --- a/chrome/browser/extensions/extension_browsertest.cc +++ b/chrome/browser/extensions/extension_browsertest.cc @@ -81,7 +81,8 @@ bool ExtensionBrowserTest::LoadExtensionImpl(const FilePath& path, // OnExtensionInstalled ensures the other extension prefs are set up with // the defaults. Extension* extension = service->extensions()->at(num_after - 1); - service->extension_prefs()->OnExtensionInstalled(extension); + service->extension_prefs()->OnExtensionInstalled( + extension, Extension::ENABLED, false); service->SetIsIncognitoEnabled(extension, true); } diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc index 0f9661c..93f29c4 100644 --- a/chrome/browser/extensions/extension_prefs.cc +++ b/chrome/browser/extensions/extension_prefs.cc @@ -444,13 +444,14 @@ void ExtensionPrefs::SetToolbarOrder( prefs_->ScheduleSavePersistentPrefs(); } -void ExtensionPrefs::OnExtensionInstalled(Extension* extension) { +void ExtensionPrefs::OnExtensionInstalled( + Extension* extension, Extension::State initial_state, + bool initial_incognito_enabled) { const std::string& id = extension->id(); - // Make sure we don't enable a disabled extension. - if (GetExtensionState(extension->id()) != Extension::DISABLED) { - UpdateExtensionPref(id, kPrefState, - Value::CreateIntegerValue(Extension::ENABLED)); - } + UpdateExtensionPref(id, kPrefState, + Value::CreateIntegerValue(initial_state)); + UpdateExtensionPref(id, kPrefIncognitoEnabled, + Value::CreateBooleanValue(initial_incognito_enabled)); UpdateExtensionPref(id, kPrefLocation, Value::CreateIntegerValue(extension->location())); FilePath::StringType path = MakePathRelative(install_directory_, diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h index 77487e9..f7733c3 100644 --- a/chrome/browser/extensions/extension_prefs.h +++ b/chrome/browser/extensions/extension_prefs.h @@ -47,7 +47,9 @@ class ExtensionPrefs { void SetToolbarOrder(const std::vector<std::string>& extension_ids); // Called when an extension is installed, so that prefs get created. - void OnExtensionInstalled(Extension* extension); + void OnExtensionInstalled(Extension* extension, + Extension::State initial_state, + bool initial_incognito_enabled); // Called when an extension is uninstalled, so that prefs get cleaned up. void OnExtensionUninstalled(const std::string& extension_id, diff --git a/chrome/browser/extensions/extension_prefs_unittest.cc b/chrome/browser/extensions/extension_prefs_unittest.cc index 5f45ad5..26df5b1 100644 --- a/chrome/browser/extensions/extension_prefs_unittest.cc +++ b/chrome/browser/extensions/extension_prefs_unittest.cc @@ -345,3 +345,26 @@ class ExtensionPrefsAppToolbars : public ExtensionPrefsTest { std::string extension_id_overridden_off_; }; TEST_F(ExtensionPrefsAppToolbars, ExtensionPrefsAppToolbars) {} + +class ExtensionPrefsOnExtensionInstalled : public ExtensionPrefsTest { + public: + virtual void Initialize() { + extension_.reset(prefs_.AddExtension("on_extension_installed")); + EXPECT_EQ(Extension::ENABLED, + prefs()->GetExtensionState(extension_->id())); + EXPECT_FALSE(prefs()->IsIncognitoEnabled(extension_->id())); + prefs()->OnExtensionInstalled(extension_.get(), + Extension::DISABLED, true); + } + + virtual void Verify() { + EXPECT_EQ(Extension::DISABLED, + prefs()->GetExtensionState(extension_->id())); + EXPECT_TRUE(prefs()->IsIncognitoEnabled(extension_->id())); + } + + private: + scoped_ptr<Extension> extension_; +}; +TEST_F(ExtensionPrefsOnExtensionInstalled, + ExtensionPrefsOnExtensionInstalled) {} diff --git a/chrome/browser/extensions/extension_updater.cc b/chrome/browser/extensions/extension_updater.cc index 9c428e3..95177c6 100644 --- a/chrome/browser/extensions/extension_updater.cc +++ b/chrome/browser/extensions/extension_updater.cc @@ -169,7 +169,12 @@ void ManifestFetchesBuilder::AddExtension(const Extension& extension) { void ManifestFetchesBuilder::AddPendingExtension( const std::string& id, const PendingExtensionInfo& info) { - AddExtensionData(Extension::INTERNAL, id, info.version, + // Use a zero version to ensure that a pending extension will always + // be updated, and thus installed (assuming all extensions have + // non-zero versions). + scoped_ptr<Version> version( + Version::GetVersionFromString("0.0.0.0")); + AddExtensionData(Extension::INTERNAL, id, *version, false, info.is_theme, info.update_url); } @@ -716,12 +721,6 @@ bool ExtensionUpdater::GetExistingVersion(const std::string& id, WideToASCII(prefs_->GetString(kExtensionBlacklistUpdateVersion)); return true; } - PendingExtensionMap::const_iterator it = - service_->pending_extensions().find(id); - if (it != service_->pending_extensions().end()) { - *version = it->second.version.GetString(); - return true; - } Extension* extension = service_->GetExtensionById(id, false); if (!extension) { return false; @@ -745,20 +744,24 @@ std::vector<int> ExtensionUpdater::DetermineUpdates( if (!fetch_data.Includes(update->extension_id)) continue; - std::string version; - if (!GetExistingVersion(update->extension_id, &version)) - continue; + if (service_->pending_extensions().find(update->extension_id) == + service_->pending_extensions().end()) { + // If we're not installing pending extension, and the update + // version is the same or older than what's already installed, + // we don't want it. + std::string version; + if (!GetExistingVersion(update->extension_id, &version)) + continue; - // If the update version is the same or older than what's already installed, - // we don't want it. - scoped_ptr<Version> existing_version( - Version::GetVersionFromString(version)); - scoped_ptr<Version> update_version( - Version::GetVersionFromString(update->version)); + scoped_ptr<Version> existing_version( + Version::GetVersionFromString(version)); + scoped_ptr<Version> update_version( + Version::GetVersionFromString(update->version)); - if (!update_version.get() || - update_version->CompareTo(*(existing_version.get())) <= 0) { - continue; + if (!update_version.get() || + update_version->CompareTo(*(existing_version.get())) <= 0) { + continue; + } } // If the update specifies a browser minimum version, do we qualify? diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc index 78fbbf3..5266888 100644 --- a/chrome/browser/extensions/extension_updater_unittest.cc +++ b/chrome/browser/extensions/extension_updater_unittest.cc @@ -115,13 +115,12 @@ void CreateTestPendingExtensions(int count, const GURL& update_url, for (int i = 1; i <= count; i++) { bool is_theme = (i % 2) == 0; const bool kInstallSilently = true; - scoped_ptr<Version> version( - Version::GetVersionFromString(StringPrintf("%d.0.0.0", i))); - ASSERT_TRUE(version.get()); + const Extension::State kInitialState = Extension::ENABLED; + const bool kInitialIncognitoEnabled = false; std::string id = GenerateId(StringPrintf("extension%i", i)); (*pending_extensions)[id] = - PendingExtensionInfo(update_url, *version, - is_theme, kInstallSilently); + PendingExtensionInfo(update_url, is_theme, kInstallSilently, + kInitialState, kInitialIncognitoEnabled); } } @@ -326,7 +325,7 @@ class ExtensionUpdaterTest : public testing::Test { ExtractParameters(decoded, ¶ms); if (pending) { EXPECT_EQ(pending_extensions.begin()->first, params["id"]); - EXPECT_EQ("1.0.0.0", params["v"]); + EXPECT_EQ("0.0.0.0", params["v"]); } else { EXPECT_EQ(extensions[0]->id(), params["id"]); EXPECT_EQ(extensions[0]->VersionString(), params["v"]); @@ -446,16 +445,15 @@ class ExtensionUpdaterTest : public testing::Test { UpdateManifest::Results updates; for (PendingExtensionMap::const_iterator it = pending_extensions.begin(); it != pending_extensions.end(); ++it) { - fetch_data.AddExtension(it->first, - it->second.version.GetString(), + fetch_data.AddExtension(it->first, "1.0.0.0", ManifestFetchData::kNeverPinged); AddParseResult(it->first, "1.1", "http://localhost/e1_1.1.crx", &updates); } std::vector<int> updateable = updater->DetermineUpdates(fetch_data, updates); - // Only the first one is updateable. - EXPECT_EQ(1u, updateable.size()); + // All the apps should be updateable. + EXPECT_EQ(3u, updateable.size()); for (std::vector<int>::size_type i = 0; i < updateable.size(); ++i) { EXPECT_EQ(static_cast<int>(i), updateable[i]); } @@ -552,10 +550,12 @@ class ExtensionUpdaterTest : public testing::Test { if (pending) { const bool kIsTheme = false; const bool kInstallSilently = true; + const Extension::State kInitialState = Extension::ENABLED; + const bool kInitialIncognitoEnabled = false; PendingExtensionMap pending_extensions; pending_extensions[id] = - PendingExtensionInfo(test_url, *version, - kIsTheme, kInstallSilently); + PendingExtensionInfo(test_url, kIsTheme, kInstallSilently, + kInitialState, kInitialIncognitoEnabled); service.set_pending_extensions(pending_extensions); } @@ -880,18 +880,15 @@ TEST(ExtensionUpdaterTest, TestManifestFetchesBuilderAddExtension) { STLDeleteElements(&extensions); } - scoped_ptr<Version> version(Version::GetVersionFromString("0")); - ASSERT_TRUE(version.get()); - // Extensions with invalid update URLs should be rejected. builder.AddPendingExtension( GenerateId("foo"), PendingExtensionInfo(GURL("http:google.com:foo"), - *version, false, false)); + false, false, true, false)); EXPECT_TRUE(builder.GetFetches().empty()); // Extensions with empty IDs should be rejected. builder.AddPendingExtension( - "", PendingExtensionInfo(GURL(), *version, false, false)); + "", PendingExtensionInfo(GURL(), false, false, true, false)); EXPECT_TRUE(builder.GetFetches().empty()); // TODO(akalin): Test that extensions with empty update URLs @@ -900,7 +897,8 @@ TEST(ExtensionUpdaterTest, TestManifestFetchesBuilderAddExtension) { // Extensions with empty update URLs should have a default one // filled in. builder.AddPendingExtension( - GenerateId("foo"), PendingExtensionInfo(GURL(), *version, false, false)); + GenerateId("foo"), PendingExtensionInfo(GURL(), + false, false, true, false)); std::vector<ManifestFetchData*> fetches = builder.GetFetches(); ASSERT_EQ(1u, fetches.size()); scoped_ptr<ManifestFetchData> fetch(fetches[0]); diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc index a416f17..5c4531d 100644 --- a/chrome/browser/extensions/extensions_service.cc +++ b/chrome/browser/extensions/extensions_service.cc @@ -73,19 +73,22 @@ static bool ShouldReloadExtensionManifest(const ExtensionInfo& info) { } // namespace PendingExtensionInfo::PendingExtensionInfo(const GURL& update_url, - const Version& version, bool is_theme, - bool install_silently) + bool install_silently, + bool enable_on_install, + bool enable_incognito_on_install) : update_url(update_url), - version(version), is_theme(is_theme), - install_silently(install_silently) {} + install_silently(install_silently), + enable_on_install(enable_on_install), + enable_incognito_on_install(enable_incognito_on_install) {} PendingExtensionInfo::PendingExtensionInfo() : update_url(), - version(), is_theme(false), - install_silently(false) {} + install_silently(false), + enable_on_install(false), + enable_incognito_on_install(false) {} // ExtensionsService. @@ -271,21 +274,25 @@ void ExtensionsService::UpdateExtension(const std::string& id, void ExtensionsService::AddPendingExtension( const std::string& id, const GURL& update_url, - const Version& version, bool is_theme, bool install_silently) { + bool is_theme, bool install_silently, + bool enable_on_install, bool enable_incognito_on_install) { if (GetExtensionByIdInternal(id, true, true)) { LOG(DFATAL) << "Trying to add pending extension " << id << " which already exists"; return; } - AddPendingExtensionInternal(id, update_url, version, - is_theme, install_silently); + AddPendingExtensionInternal( + id, update_url, is_theme, install_silently, + enable_on_install, enable_incognito_on_install); } void ExtensionsService::AddPendingExtensionInternal( const std::string& id, const GURL& update_url, - const Version& version, bool is_theme, bool install_silently) { + bool is_theme, bool install_silently, + bool enable_on_install, bool enable_incognito_on_install) { pending_extensions_[id] = - PendingExtensionInfo(update_url, version, is_theme, install_silently); + PendingExtensionInfo(update_url, is_theme, install_silently, + enable_on_install, enable_incognito_on_install); } void ExtensionsService::ReloadExtension(const std::string& extension_id) { @@ -905,23 +912,52 @@ void ExtensionsService::UpdateActiveExtensionsInCrashReporter() { void ExtensionsService::OnExtensionInstalled(Extension* extension, bool allow_privilege_increase) { + // Ensure extension is deleted unless we transfer ownership. + scoped_ptr<Extension> scoped_extension(extension); + Extension::State initial_state = Extension::DISABLED; + bool initial_enable_incognito = false; PendingExtensionMap::iterator it = pending_extensions_.find(extension->id()); - if (it != pending_extensions_.end() && - (it->second.is_theme != extension->is_theme())) { - LOG(WARNING) << "Not installing pending extension " << extension->id() - << " with is_theme = " << extension->is_theme() - << "; expected is_theme = " << it->second.is_theme; - // Delete the extension directory since we're not going to load - // it. - ChromeThread::PostTask( - ChromeThread::FILE, FROM_HERE, - NewRunnableFunction(&DeleteFileHelper, extension->path(), true)); - delete extension; - return; + if (it != pending_extensions_.end()) { + // Set initial state from pending extension data. + if (it->second.is_theme != extension->is_theme()) { + LOG(WARNING) + << "Not installing pending extension " << extension->id() + << " with is_theme = " << extension->is_theme() + << "; expected is_theme = " << it->second.is_theme; + // Delete the extension directory since we're not going to + // load it. + ChromeThread::PostTask( + ChromeThread::FILE, FROM_HERE, + NewRunnableFunction(&DeleteFileHelper, extension->path(), true)); + return; + } + if (it->second.is_theme) { + DCHECK(it->second.enable_on_install); + initial_state = Extension::ENABLED; + DCHECK(!it->second.enable_incognito_on_install); + initial_enable_incognito = false; + } else { + initial_state = + it->second.enable_on_install ? + Extension::ENABLED : Extension::DISABLED; + initial_enable_incognito = + it->second.enable_incognito_on_install; + } + + pending_extensions_.erase(it); + } else { + // Make sure we don't enable a disabled extension. + Extension::State existing_state = + extension_prefs_->GetExtensionState(extension->id()); + initial_state = + (existing_state == Extension::DISABLED) ? + Extension::DISABLED : Extension::ENABLED; + initial_enable_incognito = false; } - extension_prefs_->OnExtensionInstalled(extension); + extension_prefs_->OnExtensionInstalled( + extension, initial_state, initial_enable_incognito); // If the extension is a theme, tell the profile (and therefore ThemeProvider) // to apply it. @@ -937,16 +973,11 @@ void ExtensionsService::OnExtensionInstalled(Extension* extension, Details<Extension>(extension)); } - // Also load the extension. - OnExtensionLoaded(extension, allow_privilege_increase); - - // Erase any pending extension. - if (it != pending_extensions_.end()) { - pending_extensions_.erase(it); - } - if (profile_->GetTemplateURLModel()) profile_->GetTemplateURLModel()->RegisterExtensionKeyword(extension); + + // Transfer ownership of |extension| to OnExtensionLoaded. + OnExtensionLoaded(scoped_extension.release(), allow_privilege_increase); } Extension* ExtensionsService::GetExtensionByIdInternal(const std::string& id, diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h index f019630..f49a440 100644 --- a/chrome/browser/extensions/extensions_service.h +++ b/chrome/browser/extensions/extensions_service.h @@ -48,16 +48,18 @@ class Version; // default one is assumed. struct PendingExtensionInfo { PendingExtensionInfo(const GURL& update_url, - const Version& version, bool is_theme, - bool install_silently); + bool install_silently, + bool enable_on_install, + bool enable_incognito_on_install); PendingExtensionInfo(); GURL update_url; - Version version; bool is_theme; bool install_silently; + bool enable_on_install; + bool enable_incognito_on_install; }; // A PendingExtensionMap is a map from IDs of pending extensions to @@ -206,11 +208,12 @@ class ExtensionsService // It is an error to call this with an already-installed extension // (even a disabled one). // - // TODO(akalin): Make sure that all the places that check for - // existing versions also consult the pending extension info. + // TODO(akalin): Replace |install_silently| with a list of + // pre-enabled permissions. void AddPendingExtension( const std::string& id, const GURL& update_url, - const Version& version, bool is_theme, bool install_silently); + bool is_theme, bool install_silently, + bool enable_on_install, bool enable_incognito_on_install); // Reloads the specified extension. void ReloadExtension(const std::string& extension_id); @@ -365,7 +368,8 @@ class ExtensionsService // id is not already installed. void AddPendingExtensionInternal( const std::string& id, const GURL& update_url, - const Version& version, bool is_theme, bool install_silently); + bool is_theme, bool install_silently, + bool enable_on_install, bool enable_incognito_on_install); // Handles sending notification that |extension| was loaded. void NotifyExtensionLoaded(Extension* extension); diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc index cc50bab..6ce579c 100644 --- a/chrome/browser/extensions/extensions_service_unittest.cc +++ b/chrome/browser/extensions/extensions_service_unittest.cc @@ -429,9 +429,16 @@ class ExtensionsServiceTest ExtensionErrorReporter::GetInstance()->ClearErrors(); } + enum UpdateState { + FAILED_SILENTLY, + FAILED, + UPDATED, + INSTALLED, + ENABLED + }; + void UpdateExtension(const std::string& id, const FilePath& in_path, - bool should_succeed, bool should_install, - bool expect_report_on_failure) { + UpdateState expected_state) { ASSERT_TRUE(file_util::PathExists(in_path)); // We need to copy this to a temporary location because Update() will delete @@ -440,17 +447,39 @@ class ExtensionsServiceTest path = path.Append(in_path.BaseName()); ASSERT_TRUE(file_util::CopyFile(in_path, path)); + int previous_enabled_extension_count = + service_->extensions()->size(); + int previous_installed_extension_count = + previous_enabled_extension_count + + service_->disabled_extensions()->size(); + service_->UpdateExtension(id, path, GURL()); loop_.RunAllPending(); - std::vector<std::string> errors = GetErrors(); - if (should_succeed) { - EXPECT_EQ(0u, errors.size()) << path.value(); - EXPECT_EQ(should_install ? 1u : 0u, service_->extensions()->size()); + std::vector<std::string> errors = GetErrors(); + int error_count = errors.size(); + int enabled_extension_count = + service_->extensions()->size(); + int installed_extension_count = + enabled_extension_count + service_->disabled_extensions()->size(); + + int expected_error_count = (expected_state == FAILED) ? 1 : 0; + EXPECT_EQ(expected_error_count, error_count) << path.value(); + + if (expected_state <= FAILED) { + EXPECT_EQ(previous_enabled_extension_count, + enabled_extension_count); + EXPECT_EQ(previous_installed_extension_count, + installed_extension_count); } else { - if (expect_report_on_failure) { - EXPECT_EQ(1u, errors.size()) << path.value(); - } + int expected_installed_extension_count = + (expected_state >= INSTALLED) ? 1 : 0; + int expected_enabled_extension_count = + (expected_state >= ENABLED) ? 1 : 0; + EXPECT_EQ(expected_installed_extension_count, + installed_extension_count); + EXPECT_EQ(expected_enabled_extension_count, + enabled_extension_count); } // Update() should delete the temporary input file. @@ -1142,7 +1171,7 @@ TEST_F(ExtensionsServiceTest, UpdateExtension) { ASSERT_EQ(good_crx, good->id()); path = extensions_path.AppendASCII("good2.crx"); - UpdateExtension(good_crx, path, true, true, true); + UpdateExtension(good_crx, path, ENABLED); ASSERT_EQ("1.0.0.1", loaded_[0]->version()->GetString()); } @@ -1154,7 +1183,7 @@ TEST_F(ExtensionsServiceTest, UpdateNotInstalledExtension) { extensions_path = extensions_path.AppendASCII("extensions"); FilePath path = extensions_path.AppendASCII("good.crx"); - UpdateExtension(good_crx, path, true, false, true); + UpdateExtension(good_crx, path, UPDATED); loop_.RunAllPending(); ASSERT_EQ(0u, service_->extensions()->size()); @@ -1178,7 +1207,7 @@ TEST_F(ExtensionsServiceTest, UpdateWillNotDowngrade) { // Change path from good2.crx -> good.crx path = extensions_path.AppendASCII("good.crx"); - UpdateExtension(good_crx, path, false, false, true); + UpdateExtension(good_crx, path, FAILED); ASSERT_EQ("1.0.0.1", service_->extensions()->at(0)->VersionString()); } @@ -1194,7 +1223,7 @@ TEST_F(ExtensionsServiceTest, UpdateToSameVersionIsNoop) { InstallExtension(path, true); Extension* good = service_->extensions()->at(0); ASSERT_EQ(good_crx, good->id()); - UpdateExtension(good_crx, path, false, false, false); + UpdateExtension(good_crx, path, FAILED_SILENTLY); } // Test adding a pending extension. @@ -1203,21 +1232,20 @@ TEST_F(ExtensionsServiceTest, AddPendingExtension) { const std::string kFakeId("fake-id"); const GURL kFakeUpdateURL("http:://fake.update/url"); - scoped_ptr<Version> fake_version(Version::GetVersionFromString("4.3.2.1")); - ASSERT_TRUE(fake_version.get()); const bool kFakeIsTheme(false); const bool kFakeInstallSilently(true); + const Extension::State kFakeInitialState(Extension::ENABLED); + const bool kFakeInitialIncognitoEnabled(false); - service_->AddPendingExtension(kFakeId, kFakeUpdateURL, - *fake_version, kFakeIsTheme, - kFakeInstallSilently); + service_->AddPendingExtension( + kFakeId, kFakeUpdateURL, kFakeIsTheme, kFakeInstallSilently, + kFakeInitialState, kFakeInitialIncognitoEnabled); PendingExtensionMap::const_iterator it = service_->pending_extensions().find(kFakeId); ASSERT_TRUE(it != service_->pending_extensions().end()); EXPECT_EQ(kFakeUpdateURL, it->second.update_url); EXPECT_EQ(kFakeIsTheme, it->second.is_theme); EXPECT_EQ(kFakeInstallSilently, it->second.install_silently); - EXPECT_TRUE(it->second.version.Equals(*fake_version)); } namespace { @@ -1225,27 +1253,36 @@ const char kGoodId[] = "ldnnhddmnhbkjipkidpdiheffobcpfmf"; const char kGoodUpdateURL[] = "http://good.update/url"; const bool kGoodIsTheme = false; const bool kGoodInstallSilently = true; -const char kGoodVersion[] = "1.2.3.4"; +const Extension::State kGoodInitialState = Extension::DISABLED; +const bool kGoodInitialIncognitoEnabled = true; } // namespace // Test updating a pending extension. TEST_F(ExtensionsServiceTest, UpdatePendingExtension) { InitializeEmptyExtensionsService(); - scoped_ptr<Version> good_version( - Version::GetVersionFromString(kGoodVersion)); - ASSERT_TRUE(good_version.get()); - service_->AddPendingExtension(kGoodId, GURL(kGoodUpdateURL), - *good_version, kGoodIsTheme, - kGoodInstallSilently); + service_->AddPendingExtension( + kGoodId, GURL(kGoodUpdateURL), kGoodIsTheme, + kGoodInstallSilently, kGoodInitialState, + kGoodInitialIncognitoEnabled); EXPECT_TRUE(ContainsKey(service_->pending_extensions(), kGoodId)); FilePath extensions_path; ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); extensions_path = extensions_path.AppendASCII("extensions"); FilePath path = extensions_path.AppendASCII("good.crx"); - UpdateExtension(kGoodId, path, true, true, false); + UpdateExtension(kGoodId, path, INSTALLED); EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId)); + + Extension* extension = service_->GetExtensionById(kGoodId, true); + ASSERT_TRUE(extension); + + bool enabled = service_->GetExtensionById(kGoodId, false); + EXPECT_EQ(kGoodInitialState == Extension::ENABLED, enabled); + EXPECT_EQ(kGoodInitialState, + service_->extension_prefs()->GetExtensionState(extension->id())); + EXPECT_EQ(kGoodInitialIncognitoEnabled, + service_->IsIncognitoEnabled(extension)); } // TODO(akalin): Test updating a pending extension non-silently once @@ -1255,20 +1292,18 @@ TEST_F(ExtensionsServiceTest, UpdatePendingExtension) { // Test updating a pending extension with wrong is_theme. TEST_F(ExtensionsServiceTest, UpdatePendingExtensionWrongIsTheme) { InitializeEmptyExtensionsService(); - scoped_ptr<Version> good_version( - Version::GetVersionFromString(kGoodVersion)); - ASSERT_TRUE(good_version.get()); // Add pending extension with a flipped is_theme. - service_->AddPendingExtension(kGoodId, GURL(kGoodUpdateURL), - *good_version, !kGoodIsTheme, - kGoodInstallSilently); + service_->AddPendingExtension( + kGoodId, GURL(kGoodUpdateURL), !kGoodIsTheme, + kGoodInstallSilently, kGoodInitialState, + kGoodInitialIncognitoEnabled); EXPECT_TRUE(ContainsKey(service_->pending_extensions(), kGoodId)); FilePath extensions_path; ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); extensions_path = extensions_path.AppendASCII("extensions"); FilePath path = extensions_path.AppendASCII("good.crx"); - UpdateExtension(kGoodId, path, true, false, false); + UpdateExtension(kGoodId, path, UPDATED); // TODO(akalin): Figure out how to check that the extensions // directory is cleaned up properly in OnExtensionInstalled(). @@ -1284,7 +1319,7 @@ TEST_F(ExtensionsServiceTest, UpdatePendingExtensionNotPending) { ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); extensions_path = extensions_path.AppendASCII("extensions"); FilePath path = extensions_path.AppendASCII("good.crx"); - UpdateExtension(kGoodId, path, true, false, false); + UpdateExtension(kGoodId, path, UPDATED); EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId)); } @@ -1305,10 +1340,11 @@ TEST_F(ExtensionsServiceTest, UpdatePendingExtensionAlreadyInstalled) { // Use AddPendingExtensionInternal() as AddPendingExtension() would // balk. service_->AddPendingExtensionInternal( - good->id(), good->update_url(), *good->version(), - good->is_theme(), kGoodInstallSilently); + good->id(), good->update_url(), good->is_theme(), + kGoodInstallSilently, kGoodInitialState, + kGoodInitialIncognitoEnabled); - UpdateExtension(good->id(), path, true, true, false); + UpdateExtension(good->id(), path, INSTALLED); EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId)); } @@ -1354,7 +1390,7 @@ TEST_F(ExtensionsServiceTest, UnloadBlacklistedExtension) { InstallExtension(path, true); Extension* good = service_->extensions()->at(0); EXPECT_EQ(good_crx, good->id()); - UpdateExtension(good_crx, path, false, false, false); + UpdateExtension(good_crx, path, FAILED_SILENTLY); std::vector<std::string> blacklist; blacklist.push_back(good_crx); diff --git a/chrome/browser/extensions/test_extension_prefs.cc b/chrome/browser/extensions/test_extension_prefs.cc index 5a0c9f8..97c47e8 100644 --- a/chrome/browser/extensions/test_extension_prefs.cc +++ b/chrome/browser/extensions/test_extension_prefs.cc @@ -69,7 +69,9 @@ Extension* TestExtensionPrefs::AddExtensionWithManifest( EXPECT_TRUE(extension->InitFromValue(manifest, false, &errors)); extension->set_location(Extension::INTERNAL); EXPECT_TRUE(Extension::IdIsValid(extension->id())); - prefs_->OnExtensionInstalled(extension); + const bool kInitialIncognitoEnabled = false; + prefs_->OnExtensionInstalled(extension, Extension::ENABLED, + kInitialIncognitoEnabled); return extension; } diff --git a/chrome/browser/sync/glue/extension_model_associator.cc b/chrome/browser/sync/glue/extension_model_associator.cc index d8200b2..8a33951 100644 --- a/chrome/browser/sync/glue/extension_model_associator.cc +++ b/chrome/browser/sync/glue/extension_model_associator.cc @@ -385,23 +385,11 @@ void ExtensionModelAssociator::TryUpdateClient( } } else { GURL update_url(specifics.update_url()); - // TODO(akalin): The version number should be used only to - // determine auto-updating permissions, not to send to the - // auto-update server. - scoped_ptr<Version> version( - Version::GetVersionFromString("0.0.0.0")); - CHECK(version.get()); // TODO(akalin): Replace silent update with a list of enabled // permissions. - // - // TODO(akalin): Pass through the enabled/incognito_enabled state. - // We can't do it on OnClientUpdate() because we'd run into - // problems with triggering notifications while we're in a - // notification handler. The bug that this causes is that syncing - // a fresh client (i.e., no extensions) re-enables disabled - // extensions. extensions_service->AddPendingExtension( - id, update_url, *version, false, true); + id, update_url, false, true, + specifics.enabled(), specifics.incognito_enabled()); } DCHECK(!extension_data->NeedsUpdate(ExtensionData::SERVER)); } diff --git a/chrome/browser/sync/glue/theme_util.cc b/chrome/browser/sync/glue/theme_util.cc index 4ea65a4..1bd078e 100644 --- a/chrome/browser/sync/glue/theme_util.cc +++ b/chrome/browser/sync/glue/theme_util.cc @@ -127,15 +127,16 @@ void SetCurrentThemeFromThemeSpecifics( // No extension with this id exists -- we must install it; we do // so by adding it as a pending extension and then triggering an // auto-update cycle. - scoped_ptr<Version> version(Version::GetVersionFromString("0.0.0.0")); - DCHECK(version.get()); const bool kIsTheme = true; // Themes don't need to install silently as they just pop up an // informational dialog after installation instead of a // confirmation dialog. const bool kInstallSilently = false; - extensions_service->AddPendingExtension(id, update_url, *version, - kIsTheme, kInstallSilently); + const bool kEnableOnInstall = true; + const bool kEnableIncognitoOnInstall = false; + extensions_service->AddPendingExtension( + id, update_url, kIsTheme, kInstallSilently, + kEnableOnInstall, kEnableIncognitoOnInstall); ExtensionUpdater* extension_updater = extensions_service->updater(); // Auto-updates should now be on always (see the construction of // the ExtensionsService in ProfileImpl::InitExtensions()). |