When extension is blacklisted by admin policy, it should be removed if already running.

BUG=51689
TEST=ExtensionsServiceTest.BlacklistedByPolicyRemovedIfRunning

Review URL: http://codereview.chromium.org/3161020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@56683 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 78 insertions, 9 deletions

diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 9b28456..c7799c7 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -241,6 +241,10 @@ bool ExtensionPrefs::IsExtensionAllowedByPolicy(
     const std::string& extension_id) {
   std::string string_value;
 
+  const ListValue* blacklist = prefs_->GetList(kExtensionInstallDenyList);
+  if (!blacklist || blacklist->empty())
+    return true;
+
   // Check the whitelist first.
   const ListValue* whitelist = prefs_->GetList(kExtensionInstallAllowList);
   if (whitelist) {
@@ -254,7 +258,6 @@ bool ExtensionPrefs::IsExtensionAllowedByPolicy(
   }
 
   // Then check the blacklist (the admin blacklist, not the Google blacklist).
-  const ListValue* blacklist = prefs_->GetList(kExtensionInstallDenyList);
   if (blacklist) {
     for (ListValue::const_iterator it = blacklist->begin();
          it != blacklist->end(); ++it) {
diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc
index c9d472e7..842021e 100644
--- a/chrome/browser/extensions/extension_updater_unittest.cc
+++ b/chrome/browser/extensions/extension_updater_unittest.cc
@@ -40,33 +40,37 @@ class MockService : public ExtensionUpdateService {
   virtual ~MockService() {}
 
   virtual const ExtensionList* extensions() const {
-    EXPECT_TRUE(false);
+    ADD_FAILURE();
     return NULL;
   }
 
   virtual const PendingExtensionMap& pending_extensions() const {
-    EXPECT_TRUE(false);
+    ADD_FAILURE();
     return pending_extensions_;
   }
 
   virtual void UpdateExtension(const std::string& id,
                                const FilePath& extension_path,
                                const GURL& download_url) {
-    EXPECT_TRUE(false);
+    FAIL();
   }
 
   virtual Extension* GetExtensionById(const std::string& id, bool) {
-    EXPECT_TRUE(false);
+    ADD_FAILURE();
     return NULL;
   }
 
   virtual void UpdateExtensionBlacklist(
-    const std::vector<std::string>& blacklist) {
-    EXPECT_TRUE(false);
+      const std::vector<std::string>& blacklist) {
+    FAIL();
+  }
+
+  virtual void CheckAdminBlacklist() {
+    FAIL();
   }
 
   virtual bool HasInstalledExtensions() {
-    EXPECT_TRUE(false);
+    ADD_FAILURE();
     return false;
   }
 
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 61de28b..99ca4cc 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -200,6 +200,9 @@ ExtensionsService::ExtensionsService(Profile* profile,
                  NotificationService::AllSources());
   registrar_.Add(this, NotificationType::EXTENSION_PROCESS_TERMINATED,
                  Source<Profile>(profile_));
+  // We outlive the profile, so we don't unregister these.
+  prefs->AddPrefObserver(prefs::kExtensionInstallAllowList, this);
+  prefs->AddPrefObserver(prefs::kExtensionInstallDenyList, this);
 
   // Set up the ExtensionUpdater
   if (autoupdate_enabled) {
@@ -214,7 +217,7 @@ ExtensionsService::ExtensionsService(Profile* profile,
 
   backend_ = new ExtensionsServiceBackend(install_directory_);
 
-  // Use monochrome icons for omnibox icons.
+  // Use monochrome icons for Omnibox icons.
   omnibox_icon_manager_.set_monochrome(true);
 }
 
@@ -761,6 +764,22 @@ void ExtensionsService::UpdateExtensionBlacklist(
   }
 }
 
+void ExtensionsService::CheckAdminBlacklist() {
+  std::vector<std::string> to_be_removed;
+  // Loop through extensions list, unload installed extensions.
+  for (ExtensionList::const_iterator iter = extensions_.begin();
+       iter != extensions_.end(); ++iter) {
+    Extension* extension = (*iter);
+    if (!extension_prefs_->IsExtensionAllowedByPolicy(extension->id()))
+      to_be_removed.push_back(extension->id());
+  }
+
+  // UnloadExtension will change the extensions_ list. So, we should
+  // call it outside the iterator loop.
+  for (unsigned int i = 0; i < to_be_removed.size(); ++i)
+    UnloadExtension(to_be_removed[i]);
+}
+
 bool ExtensionsService::IsIncognitoEnabled(const Extension* extension) {
   // If this is a component extension we always allow it to work in incognito
   // mode.
@@ -1279,6 +1298,14 @@ void ExtensionsService::Observe(NotificationType type,
       break;
     }
 
+    case NotificationType::PREF_CHANGED: {
+      std::string* pref_name = Details<std::string>(details).ptr();
+      DCHECK(*pref_name == prefs::kExtensionInstallAllowList ||
+             *pref_name == prefs::kExtensionInstallDenyList);
+      CheckAdminBlacklist();
+      break;
+    }
+
     default:
       NOTREACHED() << "Unexpected notification type.";
   }
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index 25182ee..af0893b 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -77,6 +77,7 @@ class ExtensionUpdateService {
                                       bool include_disabled) = 0;
   virtual void UpdateExtensionBlacklist(
     const std::vector<std::string>& blacklist) = 0;
+  virtual void CheckAdminBlacklist() = 0;
   virtual bool HasInstalledExtensions() = 0;
 
   virtual ExtensionPrefs* extension_prefs() = 0;
@@ -303,6 +304,11 @@ class ExtensionsService
   virtual void UpdateExtensionBlacklist(
     const std::vector<std::string>& blacklist);
 
+  // Go through each extension and unload those that the network admin has
+  // put on the blacklist (not to be confused with the Google managed blacklist
+  // set of extensions.
+  virtual void CheckAdminBlacklist();
+
   void set_extensions_enabled(bool enabled) { extensions_enabled_ = enabled; }
   bool extensions_enabled() { return extensions_enabled_; }
 
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index b8c176b..20421cf 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -1772,6 +1772,35 @@ TEST_F(ExtensionsServiceTest, BlacklistedByPolicyWillNotInstall) {
   EXPECT_EQ(1u, service_->extensions()->size());
 }
 
+// Extension blacklisted by policy get unloaded after installing.
+TEST_F(ExtensionsServiceTest, BlacklistedByPolicyRemovedIfRunning) {
+  InitializeEmptyExtensionsService();
+
+  // Install good_crx.
+  FilePath extensions_path;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path));
+  extensions_path = extensions_path.AppendASCII("extensions");
+  FilePath path = extensions_path.AppendASCII("good.crx");
+  service_->InstallExtension(path);
+  loop_.RunAllPending();
+  EXPECT_EQ(1u, service_->extensions()->size());
+
+  ListValue* blacklist = prefs_->GetMutableList("extensions.install.denylist");
+  ASSERT_TRUE(blacklist != NULL);
+
+  // Blacklist this extension.
+  blacklist->Append(Value::CreateStringValue(good_crx));
+  prefs_->ScheduleSavePersistentPrefs();
+
+  // Programmatically appending to the prefs doesn't seem to notify the
+  // observers... :/
+  prefs_->pref_notifier()->FireObservers("extensions.install.denylist");
+
+  // Extension should not be running now.
+  loop_.RunAllPending();
+  EXPECT_EQ(0u, service_->extensions()->size());
+}
+
 // Tests disabling extensions
 TEST_F(ExtensionsServiceTest, DisableExtension) {
   InitializeEmptyExtensionsService();