Extend blacklist to support different typed of blacklisted extensions.

BUG=267514

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

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

9 files changed, 154 insertions, 39 deletions

diff --git a/chrome/browser/extensions/blacklist.cc b/chrome/browser/extensions/blacklist.cc
index d69f1d9..f39d113 100644
--- a/chrome/browser/extensions/blacklist.cc
+++ b/chrome/browser/extensions/blacklist.cc
@@ -115,10 +115,23 @@ class SafeBrowsingClientImpl
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingClientImpl);
 };
 
-void IsNotEmpty(const Blacklist::IsBlacklistedCallback& callback,
-                const std::set<std::string>& set) {
-  callback.Run(set.empty() ? Blacklist::NOT_BLACKLISTED
-                           : Blacklist::BLACKLISTED);
+void CheckOneExtensionState(
+    const Blacklist::IsBlacklistedCallback& callback,
+    const Blacklist::BlacklistStateMap& state_map) {
+  callback.Run(state_map.empty() ? Blacklist::NOT_BLACKLISTED
+                                 : state_map.begin()->second);
+}
+
+void GetMalwareFromBlacklistStateMap(
+    const Blacklist::GetMalwareIDsCallback& callback,
+    const Blacklist::BlacklistStateMap& state_map) {
+  std::set<std::string> malware;
+  for (Blacklist::BlacklistStateMap::const_iterator it = state_map.begin();
+       it != state_map.end(); ++it) {
+    if (it->second == Blacklist::BLACKLISTED_MALWARE)
+      malware.insert(it->first);
+  }
+  callback.Run(malware);
 }
 
 }  // namespace
@@ -172,20 +185,93 @@ void Blacklist::GetBlacklistedIDs(const std::set<std::string>& ids,
 
   if (ids.empty() || !g_database_manager.Get().get().get()) {
     base::MessageLoopProxy::current()->PostTask(
-        FROM_HERE, base::Bind(callback, std::set<std::string>()));
+        FROM_HERE, base::Bind(callback, BlacklistStateMap()));
     return;
   }
 
   // Constructing the SafeBrowsingClientImpl begins the process of asking
-  // safebrowsing for the blacklisted extensions.
-  new SafeBrowsingClientImpl(ids, callback);
+  // safebrowsing for the blacklisted extensions. The set of blacklisted
+  // extensions returned by SafeBrowsing will then be passed to
+  // GetBlacklistStateIDs to get the particular BlacklistState for each id.
+  new SafeBrowsingClientImpl(
+      ids, base::Bind(&Blacklist::GetBlacklistStateForIDs, AsWeakPtr(),
+                      callback));
 }
 
+void Blacklist::GetMalwareIDs(const std::set<std::string>& ids,
+                              const GetMalwareIDsCallback& callback) {
+  GetBlacklistedIDs(ids, base::Bind(&GetMalwareFromBlacklistStateMap,
+                                    callback));
+}
+
+
 void Blacklist::IsBlacklisted(const std::string& extension_id,
                               const IsBlacklistedCallback& callback) {
   std::set<std::string> check;
   check.insert(extension_id);
-  GetBlacklistedIDs(check, base::Bind(&IsNotEmpty, callback));
+  GetBlacklistedIDs(check, base::Bind(&CheckOneExtensionState, callback));
+}
+
+void Blacklist::GetBlacklistStateForIDs(
+    const GetBlacklistedIDsCallback& callback,
+    const std::set<std::string>& blacklisted_ids) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  std::set<std::string> ids_unknown_state;
+  BlacklistStateMap extensions_state;
+  for (std::set<std::string>::const_iterator it = blacklisted_ids.begin();
+       it != blacklisted_ids.end(); ++it) {
+    BlacklistStateMap::const_iterator cache_it =
+        blacklist_state_cache_.find(*it);
+    if (cache_it == blacklist_state_cache_.end())
+      ids_unknown_state.insert(*it);
+    else
+      extensions_state[*it] = cache_it->second;
+  }
+
+  if (ids_unknown_state.empty()) {
+    callback.Run(extensions_state);
+  } else {
+    // After the extension blacklist states have been downloaded, call this
+    // functions again, but prevent infinite cycle in case server is offline
+    // or some other reason prevents us from receiving the blacklist state for
+    // these extensions.
+    RequestExtensionsBlacklistState(
+        ids_unknown_state,
+        base::Bind(&Blacklist::ReturnBlacklistStateMap, AsWeakPtr(),
+                   callback, blacklisted_ids));
+  }
+}
+
+void Blacklist::ReturnBlacklistStateMap(
+    const GetBlacklistedIDsCallback& callback,
+    const std::set<std::string>& blacklisted_ids) {
+  BlacklistStateMap extensions_state;
+  for (std::set<std::string>::const_iterator it = blacklisted_ids.begin();
+       it != blacklisted_ids.end(); ++it) {
+    BlacklistStateMap::const_iterator cache_it =
+        blacklist_state_cache_.find(*it);
+    if (cache_it != blacklist_state_cache_.end())
+      extensions_state[*it] = cache_it->second;
+    // If for some reason we still haven't cached the state of this extension,
+    // we silently skip it.
+  }
+
+  callback.Run(extensions_state);
+}
+
+void Blacklist::RequestExtensionsBlacklistState(
+    const std::set<std::string> ids, base::Callback<void()> callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  // This is a stub. The request will be made here, but the server is not up
+  // yet. For compatibility with current blacklist logic, mark all extensions
+  // as malicious.
+  for (std::set<std::string>::const_iterator it = ids.begin();
+       it != ids.end();
+       ++it) {
+    blacklist_state_cache_[*it] = BLACKLISTED_MALWARE;
+  }
+  callback.Run();
 }
 
 void Blacklist::AddObserver(Observer* observer) {
diff --git a/chrome/browser/extensions/blacklist.h b/chrome/browser/extensions/blacklist.h
index f005eaf..200b5e6 100644
--- a/chrome/browser/extensions/blacklist.h
+++ b/chrome/browser/extensions/blacklist.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_BLACKLIST_H_
 #define CHROME_BROWSER_EXTENSIONS_BLACKLIST_H_
 
+#include <map>
 #include <set>
 #include <string>
 #include <vector>
@@ -52,14 +53,24 @@ class Blacklist : public content::NotificationObserver,
     DISALLOW_COPY_AND_ASSIGN(ScopedDatabaseManagerForTest);
   };
 
+  // The numeric values here match the values of the respective enum in proto
+  // received from SafeBrowsing server.
   enum BlacklistState {
-    NOT_BLACKLISTED,
-    BLACKLISTED,
+    NOT_BLACKLISTED = 0,
+    BLACKLISTED_MALWARE = 1,
+    BLACKLISTED_SECURITY_VULNERABILITY = 2,
+    BLACKLISTED_CWS_POLICY_VIOLATION = 3,
+    BLACKLISTED_POTENTIALLY_UNWANTED = 4
   };
 
-  typedef base::Callback<void(const std::set<std::string>&)>
+  typedef std::map<std::string, BlacklistState> BlacklistStateMap;
+
+  typedef base::Callback<void(const BlacklistStateMap&)>
       GetBlacklistedIDsCallback;
 
+  typedef base::Callback<void(const std::set<std::string>&)>
+      GetMalwareIDsCallback;
+
   typedef base::Callback<void(BlacklistState)> IsBlacklistedCallback;
 
   explicit Blacklist(ExtensionPrefs* prefs);
@@ -67,14 +78,23 @@ class Blacklist : public content::NotificationObserver,
   virtual ~Blacklist();
 
   // 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.
+  // which are blacklisted and includes them in the resulting map passed
+  // via |callback|, which will be sent on the caller's message loop. The values
+  // of the map are the blacklist state for each extension. Extensions with
+  // a BlacklistState of NOT_BLACKLISTED are not included in the result.
   //
   // For a synchronous version which ONLY CHECKS CURRENTLY INSTALLED EXTENSIONS
   // see ExtensionPrefs::IsExtensionBlacklisted.
   void GetBlacklistedIDs(const std::set<std::string>& ids,
                          const GetBlacklistedIDsCallback& callback);
 
+  // From the subset of extension IDs passed in via |ids|, select the ones
+  // marked in the blacklist as BLACKLISTED_MALWARE and asynchronously pass
+  // to |callback|. Basically, will call GetBlacklistedIDs and filter its
+  // results.
+  void GetMalwareIDs(const std::set<std::string>& ids,
+                     const GetMalwareIDsCallback& callback);
+
   // More convenient form of GetBlacklistedIDs for checking a single extension.
   void IsBlacklisted(const std::string& extension_id,
                      const IsBlacklistedCallback& callback);
@@ -94,10 +114,21 @@ class Blacklist : public content::NotificationObserver,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
+  void GetBlacklistStateForIDs(const GetBlacklistedIDsCallback& callback,
+                               const std::set<std::string>& blacklisted_ids);
+
+  void RequestExtensionsBlacklistState(const std::set<std::string> ids,
+                                       base::Callback<void()> callback);
+
+  void ReturnBlacklistStateMap(const GetBlacklistedIDsCallback& callback,
+                               const std::set<std::string>& blacklisted_ids);
+
   ObserverList<Observer> observers_;
 
   content::NotificationRegistrar registrar_;
 
+  BlacklistStateMap blacklist_state_cache_;
+
   DISALLOW_COPY_AND_ASSIGN(Blacklist);
 };
 
diff --git a/chrome/browser/extensions/blacklist_unittest.cc b/chrome/browser/extensions/blacklist_unittest.cc
index 1ebf5e2..18d4bb8 100644
--- a/chrome/browser/extensions/blacklist_unittest.cc
+++ b/chrome/browser/extensions/blacklist_unittest.cc
@@ -87,12 +87,12 @@ TEST_F(BlacklistTest, OnlyIncludesRequestedIDs) {
   blacklist_db()->Enable();
   blacklist_db()->SetUnsafe(a, b);
 
-  EXPECT_TRUE(tester.IsBlacklisted(a));
-  EXPECT_TRUE(tester.IsBlacklisted(b));
-  EXPECT_FALSE(tester.IsBlacklisted(c));
+  EXPECT_EQ(Blacklist::BLACKLISTED_MALWARE, tester.GetBlacklistState(a));
+  EXPECT_EQ(Blacklist::BLACKLISTED_MALWARE, tester.GetBlacklistState(b));
+  EXPECT_EQ(Blacklist::NOT_BLACKLISTED, tester.GetBlacklistState(c));
 
   std::set<std::string> blacklisted_ids;
-  blacklist.GetBlacklistedIDs(Set(a, c), base::Bind(&Assign, &blacklisted_ids));
+  blacklist.GetMalwareIDs(Set(a, c), base::Bind(&Assign, &blacklisted_ids));
   base::RunLoop().RunUntilIdle();
 
   EXPECT_EQ(Set(a), blacklisted_ids);
@@ -104,20 +104,20 @@ TEST_F(BlacklistTest, SafeBrowsing) {
   Blacklist blacklist(prefs());
   TestBlacklist tester(&blacklist);
 
-  EXPECT_FALSE(tester.IsBlacklisted(a));
+  EXPECT_EQ(Blacklist::NOT_BLACKLISTED, tester.GetBlacklistState(a));
 
   blacklist_db()->SetUnsafe(a);
   // The manager is still disabled at this point, so it won't be blacklisted.
-  EXPECT_FALSE(tester.IsBlacklisted(a));
+  EXPECT_EQ(Blacklist::NOT_BLACKLISTED, tester.GetBlacklistState(a));
 
   blacklist_db()->Enable().NotifyUpdate();
   base::RunLoop().RunUntilIdle();
   // Now it should be.
-  EXPECT_TRUE(tester.IsBlacklisted(a));
+  EXPECT_EQ(Blacklist::BLACKLISTED_MALWARE, tester.GetBlacklistState(a));
 
   blacklist_db()->ClearUnsafe().NotifyUpdate();
   // Safe browsing blacklist empty, now enabled.
-  EXPECT_FALSE(tester.IsBlacklisted(a));
+  EXPECT_EQ(Blacklist::NOT_BLACKLISTED, tester.GetBlacklistState(a));
 }
 
 // Tests that Blacklist clears the old prefs blacklist on startup.
@@ -149,8 +149,8 @@ TEST_F(BlacklistTest, ClearsPreferencesBlacklist) {
   // safebrowsing. Blacklist no longer reads from prefs. This is purely a
   // concern of somebody else (currently, ExtensionService).
   std::set<std::string> blacklisted_ids;
-  blacklist.GetBlacklistedIDs(Set(a, b, c, d),
-                              base::Bind(&Assign, &blacklisted_ids));
+  blacklist.GetMalwareIDs(Set(a, b, c, d),
+                          base::Bind(&Assign, &blacklisted_ids));
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(std::set<std::string>(), blacklisted_ids);
 
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index b40c1c6..9fdb2f4 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -523,7 +523,7 @@ void CrxInstaller::OnBlacklistChecked(
 
   blacklist_state_ = blacklist_state;
 
-  if (blacklist_state_ == extensions::Blacklist::BLACKLISTED &&
+  if (blacklist_state_ == extensions::Blacklist::BLACKLISTED_MALWARE &&
       !allow_silent_install_) {
     // User tried to install a blacklisted extension. Show an error and
     // refuse to install it.
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index d016563..93c43b2 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -1852,7 +1852,7 @@ void ExtensionPrefs::PopulateExtensionInfoPrefs(
   extension_dict->Set(kPrefInstallTime,
                       new base::StringValue(
                           base::Int64ToString(install_time.ToInternalValue())));
-  if (blacklist_state == Blacklist::BLACKLISTED)
+  if (blacklist_state == Blacklist::BLACKLISTED_MALWARE)
     extension_dict->Set(kPrefBlacklist, new base::FundamentalValue(true));
 
   base::FilePath::StringType path = MakePathRelative(install_directory_,
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index cce9f3d..ad5ec36 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -2430,7 +2430,7 @@ void ExtensionService::OnExtensionInstalled(
     extension_prefs_->ClearDisableReasons(id);
   }
 
-  if (blacklist_state == extensions::Blacklist::BLACKLISTED) {
+  if (blacklist_state == extensions::Blacklist::BLACKLISTED_MALWARE) {
     // Installation of a blacklisted extension can happen from sync, policy,
     // etc, where to maintain consistency we need to install it, just never
     // load it (see AddExtension). Usually it should be the job of callers to
@@ -3076,7 +3076,7 @@ void ExtensionService::OnNeedsToGarbageCollectIsolatedStorage() {
 }
 
 void ExtensionService::OnBlacklistUpdated() {
-  blacklist_->GetBlacklistedIDs(
+  blacklist_->GetMalwareIDs(
       GenerateInstalledExtensionsSet()->GetIDs(),
       base::Bind(&ExtensionService::ManageBlacklist, AsWeakPtr()));
 }
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index 3f77e82..4ec2b67 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -6441,7 +6441,7 @@ TEST_F(ExtensionServiceTest, InstallBlacklistedExtension) {
       extension.get(),
       syncer::StringOrdinal(),
       false /* has requirement errors */,
-      extensions::Blacklist::BLACKLISTED,
+      extensions::Blacklist::BLACKLISTED_MALWARE,
       false /* wait for idle */);
   base::RunLoop().RunUntilIdle();
 
diff --git a/chrome/browser/extensions/test_blacklist.cc b/chrome/browser/extensions/test_blacklist.cc
index 77b75b2..40dcbef 100644
--- a/chrome/browser/extensions/test_blacklist.cc
+++ b/chrome/browser/extensions/test_blacklist.cc
@@ -19,20 +19,19 @@ TestBlacklist::TestBlacklist(Blacklist* blacklist)
 
 namespace {
 
-void Assign(std::set<std::string>* out, const std::set<std::string>& in) {
+void Assign(Blacklist::BlacklistState *out, Blacklist::BlacklistState 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));
+Blacklist::BlacklistState TestBlacklist::GetBlacklistState(
+    const std::string& extension_id) {
+  Blacklist::BlacklistState blacklist_state;
+  blacklist_->IsBlacklisted(extension_id,
+                            base::Bind(&Assign, &blacklist_state));
   base::RunLoop().RunUntilIdle();
-  return blacklist_set.count(extension_id) > 0;
+  return blacklist_state;
 }
 
 }  // namespace extensions
diff --git a/chrome/browser/extensions/test_blacklist.h b/chrome/browser/extensions/test_blacklist.h
index a9b5690..9aa3041 100644
--- a/chrome/browser/extensions/test_blacklist.h
+++ b/chrome/browser/extensions/test_blacklist.h
@@ -8,11 +8,10 @@
 #include <string>
 
 #include "base/basictypes.h"
+#include "chrome/browser/extensions/blacklist.h"
 
 namespace extensions {
 
-class Blacklist;
-
 // A wrapper for an extensions::Blacklist that provides functionality for
 // testing.
 class TestBlacklist {
@@ -21,7 +20,7 @@ class TestBlacklist {
 
   Blacklist* blacklist() { return blacklist_; }
 
-  bool IsBlacklisted(const std::string& extension_id);
+  Blacklist::BlacklistState GetBlacklistState(const std::string& extension_id);
 
  private:
   Blacklist* blacklist_;