Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Revert of Enable forced extension updates on NaCl arch mismatch (patc…
Browse files Browse the repository at this point in the history 
…hset #5 id:100001 of https://codereview.chromium.org/516293007/)

Reason for revert:
Chrome OS build with DCHECKs enabled fails on start. This CL breaks assumption, that GAIA extension can be loaded without performing IO-operations.

Log and stack trace:
[16275:16275:0903/161435:WARNING:renderer_freezer.cc(55)] Cgroup freezer does not exist or is not writable. Processes will not be frozen during suspend.
[16275:16275:0903/161435:WARNING:configuration_policy_pref_store.cc(30)] Policy RemoteAccessClientFirewallTraversal: This policy has been deprecated.
[16275:16275:0903/161437:FATAL:thread_restrictions.cc(38)] Function marked as IO-only was called from a thread that disallows IO!  If this thread really should be allowed to make IO calls, adjust the call to base::ThreadRestrictions::SetIOAllowed() in this thread's startup.
#0 0x7fe3122e555e base::debug::StackTrace::StackTrace()
#1 0x7fe31237c462 logging::LogMessage::~LogMessage()
#2 0x7fe3124824af base::ThreadRestrictions::AssertIOAllowed()
#3 0x7fe3123630a1 base::PathExists()
#4 0x7fe321cfd5f3 extensions::(anonymous namespace)::CollectPlatformSpecificResourceArchs()
#5 0x7fe321cfad22 extensions::Extension::InitFromValue()
#6 0x7fe321cfa490 extensions::Extension::Create()
#7 0x7fe321cfa114 extensions::Extension::Create()
#8 0x7fe3233ebe1b extensions::ComponentLoader::Load()
#9 0x7fe3233ec3cf extensions::ComponentLoader::Add()
#10 0x7fe3233ec32a extensions::ComponentLoader::Add()
#11 0x7fe3233ec2b0 extensions::ComponentLoader::Add()
#12 0x7fe32437505f (anonymous namespace)::LoadGaiaAuthExtension()
#13 0x7fe324374d5e extensions::GaiaAuthExtensionLoader::LoadIfNeeded()
#14 0x7fe324237f7e ScopedGaiaAuthExtension::ScopedGaiaAuthExtension()
#15 0x7fe322f68bb2 chromeos::WebUILoginView::Init()
#16 0x7fe322f556ef chromeos::LoginDisplayHostImpl::InitLoginWindowAndView()
#17 0x7fe322f52640 chromeos::LoginDisplayHostImpl::LoadURL()
#18 0x7fe322f5226a chromeos::LoginDisplayHostImpl::StartWizard()
#19 0x7fe322f54acd chromeos::LoginDisplayHostImpl::StartPostponedWebUI()
#20 0x7fe322f541f8 chromeos::LoginDisplayHostImpl::Observe()
#21 0x7fe322f54bfd chromeos::LoginDisplayHostImpl::Observe()
#22 0x7fe31b222377 content::NotificationServiceImpl::Notify()
#23 0x7fe322d51a24 chromeos::(anonymous namespace)::UserWallpaperDelegate::OnWallpaperAnimationFinished()
#24 0x7fe31188692f ash::RootWindowController::OnWallpaperAnimationFinished()
#25 0x7fe3117c633a ash::(anonymous namespace)::ShowWallpaperAnimationObserver::OnImplicitAnimationsCompleted()
#26 0x7fe311f72d30 ui::ImplicitAnimationObserver::CheckCompleted()
#27 0x7fe311f72cd5 ui::ImplicitAnimationObserver::SetActive()
#28 0x7fe311f965d5 ui::ScopedLayerAnimationSettings::~ScopedLayerAnimationSettings()
#29 0x7fe3117c5ec7 ash::DesktopBackgroundWidgetController::StartAnimating()
#30 0x7fe3117bbb0c ash::DesktopBackgroundController::InstallDesktopController()
#31 0x7fe3117bbc03 ash::DesktopBackgroundController::InstallDesktopControllerForAllWindows()
#32 0x7fe3117bb3cc ash::DesktopBackgroundController::SetDesktopBackgroundImageMode()
#33 0x7fe3117bb096 ash::DesktopBackgroundController::SetWallpaperImage()
#34 0x7fe322f9a6b9 chromeos::WallpaperManager::DoSetDefaultWallpaper()
#35 0x7fe322f99b73 chromeos::WallpaperManager::PendingWallpaper::ProcessRequest()
#36 0x7fe322fbac72 base::internal::RunnableAdapter<>::Run()
#37 0x7fe322fbabe9 base::internal::InvokeHelper<>::MakeItSo()
#38 0x7fe322fbaba5 base::internal::Invoker<>::Run()
#39 0x7fe3122cc9ce base::Callback<>::Run()
#40 0x7fe312488f86 base::Timer::RunScheduledTask()
#41 0x7fe3124890bc base::BaseTimerTaskInternal::Run()
#42 0x7fe312489382 base::internal::RunnableAdapter<>::Run()
#43 0x7fe3124892ec base::internal::InvokeHelper<>::MakeItSo()
#44 0x7fe312489295 base::internal::Invoker<>::Run()
#45 0x7fe3122cc9ce base::Callback<>::Run()
#46 0x7fe3122eb9b3 base::debug::TaskAnnotator::RunTask()
#47 0x7fe3123a1c57 base::MessageLoop::RunTask()
#48 0x7fe3123a1d9b base::MessageLoop::DeferOrRunPendingTask()
#49 0x7fe3123a228d base::MessageLoop::DoDelayedWork()
#50 0x7fe3122a1f25 base::MessagePumpGlib::Run()
#51 0x7fe3123a17f0 base::MessageLoop::RunHandler()
#52 0x7fe3124093b2 base::RunLoop::Run()
#53 0x7fe324217e4d ChromeBrowserMainParts::MainMessageLoopRun()
#54 0x7fe31ad2a8bf content::BrowserMainLoop::RunMainMessageLoopParts()
#55 0x7fe31ad344c7 content::BrowserMainRunnerImpl::Run()
#56 0x7fe31ad251b1 content::BrowserMain()
#57 0x7fe31abb997f content::RunNamedProcessTypeMain()
#58 0x7fe31abbcce8 content::ContentMainRunnerImpl::Run()
#59 0x7fe31abb8ee5 content::ContentMain()
#60 0x7fe3206d3505 ChromeMain
#61 0x7fe3206d34b2 main

Cannot upload crash dump: cannot exec /sbin/crash_reporter

Crash_reporter failed to process crash report

Original issue's description:
> Enable forced extension updates on NaCl arch mismatch
>
> This makes extensions aware of the platforms for which
> they have platform-specific resources installed, if any.
>
> This also hooks up the extension update code with some
> additional logic to place an extension in forced-update
> mode if it has platform-specific resources which don't
> match the current NaCl architecture.
>
> BUG=409948
> TEST=install an extension which uses NaCl (QuickOffice for example). Rename the _platform-specific/<your-nacl-arch> directory some something else and force an update (e.g. via chrome://extensions button). Observe that a new CRX is downloaded and installed.
>
> Committed: https://chromium.googlesource.com/chromium/src/+/4a92281fa5d331860d65a59ba45dc882a5c71df4

[email protected],[email protected],[email protected],[email protected]
NOTREECHECKS=true
NOTRY=true
BUG=409948

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

Cr-Commit-Position: refs/heads/master@{#293128}
  • Loading branch information
@dzhioev
dzhioev authored and Commit bot committed Sep 3, 2014
1 parent 037a83c commit dd6d036
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 134 deletions.
66 changes: 18 additions & 48 deletions chrome/browser/extensions/updater/extension_downloader.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include "chrome/common/chrome_version_info.h"
#include "chrome/common/extensions/extension_constants.h"
#include "chrome/common/extensions/manifest_url_handler.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"
Expand All @@ -45,7 +44,6 @@
using base::Time;
using base::TimeDelta;
using content::BrowserThread;
using omaha_query_params::OmahaQueryParams;

namespace extensions {

Expand Down Expand Up @@ -86,7 +84,6 @@ const int kMaxOAuth2Attempts = 3;

const char kNotFromWebstoreInstallSource[] = "notfromwebstore";
const char kDefaultInstallSource[] = "";
const char kWrongMultiCrxInstallSource[] = "wrong_multi_crx";

const char kGoogleDotCom[] = "google.com";
const char kTokenServiceConsumerId[] = "extension_downloader";
Expand Down Expand Up @@ -213,24 +210,10 @@ bool ExtensionDownloader::AddExtension(const Extension& extension,
if (!ManifestURL::UpdatesFromGallery(&extension))
update_url_data = delegate_->GetUpdateUrlData(extension.id());

// If the browser's native architecture has changed since this extension was
// installed, we need to force an update.
bool force_update = false;
std::string install_source;
if (extension.HasPlatformSpecificResources() &&
!extension.HasResourcesForPlatform(OmahaQueryParams::GetNaclArch())) {
force_update = true;
install_source = kWrongMultiCrxInstallSource;
}

return AddExtensionData(extension.id(),
*extension.version(),
return AddExtensionData(extension.id(), *extension.version(),
extension.GetType(),
ManifestURL::GetUpdateURL(&extension),
update_url_data,
request_id,
force_update,
install_source);
update_url_data, request_id);
}

bool ExtensionDownloader::AddPendingExtension(const std::string& id,
Expand All @@ -247,9 +230,7 @@ bool ExtensionDownloader::AddPendingExtension(const std::string& id,
Manifest::TYPE_UNKNOWN,
update_url,
std::string(),
request_id,
false,
std::string());
request_id);
}

void ExtensionDownloader::StartAllPending(ExtensionCache* cache) {
Expand Down Expand Up @@ -292,8 +273,7 @@ void ExtensionDownloader::StartBlacklistUpdate(
version,
&ping_data,
std::string(),
kDefaultInstallSource,
false);
kDefaultInstallSource);
StartUpdateCheck(blacklist_fetch.Pass());
}

Expand All @@ -302,15 +282,12 @@ 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 force_update,
const std::string& install_source_override) {
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) {
GURL update_url(extension_update_url);
// Skip extensions with non-empty invalid update URLs.
if (!update_url.is_empty() && !update_url.is_valid()) {
Expand Down Expand Up @@ -376,9 +353,6 @@ bool ExtensionDownloader::AddExtensionData(

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;
Expand All @@ -395,8 +369,7 @@ bool ExtensionDownloader::AddExtensionData(
ManifestFetchData* existing_fetch = existing_iter->second.back().get();
if (existing_fetch->AddExtension(id, version.GetString(),
optional_ping_data, update_url_data,
install_source,
force_update)) {
install_source)) {
added = true;
}
}
Expand All @@ -410,8 +383,7 @@ bool ExtensionDownloader::AddExtensionData(
added = fetch->AddExtension(id, version.GetString(),
optional_ping_data,
update_url_data,
install_source,
force_update);
install_source);
DCHECK(added);
}
}
Expand Down Expand Up @@ -667,14 +639,12 @@ void ExtensionDownloader::DetermineUpdates(

VLOG(2) << id << " is at '" << version << "'";

// 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;
}
Version existing_version(version);
Version update_version(update->version);

if (!update_version.IsValid() ||
update_version.CompareTo(existing_version) <= 0) {
continue;
}
}

Expand Down
4 changes: 1 addition & 3 deletions chrome/browser/extensions/updater/extension_downloader.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,7 @@ class ExtensionDownloader
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);
int request_id);

// Adds all recorded stats taken so far to histogram counts.
void ReportStats() const;
Expand Down
32 changes: 14 additions & 18 deletions chrome/browser/extensions/updater/extension_updater_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,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(), false);
id, version, &kNeverPingedData, std::string(), std::string());

std::map<std::string, std::string> params;
VerifyQueryAndExtractParameters(fetch_data.full_url().query(), &params);
Expand All @@ -720,7 +720,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(), false);
id, version, &kNeverPingedData, "bar", std::string());
std::map<std::string, std::string> params;
VerifyQueryAndExtractParameters(fetch_data.full_url().query(), &params);
EXPECT_EQ(id, params["id"]);
Expand All @@ -736,7 +736,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(), false);
id, version, &kNeverPingedData, "a=1&b=2&c", std::string());
std::map<std::string, std::string> params;
VerifyQueryAndExtractParameters(fetch_data.full_url().query(), &params);
EXPECT_EQ(id, params["id"]);
Expand Down Expand Up @@ -780,7 +780,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, false);
kEmptyUpdateUrlData, install_source);
std::map<std::string, std::string> params;
VerifyQueryAndExtractParameters(fetch_data.full_url().query(), &params);
EXPECT_EQ(id, params["id"]);
Expand All @@ -806,12 +806,10 @@ 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(),
false);
id1, "1.0.0.0", &kNeverPingedData, kEmptyUpdateUrlData, std::string());
AddParseResult(id1, "1.1", "http://localhost/e1_1.1.crx", &updates);
fetch_data.AddExtension(
id2, "2.0.0.0", &kNeverPingedData, kEmptyUpdateUrlData, std::string(),
false);
id2, "2.0.0.0", &kNeverPingedData, kEmptyUpdateUrlData, std::string());
AddParseResult(id2, "2.0.0.0", "http://localhost/e2_2.0.crx", &updates);

EXPECT_CALL(delegate, IsExtensionPending(_)).WillRepeatedly(Return(false));
Expand Down Expand Up @@ -852,8 +850,7 @@ class ExtensionUpdaterTest : public testing::Test {
"1.0.0.0",
&kNeverPingedData,
kEmptyUpdateUrlData,
std::string(),
false);
std::string());
AddParseResult(*it, "1.1", "http://localhost/e1_1.1.crx", &updates);
}

Expand Down Expand Up @@ -887,13 +884,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(), false);
"1111", "1.0", &zeroDays, kEmptyUpdateUrlData, std::string());
fetch2->AddExtension(
"2222", "2.0", &zeroDays, kEmptyUpdateUrlData, std::string(), false);
"2222", "2.0", &zeroDays, kEmptyUpdateUrlData, std::string());
fetch3->AddExtension(
"3333", "3.0", &zeroDays, kEmptyUpdateUrlData, std::string(), false);
"3333", "3.0", &zeroDays, kEmptyUpdateUrlData, std::string());
fetch4->AddExtension(
"4444", "4.0", &zeroDays, kEmptyUpdateUrlData, std::string(), false);
"4444", "4.0", &zeroDays, kEmptyUpdateUrlData, std::string());

// 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
Expand Down Expand Up @@ -1024,7 +1021,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(), false);
"1111", "1.0", &zeroDays, kEmptyUpdateUrlData, std::string());

// This will start the first fetcher.
downloader.StartUpdateCheck(fetch.Pass());
Expand Down Expand Up @@ -1052,7 +1049,7 @@ class ExtensionUpdaterTest : public testing::Test {
// should not retry.
fetch.reset(new ManifestFetchData(kUpdateUrl, 0));
fetch->AddExtension(
"1111", "1.0", &zeroDays, kEmptyUpdateUrlData, std::string(), false);
"1111", "1.0", &zeroDays, kEmptyUpdateUrlData, std::string());

// This will start the first fetcher.
downloader.StartUpdateCheck(fetch.Pass());
Expand Down Expand Up @@ -1710,8 +1707,7 @@ class ExtensionUpdaterTest : public testing::Test {
extension->VersionString(),
&kNeverPingedData,
kEmptyUpdateUrlData,
std::string(),
false);
std::string());
UpdateManifest::Results results;
results.daystart_elapsed_seconds = 750;

Expand Down
16 changes: 2 additions & 14 deletions chrome/browser/extensions/updater/manifest_fetch_data.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,24 +67,16 @@ 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,
bool force_update) {
const std::string& install_source) {
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=" + installed_version);
parts.push_back("v=" + version);
if (!install_source.empty())
parts.push_back("installsource=" + install_source);
parts.push_back("uc");
Expand Down Expand Up @@ -171,8 +163,4 @@ 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
9 changes: 1 addition & 8 deletions chrome/browser/extensions/updater/manifest_fetch_data.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ class ManifestFetchData {
const std::string& version,
const PingData* ping_data,
const std::string& update_url_data,
const std::string& install_source,
bool force_update);
const std::string& install_source);

const GURL& base_url() const { return base_url_; }
const GURL& full_url() const { return full_url_; }
Expand All @@ -77,9 +76,6 @@ 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_;
Expand All @@ -100,9 +96,6 @@ 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);
};

Expand Down
36 changes: 0 additions & 36 deletions extensions/common/extension.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,10 @@

#include "extensions/common/extension.h"

#include <algorithm>

#include "base/base64.h"
#include "base/basictypes.h"
#include "base/command_line.h"
#include "base/files/file_enumerator.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/i18n/rtl.h"
#include "base/logging.h"
#include "base/memory/singleton.h"
Expand Down Expand Up @@ -69,26 +65,6 @@ bool ContainsReservedCharacters(const base::FilePath& path) {
return !net::IsSafePortableRelativePath(path);
}

void CollectPlatformSpecificResourceArchs(const base::FilePath& extension_path,
std::set<std::string>* archs) {
archs->clear();
base::FilePath platform_specific_path = extension_path.Append(
kPlatformSpecificFolder);
if (!base::PathExists(platform_specific_path)) {
return;
}

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(), '_', '-');
archs->insert(arch_name);
}
}

} // namespace

const int Extension::kInitFromValueFlagBits = 13;
Expand Down Expand Up @@ -459,15 +435,6 @@ void Extension::AddWebExtentPattern(const URLPattern& pattern) {
extent_.AddPattern(pattern);
}

bool Extension::HasPlatformSpecificResources() const {
return !platform_specific_resource_archs_.empty();
}

bool Extension::HasResourcesForPlatform(const std::string& arch) const {
return platform_specific_resource_archs_.find(arch) !=
platform_specific_resource_archs_.end();
}

// static
bool Extension::InitExtensionID(extensions::Manifest* manifest,
const base::FilePath& path,
Expand Down Expand Up @@ -569,9 +536,6 @@ bool Extension::InitFromValue(int flags, base::string16* error) {

permissions_data_.reset(new PermissionsData(this));

CollectPlatformSpecificResourceArchs(path_,
&platform_specific_resource_archs_);

return true;
}

Expand Down
Loading

0 comments on commit dd6d036

Please sign in to comment.