Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(5493)

Unified Diff: chrome/browser/extensions/extension_install_checker.cc

Issue 2751013002: Simplify ExtensionInstallChecker into a single-use class (Closed)
Patch Set: todo Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/extensions/extension_install_checker.cc
diff --git a/chrome/browser/extensions/extension_install_checker.cc b/chrome/browser/extensions/extension_install_checker.cc
index 313225df59060d09c967dd4cf239bc78eb43db8f..d027389d6e26b672969e8e8c84a9c644216ccfc1 100644
--- a/chrome/browser/extensions/extension_install_checker.cc
+++ b/chrome/browser/extensions/extension_install_checker.cc
@@ -4,6 +4,7 @@
#include "chrome/browser/extensions/extension_install_checker.h"
+#include "base/callback_helpers.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/extensions/blacklist.h"
#include "chrome/browser/extensions/chrome_requirements_checker.h"
@@ -14,55 +15,56 @@
namespace extensions {
-ExtensionInstallChecker::ExtensionInstallChecker(Profile* profile)
+ExtensionInstallChecker::ExtensionInstallChecker(
+ Profile* profile,
+ scoped_refptr<const Extension> extension,
+ int enabled_checks,
+ bool fail_fast)
: profile_(profile),
+ extension_(extension),
blacklist_state_(NOT_BLACKLISTED),
policy_allows_load_(true),
- current_sequence_number_(0),
+ enabled_checks_(enabled_checks),
running_checks_(0),
- fail_fast_(false),
- weak_ptr_factory_(this) {
-}
+ fail_fast_(fail_fast),
+ weak_ptr_factory_(this) {}
ExtensionInstallChecker::~ExtensionInstallChecker() {
}
-void ExtensionInstallChecker::Start(int enabled_checks,
- bool fail_fast,
- const Callback& callback) {
+void ExtensionInstallChecker::Start(const Callback& callback) {
// Profile is null in tests.
if (profile_) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+ // TODO(michaelpg): change NOTREACHED to [D]CHECK here and below.
if (!extension_.get()) {
NOTREACHED();
return;
}
}
- if (is_running() || !enabled_checks || callback.is_null()) {
+ if (is_running() || !enabled_checks_ || callback.is_null()) {
NOTREACHED();
return;
}
- running_checks_ = enabled_checks;
- fail_fast_ = fail_fast;
+ running_checks_ = enabled_checks_;
callback_ = callback;
- ResetResults();
// Execute the management policy check first as it is synchronous.
- if (enabled_checks & CHECK_MANAGEMENT_POLICY) {
+ if (enabled_checks_ & CHECK_MANAGEMENT_POLICY) {
CheckManagementPolicy();
if (!is_running())
return;
}
- if (enabled_checks & CHECK_REQUIREMENTS) {
+ if (enabled_checks_ & CHECK_REQUIREMENTS) {
CheckRequirements();
if (!is_running())
return;
}
- if (enabled_checks & CHECK_BLACKLIST)
+ if (enabled_checks_ & CHECK_BLACKLIST)
CheckBlacklistState();
}
@@ -89,21 +91,15 @@ void ExtensionInstallChecker::OnManagementPolicyCheckDone(
void ExtensionInstallChecker::CheckRequirements() {
DCHECK(extension_.get());
- if (!requirements_checker_.get())
- requirements_checker_.reset(new ChromeRequirementsChecker());
+ requirements_checker_ = base::MakeUnique<ChromeRequirementsChecker>();
requirements_checker_->Check(
- extension_,
- base::Bind(&ExtensionInstallChecker::OnRequirementsCheckDone,
- weak_ptr_factory_.GetWeakPtr(),
- current_sequence_number_));
+ extension_, base::Bind(&ExtensionInstallChecker::OnRequirementsCheckDone,
+ weak_ptr_factory_.GetWeakPtr()));
}
void ExtensionInstallChecker::OnRequirementsCheckDone(
- int sequence_number,
const std::vector<std::string>& errors) {
- // Some pending results may arrive after fail fast.
- if (sequence_number != current_sequence_number_)
- return;
+ DCHECK(is_running());
requirement_errors_ = errors;
@@ -118,15 +114,11 @@ void ExtensionInstallChecker::CheckBlacklistState() {
blacklist->IsBlacklisted(
extension_->id(),
base::Bind(&ExtensionInstallChecker::OnBlacklistStateCheckDone,
- weak_ptr_factory_.GetWeakPtr(),
- current_sequence_number_));
+ weak_ptr_factory_.GetWeakPtr()));
}
-void ExtensionInstallChecker::OnBlacklistStateCheckDone(int sequence_number,
- BlacklistState state) {
- // Some pending results may arrive after fail fast.
- if (sequence_number != current_sequence_number_)
- return;
+void ExtensionInstallChecker::OnBlacklistStateCheckDone(BlacklistState state) {
+ DCHECK(is_running());
blacklist_state_ = state;
@@ -134,13 +126,6 @@ void ExtensionInstallChecker::OnBlacklistStateCheckDone(int sequence_number,
MaybeInvokeCallback();
}
-void ExtensionInstallChecker::ResetResults() {
- requirement_errors_.clear();
- blacklist_state_ = NOT_BLACKLISTED;
- policy_allows_load_ = true;
- policy_error_.clear();
-}
-
void ExtensionInstallChecker::MaybeInvokeCallback() {
if (callback_.is_null())
return;
@@ -160,14 +145,7 @@ void ExtensionInstallChecker::MaybeInvokeCallback() {
// If we are failing fast, discard any pending results.
weak_ptr_factory_.InvalidateWeakPtrs();
running_checks_ = 0;
- ++current_sequence_number_;
-
- 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);
+ base::ResetAndReturn(&callback_).Run(failed_mask);
}
}
« no previous file with comments | « chrome/browser/extensions/extension_install_checker.h ('k') | chrome/browser/extensions/extension_install_checker_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698