Commit b864d8b8 authored by rockot's avatar rockot Committed by Commit bot

Enable forced extension updates on NaCl arch mismatch.

This causes ExtensionUpdater to scan the contents of
installed extensions prior to initiating update queries. If
any extensions are found with a _platform_specific
directory that does NOT contain a subdirectory which
corresponds to the current NaCl arch, that extension is
placed into forced-update mode.

An extension in forced-update mode will be treated as
version 0.0.0.0 when querying the update server.

BUG=409948

Review URL: https://codereview.chromium.org/540673002

Cr-Commit-Position: refs/heads/master@{#293548}
parent 4af8c212
......@@ -210,10 +210,17 @@ bool ExtensionDownloader::AddExtension(const Extension& extension,
if (!ManifestURL::UpdatesFromGallery(&extension))
update_url_data = delegate_->GetUpdateUrlData(extension.id());
return AddExtensionData(extension.id(), *extension.version(),
std::string install_source;
bool force_update = delegate_->ShouldForceUpdate(extension.id(),
&install_source);
return AddExtensionData(extension.id(),
*extension.version(),
extension.GetType(),
ManifestURL::GetUpdateURL(&extension),
update_url_data, request_id);
update_url_data,
request_id,
force_update,
install_source);
}
bool ExtensionDownloader::AddPendingExtension(const std::string& id,
......@@ -230,7 +237,9 @@ bool ExtensionDownloader::AddPendingExtension(const std::string& id,
Manifest::TYPE_UNKNOWN,
update_url,
std::string(),
request_id);
request_id,
false,
std::string());
}
void ExtensionDownloader::StartAllPending(ExtensionCache* cache) {
......@@ -273,7 +282,8 @@ void ExtensionDownloader::StartBlacklistUpdate(
version,
&ping_data,
std::string(),
kDefaultInstallSource);
kDefaultInstallSource,
false);
StartUpdateCheck(blacklist_fetch.Pass());
}
......@@ -282,12 +292,15 @@ void ExtensionDownloader::SetWebstoreIdentityProvider(
identity_provider_.swap(identity_provider);
}
bool ExtensionDownloader::AddExtensionData(const std::string& id,
const Version& version,
Manifest::Type extension_type,
const GURL& extension_update_url,
const std::string& update_url_data,
int request_id) {
bool ExtensionDownloader::AddExtensionData(
const std::string& id,
const Version& version,
Manifest::Type extension_type,
const GURL& extension_update_url,
const std::string& update_url_data,
int request_id,
bool force_update,
const std::string& install_source_override) {
GURL update_url(extension_update_url);
// Skip extensions with non-empty invalid update URLs.
if (!update_url.is_empty() && !update_url.is_valid()) {
......@@ -353,6 +366,9 @@ bool ExtensionDownloader::AddExtensionData(const std::string& id,
std::string install_source = i == 0 ?
kDefaultInstallSource : kNotFromWebstoreInstallSource;
if (!install_source_override.empty()) {
install_source = install_source_override;
}
ManifestFetchData::PingData ping_data;
ManifestFetchData::PingData* optional_ping_data = NULL;
......@@ -369,7 +385,8 @@ bool ExtensionDownloader::AddExtensionData(const std::string& id,
ManifestFetchData* existing_fetch = existing_iter->second.back().get();
if (existing_fetch->AddExtension(id, version.GetString(),
optional_ping_data, update_url_data,
install_source)) {
install_source,
force_update)) {
added = true;
}
}
......@@ -383,7 +400,8 @@ bool ExtensionDownloader::AddExtensionData(const std::string& id,
added = fetch->AddExtension(id, version.GetString(),
optional_ping_data,
update_url_data,
install_source);
install_source,
force_update);
DCHECK(added);
}
}
......@@ -639,12 +657,14 @@ void ExtensionDownloader::DetermineUpdates(
VLOG(2) << id << " is at '" << version << "'";
Version existing_version(version);
Version update_version(update->version);
if (!update_version.IsValid() ||
update_version.CompareTo(existing_version) <= 0) {
continue;
// We should skip the version check if update was forced.
if (!fetch_data.DidForceUpdate(id)) {
Version existing_version(version);
Version update_version(update->version);
if (!update_version.IsValid() ||
update_version.CompareTo(existing_version) <= 0) {
continue;
}
}
}
......
......@@ -166,7 +166,9 @@ class ExtensionDownloader
Manifest::Type extension_type,
const GURL& extension_update_url,
const std::string& update_url_data,
int request_id);
int request_id,
bool force_update,
const std::string& install_source_override);
// Adds all recorded stats taken so far to histogram counts.
void ReportStats() const;
......
......@@ -32,4 +32,10 @@ std::string ExtensionDownloaderDelegate::GetUpdateUrlData(
return std::string();
}
bool ExtensionDownloaderDelegate::ShouldForceUpdate(
const std::string& id,
std::string* source) {
return false;
}
} // namespace extensions
......@@ -111,6 +111,13 @@ class ExtensionDownloaderDelegate {
// that extension is not installed.
virtual bool GetExtensionExistingVersion(const std::string& id,
std::string* version) = 0;
// Determines if a given extension should be forced to update and (if so)
// what the source of this forcing is (i.e. what string will be passed
// in |installsource| as part of the update query parameters). The default
// implementation always returns |false|.
virtual bool ShouldForceUpdate(const std::string& id,
std::string* source);
};
} // namespace extensions
......
......@@ -8,6 +8,8 @@
#include <set>
#include "base/bind.h"
#include "base/files/file_enumerator.h"
#include "base/files/file_util.h"
#include "base/logging.h"
#include "base/metrics/histogram.h"
#include "base/prefs/pref_service.h"
......@@ -22,6 +24,7 @@
#include "chrome/browser/extensions/pending_extension_manager.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/pref_names.h"
#include "components/omaha_query_params/omaha_query_params.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/notification_details.h"
#include "content/public/browser/notification_service.h"
......@@ -40,6 +43,9 @@ using base::RandInt;
using base::Time;
using base::TimeDelta;
using content::BrowserThread;
using extensions::Extension;
using extensions::ExtensionSet;
using omaha_query_params::OmahaQueryParams;
typedef extensions::ExtensionDownloaderDelegate::Error Error;
typedef extensions::ExtensionDownloaderDelegate::PingResult PingResult;
......@@ -60,6 +66,10 @@ const int kMaxUpdateFrequencySeconds = 60 * 60 * 24 * 7; // 7 days
// checks.
const int kMinUpdateThrottleTime = 5;
// The installsource query parameter to use when forcing updates due to NaCl
// arch mismatch.
const char kWrongMultiCrxInstallSource[] = "wrong_multi_crx";
// When we've computed a days value, we want to make sure we don't send a
// negative value (due to the system clock being set backwards, etc.), since -1
// is a special sentinel value that means "never pinged", and other negative
......@@ -88,6 +98,73 @@ int CalculateActivePingDays(const Time& last_active_ping_day,
return SanitizeDays((Time::Now() - last_active_ping_day).InDays());
}
void RespondWithForcedUpdates(
const base::Callback<void(const std::set<std::string>&)>& callback,
scoped_ptr<std::set<std::string> > forced_updates) {
callback.Run(*forced_updates.get());
}
void DetermineForcedUpdatesOnBlockingPool(
scoped_ptr<std::vector<scoped_refptr<const Extension> > > extensions,
const base::Callback<void(const std::set<std::string>&)>& callback) {
scoped_ptr<std::set<std::string> > forced_updates(
new std::set<std::string>());
DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
for (std::vector<scoped_refptr<const Extension> >::const_iterator iter =
extensions->begin();
iter != extensions->end();
++iter) {
scoped_refptr<const Extension> extension = *iter;
base::FilePath platform_specific_path = extension->path().Append(
extensions::kPlatformSpecificFolder);
if (base::PathExists(platform_specific_path)) {
bool force = true;
base::FileEnumerator all_archs(platform_specific_path,
false,
base::FileEnumerator::DIRECTORIES);
base::FilePath arch;
while (!(arch = all_archs.Next()).empty()) {
std::string arch_name = arch.BaseName().AsUTF8Unsafe();
std::replace(arch_name.begin(), arch_name.end(), '_', '-');
if (arch_name == OmahaQueryParams::GetNaclArch())
force = false;
}
if (force)
forced_updates->insert(extension->id());
}
}
BrowserThread::PostTask(
BrowserThread::UI,
FROM_HERE,
base::Bind(&RespondWithForcedUpdates,
callback,
base::Passed(&forced_updates)));
}
void CollectExtensionsFromSet(
const ExtensionSet& extensions,
std::vector<scoped_refptr<const Extension> >* paths) {
std::copy(extensions.begin(), extensions.end(), std::back_inserter(*paths));
}
void DetermineForcedUpdates(
content::BrowserContext* browser_context,
const base::Callback<void(const std::set<std::string>&)>& callback) {
scoped_ptr<std::vector<scoped_refptr<const Extension> > > extensions(
new std::vector<scoped_refptr<const Extension> >());
const extensions::ExtensionRegistry* registry =
extensions::ExtensionRegistry::Get(browser_context);
scoped_ptr<ExtensionSet> installed_extensions =
registry->GenerateInstalledExtensionsSet();
CollectExtensionsFromSet(*installed_extensions.get(), extensions.get());
BrowserThread::PostBlockingPoolTask(
FROM_HERE,
base::Bind(&DetermineForcedUpdatesOnBlockingPool,
base::Passed(&extensions),
callback));
}
} // namespace
namespace extensions {
......@@ -335,6 +412,16 @@ void ExtensionUpdater::AddToDownloader(
}
void ExtensionUpdater::CheckNow(const CheckParams& params) {
DetermineForcedUpdates(
profile_,
base::Bind(&ExtensionUpdater::OnForcedUpdatesDetermined,
weak_ptr_factory_.GetWeakPtr(),
params));
}
void ExtensionUpdater::OnForcedUpdatesDetermined(
const CheckParams& params,
const std::set<std::string>& forced_updates) {
int request_id = next_request_id_++;
VLOG(2) << "Starting update check " << request_id;
......@@ -349,6 +436,8 @@ void ExtensionUpdater::CheckNow(const CheckParams& params) {
EnsureDownloaderCreated();
forced_updates_ = forced_updates;
// Add fetch records for extensions that should be fetched by an update URL.
// These extensions are not yet installed. They come from group policy
// and external install sources.
......@@ -536,6 +625,18 @@ bool ExtensionUpdater::GetExtensionExistingVersion(const std::string& id,
return true;
}
bool ExtensionUpdater::ShouldForceUpdate(
const std::string& extension_id,
std::string* source) {
bool force = forced_updates_.find(extension_id) != forced_updates_.end();
// Currently the only reason to force is a NaCl arch mismatch with the
// installed extension contents.
if (force) {
*source = kWrongMultiCrxInstallSource;
}
return force;
}
void ExtensionUpdater::UpdatePingData(const std::string& id,
const PingResult& ping_result) {
DCHECK(alive_);
......
......@@ -157,6 +157,11 @@ class ExtensionUpdater : public ExtensionDownloaderDelegate,
struct ThrottleInfo;
// Callback used to continue CheckNow after determining which extensions
// should be force-updated.
void OnForcedUpdatesDetermined(const CheckParams& params,
const std::set<std::string>& forced_updates);
// Ensure that we have a valid ExtensionDownloader instance referenced by
// |downloader|.
void EnsureDownloaderCreated();
......@@ -188,7 +193,6 @@ class ExtensionUpdater : public ExtensionDownloaderDelegate,
Error error,
const PingResult& ping,
const std::set<int>& request_ids) OVERRIDE;
virtual void OnExtensionDownloadFinished(
const std::string& id,
const base::FilePath& path,
......@@ -197,25 +201,15 @@ class ExtensionUpdater : public ExtensionDownloaderDelegate,
const std::string& version,
const PingResult& ping,
const std::set<int>& request_id) OVERRIDE;
// Implementation of ExtensionRegistryObserver.
virtual void OnExtensionWillBeInstalled(
content::BrowserContext* browser_context,
const Extension* extension,
bool is_update,
bool from_ephemeral,
const std::string& old_name) OVERRIDE;
virtual bool GetPingDataForExtension(
const std::string& id,
ManifestFetchData::PingData* ping_data) OVERRIDE;
virtual std::string GetUpdateUrlData(const std::string& id) OVERRIDE;
virtual bool IsExtensionPending(const std::string& id) OVERRIDE;
virtual bool GetExtensionExistingVersion(const std::string& id,
std::string* version) OVERRIDE;
virtual bool ShouldForceUpdate(const std::string& extension_id,
std::string* source) OVERRIDE;
void UpdatePingData(const std::string& id, const PingResult& ping_result);
......@@ -227,6 +221,14 @@ class ExtensionUpdater : public ExtensionDownloaderDelegate,
const content::NotificationSource& source,
const content::NotificationDetails& details) OVERRIDE;
// Implementation of ExtensionRegistryObserver.
virtual void OnExtensionWillBeInstalled(
content::BrowserContext* browser_context,
const Extension* extension,
bool is_update,
bool from_ephemeral,
const std::string& old_name) OVERRIDE;
// Send a notification that update checks are starting.
void NotifyStarted();
......@@ -284,6 +286,10 @@ class ExtensionUpdater : public ExtensionDownloaderDelegate,
// checks to prevent too many requests from being made.
std::map<std::string, ThrottleInfo> throttle_info_;
// Keeps track of extensions (by ID) whose update should be forced during the
// next update check.
std::set<std::string> forced_updates_;
DISALLOW_COPY_AND_ASSIGN(ExtensionUpdater);
};
......
......@@ -613,6 +613,7 @@ class ExtensionUpdaterTest : public testing::Test {
EXPECT_TRUE(updater->timer_.IsRunning());
updater->timer_.Stop();
updater->TimerFired();
content::RunAllBlockingPoolTasksUntilIdle();
}
// Adds a Result with the given data to results.
......@@ -703,7 +704,7 @@ class ExtensionUpdaterTest : public testing::Test {
// option to appear in the x= parameter.
ManifestFetchData fetch_data(GURL("http://localhost/foo"), 0);
fetch_data.AddExtension(
id, version, &kNeverPingedData, std::string(), std::string());
id, version, &kNeverPingedData, std::string(), std::string(), false);
std::map<std::string, std::string> params;
VerifyQueryAndExtractParameters(fetch_data.full_url().query(), &params);
......@@ -720,7 +721,7 @@ class ExtensionUpdaterTest : public testing::Test {
// option to appear in the x= parameter.
ManifestFetchData fetch_data(GURL("http://localhost/foo"), 0);
fetch_data.AddExtension(
id, version, &kNeverPingedData, "bar", std::string());
id, version, &kNeverPingedData, "bar", std::string(), false);
std::map<std::string, std::string> params;
VerifyQueryAndExtractParameters(fetch_data.full_url().query(), &params);
EXPECT_EQ(id, params["id"]);
......@@ -736,7 +737,7 @@ class ExtensionUpdaterTest : public testing::Test {
// option to appear in the x= parameter.
ManifestFetchData fetch_data(GURL("http://localhost/foo"), 0);
fetch_data.AddExtension(
id, version, &kNeverPingedData, "a=1&b=2&c", std::string());
id, version, &kNeverPingedData, "a=1&b=2&c", std::string(), false);
std::map<std::string, std::string> params;
VerifyQueryAndExtractParameters(fetch_data.full_url().query(), &params);
EXPECT_EQ(id, params["id"]);
......@@ -780,7 +781,7 @@ class ExtensionUpdaterTest : public testing::Test {
// Make sure that an installsource= appears in the x= parameter.
ManifestFetchData fetch_data(GURL("http://localhost/foo"), 0);
fetch_data.AddExtension(id, version, &kNeverPingedData,
kEmptyUpdateUrlData, install_source);
kEmptyUpdateUrlData, install_source, false);
std::map<std::string, std::string> params;
VerifyQueryAndExtractParameters(fetch_data.full_url().query(), &params);
EXPECT_EQ(id, params["id"]);
......@@ -806,10 +807,12 @@ class ExtensionUpdaterTest : public testing::Test {
const std::string id1 = crx_file::id_util::GenerateId("1");
const std::string id2 = crx_file::id_util::GenerateId("2");
fetch_data.AddExtension(
id1, "1.0.0.0", &kNeverPingedData, kEmptyUpdateUrlData, std::string());
id1, "1.0.0.0", &kNeverPingedData, kEmptyUpdateUrlData, std::string(),
false);
AddParseResult(id1, "1.1", "http://localhost/e1_1.1.crx", &updates);
fetch_data.AddExtension(
id2, "2.0.0.0", &kNeverPingedData, kEmptyUpdateUrlData, std::string());
id2, "2.0.0.0", &kNeverPingedData, kEmptyUpdateUrlData, std::string(),
false);
AddParseResult(id2, "2.0.0.0", "http://localhost/e2_2.0.crx", &updates);
EXPECT_CALL(delegate, IsExtensionPending(_)).WillRepeatedly(Return(false));
......@@ -850,7 +853,8 @@ class ExtensionUpdaterTest : public testing::Test {
"1.0.0.0",
&kNeverPingedData,
kEmptyUpdateUrlData,
std::string());
std::string(),
false);
AddParseResult(*it, "1.1", "http://localhost/e1_1.1.crx", &updates);
}
......@@ -884,13 +888,13 @@ class ExtensionUpdaterTest : public testing::Test {
scoped_ptr<ManifestFetchData> fetch4(new ManifestFetchData(kUpdateUrl, 0));
ManifestFetchData::PingData zeroDays(0, 0, true);
fetch1->AddExtension(
"1111", "1.0", &zeroDays, kEmptyUpdateUrlData, std::string());
"1111", "1.0", &zeroDays, kEmptyUpdateUrlData, std::string(), false);
fetch2->AddExtension(
"2222", "2.0", &zeroDays, kEmptyUpdateUrlData, std::string());
"2222", "2.0", &zeroDays, kEmptyUpdateUrlData, std::string(), false);
fetch3->AddExtension(
"3333", "3.0", &zeroDays, kEmptyUpdateUrlData, std::string());
"3333", "3.0", &zeroDays, kEmptyUpdateUrlData, std::string(), false);
fetch4->AddExtension(
"4444", "4.0", &zeroDays, kEmptyUpdateUrlData, std::string());
"4444", "4.0", &zeroDays, kEmptyUpdateUrlData, std::string(), false);
// This will start the first fetcher and queue the others. The next in queue
// is started as each fetcher receives its response. Note that the fetchers
......@@ -1021,7 +1025,7 @@ class ExtensionUpdaterTest : public testing::Test {
scoped_ptr<ManifestFetchData> fetch(new ManifestFetchData(kUpdateUrl, 0));
ManifestFetchData::PingData zeroDays(0, 0, true);
fetch->AddExtension(
"1111", "1.0", &zeroDays, kEmptyUpdateUrlData, std::string());
"1111", "1.0", &zeroDays, kEmptyUpdateUrlData, std::string(), false);
// This will start the first fetcher.
downloader.StartUpdateCheck(fetch.Pass());
......@@ -1049,7 +1053,7 @@ class ExtensionUpdaterTest : public testing::Test {
// should not retry.
fetch.reset(new ManifestFetchData(kUpdateUrl, 0));
fetch->AddExtension(
"1111", "1.0", &zeroDays, kEmptyUpdateUrlData, std::string());
"1111", "1.0", &zeroDays, kEmptyUpdateUrlData, std::string(), false);
// This will start the first fetcher.
downloader.StartUpdateCheck(fetch.Pass());
......@@ -1604,6 +1608,7 @@ class ExtensionUpdaterTest : public testing::Test {
ExtensionUpdater::CheckParams params;
updater.Start();
updater.CheckNow(params);
content::RunAllBlockingPoolTasksUntilIdle();
// Make the updater do manifest fetching, and note the urls it tries to
// fetch.
......@@ -1707,7 +1712,8 @@ class ExtensionUpdaterTest : public testing::Test {
extension->VersionString(),
&kNeverPingedData,
kEmptyUpdateUrlData,
std::string());
std::string(),
false);
UpdateManifest::Results results;
results.daystart_elapsed_seconds = 750;
......@@ -1905,6 +1911,7 @@ TEST_F(ExtensionUpdaterTest, TestNonAutoUpdateableLocations) {
ExtensionUpdater::CheckParams params;
updater.Start();
updater.CheckNow(params);
content::RunAllBlockingPoolTasksUntilIdle();
}
TEST_F(ExtensionUpdaterTest, TestUpdatingDisabledExtensions) {
......@@ -1943,6 +1950,7 @@ TEST_F(ExtensionUpdaterTest, TestUpdatingDisabledExtensions) {
ExtensionUpdater::CheckParams params;
updater.Start();
updater.CheckNow(params);
content::RunAllBlockingPoolTasksUntilIdle();
}
TEST_F(ExtensionUpdaterTest, TestManifestFetchesBuilderAddExtension) {
......
......@@ -67,16 +67,24 @@ bool ManifestFetchData::AddExtension(const std::string& id,
const std::string& version,
const PingData* ping_data,
const std::string& update_url_data,
const std::string& install_source) {
const std::string& install_source,
bool force_update) {
if (extension_ids_.find(id) != extension_ids_.end()) {
NOTREACHED() << "Duplicate extension id " << id;
return false;
}
if (force_update)
forced_updates_.insert(id);
// If we want to force an update, we send 0.0.0.0 as the installed version
// number.
const std::string installed_version = force_update ? "0.0.0.0" : version;
// Compute the string we'd append onto the full_url_, and see if it fits.
std::vector<std::string> parts;
parts.push_back("id=" + id);
parts.push_back("v=" + version);
parts.push_back("v=" + installed_version);
if (!install_source.empty())
parts.push_back("installsource=" + install_source);
parts.push_back("uc");
......@@ -163,4 +171,8 @@ void ManifestFetchData::Merge(const ManifestFetchData& other) {
request_ids_.insert(other.request_ids_.begin(), other.request_ids_.end());
}
bool ManifestFetchData::DidForceUpdate(const std::string& extension_id) const {
return forced_updates_.find(extension_id) != forced_updates_.end();
}
} // namespace extensions
......@@ -56,7 +56,8 @@ class ManifestFetchData {
const std::string& version,
const PingData* ping_data,
const std::string& update_url_data,
const std::string& install_source);
const std::string& install_source,
bool force_update);
const GURL& base_url() const { return base_url_; }
const GURL& full_url() const { return full_url_; }
......@@ -76,6 +77,9 @@ class ManifestFetchData {
// to this ManifestFetchData).
void Merge(const ManifestFetchData& other);
// Returns |true| if a given extension was forced to update.
bool DidForceUpdate(const std::string& extension_id) const;
private:
// The set of extension id's for this ManifestFetchData.
std::set<std::string> extension_ids_;
......@@ -96,6 +100,9 @@ class ManifestFetchData {
// one ManifestFetchData.
std::set<int> request_ids_;
// The set of extension IDs for which this fetch forced a CRX update.
std::set<std::string> forced_updates_;
DISALLOW_COPY_AND_ASSIGN(ManifestFetchData);
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment