Commit 253fc2bb authored by tmdiep@chromium.org's avatar tmdiep@chromium.org

Update the CrxInstaller and UnpackedInstaller to use the ExtensionInstallChecker

This patch removes the deprecated ExtensionInstaller and uses
ExtensionInstallChecker in CrxInstaller and UnpackedInstaller.

BUG=386404
TEST=browser_tests
TBR=pam@chromium.org (for include in supervised_user_service_unittest.cc)

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282250 0039d316-1c4b-4281-b951-d872f2087c98
parent c69ecc30
......@@ -133,7 +133,7 @@ CrxInstaller::CrxInstaller(base::WeakPtr<ExtensionService> service_weak,
error_on_unsupported_requirements_(false),
update_from_settings_page_(false),
install_flags_(kInstallFlagNone),
installer_(service_weak->profile()) {
install_checker_(service_weak->profile()) {
installer_task_runner_ = service_weak->GetFileTaskRunner();
if (!approval)
return;
......@@ -369,7 +369,7 @@ CrxInstallerError CrxInstaller::AllowInstall(const Extension* extension) {
}
}
if (installer_.extension()->is_app()) {
if (install_checker_.extension()->is_app()) {
// If the app was downloaded, apps_require_extension_mime_type_
// will be set. In this case, check that it was served with the
// right mime type. Make an exception for file URLs, which come
......@@ -404,7 +404,7 @@ CrxInstallerError CrxInstaller::AllowInstall(const Extension* extension) {
pattern.SetHost(download_url_.host());
pattern.SetMatchSubdomains(true);
URLPatternSet patterns = installer_.extension()->web_extent();
URLPatternSet patterns = install_checker_.extension()->web_extent();
for (URLPatternSet::const_iterator i = patterns.begin();
i != patterns.end(); ++i) {
if (!pattern.MatchesHost(i->host())) {
......@@ -448,7 +448,7 @@ void CrxInstaller::OnUnpackSuccess(
install_cause(),
extension_misc::NUM_INSTALL_CAUSES);
installer_.set_extension(extension);
install_checker_.set_extension(extension);
temp_dir_ = temp_dir;
if (!install_icon.empty())
install_icon_.reset(new SkBitmap(install_icon));
......@@ -468,13 +468,13 @@ void CrxInstaller::OnUnpackSuccess(
return;
}
if (!BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
base::Bind(&CrxInstaller::CheckImportsAndRequirements, this)))
if (!BrowserThread::PostTask(BrowserThread::UI,
FROM_HERE,
base::Bind(&CrxInstaller::CheckInstall, this)))
NOTREACHED();
}
void CrxInstaller::CheckImportsAndRequirements() {
void CrxInstaller::CheckInstall() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
ExtensionService* service = service_weak_.get();
if (!service || service->browser_terminating())
......@@ -505,42 +505,36 @@ void CrxInstaller::CheckImportsAndRequirements() {
}
}
}
installer_.CheckRequirements(base::Bind(&CrxInstaller::OnRequirementsChecked,
this));
// Run the policy, requirements and blacklist checks in parallel.
install_checker_.Start(
ExtensionInstallChecker::CHECK_ALL,
false /* fail fast */,
base::Bind(&CrxInstaller::OnInstallChecksComplete, this));
}
void CrxInstaller::OnRequirementsChecked(
std::vector<std::string> requirement_errors) {
void CrxInstaller::OnInstallChecksComplete(int failed_checks) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (!service_weak_)
return;
if (!requirement_errors.empty()) {
// Check for requirement errors.
if (!install_checker_.requirement_errors().empty()) {
if (error_on_unsupported_requirements_) {
ReportFailureFromUIThread(CrxInstallerError(
base::UTF8ToUTF16(JoinString(requirement_errors, ' '))));
ReportFailureFromUIThread(CrxInstallerError(base::UTF8ToUTF16(
JoinString(install_checker_.requirement_errors(), ' '))));
return;
}
install_flags_ |= kInstallFlagHasRequirementErrors;
}
ExtensionSystem::Get(profile())->blacklist()->IsBlacklisted(
extension()->id(),
base::Bind(&CrxInstaller::OnBlacklistChecked, this));
}
void CrxInstaller::OnBlacklistChecked(
extensions::BlacklistState blacklist_state) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (!service_weak_)
return;
if (blacklist_state == extensions::BLACKLISTED_MALWARE) {
// Check the blacklist state.
if (install_checker_.blacklist_state() == extensions::BLACKLISTED_MALWARE) {
install_flags_ |= kInstallFlagIsBlacklistedForMalware;
}
if ((blacklist_state == extensions::BLACKLISTED_MALWARE ||
blacklist_state == extensions::BLACKLISTED_UNKNOWN) &&
if ((install_checker_.blacklist_state() == extensions::BLACKLISTED_MALWARE ||
install_checker_.blacklist_state() == extensions::BLACKLISTED_UNKNOWN) &&
!allow_silent_install_) {
// User tried to install a blacklisted extension. Show an error and
// refuse to install it.
......@@ -556,6 +550,19 @@ void CrxInstaller::OnBlacklistChecked(
// NOTE: extension may still be blacklisted, but we're forced to silently
// install it. In this case, ExtensionService::OnExtensionInstalled needs to
// deal with it.
// Check for policy errors.
if (!install_checker_.policy_error().empty()) {
// We don't want to show the error infobar for installs from the WebStore,
// because the WebStore already shows an error dialog itself.
// Note: |client_| can be NULL in unit_tests!
if (extension()->from_webstore() && client_)
client_->install_ui()->set_skip_post_install_ui(true);
ReportFailureFromUIThread(
CrxInstallerError(base::UTF8ToUTF16(install_checker_.policy_error())));
return;
}
ConfirmInstall();
}
......@@ -565,7 +572,7 @@ void CrxInstaller::ConfirmInstall() {
if (!service || service->browser_terminating())
return;
if (KioskModeInfo::IsKioskOnly(installer_.extension())) {
if (KioskModeInfo::IsKioskOnly(install_checker_.extension())) {
bool in_kiosk_mode = false;
#if defined(OS_CHROMEOS)
chromeos::UserManager* user_manager = chromeos::UserManager::Get();
......@@ -579,17 +586,6 @@ void CrxInstaller::ConfirmInstall() {
}
}
base::string16 error = installer_.CheckManagementPolicy();
if (!error.empty()) {
// We don't want to show the error infobar for installs from the WebStore,
// because the WebStore already shows an error dialog itself.
// Note: |client_| can be NULL in unit_tests!
if (extension()->from_webstore() && client_)
client_->install_ui()->set_skip_post_install_ui(true);
ReportFailureFromUIThread(CrxInstallerError(error));
return;
}
// Check whether this install is initiated from the settings page to
// update an existing extension or app.
CheckUpdateFromSettingsPage();
......@@ -722,7 +718,7 @@ void CrxInstaller::ReloadExtensionAfterInstall(
// with base::string16
std::string extension_id = extension()->id();
std::string error;
installer_.set_extension(
install_checker_.set_extension(
file_util::LoadExtension(
version_dir,
install_source_,
......
......@@ -14,8 +14,8 @@
#include "base/memory/weak_ptr.h"
#include "base/version.h"
#include "chrome/browser/extensions/blacklist.h"
#include "chrome/browser/extensions/extension_install_checker.h"
#include "chrome/browser/extensions/extension_install_prompt.h"
#include "chrome/browser/extensions/extension_installer.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/sandboxed_unpacker.h"
#include "chrome/browser/extensions/webstore_installer.h"
......@@ -195,9 +195,9 @@ class CrxInstaller
bool did_handle_successfully() const { return did_handle_successfully_; }
Profile* profile() { return installer_.profile(); }
Profile* profile() { return install_checker_.profile(); }
const Extension* extension() { return installer_.extension().get(); }
const Extension* extension() { return install_checker_.extension().get(); }
const std::string& current_version() const { return current_version_; }
......@@ -229,11 +229,12 @@ class CrxInstaller
const Extension* extension,
const SkBitmap& install_icon) OVERRIDE;
// Called on the UI thread to start the requirements check on the extension.
void CheckImportsAndRequirements();
// Called on the UI thread to start the requirements, policy and blacklist
// checks on the extension.
void CheckInstall();
// Runs on the UI thread. Callback from RequirementsChecker.
void OnRequirementsChecked(std::vector<std::string> requirement_errors);
// Runs on the UI thread. Callback from ExtensionInstallChecker.
void OnInstallChecksComplete(int failed_checks);
// Runs on the UI thread. Callback from Blacklist.
void OnBlacklistChecked(
......@@ -417,8 +418,8 @@ class CrxInstaller
// The flags for ExtensionService::OnExtensionInstalled.
int install_flags_;
// Gives access to common methods and data of an extension installer.
ExtensionInstaller installer_;
// Performs requirements, policy and blacklist checks on the extension.
ExtensionInstallChecker install_checker_;
DISALLOW_COPY_AND_ASSIGN(CrxInstaller);
};
......
......@@ -4,6 +4,7 @@
#include "base/at_exit.h"
#include "base/memory/ref_counted.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/download/download_crx_util.h"
#include "chrome/browser/extensions/browser_action_test_util.h"
#include "chrome/browser/extensions/crx_installer.h"
......@@ -24,6 +25,7 @@
#include "content/public/test/download_test_observer.h"
#include "extensions/browser/extension_prefs.h"
#include "extensions/browser/extension_system.h"
#include "extensions/browser/management_policy.h"
#include "extensions/common/extension.h"
#include "extensions/common/feature_switch.h"
#include "extensions/common/file_util.h"
......@@ -137,6 +139,21 @@ scoped_refptr<MockPromptProxy> CreateMockPromptProxyForBrowser(
browser->tab_strip_model()->GetActiveWebContents());
}
class ManagementPolicyMock : public extensions::ManagementPolicy::Provider {
public:
ManagementPolicyMock() {}
virtual std::string GetDebugPolicyProviderName() const OVERRIDE {
return "ManagementPolicyMock";
}
virtual bool UserMayLoad(const Extension* extension,
base::string16* error) const OVERRIDE {
*error = base::UTF8ToUTF16("Dummy error message");
return false;
}
};
} // namespace
class ExtensionCrxInstallerTest : public ExtensionBrowserTest {
......@@ -550,4 +567,14 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrxInstallerTest, DoNotSync) {
browser()->profile()));
}
IN_PROC_BROWSER_TEST_F(ExtensionCrxInstallerTest, ManagementPolicy) {
ManagementPolicyMock policy;
extensions::ExtensionSystem::Get(profile())
->management_policy()
->RegisterProvider(&policy);
base::FilePath crx_path = test_data_dir_.AppendASCII("crx_installer/v1.crx");
EXPECT_FALSE(InstallExtension(crx_path, 0));
}
} // namespace extensions
......@@ -163,8 +163,12 @@ void ExtensionInstallChecker::MaybeInvokeCallback() {
running_checks_ = 0;
++current_sequence_number_;
callback_.Run(failed_mask);
Callback callback_copy = callback_;
callback_.Reset();
// This instance may be owned by the callback recipient and deleted here,
// so reset |callback_| first and invoke a copy of the callback.
callback_copy.Run(failed_mask);
}
}
......
// Copyright (c) 2013 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/extensions/extension_installer.h"
#include "base/bind.h"
#include "chrome/browser/profiles/profile.h"
#include "content/public/browser/browser_thread.h"
#include "extensions/browser/extension_system.h"
#include "extensions/browser/management_policy.h"
namespace extensions {
ExtensionInstaller::ExtensionInstaller(Profile* profile)
: requirements_checker_(new RequirementsChecker()),
profile_(profile),
weak_ptr_factory_(this) {
}
ExtensionInstaller::~ExtensionInstaller() {
}
void ExtensionInstaller::CheckRequirements(
const RequirementsCallback& callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
requirements_checker_->Check(extension_, callback);
}
base::string16 ExtensionInstaller::CheckManagementPolicy() {
base::string16 error;
bool allowed = ExtensionSystem::Get(profile_)->management_policy()
->UserMayLoad(extension_.get(), &error);
DCHECK(allowed || !error.empty());
return error;
}
} // namespace extensions
// Copyright 2013 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_INSTALLER_H_
#define CHROME_BROWSER_EXTENSIONS_EXTENSION_INSTALLER_H_
#include <string>
#include <vector>
#include "base/memory/weak_ptr.h"
#include "base/strings/string16.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/requirements_checker.h"
#include "extensions/common/extension.h"
class Profile;
namespace content {
class WebContents;
}
namespace extensions {
// Holds common methods and data of extension installers.
//
// NOTE: This class is deprecated and has been replaced by
// ExtensionInstallChecker.
// TODO(tmdiep): CrxInstaller and UnpackedInstaller should be refactored to use
// this class (crbug.com/386404).
class ExtensionInstaller {
public:
typedef base::Callback<void(std::vector<std::string>)> RequirementsCallback;
explicit ExtensionInstaller(Profile* profile);
~ExtensionInstaller();
// Called on the UI thread to start the requirements check on the extension
void CheckRequirements(const RequirementsCallback& callback);
// Checks the management policy if the extension can be installed.
base::string16 CheckManagementPolicy();
Profile* profile() const {
return profile_;
}
void set_extension(const Extension* extension) {
extension_ = extension;
}
scoped_refptr<const Extension> extension() {
return extension_;
}
private:
scoped_ptr<RequirementsChecker> requirements_checker_;
// The Profile where the extension is being installed in.
Profile* profile_;
// The extension we're installing. We either pass it off to ExtensionService
// on success, or drop the reference on failure.
scoped_refptr<const Extension> extension_;
base::WeakPtrFactory<ExtensionInstaller> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(ExtensionInstaller);
};
} // namespace extensions
#endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_INSTALLER_H_
......@@ -9,7 +9,6 @@
#include "base/command_line.h"
#include "base/file_util.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "base/threading/thread_restrictions.h"
#include "chrome/browser/extensions/extension_error_reporter.h"
#include "chrome/browser/extensions/extension_install_prompt.h"
......@@ -115,7 +114,7 @@ UnpackedInstaller::UnpackedInstaller(ExtensionService* extension_service)
prompt_for_plugins_(true),
require_modern_manifest_version_(true),
be_noisy_on_failure_(true),
installer_(extension_service->profile()) {
install_checker_(extension_service->profile()) {
CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
}
......@@ -152,22 +151,20 @@ bool UnpackedInstaller::LoadFromCommandLine(const base::FilePath& path_in,
}
std::string error;
installer_.set_extension(
install_checker_.set_extension(
file_util::LoadExtension(
extension_path_, Manifest::COMMAND_LINE, GetFlags(), &error).get());
if (!installer_.extension().get() ||
if (!extension() ||
!extension_l10n_util::ValidateExtensionLocales(
extension_path_,
installer_.extension()->manifest()->value(),
&error)) {
extension_path_, extension()->manifest()->value(), &error)) {
ReportExtensionLoadError(error);
return false;
}
ShowInstallPrompt();
*extension_id = installer_.extension()->id();
*extension_id = extension()->id();
return true;
}
......@@ -179,33 +176,41 @@ void UnpackedInstaller::ShowInstallPrompt() {
const ExtensionSet& disabled_extensions =
ExtensionRegistry::Get(service_weak_->profile())->disabled_extensions();
if (service_weak_->show_extensions_prompts() && prompt_for_plugins_ &&
PluginInfo::HasPlugins(installer_.extension().get()) &&
!disabled_extensions.Contains(installer_.extension()->id())) {
PluginInfo::HasPlugins(extension()) &&
!disabled_extensions.Contains(extension()->id())) {
SimpleExtensionLoadPrompt* prompt = new SimpleExtensionLoadPrompt(
installer_.extension().get(),
installer_.profile(),
base::Bind(&UnpackedInstaller::CallCheckRequirements, this));
extension(),
install_checker_.profile(),
base::Bind(&UnpackedInstaller::StartInstallChecks, this));
prompt->ShowPrompt();
return;
}
CallCheckRequirements();
StartInstallChecks();
}
void UnpackedInstaller::CallCheckRequirements() {
installer_.CheckRequirements(
base::Bind(&UnpackedInstaller::OnRequirementsChecked, this));
void UnpackedInstaller::StartInstallChecks() {
install_checker_.Start(
ExtensionInstallChecker::CHECK_REQUIREMENTS |
ExtensionInstallChecker::CHECK_MANAGEMENT_POLICY,
true /* fail fast */,
base::Bind(&UnpackedInstaller::OnInstallChecksComplete, this));
}
void UnpackedInstaller::OnRequirementsChecked(
std::vector<std::string> requirement_errors) {
void UnpackedInstaller::OnInstallChecksComplete(int failed_checks) {
CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (!requirement_errors.empty()) {
ReportExtensionLoadError(JoinString(requirement_errors, ' '));
if (!install_checker_.policy_error().empty()) {
ReportExtensionLoadError(install_checker_.policy_error());
return;
}
ConfirmInstall();
if (!install_checker_.requirement_errors().empty()) {
ReportExtensionLoadError(
JoinString(install_checker_.requirement_errors(), ' '));
return;
}
InstallExtension();
}
int UnpackedInstaller::GetFlags() {
......@@ -272,15 +277,13 @@ void UnpackedInstaller::LoadWithFileAccess(int flags) {
CHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
std::string error;
installer_.set_extension(
install_checker_.set_extension(
file_util::LoadExtension(
extension_path_, Manifest::UNPACKED, flags, &error).get());
if (!installer_.extension().get() ||
if (!extension() ||
!extension_l10n_util::ValidateExtensionLocales(
extension_path_,
installer_.extension()->manifest()->value(),
&error)) {
extension_path_, extension()->manifest()->value(), &error)) {
BrowserThread::PostTask(
BrowserThread::UI,
FROM_HERE,
......@@ -308,20 +311,14 @@ void UnpackedInstaller::ReportExtensionLoadError(const std::string &error) {
}
}
void UnpackedInstaller::ConfirmInstall() {
void UnpackedInstaller::InstallExtension() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::string16 error = installer_.CheckManagementPolicy();
if (!error.empty()) {
ReportExtensionLoadError(base::UTF16ToUTF8(error));
return;
}
PermissionsUpdater perms_updater(service_weak_->profile());
perms_updater.GrantActivePermissions(installer_.extension().get());
perms_updater.GrantActivePermissions(extension());
service_weak_->OnExtensionInstalled(installer_.extension().get(),
syncer::StringOrdinal(),
kInstallFlagInstallImmediately);
service_weak_->OnExtensionInstalled(
extension(), syncer::StringOrdinal(), kInstallFlagInstallImmediately);
}
} // namespace extensions
......@@ -13,14 +13,13 @@
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/extensions/extension_installer.h"
#include "chrome/browser/extensions/extension_install_checker.h"
class ExtensionService;
namespace extensions {
class Extension;
class RequirementsChecker;
// Installs and loads an unpacked extension. Because internal state needs to be
// held about the instalation process, only one call to Load*() should be made
......@@ -82,11 +81,11 @@ class UnpackedInstaller
// Must be called from the UI thread.
void ShowInstallPrompt();
// Calls CheckRequirements.
void CallCheckRequirements();
// Begin management policy and requirements checks.
void StartInstallChecks();
// Callback from RequirementsChecker.
void OnRequirementsChecked(std::vector<std::string> requirement_errors);
// Callback from ExtensionInstallChecker.
void OnInstallChecksComplete(int failed_checks);
// Verifies if loading unpacked extensions is allowed.
bool IsLoadingUnpackedAllowed() const;
......@@ -108,12 +107,14 @@ class UnpackedInstaller
// Notify the frontend that there was an error loading an extension.
void ReportExtensionLoadError(const std::string& error);
// Called when an unpacked extension has been loaded and installed.
void ConfirmInstall();
// Passes the extension onto extension service.
void InstallExtension();
// Helper to get the Extension::CreateFlags for the installing extension.
int GetFlags();
const Extension* extension() { return install_checker_.extension().get(); }
// The service we will report results back to.
base::WeakPtr<ExtensionService> service_weak_;
......@@ -135,8 +136,9 @@ class UnpackedInstaller
// Whether or not to be noisy (show a dialog) on failure. Defaults to true.
bool be_noisy_on_failure_;
// Gives access to common methods and data of an extension installer.
ExtensionInstaller installer_;
// Checks management policies and requirements before the extension can be
// installed.
ExtensionInstallChecker install_checker_;
DISALLOW_COPY_AND_ASSIGN(UnpackedInstaller);
};
......
......@@ -7,6 +7,7 @@
#include "base/prefs/scoped_user_pref_update.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_service_test_base.h"
#include "chrome/browser/extensions/unpacked_installer.h"
#include "chrome/browser/profiles/profile.h"
......
......@@ -168,8 +168,6 @@
'browser/extensions/extension_install_prompt.h',
'browser/extensions/extension_install_prompt_experiment.cc',
'browser/extensions/extension_install_prompt_experiment.h',
'browser/extensions/extension_installer.cc',
'browser/extensions/extension_installer.h',
'browser/extensions/extension_keybinding_registry.cc',
'browser/extensions/extension_keybinding_registry.h',
'browser/extensions/extension_message_bubble_controller.cc',
......
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