Reland 171079, as it wasn't the CL that caused the perf regression
> Revert 171079 - investigating perf regression.
>
> > Make Blacklist::IsBlacklist asynchronous (it will need to be for safe
> > browsing), and unravel the knots that result from it:
> > - Decouple it from the admin policy.
> > - Take the other half of blacklist logic out of ExtensionPrefs and into
> > Blacklist.
> > - Fix bug where blacklisted extensions couldn't be re-installed (let alone
> > re-enabled) if they get taken off the blacklist.
> >
> >
> > [email protected]
> > BUG=154149,156750
> >
> >
> > Review URL: https://chromiumcodereview.appspot.com/11415216
>
> [email protected]
>
> Review URL: https://codereview.chromium.org/11478003
[email protected]
Review URL: https://codereview.chromium.org/11465014
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@171642 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/background/background_contents_service.cc b/chrome/browser/background/background_contents_service.cc
index d08e84fd..bb25f9b 100644
--- a/chrome/browser/background/background_contents_service.cc
+++ b/chrome/browser/background/background_contents_service.cc
@@ -325,7 +325,8 @@
switch (content::Details<UnloadedExtensionInfo>(details)->reason) {
case extension_misc::UNLOAD_REASON_DISABLE: // Fall through.
case extension_misc::UNLOAD_REASON_TERMINATE: // Fall through.
- case extension_misc::UNLOAD_REASON_UNINSTALL:
+ case extension_misc::UNLOAD_REASON_UNINSTALL: // Fall through.
+ case extension_misc::UNLOAD_REASON_BLACKLIST:
ShutdownAssociatedBackgroundContents(
ASCIIToUTF16(content::Details<UnloadedExtensionInfo>(details)->
extension->id()));
diff --git a/chrome/browser/extensions/admin_policy.cc b/chrome/browser/extensions/admin_policy.cc
index c25e31b..ea69dab 100644
--- a/chrome/browser/extensions/admin_policy.cc
+++ b/chrome/browser/extensions/admin_policy.cc
@@ -41,8 +41,7 @@
return blacklist && blacklist->Find(wildcard) != blacklist->end();
}
-bool UserMayLoad(bool is_google_blacklisted,
- const base::ListValue* blacklist,
+bool UserMayLoad(const base::ListValue* blacklist,
const base::ListValue* whitelist,
const base::ListValue* forcelist,
const Extension* extension,
@@ -50,19 +49,17 @@
if (IsRequired(extension))
return true;
- if ((!blacklist || blacklist->empty()) && !is_google_blacklisted)
+ if (!blacklist || blacklist->empty())
return true;
- // Check the whitelist/forcelist first (takes precedence over Google
- // blacklist).
+ // Check the whitelist/forcelist first.
base::StringValue id_value(extension->id());
if ((whitelist && whitelist->Find(id_value) != whitelist->end()) ||
(forcelist && forcelist->Find(id_value) != forcelist->end()))
return true;
- // Then check both admin and Google blacklists.
- bool result = !is_google_blacklisted &&
- (!blacklist || blacklist->Find(id_value) == blacklist->end()) &&
+ // Then check the admin blacklist.
+ bool result = (!blacklist || blacklist->Find(id_value) == blacklist->end()) &&
!BlacklistedByDefault(blacklist);
if (error && !result) {
*error = l10n_util::GetStringFUTF16(
diff --git a/chrome/browser/extensions/admin_policy.h b/chrome/browser/extensions/admin_policy.h
index 5d69d07..61cd987 100644
--- a/chrome/browser/extensions/admin_policy.h
+++ b/chrome/browser/extensions/admin_policy.h
@@ -22,10 +22,8 @@
// from the command line, or when loaded as an unpacked extension).
bool BlacklistedByDefault(const base::ListValue* blacklist);
-// Returns true if the extension is allowed by Google blacklist and admin policy
-// white-, black- and forcelists.
-bool UserMayLoad(bool is_google_blacklisted,
- const base::ListValue* blacklist,
+// Returns true if the extension is allowed by the admin policy.
+bool UserMayLoad(const base::ListValue* blacklist,
const base::ListValue* whitelist,
const base::ListValue* forcelist,
const Extension* extension,
diff --git a/chrome/browser/extensions/admin_policy_unittest.cc b/chrome/browser/extensions/admin_policy_unittest.cc
index 875312f..59de6f2 100644
--- a/chrome/browser/extensions/admin_policy_unittest.cc
+++ b/chrome/browser/extensions/admin_policy_unittest.cc
@@ -50,32 +50,32 @@
// Tests UserMayLoad for required extensions.
TEST_F(ExtensionAdminPolicyTest, UserMayLoadRequired) {
CreateExtension(Extension::EXTERNAL_POLICY_DOWNLOAD, true);
- EXPECT_TRUE(ap::UserMayLoad(false, NULL, NULL, NULL, extension_.get(), NULL));
+ EXPECT_TRUE(ap::UserMayLoad(NULL, NULL, NULL, extension_.get(), NULL));
string16 error;
- EXPECT_TRUE(ap::UserMayLoad(false, NULL, NULL, NULL, extension_.get(),
+ EXPECT_TRUE(ap::UserMayLoad(NULL, NULL, NULL, extension_.get(),
&error));
EXPECT_TRUE(error.empty());
// Required extensions may load even if they're on the blacklist.
base::ListValue blacklist;
blacklist.Append(Value::CreateStringValue(extension_->id()));
- EXPECT_TRUE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(),
+ EXPECT_TRUE(ap::UserMayLoad(&blacklist, NULL, NULL, extension_.get(),
NULL));
blacklist.Append(Value::CreateStringValue("*"));
- EXPECT_TRUE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(),
+ EXPECT_TRUE(ap::UserMayLoad(&blacklist, NULL, NULL, extension_.get(),
NULL));
}
// Tests UserMayLoad when no blacklist exists, or it's empty.
TEST_F(ExtensionAdminPolicyTest, UserMayLoadNoBlacklist) {
CreateExtension(Extension::INTERNAL, false);
- EXPECT_TRUE(ap::UserMayLoad(false, NULL, NULL, NULL, extension_.get(), NULL));
+ EXPECT_TRUE(ap::UserMayLoad(NULL, NULL, NULL, extension_.get(), NULL));
base::ListValue blacklist;
- EXPECT_TRUE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(),
+ EXPECT_TRUE(ap::UserMayLoad(&blacklist, NULL, NULL, extension_.get(),
NULL));
string16 error;
- EXPECT_TRUE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(),
+ EXPECT_TRUE(ap::UserMayLoad(&blacklist, NULL, NULL, extension_.get(),
&error));
EXPECT_TRUE(error.empty());
}
@@ -86,15 +86,15 @@
base::ListValue whitelist;
whitelist.Append(Value::CreateStringValue(extension_->id()));
- EXPECT_TRUE(ap::UserMayLoad(false, NULL, &whitelist, NULL, extension_.get(),
+ EXPECT_TRUE(ap::UserMayLoad(NULL, &whitelist, NULL, extension_.get(),
NULL));
base::ListValue blacklist;
blacklist.Append(Value::CreateStringValue(extension_->id()));
- EXPECT_TRUE(ap::UserMayLoad(false, NULL, &whitelist, NULL, extension_.get(),
+ EXPECT_TRUE(ap::UserMayLoad(NULL, &whitelist, NULL, extension_.get(),
NULL));
string16 error;
- EXPECT_TRUE(ap::UserMayLoad(false, NULL, &whitelist, NULL, extension_.get(),
+ EXPECT_TRUE(ap::UserMayLoad(NULL, &whitelist, NULL, extension_.get(),
&error));
EXPECT_TRUE(error.empty());
}
@@ -106,30 +106,30 @@
// Blacklisted by default.
base::ListValue blacklist;
blacklist.Append(Value::CreateStringValue("*"));
- EXPECT_FALSE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(),
+ EXPECT_FALSE(ap::UserMayLoad(&blacklist, NULL, NULL, extension_.get(),
NULL));
string16 error;
- EXPECT_FALSE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(),
+ EXPECT_FALSE(ap::UserMayLoad(&blacklist, NULL, NULL, extension_.get(),
&error));
EXPECT_FALSE(error.empty());
// Extension on the blacklist, with and without wildcard.
blacklist.Append(Value::CreateStringValue(extension_->id()));
- EXPECT_FALSE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(),
+ EXPECT_FALSE(ap::UserMayLoad(&blacklist, NULL, NULL, extension_.get(),
NULL));
blacklist.Clear();
blacklist.Append(Value::CreateStringValue(extension_->id()));
- EXPECT_FALSE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(),
+ EXPECT_FALSE(ap::UserMayLoad(&blacklist, NULL, NULL, extension_.get(),
NULL));
// With a whitelist. There's no such thing as a whitelist wildcard.
base::ListValue whitelist;
whitelist.Append(
Value::CreateStringValue("behllobkkfkfnphdnhnkndlbkcpglgmj"));
- EXPECT_FALSE(ap::UserMayLoad(false, &blacklist, &whitelist, NULL,
+ EXPECT_FALSE(ap::UserMayLoad(&blacklist, &whitelist, NULL,
extension_.get(), NULL));
whitelist.Append(Value::CreateStringValue("*"));
- EXPECT_FALSE(ap::UserMayLoad(false, &blacklist, &whitelist, NULL,
+ EXPECT_FALSE(ap::UserMayLoad(&blacklist, &whitelist, NULL,
extension_.get(), NULL));
}
diff --git a/chrome/browser/extensions/blacklist.cc b/chrome/browser/extensions/blacklist.cc
index 834d2621..3d745a63 100644
--- a/chrome/browser/extensions/blacklist.cc
+++ b/chrome/browser/extensions/blacklist.cc
@@ -4,6 +4,10 @@
#include "chrome/browser/extensions/blacklist.h"
+#include <algorithm>
+
+#include "base/bind.h"
+#include "base/message_loop.h"
#include "chrome/browser/extensions/extension_prefs.h"
#include "chrome/browser/prefs/pref_service.h"
#include "chrome/common/pref_names.h"
@@ -24,12 +28,18 @@
Blacklist::~Blacklist() {
}
-bool Blacklist::IsBlacklisted(const Extension* extension) const {
- return prefs_->IsExtensionBlacklisted(extension->id());
-}
-
-bool Blacklist::IsBlacklisted(const std::string& extension_id) const {
- return prefs_->IsExtensionBlacklisted(extension_id);
+void Blacklist::GetBlacklistedIDs(const std::set<std::string>& ids,
+ const GetBlacklistedIDsCallback& callback) {
+ // TODO(kalman): Get the blacklisted IDs from the safebrowsing list.
+ // This will require going to the IO thread and back.
+ std::set<std::string> blacklisted_ids;
+ for (std::set<std::string>::const_iterator it = ids.begin();
+ it != ids.end(); ++it) {
+ if (prefs_->IsExtensionBlacklisted(*it))
+ blacklisted_ids.insert(*it);
+ }
+ MessageLoop::current()->PostTask(FROM_HERE,
+ base::Bind(callback, blacklisted_ids));
}
void Blacklist::SetFromUpdater(const std::vector<std::string>& ids,
@@ -43,7 +53,28 @@
LOG(WARNING) << "Got invalid extension ID \"" << *it << "\"";
}
- prefs_->UpdateBlacklist(ids_as_set);
+ std::set<std::string> from_prefs = prefs_->GetBlacklistedExtensions();
+
+ std::set<std::string> no_longer_blacklisted;
+ std::set_difference(from_prefs.begin(), from_prefs.end(),
+ ids_as_set.begin(), ids_as_set.end(),
+ std::inserter(no_longer_blacklisted,
+ no_longer_blacklisted.begin()));
+ std::set<std::string> not_yet_blacklisted;
+ std::set_difference(ids_as_set.begin(), ids_as_set.end(),
+ from_prefs.begin(), from_prefs.end(),
+ std::inserter(not_yet_blacklisted,
+ not_yet_blacklisted.begin()));
+
+ for (std::set<std::string>::iterator it = no_longer_blacklisted.begin();
+ it != no_longer_blacklisted.end(); ++it) {
+ prefs_->SetExtensionBlacklisted(*it, false);
+ }
+ for (std::set<std::string>::iterator it = not_yet_blacklisted.begin();
+ it != not_yet_blacklisted.end(); ++it) {
+ prefs_->SetExtensionBlacklisted(*it, true);
+ }
+
prefs_->pref_service()->SetString(prefs::kExtensionBlacklistUpdateVersion,
version);
diff --git a/chrome/browser/extensions/blacklist.h b/chrome/browser/extensions/blacklist.h
index 58b12db..89705a5 100644
--- a/chrome/browser/extensions/blacklist.h
+++ b/chrome/browser/extensions/blacklist.h
@@ -5,9 +5,11 @@
#ifndef CHROME_BROWSER_EXTENSIONS_BLACKLIST_H_
#define CHROME_BROWSER_EXTENSIONS_BLACKLIST_H_
+#include <set>
#include <string>
#include <vector>
+#include "base/callback.h"
#include "base/observer_list.h"
namespace extensions {
@@ -32,18 +34,19 @@
Blacklist* blacklist_;
};
+ typedef base::Callback<void(const std::set<std::string>&)>
+ GetBlacklistedIDsCallback;
+
// |prefs_| must outlive this.
explicit Blacklist(ExtensionPrefs* prefs);
~Blacklist();
- // Gets whether an extension is blacklisted.
- //
- // Note that this doesn't entirely determine whether an extension is allowed
- // to be loaded; there are other considerations (e.g. admin settings).
- // See extensions::ManagementPolicy (in particular UserMayLoad).
- bool IsBlacklisted(const std::string& extension_id) const;
- bool IsBlacklisted(const Extension* extension) const;
+ // From the set of extension IDs passed in via |ids|, asynchronously checks
+ // which are blacklisted and includes them in the resulting set passed
+ // via |callback|, which will be sent on the caller's message loop.
+ void GetBlacklistedIDs(const std::set<std::string>& ids,
+ const GetBlacklistedIDsCallback& callback);
// Sets the blacklist from the updater to contain the extension IDs in |ids|
void SetFromUpdater(const std::vector<std::string>& ids,
@@ -58,6 +61,8 @@
ExtensionPrefs* const prefs_;
+ std::set<std::string> prefs_blacklist_;
+
DISALLOW_COPY_AND_ASSIGN(Blacklist);
};
diff --git a/chrome/browser/extensions/blacklist_unittest.cc b/chrome/browser/extensions/blacklist_unittest.cc
new file mode 100644
index 0000000..9572a5c0
--- /dev/null
+++ b/chrome/browser/extensions/blacklist_unittest.cc
@@ -0,0 +1,145 @@
+// Copyright 2012 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 "base/bind.h"
+#include "base/message_loop.h"
+#include "base/run_loop.h"
+#include "chrome/browser/extensions/blacklist.h"
+#include "chrome/browser/extensions/extension_prefs.h"
+#include "chrome/browser/extensions/test_blacklist.h"
+#include "chrome/browser/extensions/test_extension_prefs.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace extensions {
+namespace {
+
+class BlacklistTest : public testing::Test {
+ public:
+ BlacklistTest()
+ : prefs_(message_loop_.message_loop_proxy()),
+ blacklist_(prefs_.prefs()) {
+ }
+
+ bool IsBlacklisted(const Extension* extension) {
+ return TestBlacklist(&blacklist_).IsBlacklisted(extension->id());
+ }
+
+ protected:
+ MessageLoop message_loop_;
+
+ TestExtensionPrefs prefs_;
+
+ Blacklist blacklist_;
+};
+
+TEST_F(BlacklistTest, SetFromUpdater) {
+ scoped_refptr<const Extension> extension_a = prefs_.AddExtension("a");
+ scoped_refptr<const Extension> extension_b = prefs_.AddExtension("b");
+ scoped_refptr<const Extension> extension_c = prefs_.AddExtension("c");
+ scoped_refptr<const Extension> extension_d = prefs_.AddExtension("d");
+
+ // c, d, start blacklisted.
+ prefs_.prefs()->SetExtensionBlacklisted(extension_c->id(), true);
+ prefs_.prefs()->SetExtensionBlacklisted(extension_d->id(), true);
+
+ EXPECT_FALSE(IsBlacklisted(extension_a));
+ EXPECT_FALSE(IsBlacklisted(extension_b));
+ EXPECT_TRUE(IsBlacklisted(extension_c));
+ EXPECT_TRUE(IsBlacklisted(extension_d));
+
+ // Mix up the blacklist.
+ {
+ std::vector<std::string> blacklist;
+ blacklist.push_back(extension_b->id());
+ blacklist.push_back(extension_c->id());
+ blacklist_.SetFromUpdater(blacklist, "1");
+ }
+ EXPECT_FALSE(IsBlacklisted(extension_a));
+ EXPECT_TRUE(IsBlacklisted(extension_b));
+ EXPECT_TRUE(IsBlacklisted(extension_c));
+ EXPECT_FALSE(IsBlacklisted(extension_d));
+
+ // No-op, just in case.
+ {
+ std::vector<std::string> blacklist;
+ blacklist.push_back(extension_b->id());
+ blacklist.push_back(extension_c->id());
+ blacklist_.SetFromUpdater(blacklist, "2");
+ }
+ EXPECT_FALSE(IsBlacklisted(extension_a));
+ EXPECT_TRUE(IsBlacklisted(extension_b));
+ EXPECT_TRUE(IsBlacklisted(extension_c));
+ EXPECT_FALSE(IsBlacklisted(extension_d));
+
+ // Strictly increase the blacklist.
+ {
+ std::vector<std::string> blacklist;
+ blacklist.push_back(extension_a->id());
+ blacklist.push_back(extension_b->id());
+ blacklist.push_back(extension_c->id());
+ blacklist.push_back(extension_d->id());
+ blacklist_.SetFromUpdater(blacklist, "3");
+ }
+ EXPECT_TRUE(IsBlacklisted(extension_a));
+ EXPECT_TRUE(IsBlacklisted(extension_b));
+ EXPECT_TRUE(IsBlacklisted(extension_c));
+ EXPECT_TRUE(IsBlacklisted(extension_d));
+
+ // Strictly decrease the blacklist.
+ {
+ std::vector<std::string> blacklist;
+ blacklist.push_back(extension_a->id());
+ blacklist.push_back(extension_b->id());
+ blacklist_.SetFromUpdater(blacklist, "4");
+ }
+ EXPECT_TRUE(IsBlacklisted(extension_a));
+ EXPECT_TRUE(IsBlacklisted(extension_b));
+ EXPECT_FALSE(IsBlacklisted(extension_c));
+ EXPECT_FALSE(IsBlacklisted(extension_d));
+
+ // Clear the blacklist.
+ {
+ std::vector<std::string> blacklist;
+ blacklist_.SetFromUpdater(blacklist, "5");
+ }
+ EXPECT_FALSE(IsBlacklisted(extension_a));
+ EXPECT_FALSE(IsBlacklisted(extension_b));
+ EXPECT_FALSE(IsBlacklisted(extension_c));
+ EXPECT_FALSE(IsBlacklisted(extension_d));
+}
+
+void Assign(std::set<std::string> *to, const std::set<std::string>& from) {
+ *to = from;
+}
+
+TEST_F(BlacklistTest, OnlyIncludesRequestedIDs) {
+ scoped_refptr<const Extension> extension_a = prefs_.AddExtension("a");
+ scoped_refptr<const Extension> extension_b = prefs_.AddExtension("b");
+ scoped_refptr<const Extension> extension_c = prefs_.AddExtension("c");
+
+ {
+ std::vector<std::string> blacklist;
+ blacklist.push_back(extension_a->id());
+ blacklist.push_back(extension_b->id());
+ blacklist_.SetFromUpdater(blacklist, "1");
+ base::RunLoop().RunUntilIdle();
+ }
+
+ std::set<std::string> blacklist_actual;
+ {
+ std::set<std::string> blacklist_query;
+ blacklist_query.insert(extension_a->id());
+ blacklist_query.insert(extension_c->id());
+ blacklist_.GetBlacklistedIDs(blacklist_query,
+ base::Bind(&Assign, &blacklist_actual));
+ base::RunLoop().RunUntilIdle();
+ }
+
+ std::set<std::string> blacklist_expected;
+ blacklist_expected.insert(extension_a->id());
+ EXPECT_EQ(blacklist_expected, blacklist_actual);
+}
+
+} // namespace extensions
+} // namespace
diff --git a/chrome/browser/extensions/extension_blacklist_browsertest.cc b/chrome/browser/extensions/extension_blacklist_browsertest.cc
new file mode 100644
index 0000000..cc6bc241
--- /dev/null
+++ b/chrome/browser/extensions/extension_blacklist_browsertest.cc
@@ -0,0 +1,389 @@
+// Copyright 2012 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 "base/run_loop.h"
+#include "base/stringprintf.h"
+#include "chrome/browser/extensions/blacklist.h"
+#include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/extension_constants.h"
+#include "content/public/browser/notification_details.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/notification_source.h"
+
+namespace extensions {
+
+namespace {
+
+// Records notifications, but only for extensions with specific IDs.
+class FilteringNotificationObserver : public content::NotificationObserver {
+ public:
+ FilteringNotificationObserver(
+ content::NotificationSource source,
+ const std::set<std::string>& extension_ids)
+ : extension_ids_(extension_ids) {
+ registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_LOADED, source);
+ registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_INSTALLED, source);
+ registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED, source);
+ }
+
+ // Checks then clears notifications for our extensions.
+ testing::AssertionResult CheckNotifications(chrome::NotificationType type) {
+ return CheckNotifications(std::vector<chrome::NotificationType>(1, type));
+ }
+
+ // Checks then clears notifications for our extensions.
+ testing::AssertionResult CheckNotifications(chrome::NotificationType t1,
+ chrome::NotificationType t2) {
+ std::vector<chrome::NotificationType> types;
+ types.push_back(t1);
+ types.push_back(t2);
+ return CheckNotifications(types);
+ }
+
+ // Checks then clears notifications for our extensions.
+ testing::AssertionResult CheckNotifications(chrome::NotificationType t1,
+ chrome::NotificationType t2,
+ chrome::NotificationType t3) {
+ std::vector<chrome::NotificationType> types;
+ types.push_back(t1);
+ types.push_back(t2);
+ types.push_back(t3);
+ return CheckNotifications(types);
+ }
+
+ // Checks then clears notifications for our extensions.
+ testing::AssertionResult CheckNotifications(chrome::NotificationType t1,
+ chrome::NotificationType t2,
+ chrome::NotificationType t3,
+ chrome::NotificationType t4,
+ chrome::NotificationType t5,
+ chrome::NotificationType t6) {
+ std::vector<chrome::NotificationType> types;
+ types.push_back(t1);
+ types.push_back(t2);
+ types.push_back(t3);
+ types.push_back(t4);
+ types.push_back(t5);
+ types.push_back(t6);
+ return CheckNotifications(types);
+ }
+
+ private:
+ // content::NotificationObserver implementation.
+ virtual void Observe(int type,
+ const content::NotificationSource& source,
+ const content::NotificationDetails& details) OVERRIDE {
+ switch (type) {
+ case chrome::NOTIFICATION_EXTENSION_INSTALLED: {
+ const Extension* extension =
+ content::Details<const Extension>(details).ptr();
+ if (extension_ids_.count(extension->id()))
+ notifications_.push_back(static_cast<chrome::NotificationType>(type));
+ break;
+ }
+
+ case chrome::NOTIFICATION_EXTENSION_LOADED: {
+ const Extension* extension =
+ content::Details<const Extension>(details).ptr();
+ if (extension_ids_.count(extension->id()))
+ notifications_.push_back(static_cast<chrome::NotificationType>(type));
+ break;
+ }
+
+ case chrome::NOTIFICATION_EXTENSION_UNLOADED: {
+ UnloadedExtensionInfo* reason =
+ content::Details<UnloadedExtensionInfo>(details).ptr();
+ if (extension_ids_.count(reason->extension->id())) {
+ notifications_.push_back(static_cast<chrome::NotificationType>(type));
+ // The only way that extensions are unloaded in these tests is
+ // by blacklisting.
+ EXPECT_EQ(extension_misc::UNLOAD_REASON_BLACKLIST,
+ reason->reason);
+ }
+ break;
+ }
+
+ default:
+ NOTREACHED();
+ break;
+ }
+ }
+
+ // Checks then clears notifications for our extensions.
+ testing::AssertionResult CheckNotifications(
+ const std::vector<chrome::NotificationType>& types) {
+ testing::AssertionResult result = (notifications_ == types) ?
+ testing::AssertionSuccess() :
+ testing::AssertionFailure() << "Expected " << Str(types) << ", " <<
+ "Got " << Str(notifications_);
+ notifications_.clear();
+ return result;
+ }
+
+ std::string Str(const std::vector<chrome::NotificationType>& types) {
+ std::string str = "[";
+ bool needs_comma = false;
+ for (std::vector<chrome::NotificationType>::const_iterator it =
+ types.begin(); it != types.end(); ++it) {
+ if (needs_comma)
+ str += ",";
+ needs_comma = true;
+ str += base::StringPrintf("%d", *it);
+ }
+ return str + "]";
+ }
+
+ const std::set<std::string> extension_ids_;
+
+ std::vector<chrome::NotificationType> notifications_;
+
+ content::NotificationRegistrar registrar_;
+};
+
+// Stores the paths to CRX files of extensions, and the extension's ID.
+// Use arbitrary extensions; we're just testing blacklisting behavior.
+class CrxInfo {
+ public:
+ CrxInfo(const std::string& path, const std::string& id)
+ : path_(path), id_(id) {}
+
+ const std::string& path() { return path_; }
+ const std::string& id() { return id_; }
+
+ private:
+ const std::string path_;
+ const std::string id_;
+};
+
+} // namespace
+
+class ExtensionBlacklistBrowserTest : public ExtensionBrowserTest {
+ public:
+ ExtensionBlacklistBrowserTest()
+ : info_a_("install/install.crx", "ogdbpbegnmindpdjfafpmpicikegejdj"),
+ info_b_("autoupdate/v1.crx", "ogjcoiohnmldgjemafoockdghcjciccf"),
+ info_c_("hosted_app.crx", "kbmnembihfiondgfjekmnmcbddelicoi") {}
+
+ virtual ~ExtensionBlacklistBrowserTest() {}
+
+ protected:
+ // Returns whether |extension| is strictly safe: in one of ExtensionService's
+ // non-blacklisted extension sets, and not in its blacklisted extensions.
+ testing::AssertionResult IsSafe(const Extension* extension) {
+ std::string id = extension->id();
+ int include_mask = ExtensionService::INCLUDE_EVERYTHING &
+ ~ExtensionService::INCLUDE_BLACKLISTED;
+ if (!extension_service()->GetExtensionById(id, include_mask))
+ return testing::AssertionFailure() << id << " is safe";
+ return IsInValidState(extension);
+ }
+
+ // Returns whether |extension| is strictly blacklisted: in ExtensionService's
+ // blacklist, and not in any of its other extension sets.
+ testing::AssertionResult IsBlacklisted(const Extension* extension) {
+ std::string id = extension->id();
+ if (!extension_service()->blacklisted_extensions()->Contains(id))
+ return testing::AssertionFailure() << id << " is not blacklisted";
+ return IsInValidState(extension);
+ }
+
+ std::set<std::string> GetTestExtensionIDs() {
+ std::set<std::string> extension_ids;
+ extension_ids.insert(info_a_.id());
+ extension_ids.insert(info_b_.id());
+ extension_ids.insert(info_c_.id());
+ return extension_ids;
+ }
+
+ Blacklist* blacklist() {
+ return ExtensionSystem::Get(profile())->blacklist();
+ }
+
+ CrxInfo info_a_;
+ CrxInfo info_b_;
+ CrxInfo info_c_;
+
+ private:
+ // Returns whether |extension| is either installed or blacklisted, but
+ // neither both nor neither.
+ testing::AssertionResult IsInValidState(const Extension* extension) {
+ std::string id = extension->id();
+ bool is_blacklisted =
+ extension_service()->blacklisted_extensions()->Contains(id);
+ int safe_mask = ExtensionService::INCLUDE_EVERYTHING &
+ ~ExtensionService::INCLUDE_BLACKLISTED;
+ bool is_safe = extension_service()->GetExtensionById(id, safe_mask) != NULL;
+ if (is_blacklisted && is_safe) {
+ return testing::AssertionFailure() <<
+ id << " is both safe and in blacklisted_extensions";
+ }
+ if (!is_blacklisted && !is_safe) {
+ return testing::AssertionFailure() <<
+ id << " is neither safe nor in blacklisted_extensions";
+ }
+ return testing::AssertionSuccess();
+ }
+};
+
+// Stage 1: blacklisting when there weren't any extensions installed when the
+// browser started.
+IN_PROC_BROWSER_TEST_F(ExtensionBlacklistBrowserTest, PRE_Blacklist) {
+ FilteringNotificationObserver notifications(
+ content::NotificationService::AllSources(), GetTestExtensionIDs());
+
+ scoped_refptr<const Extension> extension_a =
+ InstallExtension(test_data_dir_.AppendASCII(info_a_.path()), 1);
+ scoped_refptr<const Extension> extension_b =
+ InstallExtension(test_data_dir_.AppendASCII(info_b_.path()), 1);
+ scoped_refptr<const Extension> extension_c =
+ InstallExtension(test_data_dir_.AppendASCII(info_c_.path()), 1);
+
+ EXPECT_TRUE(notifications.CheckNotifications(
+ chrome::NOTIFICATION_EXTENSION_INSTALLED,
+ chrome::NOTIFICATION_EXTENSION_LOADED,
+ chrome::NOTIFICATION_EXTENSION_INSTALLED,
+ chrome::NOTIFICATION_EXTENSION_LOADED,
+ chrome::NOTIFICATION_EXTENSION_INSTALLED,
+ chrome::NOTIFICATION_EXTENSION_LOADED));
+
+ ASSERT_TRUE(extension_a);
+ ASSERT_TRUE(extension_b);
+ ASSERT_EQ(info_a_.id(), extension_a->id());
+ ASSERT_EQ(info_b_.id(), extension_b->id());
+ ASSERT_EQ(info_c_.id(), extension_c->id());
+
+ std::vector<std::string> empty_vector;
+ std::vector<std::string> vector_a(1, info_a_.id());
+ std::vector<std::string> vector_b(1, info_b_.id());
+ std::vector<std::string> vector_c(1, info_c_.id());
+ std::vector<std::string> vector_ab(1, info_a_.id());
+ vector_ab.push_back(info_b_.id());
+ std::vector<std::string> vector_bc(1, info_b_.id());
+ vector_bc.push_back(info_c_.id());
+ std::vector<std::string> vector_abc(1, info_a_.id());
+ vector_abc.push_back(info_b_.id());
+ vector_abc.push_back(info_c_.id());
+
+ EXPECT_TRUE(IsSafe(extension_a));
+ EXPECT_TRUE(IsSafe(extension_b));
+ EXPECT_TRUE(IsSafe(extension_c));
+
+ // Blacklist a and b.
+ blacklist()->SetFromUpdater(vector_ab, "1");
+ base::RunLoop().RunUntilIdle();
+
+ EXPECT_TRUE(IsBlacklisted(extension_a));
+ EXPECT_TRUE(IsBlacklisted(extension_b));
+ EXPECT_TRUE(IsSafe(extension_c));
+ EXPECT_TRUE(notifications.CheckNotifications(
+ chrome::NOTIFICATION_EXTENSION_UNLOADED,
+ chrome::NOTIFICATION_EXTENSION_UNLOADED));
+
+ // Un-blacklist a.
+ blacklist()->SetFromUpdater(vector_b, "2");
+ base::RunLoop().RunUntilIdle();
+
+ EXPECT_TRUE(IsSafe(extension_a));
+ EXPECT_TRUE(IsBlacklisted(extension_b));
+ EXPECT_TRUE(IsSafe(extension_c));
+ EXPECT_TRUE(notifications.CheckNotifications(
+ chrome::NOTIFICATION_EXTENSION_LOADED));
+
+ // Blacklist a then switch with c.
+ blacklist()->SetFromUpdater(vector_ab, "3");
+ base::RunLoop().RunUntilIdle();
+
+ EXPECT_TRUE(IsBlacklisted(extension_a));
+ EXPECT_TRUE(IsBlacklisted(extension_b));
+ EXPECT_TRUE(IsSafe(extension_c));
+ EXPECT_TRUE(notifications.CheckNotifications(
+ chrome::NOTIFICATION_EXTENSION_UNLOADED));
+
+ blacklist()->SetFromUpdater(vector_bc, "4");
+ base::RunLoop().RunUntilIdle();
+
+ EXPECT_TRUE(IsSafe(extension_a));
+ EXPECT_TRUE(IsBlacklisted(extension_b));
+ EXPECT_TRUE(IsBlacklisted(extension_c));
+ EXPECT_TRUE(notifications.CheckNotifications(
+ chrome::NOTIFICATION_EXTENSION_LOADED,
+ chrome::NOTIFICATION_EXTENSION_UNLOADED));
+
+ // Add a to blacklist.
+ blacklist()->SetFromUpdater(vector_abc, "5");
+ base::RunLoop().RunUntilIdle();
+
+ EXPECT_TRUE(IsBlacklisted(extension_a));
+ EXPECT_TRUE(IsBlacklisted(extension_b));
+ EXPECT_TRUE(IsBlacklisted(extension_c));
+ EXPECT_TRUE(notifications.CheckNotifications(
+ chrome::NOTIFICATION_EXTENSION_UNLOADED));
+
+ // Clear blacklist.
+ blacklist()->SetFromUpdater(empty_vector, "6");
+ base::RunLoop().RunUntilIdle();
+
+ EXPECT_TRUE(IsSafe(extension_a));
+ EXPECT_TRUE(IsSafe(extension_b));
+ EXPECT_TRUE(IsSafe(extension_c));
+ EXPECT_TRUE(notifications.CheckNotifications(
+ chrome::NOTIFICATION_EXTENSION_LOADED,
+ chrome::NOTIFICATION_EXTENSION_LOADED,
+ chrome::NOTIFICATION_EXTENSION_LOADED));
+
+ // Add a and b back again for the next test.
+ blacklist()->SetFromUpdater(vector_ab, "7");
+ base::RunLoop().RunUntilIdle();
+
+ EXPECT_TRUE(IsBlacklisted(extension_a));
+ EXPECT_TRUE(IsBlacklisted(extension_b));
+ EXPECT_TRUE(IsSafe(extension_c));
+ EXPECT_TRUE(notifications.CheckNotifications(
+ chrome::NOTIFICATION_EXTENSION_UNLOADED,
+ chrome::NOTIFICATION_EXTENSION_UNLOADED));
+}
+
+// Stage 2: blacklisting with extensions A and B having been installed,
+// with A actually in the blacklist.
+IN_PROC_BROWSER_TEST_F(ExtensionBlacklistBrowserTest, Blacklist) {
+ FilteringNotificationObserver notifications(
+ content::Source<Profile>(profile()), GetTestExtensionIDs());
+
+ scoped_refptr<const Extension> extension_a =
+ extension_service()->blacklisted_extensions()->GetByID(info_a_.id());
+ ASSERT_TRUE(extension_a);
+
+ scoped_refptr<const Extension> extension_b =
+ extension_service()->blacklisted_extensions()->GetByID(info_b_.id());
+ ASSERT_TRUE(extension_b);
+
+ scoped_refptr<const Extension> extension_c =
+ extension_service()->extensions()->GetByID(info_c_.id());
+ ASSERT_TRUE(extension_c);
+
+ EXPECT_TRUE(IsBlacklisted(extension_a));
+ EXPECT_TRUE(IsBlacklisted(extension_b));
+ EXPECT_TRUE(IsSafe(extension_c));
+
+ // Make sure that we can still blacklist c and unblacklist b.
+ std::vector<std::string> vector_ac(1, extension_a->id());
+ vector_ac.push_back(extension_c->id());
+ blacklist()->SetFromUpdater(vector_ac, "8");
+ base::RunLoop().RunUntilIdle();
+
+ EXPECT_TRUE(IsBlacklisted(extension_a));
+ EXPECT_TRUE(IsSafe(extension_b));
+ EXPECT_TRUE(IsBlacklisted(extension_c));
+ EXPECT_TRUE(notifications.CheckNotifications(
+ chrome::NOTIFICATION_EXTENSION_LOADED,
+ chrome::NOTIFICATION_EXTENSION_UNLOADED));
+}
+
+} // namespace extensions
diff --git a/chrome/browser/extensions/extension_browsertest.h b/chrome/browser/extensions/extension_browsertest.h
index b1e82a0..03743b21 100644
--- a/chrome/browser/extensions/extension_browsertest.h
+++ b/chrome/browser/extensions/extension_browsertest.h
@@ -12,6 +12,8 @@
#include "base/file_path.h"
#include "base/files/scoped_temp_dir.h"
#include "chrome/browser/extensions/extension_host.h"
+#include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/ui/browser.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/feature_switch.h"
#include "chrome/common/extensions/features/feature.h"
@@ -21,7 +23,9 @@
#include "content/public/browser/notification_types.h"
#include "content/public/browser/web_contents.h"
+class ExtensionService;
class ExtensionProcessManager;
+class Profile;
// Base class for extension browser tests. Provides utilities for loading,
// unloading, and installing extensions.
@@ -50,6 +54,12 @@
ExtensionBrowserTest();
virtual ~ExtensionBrowserTest();
+ // Useful accessors.
+ Profile* profile() { return browser()->profile(); }
+ ExtensionService* extension_service() {
+ return extensions::ExtensionSystem::Get(profile())->extension_service();
+ }
+
// InProcessBrowserTest
virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE;
diff --git a/chrome/browser/extensions/extension_context_menu_model_unittest.cc b/chrome/browser/extensions/extension_context_menu_model_unittest.cc
index 66456a0a..b431edf1 100644
--- a/chrome/browser/extensions/extension_context_menu_model_unittest.cc
+++ b/chrome/browser/extensions/extension_context_menu_model_unittest.cc
@@ -59,6 +59,9 @@
// Now the actions are disabled.
ASSERT_FALSE(menu->IsCommandIdEnabled(ExtensionContextMenuModel::UNINSTALL));
+
+ // Don't leave |policy_provider| dangling.
+ system->management_policy()->UnregisterAllProviders();
}
} // namespace
diff --git a/chrome/browser/extensions/extension_error_ui.cc b/chrome/browser/extensions/extension_error_ui.cc
index 2ba3b14..2cddf94 100644
--- a/chrome/browser/extensions/extension_error_ui.cc
+++ b/chrome/browser/extensions/extension_error_ui.cc
@@ -49,8 +49,8 @@
for (ExtensionIdSet::const_iterator iter = extensions->begin();
iter != extensions->end(); ++iter) {
- const extensions::Extension* e = extension_service_->GetExtensionById(*iter,
- true);
+ const extensions::Extension* e =
+ extension_service_->GetInstalledExtension(*iter);
message += l10n_util::GetStringFUTF16(
e->is_app() ? app_template_message_id : extension_template_message_id,
UTF8ToUTF16(e->name())) + char16('\n');
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 749f4bcf..79b11e32 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -748,57 +748,48 @@
UpdateExtensionPref(extension_id, kPrefDisableReasons, NULL);
}
-void ExtensionPrefs::UpdateBlacklist(
- const std::set<std::string>& blacklist_set) {
- ExtensionIdList remove_pref_ids;
- std::set<std::string> used_id_set;
+std::set<std::string> ExtensionPrefs::GetBlacklistedExtensions() {
+ std::set<std::string> ids;
+
const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
+ if (!extensions)
+ return ids;
- if (extensions) {
- for (DictionaryValue::key_iterator extension_id = extensions->begin_keys();
- extension_id != extensions->end_keys(); ++extension_id) {
- const DictionaryValue* ext;
- if (!extensions->GetDictionaryWithoutPathExpansion(*extension_id, &ext)) {
- NOTREACHED() << "Invalid pref for extension " << *extension_id;
- continue;
- }
- const std::string& id(*extension_id);
- if (blacklist_set.find(id) == blacklist_set.end()) {
- if (!IsBlacklistBitSet(ext)) {
- // This extension is not in blacklist. And it was not blacklisted
- // before.
- continue;
- } else {
- if (ext->size() == 1) {
- // We should remove the entry if the only flag here is blacklist.
- remove_pref_ids.push_back(id);
- } else {
- // Remove the blacklist bit.
- UpdateExtensionPref(id, kPrefBlacklist, NULL);
- }
- }
- } else {
- if (!IsBlacklistBitSet(ext)) {
- // Only set the blacklist if it was not set.
- UpdateExtensionPref(id, kPrefBlacklist,
- Value::CreateBooleanValue(true));
- }
- // Keep the record if this extension is already processed.
- used_id_set.insert(id);
- }
+ for (DictionaryValue::Iterator it(*extensions); it.HasNext(); it.Advance()) {
+ if (!it.value().IsType(Value::TYPE_DICTIONARY)) {
+ NOTREACHED() << "Invalid pref for extension " << it.key();
+ continue;
}
+ if (IsBlacklistBitSet(static_cast<const DictionaryValue*>(&it.value())))
+ ids.insert(it.key());
}
- // Iterate the leftovers to set blacklist in pref
- std::set<std::string>::const_iterator set_itr = blacklist_set.begin();
- for (; set_itr != blacklist_set.end(); ++set_itr) {
- if (used_id_set.find(*set_itr) == used_id_set.end()) {
- UpdateExtensionPref(*set_itr, kPrefBlacklist,
- Value::CreateBooleanValue(true));
- }
+ return ids;
+}
+
+void ExtensionPrefs::SetExtensionBlacklisted(const std::string& extension_id,
+ bool is_blacklisted) {
+ bool currently_blacklisted = IsExtensionBlacklisted(extension_id);
+ if (is_blacklisted == currently_blacklisted) {
+ NOTREACHED() << extension_id << " is " <<
+ (currently_blacklisted ? "already" : "not") <<
+ " blacklisted";
+ return;
}
- for (size_t i = 0; i < remove_pref_ids.size(); ++i) {
- DeleteExtensionPrefs(remove_pref_ids[i]);
+
+ // Always make sure the "acknowledged" bit is cleared since the blacklist bit
+ // is changing.
+ UpdateExtensionPref(extension_id, kPrefBlacklistAcknowledged, NULL);
+
+ if (is_blacklisted) {
+ UpdateExtensionPref(extension_id,
+ kPrefBlacklist,
+ new base::FundamentalValue(true));
+ } else {
+ UpdateExtensionPref(extension_id, kPrefBlacklist, NULL);
+ const DictionaryValue* dict = GetExtensionPref(extension_id);
+ if (dict && dict->empty())
+ DeleteExtensionPrefs(extension_id);
}
}
@@ -1656,8 +1647,6 @@
if (!extensions ||
!extensions->GetDictionaryWithoutPathExpansion(extension_id, &ext))
return scoped_ptr<ExtensionInfo>();
- if (IsBlacklistBitSet(ext))
- return scoped_ptr<ExtensionInfo>();
int state_value;
if (!ext->GetInteger(kPrefState, &state_value) ||
state_value == Extension::ENABLED_COMPONENT) {
diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h
index 6adb015..1908716 100644
--- a/chrome/browser/extensions/extension_prefs.h
+++ b/chrome/browser/extensions/extension_prefs.h
@@ -175,6 +175,13 @@
Extension::DisableReason disable_reason);
void ClearDisableReasons(const std::string& extension_id);
+ // Gets the set of extensions that have been blacklisted in prefs.
+ std::set<std::string> GetBlacklistedExtensions();
+
+ // Sets whether the extension with |id| is blacklisted.
+ void SetExtensionBlacklisted(const std::string& extension_id,
+ bool is_blacklisted);
+
// Returns the version string for the currently installed extension, or
// the empty string if not found.
std::string GetVersionString(const std::string& extension_id);
@@ -189,11 +196,12 @@
// Returns base extensions install directory.
const FilePath& install_directory() const { return install_directory_; }
- // Updates the prefs based on the blacklist.
- void UpdateBlacklist(const std::set<std::string>& blacklist_set);
-
- // Returns whether the extension with |id| is blacklisted.
- // You probably don't want to call this method, see Blacklist::IsBlacklisted.
+ // Returns whether the extension with |id| has its blacklist bit set.
+ //
+ // WARNING: this only checks the extension's entry in prefs, so by definition
+ // can only check extensions that prefs knows about. There may be other
+ // sources of blacklist information, such as safebrowsing. You probably want
+ // to use Blacklist::GetBlacklistedIDs rather than this method.
bool IsExtensionBlacklisted(const std::string& id) const;
// Based on extension id, checks prefs to see if it is orphaned.
@@ -495,7 +503,8 @@
URLPatternSet GetAllowedInstallSites();
private:
- friend class ExtensionPrefsUninstallExtension; // Unit test.
+ friend class ExtensionPrefsBlacklistedExtensions; // Unit test.
+ friend class ExtensionPrefsUninstallExtension; // Unit test.
// See the Create methods.
ExtensionPrefs(PrefService* prefs,
diff --git a/chrome/browser/extensions/extension_prefs_unittest.cc b/chrome/browser/extensions/extension_prefs_unittest.cc
index 9ddedbd1..49e450c 100644
--- a/chrome/browser/extensions/extension_prefs_unittest.cc
+++ b/chrome/browser/extensions/extension_prefs_unittest.cc
@@ -246,7 +246,6 @@
};
TEST_F(ExtensionPrefsExtensionState, ExtensionState) {}
-
class ExtensionPrefsEscalatePermissions : public ExtensionPrefsTest {
public:
virtual void Initialize() {
@@ -1189,4 +1188,83 @@
};
TEST_F(ExtensionPrefsDisableExtensions, ExtensionPrefsDisableExtensions) {}
+// Tests that blacklist state can be queried.
+class ExtensionPrefsBlacklistedExtensions : public ExtensionPrefsTest {
+ public:
+ virtual ~ExtensionPrefsBlacklistedExtensions() {}
+
+ virtual void Initialize() OVERRIDE {
+ extension_a_ = prefs_.AddExtension("a");
+ extension_b_ = prefs_.AddExtension("b");
+ extension_c_ = prefs_.AddExtension("c");
+ }
+
+ virtual void Verify() OVERRIDE {
+ {
+ std::set<std::string> ids;
+ EXPECT_EQ(ids, prefs()->GetBlacklistedExtensions());
+ }
+ prefs()->SetExtensionBlacklisted(extension_a_->id(), true);
+ {
+ std::set<std::string> ids;
+ ids.insert(extension_a_->id());
+ EXPECT_EQ(ids, prefs()->GetBlacklistedExtensions());
+ }
+ prefs()->SetExtensionBlacklisted(extension_b_->id(), true);
+ prefs()->SetExtensionBlacklisted(extension_c_->id(), true);
+ {
+ std::set<std::string> ids;
+ ids.insert(extension_a_->id());
+ ids.insert(extension_b_->id());
+ ids.insert(extension_c_->id());
+ EXPECT_EQ(ids, prefs()->GetBlacklistedExtensions());
+ }
+ prefs()->SetExtensionBlacklisted(extension_a_->id(), false);
+ {
+ std::set<std::string> ids;
+ ids.insert(extension_b_->id());
+ ids.insert(extension_c_->id());
+ EXPECT_EQ(ids, prefs()->GetBlacklistedExtensions());
+ }
+ prefs()->SetExtensionBlacklisted(extension_b_->id(), false);
+ prefs()->SetExtensionBlacklisted(extension_c_->id(), false);
+ {
+ std::set<std::string> ids;
+ EXPECT_EQ(ids, prefs()->GetBlacklistedExtensions());
+ }
+
+ // The interesting part: make sure that we're cleaning up after ourselves
+ // when we're storing *just* the fact that the extension is blacklisted.
+ std::string arbitrary_id = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+
+ prefs()->SetExtensionBlacklisted(arbitrary_id, true);
+ prefs()->SetExtensionBlacklisted(extension_a_->id(), true);
+
+ // (And make sure that the acknowledged bit is also cleared).
+ prefs()->AcknowledgeBlacklistedExtension(arbitrary_id);
+
+ EXPECT_TRUE(prefs()->GetExtensionPref(arbitrary_id));
+ {
+ std::set<std::string> ids;
+ ids.insert(arbitrary_id);
+ ids.insert(extension_a_->id());
+ EXPECT_EQ(ids, prefs()->GetBlacklistedExtensions());
+ }
+ prefs()->SetExtensionBlacklisted(arbitrary_id, false);
+ prefs()->SetExtensionBlacklisted(extension_a_->id(), false);
+ EXPECT_FALSE(prefs()->GetExtensionPref(arbitrary_id));
+ {
+ std::set<std::string> ids;
+ EXPECT_EQ(ids, prefs()->GetBlacklistedExtensions());
+ }
+ }
+
+ private:
+ scoped_refptr<const Extension> extension_a_;
+ scoped_refptr<const Extension> extension_b_;
+ scoped_refptr<const Extension> extension_c_;
+};
+TEST_F(ExtensionPrefsBlacklistedExtensions,
+ ExtensionPrefsBlacklistedExtensions) {}
+
} // namespace extensions
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 17e97936..6ca0238 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -5,6 +5,7 @@
#include "chrome/browser/extensions/extension_service.h"
#include <algorithm>
+#include <iterator>
#include <set>
#include "base/basictypes.h"
@@ -360,6 +361,7 @@
profile_(profile),
system_(extensions::ExtensionSystem::Get(profile)),
extension_prefs_(extension_prefs),
+ blacklist_(blacklist),
settings_frontend_(extensions::SettingsFrontend::Create(profile)),
pending_extension_manager_(*ALLOW_THIS_IN_INITIALIZER_LIST(this)),
install_directory_(install_directory),
@@ -465,23 +467,29 @@
return &terminated_extensions_;
}
-const ExtensionSet* ExtensionService::GenerateInstalledExtensionsSet() const {
- ExtensionSet* installed_extensions = new ExtensionSet();
+const ExtensionSet* ExtensionService::blacklisted_extensions() const {
+ return &blacklisted_extensions_;
+}
+
+scoped_ptr<const ExtensionSet>
+ ExtensionService::GenerateInstalledExtensionsSet() const {
+ scoped_ptr<ExtensionSet> installed_extensions(new ExtensionSet());
installed_extensions->InsertAll(extensions_);
installed_extensions->InsertAll(disabled_extensions_);
installed_extensions->InsertAll(terminated_extensions_);
- return installed_extensions;
+ installed_extensions->InsertAll(blacklisted_extensions_);
+ return installed_extensions.PassAs<const ExtensionSet>();
}
-const ExtensionSet* ExtensionService::GetWipedOutExtensions() const {
- ExtensionSet* extension_set = new ExtensionSet();
+scoped_ptr<const ExtensionSet> ExtensionService::GetWipedOutExtensions() const {
+ scoped_ptr<ExtensionSet> extension_set(new ExtensionSet());
for (ExtensionSet::const_iterator iter = disabled_extensions_.begin();
iter != disabled_extensions_.end(); ++iter) {
int disabled_reason = extension_prefs_->GetDisableReasons((*iter)->id());
if ((disabled_reason & Extension::DISABLE_SIDELOAD_WIPEOUT) != 0)
extension_set->Insert(*iter);
}
- return extension_set;
+ return extension_set.PassAs<const ExtensionSet>();
}
extensions::PendingExtensionManager*
@@ -573,9 +581,39 @@
const Extension* ExtensionService::GetExtensionById(
const std::string& id, bool include_disabled) const {
int include_mask = INCLUDE_ENABLED;
- if (include_disabled)
- include_mask |= INCLUDE_DISABLED;
- return GetExtensionByIdInternal(id, include_mask);
+ if (include_disabled) {
+ // Include blacklisted extensions here because there are hundreds of
+ // callers of this function, and many might assume that this includes those
+ // that have been disabled due to blacklisting.
+ include_mask |= INCLUDE_DISABLED | INCLUDE_BLACKLISTED;
+ }
+ return GetExtensionById(id, include_mask);
+}
+
+const Extension* ExtensionService::GetExtensionById(
+ const std::string& id, int include_mask) const {
+ std::string lowercase_id = StringToLowerASCII(id);
+ if (include_mask & INCLUDE_ENABLED) {
+ const Extension* extension = extensions_.GetByID(lowercase_id);
+ if (extension)
+ return extension;
+ }
+ if (include_mask & INCLUDE_DISABLED) {
+ const Extension* extension = disabled_extensions_.GetByID(lowercase_id);
+ if (extension)
+ return extension;
+ }
+ if (include_mask & INCLUDE_TERMINATED) {
+ const Extension* extension = terminated_extensions_.GetByID(lowercase_id);
+ if (extension)
+ return extension;
+ }
+ if (include_mask & INCLUDE_BLACKLISTED) {
+ const Extension* extension = blacklisted_extensions_.GetByID(lowercase_id);
+ if (extension)
+ return extension;
+ }
+ return NULL;
}
void ExtensionService::Init() {
@@ -631,9 +669,7 @@
const extensions::PendingExtensionInfo* pending_extension_info =
pending_extension_manager()->GetById(id);
- int include_mask = INCLUDE_ENABLED | INCLUDE_DISABLED;
- const Extension* extension =
- GetExtensionByIdInternal(id, include_mask);
+ const Extension* extension = GetInstalledExtension(id);
if (!pending_extension_info && !extension) {
LOG(WARNING) << "Will not update extension " << id
<< " because it is not installed or pending";
@@ -884,8 +920,10 @@
return true;
}
- if (disabled_extensions_.Contains(extension_id))
+ if (disabled_extensions_.Contains(extension_id) ||
+ blacklisted_extensions_.Contains(extension_id)) {
return false;
+ }
// If the extension hasn't been loaded yet, check the prefs for it. Assume
// enabled unless otherwise noted.
@@ -908,8 +946,7 @@
extension_prefs_->SetExtensionState(extension_id, Extension::ENABLED);
extension_prefs_->ClearDisableReasons(extension_id);
- const Extension* extension = GetExtensionByIdInternal(extension_id,
- INCLUDE_DISABLED);
+ const Extension* extension = disabled_extensions_.GetByID(extension_id);
// This can happen if sync enables an extension that is not
// installed yet.
if (!extension)
@@ -960,8 +997,8 @@
extension_prefs_->SetExtensionState(extension_id, Extension::DISABLED);
extension_prefs_->AddDisableReason(extension_id, disable_reason);
- int include_mask = INCLUDE_ENABLED | INCLUDE_TERMINATED;
- extension = GetExtensionByIdInternal(extension_id, include_mask);
+ int include_mask = INCLUDE_EVERYTHING & ~INCLUDE_DISABLED;
+ extension = GetExtensionById(extension_id, include_mask);
if (!extension)
return;
@@ -1267,6 +1304,7 @@
void ExtensionService::CheckManagementPolicy() {
std::vector<std::string> to_be_removed;
+
// Loop through extensions list, unload installed extensions.
for (ExtensionSet::const_iterator iter = extensions_.begin();
iter != extensions_.end(); ++iter) {
@@ -1830,15 +1868,31 @@
bool ExtensionService::PopulateExtensionErrorUI(
ExtensionErrorUI* extension_error_ui) {
bool needs_alert = false;
+
+ // Extensions that are blacklisted.
+ for (ExtensionSet::const_iterator it = blacklisted_extensions_.begin();
+ it != blacklisted_extensions_.end(); ++it) {
+ std::string id = (*it)->id();
+ if (!extension_prefs_->IsBlacklistedExtensionAcknowledged(id)) {
+ extension_error_ui->AddBlacklistedExtension(id);
+ needs_alert = true;
+ }
+ }
+
for (ExtensionSet::const_iterator iter = extensions_.begin();
iter != extensions_.end(); ++iter) {
const Extension* e = *iter;
+
+ // Extensions disabled by policy. Note: this no longer includes blacklisted
+ // extensions, though we still show the same UI.
if (!system_->management_policy()->UserMayLoad(e, NULL)) {
if (!extension_prefs_->IsBlacklistedExtensionAcknowledged(e->id())) {
extension_error_ui->AddBlacklistedExtension(e->id());
needs_alert = true;
}
}
+
+ // Orphaned extensions.
if (extension_prefs_->IsExtensionOrphaned(e->id())) {
if (!extension_prefs_->IsOrphanedExtensionAcknowledged(e->id())) {
extension_error_ui->AddOrphanedExtension(e->id());
@@ -1846,6 +1900,7 @@
}
}
}
+
return needs_alert;
}
@@ -1925,9 +1980,9 @@
const std::string& extension_id,
extension_misc::UnloadedExtensionReason reason) {
// Make sure the extension gets deleted after we return from this function.
- int include_mask = INCLUDE_ENABLED | INCLUDE_DISABLED;
+ int include_mask = INCLUDE_EVERYTHING & ~INCLUDE_TERMINATED;
scoped_refptr<const Extension> extension(
- GetExtensionByIdInternal(extension_id, include_mask));
+ GetExtensionById(extension_id, include_mask));
// This method can be called via PostTask, so the extension may have been
// unloaded by the time this runs.
@@ -2085,7 +2140,17 @@
// wipeout before, we might disable this extension here.
MaybeWipeout(extension);
- if (extension_prefs_->IsExtensionDisabled(extension->id())) {
+ // Communicated to the Blacklist.
+ std::set<std::string> already_in_blacklist;
+
+ if (extension_prefs_->IsExtensionBlacklisted(extension->id())) {
+ // Don't check the Blacklist yet because it's asynchronous (we do it at
+ // the end). This pre-emptive check is because we will always store the
+ // blacklisted state of *installed* extensions in prefs, and it's important
+ // not to re-enable blacklisted extensions.
+ blacklisted_extensions_.Insert(extension);
+ already_in_blacklist.insert(extension->id());
+ } else if (extension_prefs_->IsExtensionDisabled(extension->id())) {
disabled_extensions_.Insert(extension);
SyncExtensionChangeIfNeeded(*extension);
content::NotificationService::current()->Notify(
@@ -2097,20 +2162,29 @@
Extension::DISABLE_PERMISSIONS_INCREASE) {
extensions::AddExtensionDisabledError(this, extension);
}
- return;
+ } else {
+ // All apps that are displayed in the launcher are ordered by their ordinals
+ // so we must ensure they have valid ordinals.
+ if (extension->RequiresSortOrdinal()) {
+ extension_prefs_->extension_sorting()->EnsureValidOrdinals(
+ extension->id(), syncer::StringOrdinal());
+ }
+
+ extensions_.Insert(extension);
+ SyncExtensionChangeIfNeeded(*extension);
+ NotifyExtensionLoaded(extension);
+ DoPostLoadTasks(extension);
}
- // All apps that are displayed in the launcher are ordered by their ordinals
- // so we must ensure they have valid ordinals.
- if (extension->RequiresSortOrdinal()) {
- extension_prefs_->extension_sorting()->EnsureValidOrdinals(
- extension->id(), syncer::StringOrdinal());
- }
-
- extensions_.Insert(extension);
- SyncExtensionChangeIfNeeded(*extension);
- NotifyExtensionLoaded(extension);
- DoPostLoadTasks(extension);
+ // Lastly, begin the process for checking the blacklist status of extensions.
+ // This may need to go to other threads so is asynchronous.
+ std::set<std::string> id_set;
+ id_set.insert(extension->id());
+ blacklist_->GetBlacklistedIDs(
+ id_set,
+ base::Bind(&ExtensionService::ManageBlacklist,
+ AsWeakPtr(),
+ already_in_blacklist));
}
void ExtensionService::AddComponentExtension(const Extension* extension) {
@@ -2183,9 +2257,7 @@
// still remember that "omnibox" had been granted, so that if the
// extension once again includes "omnibox" in an upgrade, the extension
// can upgrade without requiring this user's approval.
- int include_mask = INCLUDE_ENABLED | INCLUDE_DISABLED;
- const Extension* old =
- GetExtensionByIdInternal(extension->id(), include_mask);
+ const Extension* old = GetInstalledExtension(extension->id());
bool is_extension_upgrade = old != NULL;
bool is_privilege_increase = false;
bool previously_disabled = false;
@@ -2366,8 +2438,7 @@
extension_prefs_->ClearDisableReasons(id);
}
- int include_mask = INCLUDE_ENABLED | INCLUDE_DISABLED;
- if (!GetExtensionByIdInternal(extension->id(), include_mask)) {
+ if (!GetInstalledExtension(extension->id())) {
UMA_HISTOGRAM_ENUMERATION("Extensions.InstallType",
extension->GetType(), 100);
UMA_HISTOGRAM_ENUMERATION("Extensions.InstallSource",
@@ -2479,27 +2550,6 @@
return pending_extension_updates_.GetByID(id);
}
-const Extension* ExtensionService::GetExtensionByIdInternal(
- const std::string& id, int include_mask) const {
- std::string lowercase_id = StringToLowerASCII(id);
- if (include_mask & INCLUDE_ENABLED) {
- const Extension* extension = extensions_.GetByID(lowercase_id);
- if (extension)
- return extension;
- }
- if (include_mask & INCLUDE_DISABLED) {
- const Extension* extension = disabled_extensions_.GetByID(lowercase_id);
- if (extension)
- return extension;
- }
- if (include_mask & INCLUDE_TERMINATED) {
- const Extension* extension = terminated_extensions_.GetByID(lowercase_id);
- if (extension)
- return extension;
- }
- return NULL;
-}
-
void ExtensionService::TrackTerminatedExtension(const Extension* extension) {
if (!terminated_extensions_.Contains(extension->id()))
terminated_extensions_.Insert(make_scoped_refptr(extension));
@@ -2514,13 +2564,16 @@
const Extension* ExtensionService::GetTerminatedExtension(
const std::string& id) const {
- return GetExtensionByIdInternal(id, INCLUDE_TERMINATED);
+ return GetExtensionById(id, INCLUDE_TERMINATED);
}
const Extension* ExtensionService::GetInstalledExtension(
const std::string& id) const {
- int include_mask = INCLUDE_ENABLED | INCLUDE_DISABLED | INCLUDE_TERMINATED;
- return GetExtensionByIdInternal(id, include_mask);
+ int include_mask = INCLUDE_ENABLED |
+ INCLUDE_DISABLED |
+ INCLUDE_TERMINATED |
+ INCLUDE_BLACKLISTED;
+ return GetExtensionById(id, include_mask);
}
bool ExtensionService::ExtensionBindingsAllowed(const GURL& url) {
@@ -3012,9 +3065,7 @@
if (!install_updates_when_idle_ || !wait_for_idle)
return false;
- int include_mask = INCLUDE_ENABLED | INCLUDE_DISABLED;
- const Extension* old =
- GetExtensionByIdInternal(extension_id, include_mask);
+ const Extension* old = GetInstalledExtension(extension_id);
// If there is no old extension, this is not an update, so don't delay.
if (!old)
return false;
@@ -3031,5 +3082,49 @@
}
void ExtensionService::OnBlacklistUpdated() {
- CheckManagementPolicy();
+ blacklist_->GetBlacklistedIDs(
+ GenerateInstalledExtensionsSet()->GetIDs(),
+ base::Bind(&ExtensionService::ManageBlacklist,
+ AsWeakPtr(),
+ blacklisted_extensions_.GetIDs()));
+}
+
+void ExtensionService::ManageBlacklist(
+ const std::set<std::string>& old_blacklisted_ids,
+ const std::set<std::string>& new_blacklisted_ids) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+ std::set<std::string> no_longer_blacklisted;
+ std::set_difference(old_blacklisted_ids.begin(), old_blacklisted_ids.end(),
+ new_blacklisted_ids.begin(), new_blacklisted_ids.end(),
+ std::inserter(no_longer_blacklisted,
+ no_longer_blacklisted.begin()));
+ std::set<std::string> not_yet_blacklisted;
+ std::set_difference(new_blacklisted_ids.begin(), new_blacklisted_ids.end(),
+ old_blacklisted_ids.begin(), old_blacklisted_ids.end(),
+ std::inserter(not_yet_blacklisted,
+ not_yet_blacklisted.begin()));
+
+ for (std::set<std::string>::iterator it = no_longer_blacklisted.begin();
+ it != no_longer_blacklisted.end(); ++it) {
+ scoped_refptr<const Extension> extension =
+ blacklisted_extensions_.GetByID(*it);
+ DCHECK(extension);
+ if (!extension)
+ continue;
+ blacklisted_extensions_.Remove(*it);
+ AddExtension(extension);
+ }
+
+ for (std::set<std::string>::iterator it = not_yet_blacklisted.begin();
+ it != not_yet_blacklisted.end(); ++it) {
+ scoped_refptr<const Extension> extension = GetInstalledExtension(*it);
+ DCHECK(extension);
+ if (!extension)
+ continue;
+ blacklisted_extensions_.Insert(extension);
+ UnloadExtension(*it, extension_misc::UNLOAD_REASON_BLACKLIST);
+ }
+
+ IdentifyAlertableExtensions();
}
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index 4b02b513..6e2d1b7 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -210,14 +210,15 @@
virtual const ExtensionSet* extensions() const OVERRIDE;
virtual const ExtensionSet* disabled_extensions() const OVERRIDE;
const ExtensionSet* terminated_extensions() const;
+ const ExtensionSet* blacklisted_extensions() const;
- // Returns a set of all installed, disabled, and terminated extensions and
- // transfers ownership to caller.
- const ExtensionSet* GenerateInstalledExtensionsSet() const;
+ // Returns a set of all installed, disabled, blacklisted, and terminated
+ // extensions.
+ scoped_ptr<const ExtensionSet> GenerateInstalledExtensionsSet() const;
// Returns a set of all extensions disabled by the sideload wipeout
// initiative.
- const ExtensionSet* GetWipedOutExtensions() const;
+ scoped_ptr<const ExtensionSet> GetWipedOutExtensions() const;
// Gets the object managing the set of pending extensions.
virtual extensions::PendingExtensionManager*
@@ -304,17 +305,34 @@
// Called when the associated Profile is going to be destroyed.
void Shutdown();
- // Look up an extension by ID. Does not include terminated
+ // Look up an extension by ID. Does not include terminated
// extensions.
virtual const extensions::Extension* GetExtensionById(
const std::string& id, bool include_disabled) const OVERRIDE;
+ enum IncludeFlag {
+ INCLUDE_NONE = 0,
+ INCLUDE_ENABLED = 1 << 0,
+ INCLUDE_DISABLED = 1 << 1,
+ INCLUDE_TERMINATED = 1 << 2,
+ INCLUDE_BLACKLISTED = 1 << 3,
+ INCLUDE_EVERYTHING = (1 << 4) - 1,
+ };
+
+ // Look up an extension by ID, selecting which sets to look in:
+ // * extensions() --> INCLUDE_ENABLED
+ // * disabled_extensions() --> INCLUDE_DISABLED
+ // * terminated_extensions() --> INCLUDE_TERMINATED
+ // * blacklisted_extensions() --> INCLUDE_BLACKLISTED
+ const extensions::Extension* GetExtensionById(const std::string& id,
+ int include_mask) const;
+
// Looks up a terminated (crashed) extension by ID.
const extensions::Extension*
GetTerminatedExtension(const std::string& id) const;
// Looks up an extension by ID, regardless of whether it's enabled,
- // disabled, or terminated.
+ // disabled, blacklisted, or terminated.
virtual const extensions::Extension* GetInstalledExtension(
const std::string& id) const OVERRIDE;
@@ -741,13 +759,6 @@
const extensions::ExtensionSyncData& extension_sync_data,
syncer::ModelType type);
- enum IncludeFlag {
- INCLUDE_NONE = 0,
- INCLUDE_ENABLED = 1 << 0,
- INCLUDE_DISABLED = 1 << 1,
- INCLUDE_TERMINATED = 1 << 2
- };
-
// Events to be fired after an extension is reloaded.
enum PostReloadEvents {
EVENT_NONE = 0,
@@ -755,12 +766,6 @@
EVENT_RESTARTED = 1 << 1,
};
- // Look up an extension by ID, optionally including either or both of enabled
- // and disabled extensions.
- const extensions::Extension* GetExtensionByIdInternal(
- const std::string& id,
- int include_mask) const;
-
// Adds the given extension to the list of terminated extensions if
// it is not already there and unloads it.
void TrackTerminatedExtension(const extensions::Extension* extension);
@@ -843,15 +848,23 @@
// extensions::Blacklist::Observer implementation.
virtual void OnBlacklistUpdated() OVERRIDE;
+ // Manages the blacklisted extensions, intended as callback from
+ // Blacklist::GetBlacklistedIDs.
+ void ManageBlacklist(const std::set<std::string>& old_blacklisted_ids,
+ const std::set<std::string>& new_blacklisted_ids);
+
// The normal profile associated with this ExtensionService.
Profile* profile_;
// The ExtensionSystem for the profile above.
extensions::ExtensionSystem* system_;
- // Preferences for the owning profile (weak reference).
+ // Preferences for the owning profile.
extensions::ExtensionPrefs* extension_prefs_;
+ // Blacklist for the owning profile.
+ extensions::Blacklist* blacklist_;
+
// Settings for the owning profile.
scoped_ptr<extensions::SettingsFrontend> settings_frontend_;
@@ -864,6 +877,12 @@
// The list of installed extensions that have been terminated.
ExtensionSet terminated_extensions_;
+ // The list of installed extensions that have been blacklisted. Generally
+ // these shouldn't be considered as installed by the extension platform: we
+ // only keep them around so that if extensions are blacklisted by mistake
+ // they can easily be un-blacklisted.
+ ExtensionSet blacklisted_extensions_;
+
// The list of extension updates that are waiting to be installed.
ExtensionSet pending_extension_updates_;
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index 847c9870..def4e8a 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -626,7 +626,8 @@
enum InstallState {
INSTALL_FAILED,
INSTALL_UPDATED,
- INSTALL_NEW
+ INSTALL_NEW,
+ INSTALL_WITHOUT_LOAD,
};
const Extension* PackAndInstallCRX(const FilePath& dir_path,
@@ -701,14 +702,19 @@
++expected_extensions_count_;
EXPECT_TRUE(installed_) << path.value();
-
- EXPECT_EQ(1u, loaded_.size()) << path.value();
EXPECT_EQ(0u, errors.size()) << path.value();
- EXPECT_EQ(expected_extensions_count_, service_->extensions()->size()) <<
- path.value();
- extension = loaded_[0];
- EXPECT_TRUE(service_->GetExtensionById(extension->id(), false)) <<
- path.value();
+
+ if (install_state == INSTALL_WITHOUT_LOAD) {
+ EXPECT_EQ(0u, loaded_.size()) << path.value();
+ } else {
+ EXPECT_EQ(1u, loaded_.size()) << path.value();
+ EXPECT_EQ(expected_extensions_count_, service_->extensions()->size()) <<
+ path.value();
+ extension = loaded_[0];
+ EXPECT_TRUE(service_->GetExtensionById(extension->id(), false)) <<
+ path.value();
+ }
+
for (std::vector<string16>::iterator err = errors.begin();
err != errors.end(); ++err) {
LOG(ERROR) << *err;
@@ -2998,7 +3004,7 @@
// We can not install good_crx.
FilePath path = data_dir_.AppendASCII("good.crx");
- InstallCRX(path, INSTALL_FAILED);
+ InstallCRX(path, INSTALL_WITHOUT_LOAD);
EXPECT_EQ(0u, service_->extensions()->size());
ValidateBooleanPref(good_crx, "blacklist", true);
}
@@ -3024,97 +3030,13 @@
"v1");
// Make sure pref is updated
+ loop_.RunUntilIdle();
- // Now, the good_crx is blacklisted but whitelist negates it.
- ValidateBooleanPref(good_crx, "blacklist", true);
- EXPECT_EQ(1u, service_->extensions()->size());
-
- whitelist.Clear();
- prefs->Set(prefs::kExtensionInstallAllowList, whitelist);
-
- // Now, the good_crx is blacklisted for good.
+ // The good_crx is blacklisted and the whitelist doesn't negate it.
ValidateBooleanPref(good_crx, "blacklist", true);
EXPECT_EQ(0u, service_->extensions()->size());
}
-// Allow Google-blacklisted extension if policy explicitly allows it (blacklist
-// then set policy).
-TEST_F(ExtensionServiceTest, WhitelistGoogleBlacklistedExtension) {
- InitializeEmptyExtensionService();
-
- std::vector<std::string> blacklist;
- blacklist.push_back(good_crx);
- ExtensionSystem::Get(profile_.get())->blacklist()->SetFromUpdater(blacklist,
- "v1");
-
- FilePath path = data_dir_.AppendASCII("good.crx");
- InstallCRX(path, INSTALL_FAILED);
-
- base::ListValue whitelist;
- whitelist.Append(base::Value::CreateStringValue(good_crx));
- service_->extension_prefs()->pref_service()->Set(
- prefs::kExtensionInstallAllowList, whitelist);
-
- InstallCRX(path, INSTALL_NEW);
-}
-
-// Allow Google-blacklisted extension if policy requires it (blacklist then set
-// policy).
-TEST_F(ExtensionServiceTest, ForcelistGoogleBlacklistedExtension) {
- InitializeEmptyExtensionService();
-
- std::vector<std::string> blacklist;
- blacklist.push_back(good_crx);
- ExtensionSystem::Get(profile_.get())->blacklist()->SetFromUpdater(blacklist,
- "v1");
-
- FilePath path = data_dir_.AppendASCII("good.crx");
- InstallCRX(path, INSTALL_FAILED);
-
- base::ListValue forcelist;
- forcelist.Append(base::Value::CreateStringValue(good_crx));
- service_->extension_prefs()->pref_service()->Set(
- prefs::kExtensionInstallAllowList, forcelist);
-
- InstallCRX(path, INSTALL_NEW);
-}
-
-// Allow Google-blacklisted extension if policy explicitly allows it (set policy
-// then blacklist).
-TEST_F(ExtensionServiceTest, GoogleBlacklistWhitelistedExtension) {
- InitializeEmptyExtensionService();
-
- base::ListValue whitelist;
- whitelist.Append(base::Value::CreateStringValue(good_crx));
- service_->extension_prefs()->pref_service()->Set(
- prefs::kExtensionInstallAllowList, whitelist);
-
- std::vector<std::string> blacklist;
- blacklist.push_back(good_crx);
- ExtensionSystem::Get(profile_.get())->blacklist()->SetFromUpdater(blacklist,
- "v1");
-
- InstallCRX(data_dir_.AppendASCII("good.crx"), INSTALL_NEW);
-}
-
-// Allow Google-blacklisted extension if policy requires it (set policy then
-// blacklist).
-TEST_F(ExtensionServiceTest, GoogleBlacklistForcelistedExtension) {
- InitializeEmptyExtensionService();
-
- base::ListValue forcelist;
- forcelist.Append(base::Value::CreateStringValue(good_crx));
- service_->extension_prefs()->pref_service()->Set(
- prefs::kExtensionInstallAllowList, forcelist);
-
- std::vector<std::string> blacklist;
- blacklist.push_back(good_crx);
- ExtensionSystem::Get(profile_.get())->blacklist()->SetFromUpdater(blacklist,
- "v1");
-
- InstallCRX(data_dir_.AppendASCII("good.crx"), INSTALL_NEW);
-}
-
// Test loading extensions from the profile directory, except
// blacklisted ones.
TEST_F(ExtensionServiceTest, WillNotLoadBlacklistedExtensionsFromDirectory) {
@@ -3148,7 +3070,10 @@
}
ASSERT_EQ(2u, loaded_.size());
- EXPECT_FALSE(service_->GetExtensionById(good1, true));
+ EXPECT_TRUE(service_->GetInstalledExtension(good1));
+ int include_mask = ExtensionService::INCLUDE_EVERYTHING &
+ ~ExtensionService::INCLUDE_BLACKLISTED;
+ EXPECT_FALSE(service_->GetExtensionById(good1, include_mask));
}
// Will not install extension blacklisted by policy.
diff --git a/chrome/browser/extensions/extension_system.cc b/chrome/browser/extensions/extension_system.cc
index 3422f6d..2881cec0 100644
--- a/chrome/browser/extensions/extension_system.cc
+++ b/chrome/browser/extensions/extension_system.cc
@@ -103,8 +103,7 @@
blacklist_.reset(new Blacklist(extension_prefs_.get()));
standard_management_policy_provider_.reset(
- new StandardManagementPolicyProvider(extension_prefs_.get(),
- blacklist_.get()));
+ new StandardManagementPolicyProvider(extension_prefs_.get()));
}
void ExtensionSystemImpl::Shared::RegisterManagementPolicyProviders() {
diff --git a/chrome/browser/extensions/standard_management_policy_provider.cc b/chrome/browser/extensions/standard_management_policy_provider.cc
index 05e2e5f..1a818f2 100644
--- a/chrome/browser/extensions/standard_management_policy_provider.cc
+++ b/chrome/browser/extensions/standard_management_policy_provider.cc
@@ -14,8 +14,8 @@
namespace extensions {
StandardManagementPolicyProvider::StandardManagementPolicyProvider(
- ExtensionPrefs* prefs, Blacklist* blacklist)
- : prefs_(prefs), blacklist_(blacklist) {
+ ExtensionPrefs* prefs)
+ : prefs_(prefs) {
}
StandardManagementPolicyProvider::~StandardManagementPolicyProvider() {
@@ -34,15 +34,14 @@
bool StandardManagementPolicyProvider::UserMayLoad(
const Extension* extension,
string16* error) const {
- bool is_google_blacklisted = blacklist_->IsBlacklisted(extension);
const base::ListValue* blacklist =
prefs_->pref_service()->GetList(prefs::kExtensionInstallDenyList);
const base::ListValue* whitelist =
prefs_->pref_service()->GetList(prefs::kExtensionInstallAllowList);
const base::ListValue* forcelist =
prefs_->pref_service()->GetList(prefs::kExtensionInstallForceList);
- return admin_policy::UserMayLoad(is_google_blacklisted, blacklist, whitelist,
- forcelist, extension, error);
+ return admin_policy::UserMayLoad(
+ blacklist, whitelist, forcelist, extension, error);
}
bool StandardManagementPolicyProvider::UserMayModifySettings(
diff --git a/chrome/browser/extensions/standard_management_policy_provider.h b/chrome/browser/extensions/standard_management_policy_provider.h
index 8c849da..4dd2a68 100644
--- a/chrome/browser/extensions/standard_management_policy_provider.h
+++ b/chrome/browser/extensions/standard_management_policy_provider.h
@@ -17,9 +17,8 @@
// extension black/whitelists and admin black/whitelists.
class StandardManagementPolicyProvider : public ManagementPolicy::Provider {
public:
- // |prefs| and |blacklist| must outlive this.
- StandardManagementPolicyProvider(ExtensionPrefs* prefs,
- Blacklist* blacklist);
+ // |prefs| must outlive this.
+ explicit StandardManagementPolicyProvider(ExtensionPrefs* prefs);
virtual ~StandardManagementPolicyProvider();
@@ -34,8 +33,6 @@
private:
ExtensionPrefs* const prefs_;
-
- Blacklist* const blacklist_;
};
} // namespace extensions
diff --git a/chrome/browser/extensions/standard_management_policy_provider_unittest.cc b/chrome/browser/extensions/standard_management_policy_provider_unittest.cc
index 134e146d..8cdc3d87 100644
--- a/chrome/browser/extensions/standard_management_policy_provider_unittest.cc
+++ b/chrome/browser/extensions/standard_management_policy_provider_unittest.cc
@@ -21,11 +21,9 @@
: ui_thread_(content::BrowserThread::UI, &message_loop_),
file_thread_(content::BrowserThread::FILE, &message_loop_),
prefs_(message_loop_.message_loop_proxy()),
- blacklist_(prefs()),
- provider_(prefs(), &blacklist_) {
+ provider_(prefs()) {
}
-
protected:
ExtensionPrefs* prefs() {
return prefs_.prefs();
@@ -49,53 +47,9 @@
TestExtensionPrefs prefs_;
- Blacklist blacklist_;
-
StandardManagementPolicyProvider provider_;
};
-// Tests various areas of blacklist functionality.
-TEST_F(StandardManagementPolicyProviderTest, Blacklist) {
- std::string not_installed_id = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
-
- // Install 5 extensions.
- ExtensionList extensions;
- for (int i = 0; i < 5; i++) {
- std::string name = "test" + base::IntToString(i);
- extensions.push_back(prefs_.AddExtension(name));
- }
- EXPECT_EQ(NULL,
- prefs()->GetInstalledExtensionInfo(not_installed_id).get());
-
- ExtensionList::const_iterator iter;
- for (iter = extensions.begin(); iter != extensions.end(); ++iter) {
- EXPECT_TRUE(provider_.UserMayLoad(*iter, NULL));
- }
- // Blacklist one installed and one not-installed extension id.
- std::vector<std::string> blacklisted_ids;
- blacklisted_ids.push_back(extensions[0]->id());
- blacklisted_ids.push_back(not_installed_id);
- blacklist_.SetFromUpdater(blacklisted_ids, "version0");
-
- // Make sure the id we expect to be blacklisted is.
- EXPECT_FALSE(provider_.UserMayLoad(extensions[0], NULL));
-
- // Make sure the other id's are not blacklisted.
- for (iter = extensions.begin() + 1; iter != extensions.end(); ++iter)
- EXPECT_TRUE(provider_.UserMayLoad(*iter, NULL));
-
- // Make sure GetInstalledExtensionsInfo returns only the non-blacklisted
- // extensions data.
- scoped_ptr<ExtensionPrefs::ExtensionsInfo> info(
- prefs()->GetInstalledExtensionsInfo());
- EXPECT_EQ(4u, info->size());
- ExtensionPrefs::ExtensionsInfo::iterator info_iter;
- for (info_iter = info->begin(); info_iter != info->end(); ++info_iter) {
- ExtensionInfo* extension_info = info_iter->get();
- EXPECT_NE(extensions[0]->id(), extension_info->extension_id);
- }
-}
-
// Tests the behavior of the ManagementPolicy provider methods for an
// extension required by policy.
TEST_F(StandardManagementPolicyProviderTest, RequiredExtension) {
diff --git a/chrome/browser/extensions/test_blacklist.cc b/chrome/browser/extensions/test_blacklist.cc
new file mode 100644
index 0000000..138804d
--- /dev/null
+++ b/chrome/browser/extensions/test_blacklist.cc
@@ -0,0 +1,40 @@
+// Copyright 2012 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/test_blacklist.h"
+
+#include <set>
+
+#include "base/bind.h"
+#include "base/message_loop.h"
+#include "base/run_loop.h"
+#include "chrome/browser/extensions/blacklist.h"
+
+namespace extensions {
+
+TestBlacklist::TestBlacklist(Blacklist* blacklist)
+ : blacklist_(blacklist) {
+}
+
+namespace {
+
+void Assign(std::set<std::string>* out, const std::set<std::string>& in) {
+ *out = in;
+}
+
+} // namespace
+
+bool TestBlacklist::IsBlacklisted(const std::string& extension_id) {
+ std::set<std::string> id_set;
+ id_set.insert(extension_id);
+ std::set<std::string> blacklist_set;
+ blacklist_->GetBlacklistedIDs(id_set,
+ base::Bind(&Assign, &blacklist_set));
+ base::RunLoop run_loop;
+ MessageLoop::current()->PostTask(FROM_HERE, run_loop.QuitClosure());
+ run_loop.Run();
+ return blacklist_set.count(extension_id) > 0;
+}
+
+} // namespace extensions
diff --git a/chrome/browser/extensions/test_blacklist.h b/chrome/browser/extensions/test_blacklist.h
new file mode 100644
index 0000000..a9b5690
--- /dev/null
+++ b/chrome/browser/extensions/test_blacklist.h
@@ -0,0 +1,34 @@
+// Copyright 2012 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_TEST_BLACKLIST_H_
+#define CHROME_BROWSER_EXTENSIONS_TEST_BLACKLIST_H_
+
+#include <string>
+
+#include "base/basictypes.h"
+
+namespace extensions {
+
+class Blacklist;
+
+// A wrapper for an extensions::Blacklist that provides functionality for
+// testing.
+class TestBlacklist {
+ public:
+ explicit TestBlacklist(Blacklist* blacklist);
+
+ Blacklist* blacklist() { return blacklist_; }
+
+ bool IsBlacklisted(const std::string& extension_id);
+
+ private:
+ Blacklist* blacklist_;
+
+ DISALLOW_COPY_AND_ASSIGN(TestBlacklist);
+};
+
+} // namespace extensions
+
+#endif // CHROME_BROWSER_EXTENSIONS_TEST_BLACKLIST_H_
diff --git a/chrome/browser/extensions/test_extension_system.cc b/chrome/browser/extensions/test_extension_system.cc
index 1b8ccd760..ed6eccf8 100644
--- a/chrome/browser/extensions/test_extension_system.cc
+++ b/chrome/browser/extensions/test_extension_system.cc
@@ -84,8 +84,7 @@
new ShellWindowGeometryCache(profile_, state_store_.get()));
blacklist_.reset(new Blacklist(extension_prefs_.get()));
standard_management_policy_provider_.reset(
- new StandardManagementPolicyProvider(extension_prefs_.get(),
- blacklist_.get()));
+ new StandardManagementPolicyProvider(extension_prefs_.get()));
management_policy_.reset(new ManagementPolicy());
management_policy_->RegisterProvider(
standard_management_policy_provider_.get());
diff --git a/chrome/browser/extensions/test_extension_system.h b/chrome/browser/extensions/test_extension_system.h
index 3f80dc0..fa66f60 100644
--- a/chrome/browser/extensions/test_extension_system.h
+++ b/chrome/browser/extensions/test_extension_system.h
@@ -78,8 +78,8 @@
scoped_ptr<Blacklist> blacklist_;
scoped_ptr<StandardManagementPolicyProvider>
standard_management_policy_provider_;
- scoped_ptr<ExtensionService> extension_service_;
scoped_ptr<ManagementPolicy> management_policy_;
+ scoped_ptr<ExtensionService> extension_service_;
scoped_ptr<ExtensionProcessManager> extension_process_manager_;
scoped_ptr<AlarmManager> alarm_manager_;
scoped_refptr<ExtensionInfoMap> info_map_;
diff --git a/chrome/browser/extensions/updater/extension_updater_unittest.cc b/chrome/browser/extensions/updater/extension_updater_unittest.cc
index 5c78d84..ea3d9a3 100644
--- a/chrome/browser/extensions/updater/extension_updater_unittest.cc
+++ b/chrome/browser/extensions/updater/extension_updater_unittest.cc
@@ -7,11 +7,16 @@
#include <set>
#include <vector>
+#include "base/bind.h"
+#include "base/bind_helpers.h"
#include "base/command_line.h"
#include "base/compiler_specific.h"
#include "base/file_util.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
+#include "base/message_loop.h"
+#include "base/run_loop.h"
+#include "base/sequenced_task_runner.h"
#include "base/stl_util.h"
#include "base/string_number_conversions.h"
#include "base/string_split.h"
@@ -24,6 +29,7 @@
#include "chrome/browser/extensions/extension_error_reporter.h"
#include "chrome/browser/extensions/extension_sync_data.h"
#include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/extensions/test_blacklist.h"
#include "chrome/browser/extensions/test_extension_prefs.h"
#include "chrome/browser/extensions/test_extension_service.h"
#include "chrome/browser/extensions/test_extension_system.h"
@@ -1035,9 +1041,10 @@
net::TestURLFetcherFactory factory;
net::TestURLFetcher* fetcher = NULL;
MockService service(prefs_.get());
+ TestBlacklist blacklist(service.blacklist());
ExtensionUpdater updater(
&service, service.extension_prefs(), service.pref_service(),
- service.profile(), service.blacklist(), kUpdateFrequencySecs);
+ service.profile(), blacklist.blacklist(), kUpdateFrequencySecs);
updater.Start();
ResetDownloader(
&updater,
@@ -1062,7 +1069,7 @@
// Call back the ExtensionUpdater with a 200 response and some test data.
std::string extension_data("aaaabbbbcccceeeeaaaabbbbcccceeee");
- EXPECT_FALSE(service.blacklist()->IsBlacklisted(extension_data));
+ EXPECT_FALSE(blacklist.IsBlacklisted(extension_data));
fetcher = factory.GetFetcherByID(ExtensionDownloader::kExtensionFetcherId);
EXPECT_TRUE(fetcher != NULL && fetcher->delegate() != NULL);
@@ -1076,7 +1083,7 @@
RunUntilIdle();
- EXPECT_TRUE(service.blacklist()->IsBlacklisted(extension_data));
+ EXPECT_TRUE(blacklist.IsBlacklisted(extension_data));
EXPECT_EQ(version, service.pref_service()->
GetString(prefs::kExtensionBlacklistUpdateVersion));
diff --git a/chrome/browser/protector/protected_prefs_watcher_unittest.cc b/chrome/browser/protector/protected_prefs_watcher_unittest.cc
index f69b043..f67d7f7 100644
--- a/chrome/browser/protector/protected_prefs_watcher_unittest.cc
+++ b/chrome/browser/protector/protected_prefs_watcher_unittest.cc
@@ -132,9 +132,7 @@
EXPECT_FALSE(IsSignatureValid());
// Blacklisting the extension does update the backup and signature.
- std::set<std::string> blacklist;
- blacklist.insert(sample_id);
- extension_prefs->UpdateBlacklist(blacklist);
+ extension_prefs->SetExtensionBlacklisted(sample_id, true);
EXPECT_TRUE(IsSignatureValid());
}
