Added support for pending extensions to ExtensionsService and
ExtensionUpdater. This is needed for theme syncing.
Basically a pending extension is an (id, update_url) pair. This change
makes it so that one can pass pending extensions to the extension service
and they will be installed if necessary on the next auto-update cycle.
BUG=32414
TEST=unittests, trybots, in-progress theme syncing change
Review URL: http://codereview.chromium.org/1232003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@42711 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/extensions/extension_updater.cc b/chrome/browser/extensions/extension_updater.cc
index de1ca96c..80621f5 100644
--- a/chrome/browser/extensions/extension_updater.cc
+++ b/chrome/browser/extensions/extension_updater.cc
@@ -378,7 +378,7 @@
safe_parser->Start();
} else {
// TODO(asargent) Do exponential backoff here. (http://crbug.com/12546).
- LOG(INFO) << "Failed to fetch manifst '" << url.possibly_invalid_spec() <<
+ LOG(INFO) << "Failed to fetch manifest '" << url.possibly_invalid_spec() <<
"' response code:" << response_code;
}
manifest_fetcher_.reset();
@@ -540,59 +540,110 @@
ScheduleNextCheck(TimeDelta::FromSeconds(frequency_seconds_));
}
-void ExtensionUpdater::CheckNow() {
- // List of data on fetches we're going to do. We limit the number of
- // extensions grouped together in one batch to avoid running into the limits
- // on the length of http GET requests, so there might be multiple
- // ManifestFetchData* objects with the same base_url.
- std::multimap<GURL, ManifestFetchData*> fetches;
+namespace {
- int no_url_count = 0;
- int google_url_count = 0;
- int other_url_count = 0;
- int theme_count = 0;
+struct URLStats {
+ URLStats()
+ : no_url_count(0),
+ google_url_count(0),
+ other_url_count(0),
+ theme_count(0) {}
- const ExtensionList* extensions = service_->extensions();
- for (ExtensionList::const_iterator iter = extensions->begin();
- iter != extensions->end(); ++iter) {
- Extension* extension = (*iter);
+ void ReportStats() const {
+ UMA_HISTOGRAM_COUNTS_100("Extensions.UpdateCheckExtensions",
+ google_url_count + other_url_count -
+ theme_count);
+ UMA_HISTOGRAM_COUNTS_100("Extensions.UpdateCheckTheme",
+ theme_count);
+ UMA_HISTOGRAM_COUNTS_100("Extensions.UpdateCheckGoogleUrl",
+ google_url_count);
+ UMA_HISTOGRAM_COUNTS_100("Extensions.UpdateCheckOtherUrl",
+ other_url_count);
+ UMA_HISTOGRAM_COUNTS_100("Extensions.UpdateCheckNoUrl",
+ no_url_count);
+ }
- // Only internal and external extensions can be autoupdated.
- if (extension->location() != Extension::INTERNAL &&
- !Extension::IsExternalLocation(extension->location())) {
- continue;
+ int no_url_count, google_url_count, other_url_count, theme_count;
+};
+
+class ManifestFetchesBuilder {
+ public:
+ explicit ManifestFetchesBuilder(ExtensionUpdateService* service)
+ : service_(service) {
+ DCHECK(service_);
+ }
+
+ void AddExtension(const Extension& extension) {
+ AddExtensionData(extension.location(),
+ extension.id(),
+ *extension.version(),
+ extension.converted_from_user_script(),
+ extension.IsTheme(),
+ extension.update_url());
+ }
+
+ void AddPendingExtension(const std::string& id,
+ const PendingExtensionInfo& info) {
+ AddExtensionData(Extension::INTERNAL, id, info.version,
+ false, info.is_theme, info.update_url);
+ }
+
+ const URLStats& url_stats() const { return url_stats_; }
+
+ // Caller takes ownership of the returned ManifestFetchData
+ // objects. Clears all counters.
+ std::vector<ManifestFetchData*> GetFetches() {
+ std::vector<ManifestFetchData*> fetches;
+ fetches.reserve(fetches_.size());
+ for (std::multimap<GURL, ManifestFetchData*>::iterator it =
+ fetches_.begin(); it != fetches_.end(); ++it) {
+ fetches.push_back(it->second);
}
+ fetches_.clear();
+ url_stats_ = URLStats();
+ return fetches;
+ }
- const GURL& update_url = extension->update_url();
+ private:
+ void AddExtensionData(Extension::Location location,
+ const std::string& id,
+ const Version& version,
+ bool converted_from_user_script,
+ bool is_theme,
+ const GURL& update_url) {
+ // Only internal and external extensions can be autoupdated.
+ if (location != Extension::INTERNAL &&
+ !Extension::IsExternalLocation(location)) {
+ return;
+ }
// Collect histogram data and skip extensions with no update url.
if (update_url.DomainIs("google.com")) {
- google_url_count++;
- } else if (update_url.is_empty() || extension->id().empty()) {
+ url_stats_.google_url_count++;
+ } else if (update_url.is_empty() || id.empty()) {
// TODO(asargent) when a default URL is added, make sure to update
// the total histogram below. Also, make sure to skip extensions that
// are "converted_from_user_script".
- if (!extension->converted_from_user_script())
- no_url_count++;
- continue;
+ if (!converted_from_user_script)
+ url_stats_.no_url_count++;
+ return;
} else {
- other_url_count++;
+ url_stats_.other_url_count++;
}
- if (extension->IsTheme())
- theme_count++;
+ if (is_theme)
+ url_stats_.theme_count++;
DCHECK(update_url.is_valid());
ManifestFetchData* fetch = NULL;
std::multimap<GURL, ManifestFetchData*>::iterator existing_iter =
- fetches.find(update_url);
+ fetches_.find(update_url);
// Find or create a ManifestFetchData to add this extension to.
- std::string id = extension->id();
- std::string version = extension->VersionString();
- int ping_days = CalculatePingDays(extension->id());
- while (existing_iter != fetches.end()) {
- if (existing_iter->second->AddExtension(id, version, ping_days)) {
+ int ping_days = CalculatePingDays(id);
+ while (existing_iter != fetches_.end()) {
+ if (existing_iter->second->AddExtension(id, version.GetString(),
+ ping_days)) {
fetch = existing_iter->second;
break;
}
@@ -600,12 +651,58 @@
}
if (!fetch) {
fetch = new ManifestFetchData(update_url);
- fetches.insert(std::pair<GURL, ManifestFetchData*>(update_url, fetch));
- bool added = fetch->AddExtension(id, version, ping_days);
+ fetches_.insert(std::pair<GURL, ManifestFetchData*>(update_url, fetch));
+ bool added = fetch->AddExtension(id, version.GetString(), ping_days);
DCHECK(added);
}
}
+ // Calculates the value to use for the ping days parameter in manifest
+ // fetches for a given extension.
+ int CalculatePingDays(const std::string& extension_id) {
+ int days = ManifestFetchData::kNeverPinged;
+ Time last_ping_day = service_->LastPingDay(extension_id);
+ if (!last_ping_day.is_null()) {
+ days = (Time::Now() - last_ping_day).InDays();
+ }
+ return days;
+ }
+
+ ExtensionUpdateService* service_;
+
+ // List of data on fetches we're going to do. We limit the number of
+ // extensions grouped together in one batch to avoid running into the limits
+ // on the length of http GET requests, so there might be multiple
+ // ManifestFetchData* objects with the same base_url.
+ std::multimap<GURL, ManifestFetchData*> fetches_;
+
+ URLStats url_stats_;
+
+ DISALLOW_COPY_AND_ASSIGN(ManifestFetchesBuilder);
+};
+
+} // namespace
+
+void ExtensionUpdater::CheckNow() {
+ ManifestFetchesBuilder fetches_builder(service_);
+
+ const ExtensionList* extensions = service_->extensions();
+ for (ExtensionList::const_iterator iter = extensions->begin();
+ iter != extensions->end(); ++iter) {
+ fetches_builder.AddExtension(**iter);
+ }
+
+ const PendingExtensionMap& pending_extensions =
+ service_->pending_extensions();
+ for (PendingExtensionMap::const_iterator iter = pending_extensions.begin();
+ iter != pending_extensions.end(); ++iter) {
+ fetches_builder.AddPendingExtension(iter->first, iter->second);
+ }
+
+ fetches_builder.url_stats().ReportStats();
+
+ std::vector<ManifestFetchData*> fetches(fetches_builder.GetFetches());
+
// Start a fetch of the blacklist if needed.
if (blacklist_checks_enabled_ && service_->HasInstalledExtensions()) {
ManifestFetchData* blacklist_fetch =
@@ -616,36 +713,16 @@
}
// Now start fetching regular extension updates
- std::multimap<GURL, ManifestFetchData*>::iterator i = fetches.begin();
- while (i != fetches.end()) {
- ManifestFetchData *fetch = i->second;
-
+ for (std::vector<ManifestFetchData*>::const_iterator it = fetches.begin();
+ it != fetches.end(); ++it) {
// StartUpdateCheck makes sure the url isn't already downloading or
// scheduled, so we don't need to check before calling it. Ownership of
// fetch is transferred here.
- StartUpdateCheck(fetch);
- fetches.erase(i++);
+ StartUpdateCheck(*it);
}
-
- UMA_HISTOGRAM_COUNTS_100("Extensions.UpdateCheckExtensions",
- google_url_count + other_url_count - theme_count);
- UMA_HISTOGRAM_COUNTS_100("Extensions.UpdateCheckTheme",
- theme_count);
- UMA_HISTOGRAM_COUNTS_100("Extensions.UpdateCheckGoogleUrl",
- google_url_count);
- UMA_HISTOGRAM_COUNTS_100("Extensions.UpdateCheckOtherUrl",
- other_url_count);
- UMA_HISTOGRAM_COUNTS_100("Extensions.UpdateCheckNoUrl",
- no_url_count);
-}
-
-int ExtensionUpdater::CalculatePingDays(const std::string& extension_id) {
- int days = ManifestFetchData::kNeverPinged;
- Time last_ping_day = service_->LastPingDay(extension_id);
- if (!last_ping_day.is_null()) {
- days = (Time::Now() - last_ping_day).InDays();
- }
- return days;
+ // We don't want to use fetches after this since StartUpdateCheck()
+ // takes ownership of its argument.
+ fetches.clear();
}
bool ExtensionUpdater::GetExistingVersion(const std::string& id,
@@ -655,6 +732,12 @@
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;
diff --git a/chrome/browser/extensions/extension_updater.h b/chrome/browser/extensions/extension_updater.h
index ae7c4881..bd48af8e 100644
--- a/chrome/browser/extensions/extension_updater.h
+++ b/chrome/browser/extensions/extension_updater.h
@@ -198,10 +198,6 @@
void HandleManifestResults(const ManifestFetchData& fetch_data,
const UpdateManifest::Results& results);
- // Calculates the value to use for the ping days parameter in manifest
- // fetches for a given extension.
- int CalculatePingDays(const std::string& extension_id);
-
// Determines the version of an existing extension.
// Returns true on success and false on failures.
bool GetExistingVersion(const std::string& id, std::string* version);
diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc
index 8368364..b613bee1 100644
--- a/chrome/browser/extensions/extension_updater_unittest.cc
+++ b/chrome/browser/extensions/extension_updater_unittest.cc
@@ -6,9 +6,11 @@
#include "base/file_util.h"
#include "base/rand_util.h"
+#include "base/scoped_ptr.h"
#include "base/stl_util-inl.h"
#include "base/string_util.h"
#include "base/thread.h"
+#include "base/version.h"
#include "chrome/browser/chrome_thread.h"
#include "chrome/browser/extensions/extension_updater.h"
#include "chrome/browser/extensions/extensions_service.h"
@@ -50,6 +52,11 @@
return NULL;
}
+ virtual const PendingExtensionMap& pending_extensions() const {
+ EXPECT_TRUE(false);
+ return pending_extensions_;
+ }
+
virtual void UpdateExtension(const std::string& id,
const FilePath& extension_path,
const GURL& download_url) {
@@ -85,6 +92,9 @@
return Time();
}
+ protected:
+ PendingExtensionMap pending_extensions_;
+
private:
std::map<std::string, Time> last_ping_days_;
DISALLOW_COPY_AND_ASSIGN(MockService);
@@ -140,6 +150,22 @@
}
}
+// Creates test pending extensions and inserts them into list. The
+// name and version are all based on their index.
+void CreateTestPendingExtensions(int count, const GURL& update_url,
+ PendingExtensionMap* pending_extensions) {
+ 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());
+ (*pending_extensions)[StringPrintf("extension%i", i)] =
+ PendingExtensionInfo(update_url, *version,
+ is_theme, kInstallSilently);
+ }
+}
+
class ServiceForManifestTests : public MockService {
public:
ServiceForManifestTests() : has_installed_extensions_(false) {}
@@ -158,10 +184,19 @@
virtual const ExtensionList* extensions() const { return &extensions_; }
+ virtual const PendingExtensionMap& pending_extensions() const {
+ return pending_extensions_;
+ }
+
void set_extensions(ExtensionList extensions) {
extensions_ = extensions;
}
+ void set_pending_extensions(
+ const PendingExtensionMap& pending_extensions) {
+ pending_extensions_ = pending_extensions;
+ }
+
virtual bool HasInstalledExtensions() {
return has_installed_extensions_;
}
@@ -185,11 +220,20 @@
download_url_ = download_url;
}
+ virtual const PendingExtensionMap& pending_extensions() const {
+ return pending_extensions_;
+ }
+
virtual Extension* GetExtensionById(const std::string& id, bool) {
last_inquired_extension_id_ = id;
return NULL;
}
+ void set_pending_extensions(
+ const PendingExtensionMap& pending_extensions) {
+ pending_extensions_ = pending_extensions;
+ }
+
const std::string& extension_id() { return extension_id_; }
const FilePath& install_path() { return install_path_; }
const GURL& download_url() { return download_url_; }
@@ -272,13 +316,19 @@
results->list.push_back(result);
}
- static void TestExtensionUpdateCheckRequests() {
+ static void TestExtensionUpdateCheckRequests(bool pending) {
// Create an extension with an update_url.
ServiceForManifestTests service;
- ExtensionList tmp;
std::string update_url("http://foo.com/bar");
- CreateTestExtensions(1, &tmp, &update_url);
- service.set_extensions(tmp);
+ ExtensionList extensions;
+ PendingExtensionMap pending_extensions;
+ if (pending) {
+ CreateTestPendingExtensions(1, GURL(update_url), &pending_extensions);
+ service.set_pending_extensions(pending_extensions);
+ } else {
+ CreateTestExtensions(1, &extensions, &update_url);
+ service.set_extensions(extensions);
+ }
// Setup and start the updater.
MessageLoop message_loop;
@@ -316,11 +366,18 @@
UnescapeRule::URL_SPECIAL_CHARS);
std::map<std::string, std::string> params;
ExtractParameters(decoded, ¶ms);
- EXPECT_EQ(tmp[0]->id(), params["id"]);
- EXPECT_EQ(tmp[0]->VersionString(), params["v"]);
+ if (pending) {
+ EXPECT_EQ(pending_extensions.begin()->first, params["id"]);
+ EXPECT_EQ("1.0.0.0", params["v"]);
+ } else {
+ EXPECT_EQ(extensions[0]->id(), params["id"]);
+ EXPECT_EQ(extensions[0]->VersionString(), params["v"]);
+ }
EXPECT_EQ("", params["uc"]);
- STLDeleteElements(&tmp);
+ if (!pending) {
+ STLDeleteElements(&extensions);
+ }
}
static void TestBlacklistUpdateCheckRequests() {
@@ -415,6 +472,37 @@
STLDeleteElements(&tmp);
}
+ static void TestDetermineUpdatesPending() {
+ // Create a set of test extensions
+ ServiceForManifestTests service;
+ PendingExtensionMap pending_extensions;
+ CreateTestPendingExtensions(3, GURL(), &pending_extensions);
+ service.set_pending_extensions(pending_extensions);
+
+ MessageLoop message_loop;
+ ScopedTempPrefService prefs;
+ scoped_refptr<ExtensionUpdater> updater =
+ new ExtensionUpdater(&service, prefs.get(), kUpdateFrequencySecs);
+
+ ManifestFetchData fetch_data(GURL("http://localhost/foo"));
+ 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(),
+ 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());
+ for (std::vector<int>::size_type i = 0; i < updateable.size(); ++i) {
+ EXPECT_EQ(static_cast<int>(i), updateable[i]);
+ }
+ }
+
static void TestMultipleManifestDownloading() {
MessageLoop ui_loop;
ChromeThread ui_thread(ChromeThread::UI, &ui_loop);
@@ -479,7 +567,7 @@
xmlCleanupGlobals();
}
- static void TestSingleExtensionDownloading() {
+ static void TestSingleExtensionDownloading(bool pending) {
MessageLoop ui_loop;
ChromeThread ui_thread(ChromeThread::UI, &ui_loop);
ChromeThread file_thread(ChromeThread::FILE);
@@ -499,9 +587,19 @@
std::string id = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
std::string hash = "";
- std::string version = "0.0.1";
+ scoped_ptr<Version> version(Version::GetVersionFromString("0.0.1"));
+ ASSERT_TRUE(version.get());
+ updater->FetchUpdatedExtension(id, test_url, hash, version->GetString());
- updater->FetchUpdatedExtension(id, test_url, hash, version);
+ if (pending) {
+ const bool kIsTheme = false;
+ const bool kInstallSilently = true;
+ PendingExtensionMap pending_extensions;
+ pending_extensions[id] =
+ PendingExtensionInfo(test_url, *version,
+ kIsTheme, kInstallSilently);
+ service.set_pending_extensions(pending_extensions);
+ }
// Call back the ExtensionUpdater with a 200 response and some test data
std::string extension_data("whatever");
@@ -756,7 +854,11 @@
// subclasses where friendship with ExtenionUpdater is not inherited.
TEST(ExtensionUpdaterTest, TestExtensionUpdateCheckRequests) {
- ExtensionUpdaterTest::TestExtensionUpdateCheckRequests();
+ ExtensionUpdaterTest::TestExtensionUpdateCheckRequests(false);
+}
+
+TEST(ExtensionUpdaterTest, TestExtensionUpdateCheckRequestsPending) {
+ ExtensionUpdaterTest::TestExtensionUpdateCheckRequests(true);
}
// This test is disabled on Mac, see http://crbug.com/26035.
@@ -768,12 +870,20 @@
ExtensionUpdaterTest::TestDetermineUpdates();
}
+TEST(ExtensionUpdaterTest, TestDetermineUpdatesPending) {
+ ExtensionUpdaterTest::TestDetermineUpdatesPending();
+}
+
TEST(ExtensionUpdaterTest, TestMultipleManifestDownloading) {
ExtensionUpdaterTest::TestMultipleManifestDownloading();
}
TEST(ExtensionUpdaterTest, TestSingleExtensionDownloading) {
- ExtensionUpdaterTest::TestSingleExtensionDownloading();
+ ExtensionUpdaterTest::TestSingleExtensionDownloading(false);
+}
+
+TEST(ExtensionUpdaterTest, TestSingleExtensionDownloadingPending) {
+ ExtensionUpdaterTest::TestSingleExtensionDownloading(true);
}
// This test is disabled on Mac, see http://crbug.com/26035.
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 28ba10c..6807f6a 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -12,6 +12,7 @@
#include "base/string_util.h"
#include "base/time.h"
#include "base/values.h"
+#include "base/version.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/chrome_thread.h"
#include "chrome/browser/debugger/devtools_manager.h"
@@ -97,6 +98,21 @@
} // namespace
+PendingExtensionInfo::PendingExtensionInfo(const GURL& update_url,
+ const Version& version,
+ bool is_theme,
+ bool install_silently)
+ : update_url(update_url),
+ version(version),
+ is_theme(is_theme),
+ install_silently(install_silently) {}
+
+PendingExtensionInfo::PendingExtensionInfo()
+ : update_url(),
+ version(),
+ is_theme(false),
+ install_silently(false) {}
+
// ExtensionsService.
const char* ExtensionsService::kInstallDirectoryName = "Extensions";
@@ -203,19 +219,41 @@
installer->InstallCrx(extension_path);
}
+namespace {
+ // TODO(akalin): Put this somewhere where both crx_installer.cc and
+ // this file can use it.
+ void DeleteFileHelper(const FilePath& path, bool recursive) {
+ DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+ file_util::Delete(path, recursive);
+ }
+} // namespace
+
void ExtensionsService::UpdateExtension(const std::string& id,
const FilePath& extension_path,
const GURL& download_url) {
- if (!GetExtensionByIdInternal(id, true, true)) {
- LOG(WARNING) << "Will not update extension " << id << " because it is not "
- << "installed";
+ PendingExtensionMap::const_iterator it = pending_extensions_.find(id);
+ if ((it == pending_extensions_.end()) &&
+ !GetExtensionByIdInternal(id, true, true)) {
+ LOG(WARNING) << "Will not update extension " << id
+ << " because it is not installed or pending";
+ // Delete extension_path since we're not creating a CrxInstaller
+ // that would do it for us.
+ ChromeThread::PostTask(
+ ChromeThread::FILE, FROM_HERE,
+ NewRunnableFunction(&DeleteFileHelper, extension_path, false));
return;
}
+ // We want a silent install only for non-pending extensions and
+ // pending extensions that have install_silently set.
+ ExtensionInstallUI* client =
+ ((it == pending_extensions_.end()) || it->second.install_silently) ?
+ NULL : new ExtensionInstallUI(profile_);
+
scoped_refptr<CrxInstaller> installer(
new CrxInstaller(install_directory_,
this, // frontend
- NULL)); // no client (silent install)
+ client));
installer->set_expected_id(id);
installer->set_delete_source(true);
installer->set_force_web_origin_to_download_url(true);
@@ -223,6 +261,23 @@
installer->InstallCrx(extension_path);
}
+void ExtensionsService::AddPendingExtension(
+ const std::string& id, const GURL& update_url,
+ const Version& version, bool is_theme, bool install_silently) {
+ if (GetExtensionByIdInternal(id, true, true)) {
+ return;
+ }
+ AddPendingExtensionInternal(id, update_url, version,
+ is_theme, install_silently);
+}
+
+void ExtensionsService::AddPendingExtensionInternal(
+ const std::string& id, const GURL& update_url,
+ const Version& version, bool is_theme, bool install_silently) {
+ pending_extensions_[id] =
+ PendingExtensionInfo(update_url, version, is_theme, install_silently);
+}
+
void ExtensionsService::ReloadExtension(const std::string& extension_id) {
FilePath path;
Extension* current_extension = GetExtensionById(extension_id, false);
@@ -823,6 +878,22 @@
void ExtensionsService::OnExtensionInstalled(Extension* extension,
bool allow_privilege_increase) {
+ PendingExtensionMap::iterator it =
+ pending_extensions_.find(extension->id());
+ if (it != pending_extensions_.end() &&
+ (it->second.is_theme != extension->IsTheme())) {
+ LOG(WARNING) << "Not installing pending extension " << extension->id()
+ << " with is_theme = " << extension->IsTheme()
+ << "; 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;
+ }
+
extension_prefs_->OnExtensionInstalled(extension);
// If the extension is a theme, tell the profile (and therefore ThemeProvider)
@@ -841,6 +912,11 @@
// Also load the extension.
OnExtensionLoaded(extension, allow_privilege_increase);
+
+ // Erase any pending extension.
+ if (it != pending_extensions_.end()) {
+ pending_extensions_.erase(it);
+ }
}
void ExtensionsService::OnExtensionOverinstallAttempted(const std::string& id) {
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index 413454c..256fa37ba 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -29,6 +29,7 @@
#include "chrome/common/notification_observer.h"
#include "chrome/common/notification_registrar.h"
#include "chrome/common/extensions/extension.h"
+#include "testing/gtest/include/gtest/gtest_prod.h"
class Browser;
class ExtensionsServiceBackend;
@@ -39,6 +40,29 @@
class Profile;
class ResourceDispatcherHost;
class SiteInstance;
+class Version;
+
+// A pending extension is an extension that hasn't been installed yet
+// and is intended to be installed in the next auto-update cycle. The
+// update URL of a pending extension may be blank, in which case a
+// default one is assumed.
+struct PendingExtensionInfo {
+ PendingExtensionInfo(const GURL& update_url,
+ const Version& version,
+ bool is_theme,
+ bool install_silently);
+
+ PendingExtensionInfo();
+
+ GURL update_url;
+ Version version;
+ bool is_theme;
+ bool install_silently;
+};
+
+// A PendingExtensionMap is a map from IDs of pending extensions to
+// their info.
+typedef std::map<std::string, PendingExtensionInfo> PendingExtensionMap;
// This is an interface class to encapsulate the dependencies that
// ExtensionUpdater has on ExtensionsService. This allows easy mocking.
@@ -46,6 +70,7 @@
public:
virtual ~ExtensionUpdateService() {}
virtual const ExtensionList* extensions() const = 0;
+ virtual const PendingExtensionMap& pending_extensions() const = 0;
virtual void UpdateExtension(const std::string& id, const FilePath& path,
const GURL& download_url) = 0;
virtual Extension* GetExtensionById(const std::string& id,
@@ -115,6 +140,11 @@
return &disabled_extensions_;
}
+ // Gets the set of pending extensions.
+ virtual const PendingExtensionMap& pending_extensions() const {
+ return pending_extensions_;
+ }
+
// Registers an extension to be loaded as a component extension.
void register_component_extension(const ComponentExtensionInfo& info) {
component_extension_manifests_.push_back(info);
@@ -159,6 +189,16 @@
const FilePath& extension_path,
const GURL& download_url);
+ // If an extension with the given id is already installed,
+ // does nothing. Otherwise, the extension will be installed in the
+ // next auto-update cycle.
+ //
+ // TODO(akalin): Make sure that all the places that check for
+ // existing versions also consult the pending extension info.
+ void AddPendingExtension(
+ const std::string& id, const GURL& update_url,
+ const Version& version, bool is_theme, bool install_silently);
+
// Reloads the specified extension.
void ReloadExtension(const std::string& extension_id);
@@ -302,6 +342,12 @@
bool include_enabled,
bool include_disabled);
+ // Like AddPendingExtension() but assumes an extension with the same
+ // id is not already installed.
+ void AddPendingExtensionInternal(
+ const std::string& id, const GURL& update_url,
+ const Version& version, bool is_theme, bool install_silently);
+
// Handles sending notification that |extension| was loaded.
void NotifyExtensionLoaded(Extension* extension);
@@ -326,6 +372,9 @@
// The list of installed extensions that have been disabled.
ExtensionList disabled_extensions_;
+ // The set of pending extensions.
+ PendingExtensionMap pending_extensions_;
+
// The full path to the directory where extensions are installed.
FilePath install_directory_;
@@ -370,6 +419,8 @@
typedef std::vector<ComponentExtensionInfo> RegisteredComponentExtensions;
RegisteredComponentExtensions component_extension_manifests_;
+ FRIEND_TEST(ExtensionsServiceTest, UpdatePendingExtensionAlreadyInstalled);
+
DISALLOW_COPY_AND_ASSIGN(ExtensionsService);
};
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index ff9a2b6..c475884 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -13,9 +13,11 @@
#include "base/json/json_reader.h"
#include "base/message_loop.h"
#include "base/path_service.h"
+#include "base/scoped_ptr.h"
#include "base/string16.h"
#include "base/string_util.h"
#include "base/task.h"
+#include "base/version.h"
#include "chrome/browser/extensions/crx_installer.h"
#include "chrome/browser/extensions/extension_creator.h"
#include "chrome/browser/extensions/extensions_service.h"
@@ -398,7 +400,8 @@
}
void UpdateExtension(const std::string& id, const FilePath& in_path,
- bool should_succeed, bool expect_report_on_failure) {
+ bool should_succeed, bool should_install,
+ bool expect_report_on_failure) {
ASSERT_TRUE(file_util::PathExists(in_path));
// We need to copy this to a temporary location because Update() will delete
@@ -413,7 +416,7 @@
if (should_succeed) {
EXPECT_EQ(0u, errors.size()) << path.value();
- EXPECT_EQ(1u, service_->extensions()->size());
+ EXPECT_EQ(should_install ? 1u : 0u, service_->extensions()->size());
} else {
if (expect_report_on_failure) {
EXPECT_EQ(1u, errors.size()) << path.value();
@@ -1048,7 +1051,7 @@
ASSERT_EQ(good_crx, good->id());
path = extensions_path.AppendASCII("good2.crx");
- UpdateExtension(good_crx, path, true, true);
+ UpdateExtension(good_crx, path, true, true, true);
ASSERT_EQ("1.0.0.1", loaded_[0]->version()->GetString());
}
@@ -1060,7 +1063,7 @@
extensions_path = extensions_path.AppendASCII("extensions");
FilePath path = extensions_path.AppendASCII("good.crx");
- service_->UpdateExtension(good_crx, path, GURL());
+ UpdateExtension(good_crx, path, true, false, true);
loop_.RunAllPending();
ASSERT_EQ(0u, service_->extensions()->size());
@@ -1084,7 +1087,7 @@
// Change path from good2.crx -> good.crx
path = extensions_path.AppendASCII("good.crx");
- UpdateExtension(good_crx, path, false, true);
+ UpdateExtension(good_crx, path, false, false, true);
ASSERT_EQ("1.0.0.1", service_->extensions()->at(0)->VersionString());
}
@@ -1100,7 +1103,141 @@
InstallExtension(path, true);
Extension* good = service_->extensions()->at(0);
ASSERT_EQ(good_crx, good->id());
- UpdateExtension(good_crx, path, false, false);
+ UpdateExtension(good_crx, path, false, false, false);
+}
+
+// Test adding a pending extension.
+TEST_F(ExtensionsServiceTest, AddPendingExtension) {
+ InitializeEmptyExtensionsService();
+
+ 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);
+
+ service_->AddPendingExtension(kFakeId, kFakeUpdateURL,
+ *fake_version, kFakeIsTheme,
+ kFakeInstallSilently);
+ 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));
+}
+
+// Test adding a pending extension for one that is already installed.
+TEST_F(ExtensionsServiceTest, AddPendingExtensionAlreadyInstalled) {
+ InitializeEmptyExtensionsService();
+
+ FilePath extensions_path;
+ ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path));
+ extensions_path = extensions_path.AppendASCII("extensions");
+ FilePath path = extensions_path.AppendASCII("good.crx");
+ InstallExtension(path, true);
+ Extension* good = service_->extensions()->at(0);
+ const bool kInstallSilently(true);
+
+ service_->AddPendingExtension(good->id(), good->update_url(),
+ *good->version(), good->IsTheme(),
+ kInstallSilently);
+ EXPECT_FALSE(ContainsKey(service_->pending_extensions(), good->id()));
+}
+
+namespace {
+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";
+} // 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);
+ 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);
+
+ EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId));
+}
+
+// TODO(akalin): Test updating a pending extension non-silently once
+// we can mock out ExtensionInstallUI and inject our version into
+// UpdateExtension().
+
+// 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);
+ 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);
+
+ // TODO(akalin): Figure out how to check that the extensions
+ // directory is cleaned up properly in OnExtensionInstalled().
+
+ EXPECT_TRUE(ContainsKey(service_->pending_extensions(), kGoodId));
+}
+
+// Test updating a pending extension for one that is not pending.
+TEST_F(ExtensionsServiceTest, UpdatePendingExtensionNotPending) {
+ InitializeEmptyExtensionsService();
+
+ FilePath extensions_path;
+ ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path));
+ extensions_path = extensions_path.AppendASCII("extensions");
+ FilePath path = extensions_path.AppendASCII("good.crx");
+ UpdateExtension(kGoodId, path, true, false, false);
+
+ EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId));
+}
+
+// Test updating a pending extension for one that is already
+// installed.
+TEST_F(ExtensionsServiceTest, UpdatePendingExtensionAlreadyInstalled) {
+ InitializeEmptyExtensionsService();
+
+ FilePath extensions_path;
+ ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path));
+ extensions_path = extensions_path.AppendASCII("extensions");
+ FilePath path = extensions_path.AppendASCII("good.crx");
+ InstallExtension(path, true);
+ ASSERT_EQ(1u, service_->extensions()->size());
+ Extension* good = service_->extensions()->at(0);
+
+ // Use AddPendingExtensionInternal() as AddPendingExtension() would
+ // balk.
+ service_->AddPendingExtensionInternal(
+ good->id(), good->update_url(), *good->version(),
+ good->IsTheme(), kGoodInstallSilently);
+
+ UpdateExtension(good->id(), path, true, true, false);
+
+ EXPECT_TRUE(ContainsKey(service_->pending_extensions(), kGoodId));
}
// Test pref settings for blacklist and unblacklist extensions.
@@ -1144,7 +1281,7 @@
InstallExtension(path, true);
Extension* good = service_->extensions()->at(0);
EXPECT_EQ(good_crx, good->id());
- UpdateExtension(good_crx, path, false, false);
+ UpdateExtension(good_crx, path, false, false, false);
std::vector<std::string> blacklist;
blacklist.push_back(good_crx);
