1. Store list information in ListInfo (was: StoreIdAndFIleName). This includes...

1. Store list information in ListInfo (was: StoreIdAndFIleName). This includes the list identifier, list filename on disk, the SBThreatType for the list, and whether to fetch updates for this list.

2. Fixed potential leaks by inserting the client into pending_clients_ before calling GetFullHashes
(based on shess@'s feedback in 2349603003)

BUG=543161, 608075

Review-Url: https://codereview.chromium.org/2353413002
Cr-Commit-Position: refs/heads/master@{#420515}

chrome/browser/extensions/BUILD.gn

@@ -816,6 +816,7 @@ static_library("extensions") {
   # browser, then we can clean up these dependencies.
   public_deps = [
     "//chrome/common/extensions/api",
+    "//components/safe_browsing_db:util",
     "//content/public/browser",
   ]
   deps = [
@@ -868,7 +869,6 @@ static_library("extensions") {
     "//components/rappor",
     "//components/resources",
     "//components/safe_browsing_db:database_manager",
-    "//components/safe_browsing_db:util",
     "//components/safe_json",
     "//components/search_engines",
     "//components/sessions",

components/safe_browsing_db/BUILD.gn

@@ -154,8 +154,10 @@ static_library("util") {
     "util.cc",
     "util.h",
   ]
-  deps = [
+  public_deps = [
     ":v4_protocol_manager_util",
+  ]
+  deps = [
     "//base",
     "//crypto",
     "//net",
@@ -226,8 +228,10 @@ source_set("v4_protocol_manager_util") {
     "v4_protocol_manager_util.cc",
     "v4_protocol_manager_util.h",
   ]
-  deps = [
+  public_deps = [
     ":safebrowsing_proto",
+  ]
+  deps = [
     "//base",
     "//net",
     "//url",

components/safe_browsing_db/database_manager_unittest.cc

@@ -78,9 +78,9 @@ class SafeBrowsingDatabaseManagerTest : public testing::Test {
     FindFullHashesResponse response;
     response.mutable_negative_cache_duration()->set_seconds(600);
     ThreatMatch* m = response.add_matches();
-    m->set_platform_type(list_id.platform_type);
-    m->set_threat_entry_type(list_id.threat_entry_type);
-    m->set_threat_type(list_id.threat_type);
+    m->set_platform_type(list_id.platform_type());
+    m->set_threat_entry_type(list_id.threat_entry_type());
+    m->set_threat_type(list_id.threat_type());
     m->mutable_threat()->set_hash(full_hash);
     m->mutable_cache_duration()->set_seconds(300);
 

components/safe_browsing_db/util.cc

@@ -8,7 +8,6 @@
 
 #include "base/macros.h"
 #include "base/trace_event/trace_event.h"
-#include "components/safe_browsing_db/v4_protocol_manager_util.h"
 #include "crypto/sha2.h"
 #include "net/base/escape.h"
 #include "url/gurl.h"

components/safe_browsing_db/util.h

@@ -16,47 +16,12 @@
 
 #include "base/strings/string_piece.h"
 #include "base/time/time.h"
+#include "components/safe_browsing_db/v4_protocol_manager_util.h"
 
 class GURL;
 
 namespace safe_browsing {
 
-// Different types of threats that SafeBrowsing protects against.
-enum SBThreatType {
-  // No threat at all.
-  SB_THREAT_TYPE_SAFE,
-
-  // The URL is being used for phishing.
-  SB_THREAT_TYPE_URL_PHISHING,
-
-  // The URL hosts malware.
-  SB_THREAT_TYPE_URL_MALWARE,
-
-  // The URL hosts unwanted programs.
-  SB_THREAT_TYPE_URL_UNWANTED,
-
-  // The download URL is malware.
-  SB_THREAT_TYPE_BINARY_MALWARE_URL,
-
-  // Url detected by the client-side phishing model.  Note that unlike the
-  // above values, this does not correspond to a downloaded list.
-  SB_THREAT_TYPE_CLIENT_SIDE_PHISHING_URL,
-
-  // The Chrome extension or app (given by its ID) is malware.
-  SB_THREAT_TYPE_EXTENSION,
-
-  // Url detected by the client-side malware IP list. This IP list is part
-  // of the client side detection model.
-  SB_THREAT_TYPE_CLIENT_SIDE_MALWARE_URL,
-
-  // Url leads to a blacklisted resource script. Note that no warnings should be
-  // shown on this threat type, but an incident report might be sent.
-  SB_THREAT_TYPE_BLACKLISTED_RESOURCE,
-
-  // Url abuses a permission API.
-  SB_THREAT_TYPE_API_ABUSE,
-};
-
 // Metadata that indicates what kind of URL match this is.
 enum class ThreatPatternType {
   NONE = 0,                        // Pattern type didn't appear in the metadata

components/safe_browsing_db/v4_database.cc

@@ -23,24 +23,24 @@ V4StoreFactory* V4Database::factory_ = NULL;
 void V4Database::Create(
     const scoped_refptr<base::SequencedTaskRunner>& db_task_runner,
     const base::FilePath& base_path,
-    const StoreIdAndFileNames& store_id_file_names,
+    const ListInfos& list_infos,
     NewDatabaseReadyCallback new_db_callback) {
   DCHECK(base_path.IsAbsolute());
-  DCHECK(!store_id_file_names.empty());
+  DCHECK(!list_infos.empty());
 
   const scoped_refptr<base::SingleThreadTaskRunner>& callback_task_runner =
       base::MessageLoop::current()->task_runner();
   db_task_runner->PostTask(
       FROM_HERE,
       base::Bind(&V4Database::CreateOnTaskRunner, db_task_runner, base_path,
-                 store_id_file_names, callback_task_runner, new_db_callback));
+                 list_infos, callback_task_runner, new_db_callback));
 }
 
 // static
 void V4Database::CreateOnTaskRunner(
     const scoped_refptr<base::SequencedTaskRunner>& db_task_runner,
     const base::FilePath& base_path,
-    const StoreIdAndFileNames& store_id_file_names,
+    const ListInfos& list_infos,
     const scoped_refptr<base::SingleThreadTaskRunner>& callback_task_runner,
     NewDatabaseReadyCallback new_db_callback) {
   DCHECK(db_task_runner->RunsTasksOnCurrentThread());
@@ -55,9 +55,14 @@ void V4Database::CreateOnTaskRunner(
   }
 
   std::unique_ptr<StoreMap> store_map = base::MakeUnique<StoreMap>();
-  for (const auto& it : store_id_file_names) {
-    const base::FilePath store_path = base_path.AppendASCII(it.filename);
-    (*store_map)[it.list_id].reset(
+  for (const auto& it : list_infos) {
+    if (!it.fetch_updates()) {
+      // This list doesn't need to be fetched or stored on disk.
+      continue;
+    }
+
+    const base::FilePath store_path = base_path.AppendASCII(it.filename());
+    (*store_map)[it.list_id()].reset(
         factory_->CreateV4Store(db_task_runner, store_path));
   }
   std::unique_ptr<V4Database> v4_database(
@@ -184,12 +189,18 @@ void V4Database::GetStoresMatchingFullHash(
   }
 }
 
-StoreIdAndFileName::StoreIdAndFileName(const ListIdentifier& list_id,
-                                       const std::string& filename)
-    : list_id(list_id), filename(filename) {
-  DCHECK(!filename.empty());
+ListInfo::ListInfo(const bool fetch_updates,
+                   const std::string& filename,
+                   const ListIdentifier& list_id,
+                   const SBThreatType sb_threat_type)
+    : fetch_updates_(fetch_updates),
+      filename_(filename),
+      list_id_(list_id),
+      sb_threat_type_(sb_threat_type) {
+  DCHECK(!fetch_updates_ || !filename_.empty());
+  DCHECK_NE(SB_THREAT_TYPE_SAFE, sb_threat_type_);
 }
 
-StoreIdAndFileName::~StoreIdAndFileName() {}
+ListInfo::~ListInfo() {}
 
 }  // namespace safe_browsing

components/safe_browsing_db/v4_database.h

@@ -30,27 +30,39 @@ typedef base::Callback<void()> DatabaseUpdatedCallback;
 // storage on disk.
 typedef base::hash_map<ListIdentifier, std::unique_ptr<V4Store>> StoreMap;
 
-// TODO(vakh): Find the canonical place where these are defined and update the
-// comment to point to that place.
-struct StoreIdAndFileName {
-  // The list being read from/written to the disk.
-  ListIdentifier list_id;
+// Associates metadata for a list with its ListIdentifier.
+struct ListInfo {
+  ListInfo(const bool fetch_updates,
+           const std::string& filename,
+           const ListIdentifier& list_id,
+           const SBThreatType sb_threat_type);
+  ~ListInfo();
+
+  ListIdentifier list_id() const { return list_id_; }
+  std::string filename() const { return filename_; }
+  SBThreatType sb_threat_type() const { return sb_threat_type_; }
+  bool fetch_updates() const { return fetch_updates_; }
+
+ private:
+  // Whether to fetch and store updates for this list.
+  bool fetch_updates_;
 
   // The ASCII name of the file on disk. This file is created inside the
   // user-data directory. For instance, the ListIdentifier could be for URL
   // expressions for UwS on Windows platform, and the corresponding file on disk
   // could be named: "UrlUws.store"
-  std::string filename;
+  std::string filename_;
 
-  StoreIdAndFileName(const ListIdentifier& list_id,
-                     const std::string& filename);
-  ~StoreIdAndFileName();
+  // The list being read from/written to the disk.
+  ListIdentifier list_id_;
 
- private:
-  StoreIdAndFileName();
+  // The threat type enum value for this store.
+  SBThreatType sb_threat_type_;
+
+  ListInfo();
 };
 
-using StoreIdAndFileNames = std::vector<StoreIdAndFileName>;
+typedef std::vector<ListInfo> ListInfos;
 
 // Factory for creating V4Database. Tests implement this factory to create fake
 // databases for testing.
@@ -60,7 +72,7 @@ class V4DatabaseFactory {
   virtual V4Database* CreateV4Database(
       const scoped_refptr<base::SequencedTaskRunner>& db_task_runner,
       const base::FilePath& base_dir_path,
-      const StoreIdAndFileNames& store_id_file_names) = 0;
+      const ListInfos& list_infos) = 0;
 };
 
 // The on-disk databases are shared among all profiles, as it doesn't contain
@@ -79,7 +91,7 @@ class V4Database {
   static void Create(
       const scoped_refptr<base::SequencedTaskRunner>& db_task_runner,
       const base::FilePath& base_path,
-      const StoreIdAndFileNames& store_id_file_names,
+      const ListInfos& list_infos,
       NewDatabaseReadyCallback callback);
 
   // Destroys the provided v4_database on its task_runner since this may be a
@@ -133,7 +145,7 @@ class V4Database {
   static void CreateOnTaskRunner(
       const scoped_refptr<base::SequencedTaskRunner>& db_task_runner,
       const base::FilePath& base_path,
-      const StoreIdAndFileNames& store_id_file_names,
+      const ListInfos& list_infos,
       const scoped_refptr<base::SingleThreadTaskRunner>& callback_task_runner,
       NewDatabaseReadyCallback callback);
 

components/safe_browsing_db/v4_database_unittest.cc

@@ -105,12 +105,14 @@ class V4DatabaseTest : public PlatformTest {
   }
 
   void SetupInfoMapAndExpectedState() {
-    store_id_file_names_.emplace_back(win_malware_id_, "win_url_malware");
+    list_infos_.emplace_back(true, "win_url_malware", win_malware_id_,
+                             SB_THREAT_TYPE_URL_MALWARE);
     expected_identifiers_.push_back(win_malware_id_);
     expected_store_paths_.push_back(
         database_dirname_.AppendASCII("win_url_malware.fake"));
 
-    store_id_file_names_.emplace_back(linux_malware_id_, "linux_url_malware");
+    list_infos_.emplace_back(true, "linux_url_malware", linux_malware_id_,
+                             SB_THREAT_TYPE_URL_MALWARE);
     expected_identifiers_.push_back(linux_malware_id_);
     expected_store_paths_.push_back(
         database_dirname_.AppendASCII("linux_url_malware.fake"));
@@ -151,9 +153,9 @@ class V4DatabaseTest : public PlatformTest {
     for (const auto& store_state_iter : store_state_map) {
       ListIdentifier identifier = store_state_iter.first;
       ListUpdateResponse* lur = new ListUpdateResponse;
-      lur->set_platform_type(identifier.platform_type);
-      lur->set_threat_entry_type(identifier.threat_entry_type);
-      lur->set_threat_type(identifier.threat_type);
+      lur->set_platform_type(identifier.platform_type());
+      lur->set_threat_entry_type(identifier.threat_entry_type());
+      lur->set_threat_type(identifier.threat_type());
       lur->set_new_client_state(store_state_iter.second);
       if (use_valid_response_type) {
         lur->set_response_type(ListUpdateResponse::FULL_UPDATE);
@@ -200,7 +202,7 @@ class V4DatabaseTest : public PlatformTest {
   content::TestBrowserThreadBundle thread_bundle_;
   bool created_but_not_called_back_;
   bool created_and_called_back_;
-  StoreIdAndFileNames store_id_file_names_;
+  ListInfos list_infos_;
   std::vector<ListIdentifier> expected_identifiers_;
   std::vector<base::FilePath> expected_store_paths_;
   bool expected_resets_successfully_;
@@ -217,7 +219,7 @@ TEST_F(V4DatabaseTest, TestSetupDatabaseWithFakeStores) {
   expected_resets_successfully_ = true;
   RegisterFactory(!expected_resets_successfully_);
 
-  V4Database::Create(task_runner_, database_dirname_, store_id_file_names_,
+  V4Database::Create(task_runner_, database_dirname_, list_infos_,
                      callback_db_ready_);
   created_but_not_called_back_ = true;
   task_runner_->RunPendingTasks();
@@ -231,7 +233,7 @@ TEST_F(V4DatabaseTest, TestSetupDatabaseWithFakeStoresFailsReset) {
   expected_resets_successfully_ = false;
   RegisterFactory(!expected_resets_successfully_);
 
-  V4Database::Create(task_runner_, database_dirname_, store_id_file_names_,
+  V4Database::Create(task_runner_, database_dirname_, list_infos_,
                      callback_db_ready_);
   created_but_not_called_back_ = true;
   task_runner_->RunPendingTasks();
@@ -245,7 +247,7 @@ TEST_F(V4DatabaseTest, TestApplyUpdateWithNewStates) {
   expected_resets_successfully_ = true;
   RegisterFactory(!expected_resets_successfully_);
 
-  V4Database::Create(task_runner_, database_dirname_, store_id_file_names_,
+  V4Database::Create(task_runner_, database_dirname_, list_infos_,
                      callback_db_ready_);
   created_but_not_called_back_ = true;
   task_runner_->RunPendingTasks();
@@ -276,7 +278,7 @@ TEST_F(V4DatabaseTest, TestApplyUpdateWithNoNewState) {
   expected_resets_successfully_ = true;
   RegisterFactory(!expected_resets_successfully_);
 
-  V4Database::Create(task_runner_, database_dirname_, store_id_file_names_,
+  V4Database::Create(task_runner_, database_dirname_, list_infos_,
                      callback_db_ready_);
   created_but_not_called_back_ = true;
   task_runner_->RunPendingTasks();
@@ -307,7 +309,7 @@ TEST_F(V4DatabaseTest, TestApplyUpdateWithEmptyUpdate) {
   expected_resets_successfully_ = true;
   RegisterFactory(!expected_resets_successfully_);
 
-  V4Database::Create(task_runner_, database_dirname_, store_id_file_names_,
+  V4Database::Create(task_runner_, database_dirname_, list_infos_,
                      callback_db_ready_);
   created_but_not_called_back_ = true;
   task_runner_->RunPendingTasks();
@@ -339,7 +341,7 @@ TEST_F(V4DatabaseTest, TestApplyUpdateWithInvalidUpdate) {
   expected_resets_successfully_ = true;
   RegisterFactory(!expected_resets_successfully_);
 
-  V4Database::Create(task_runner_, database_dirname_, store_id_file_names_,
+  V4Database::Create(task_runner_, database_dirname_, list_infos_,
                      callback_db_ready_);
   created_but_not_called_back_ = true;
   task_runner_->RunPendingTasks();
@@ -370,7 +372,7 @@ TEST_F(V4DatabaseTest, TestAllStoresMatchFullHash) {
   expected_resets_successfully_ = true;
   RegisterFactory(!expected_resets_successfully_, hash_prefix_matches);
 
-  V4Database::Create(task_runner_, database_dirname_, store_id_file_names_,
+  V4Database::Create(task_runner_, database_dirname_, list_infos_,
                      callback_db_ready_);
   created_but_not_called_back_ = true;
   task_runner_->RunPendingTasks();
@@ -397,7 +399,7 @@ TEST_F(V4DatabaseTest, TestNoStoreMatchesFullHash) {
   expected_resets_successfully_ = true;
   RegisterFactory(!expected_resets_successfully_, hash_prefix_matches);
 
-  V4Database::Create(task_runner_, database_dirname_, store_id_file_names_,
+  V4Database::Create(task_runner_, database_dirname_, list_infos_,
                      callback_db_ready_);
   created_but_not_called_back_ = true;
   task_runner_->RunPendingTasks();
@@ -420,7 +422,7 @@ TEST_F(V4DatabaseTest, TestSomeStoresMatchFullHash) {
   expected_resets_successfully_ = true;
   RegisterFactory(!expected_resets_successfully_, hash_prefix_matches);
 
-  V4Database::Create(task_runner_, database_dirname_, store_id_file_names_,
+  V4Database::Create(task_runner_, database_dirname_, list_infos_,
                      callback_db_ready_);
   created_but_not_called_back_ = true;
   task_runner_->RunPendingTasks();
@@ -451,7 +453,7 @@ TEST_F(V4DatabaseTest, TestSomeStoresMatchFullHashBecauseOfStoresToMatch) {
   expected_resets_successfully_ = true;
   RegisterFactory(!expected_resets_successfully_, hash_prefix_matches);
 
-  V4Database::Create(task_runner_, database_dirname_, store_id_file_names_,
+  V4Database::Create(task_runner_, database_dirname_, list_infos_,
                      callback_db_ready_);
   created_but_not_called_back_ = true;
   task_runner_->RunPendingTasks();

components/safe_browsing_db/v4_get_hash_protocol_manager.cc

@@ -240,9 +240,9 @@ V4GetHashProtocolManager::V4GetHashProtocolManager(
       clock_(new base::DefaultClock()) {
   DCHECK(!stores_to_request.empty());
   for (const ListIdentifier& store : stores_to_request) {
-    platform_types_.insert(store.platform_type);
-    threat_entry_types_.insert(store.threat_entry_type);
-    threat_types_.insert(store.threat_type);
+    platform_types_.insert(store.platform_type());
+    threat_entry_types_.insert(store.threat_entry_type());
+    threat_types_.insert(store.threat_type());
   }
 }
 

components/safe_browsing_db/v4_get_hash_protocol_manager_unittest.cc

@@ -140,9 +140,9 @@ class V4GetHashProtocolManagerTest : public PlatformTest {
     res.mutable_negative_cache_duration()->set_seconds(600);
     for (const ResponseInfo& info : response_infos) {
       ThreatMatch* m = res.add_matches();
-      m->set_platform_type(info.list_id.platform_type);
-      m->set_threat_entry_type(info.list_id.threat_entry_type);
-      m->set_threat_type(info.list_id.threat_type);
+      m->set_platform_type(info.list_id.platform_type());
+      m->set_threat_entry_type(info.list_id.threat_entry_type());
+      m->set_threat_type(info.list_id.threat_type());
       m->mutable_cache_duration()->set_seconds(300);
       m->mutable_threat()->set_hash(info.full_hash);
 

components/safe_browsing_db/v4_local_database_manager.cc

@@ -23,35 +23,19 @@ namespace {
 const ThreatSeverity kLeastSeverity =
     std::numeric_limits<ThreatSeverity>::max();
 
-// TODO(vakh): Implement this to populate the vector appopriately.
-// Filed as http://crbug.com/608075
-// Any stores added/removed to/from here likely need an update in
-// GetSBThreatTypeForList and GetThreatSeverity.
-// TODO(vakh): Add a compile-time check or DCHECK to enforce this.
-StoreIdAndFileNames GetStoreIdAndFileNames() {
-  return StoreIdAndFileNames(
-      {StoreIdAndFileName(GetUrlMalwareId(), "UrlMalware.store"),
-       StoreIdAndFileName(GetUrlSocEngId(), "UrlSoceng.store")});
-}
-
-// Returns the SBThreatType corresponding to a given SafeBrowsing list.
-SBThreatType GetSBThreatTypeForList(const ListIdentifier& list_id) {
-  if (list_id == GetChromeUrlApiId()) {
-    return SB_THREAT_TYPE_API_ABUSE;
-  } else if (list_id == GetUrlMalwareId()) {
-    return SB_THREAT_TYPE_URL_MALWARE;
-  } else if (list_id == GetUrlSocEngId()) {
-    return SB_THREAT_TYPE_URL_PHISHING;
-  } else {
-    NOTREACHED() << "Unknown list encountered in GetSBThreatTypeForList";
-    return SB_THREAT_TYPE_SAFE;
-  }
+ListInfos GetListInfos() {
+  return ListInfos(
+      {ListInfo(true, "UrlMalware.store", GetUrlMalwareId(),
+                SB_THREAT_TYPE_URL_MALWARE),
+       ListInfo(true, "UrlSoceng.store", GetUrlSocEngId(),
+                SB_THREAT_TYPE_URL_PHISHING),
+       ListInfo(false, "", GetChromeUrlApiId(), SB_THREAT_TYPE_API_ABUSE)});
 }
 
 // Returns the severity information about a given SafeBrowsing list. The lowest
 // value is 0, which represents the most severe list.
 ThreatSeverity GetThreatSeverity(const ListIdentifier& list_id) {
-  switch (list_id.threat_type) {
+  switch (list_id.threat_type()) {
     case MALWARE_THREAT:
     case SOCIAL_ENGINEERING_PUBLIC:
       return 0;
@@ -77,11 +61,9 @@ V4LocalDatabaseManager::PendingCheck::PendingCheck(
 V4LocalDatabaseManager::PendingCheck::~PendingCheck() {}
 
 V4LocalDatabaseManager::V4LocalDatabaseManager(const base::FilePath& base_path)
-    : base_path_(base_path),
-      enabled_(false),
-      store_id_file_names_(GetStoreIdAndFileNames()) {
+    : base_path_(base_path), enabled_(false), list_infos_(GetListInfos()) {
   DCHECK(!base_path_.empty());
-  DCHECK(!store_id_file_names_.empty());
+  DCHECK(!list_infos_.empty());
 
   DVLOG(1) << "V4LocalDatabaseManager::V4LocalDatabaseManager: "
            << "base_path_: " << base_path_.AsUTF8Unsafe();
@@ -220,13 +202,13 @@ bool V4LocalDatabaseManager::CheckBrowseUrl(const GURL& url, Client* client) {
           base::MakeUnique<PendingCheck>(
               client, ClientCallbackType::CHECK_BROWSE_URL, url);
 
+      pending_clients_.insert(client);
+
       v4_get_hash_protocol_manager_->GetFullHashes(
           full_hash_to_store_and_hash_prefixes,
           base::Bind(&V4LocalDatabaseManager::OnFullHashResponse,
                      base::Unretained(this), base::Passed(&pending_check)));
 
-      pending_clients_.insert(client);
-
       return false;
     }
   } else {
@@ -236,7 +218,6 @@ bool V4LocalDatabaseManager::CheckBrowseUrl(const GURL& url, Client* client) {
   }
 }
 
-// static
 void V4LocalDatabaseManager::GetSeverestThreatTypeAndMetadata(
     SBThreatType* result_threat_type,
     ThreatMetadata* metadata,
@@ -271,16 +252,10 @@ void V4LocalDatabaseManager::OnFullHashResponse(
     return;
   }
 
-  if (full_hash_infos.empty()) {
-    // The resource is not known to be unsafe. Respond right away.
-    RespondToClient(std::move(pending_check));
-    return;
-  }
-
+  // Find out the most severe threat, if any, to report to the client.
   GetSeverestThreatTypeAndMetadata(&pending_check->result_threat_type,
                                    &pending_check->url_metadata,
                                    full_hash_infos);
-
   RespondToClient(std::move(pending_check));
   pending_clients_.erase(it);
 }
@@ -331,7 +306,7 @@ void V4LocalDatabaseManager::SetupUpdateProtocolManager(
 
 void V4LocalDatabaseManager::SetupDatabase() {
   DCHECK(!base_path_.empty());
-  DCHECK(!store_id_file_names_.empty());
+  DCHECK(!list_infos_.empty());
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   // Only get a new task runner if there isn't one already. If the service has
@@ -347,8 +322,7 @@ void V4LocalDatabaseManager::SetupDatabase() {
   // has been created, swap it out on the IO thread.
   NewDatabaseReadyCallback db_ready_callback = base::Bind(
       &V4LocalDatabaseManager::DatabaseReady, base::Unretained(this));
-  V4Database::Create(task_runner_, base_path_, store_id_file_names_,
-                     db_ready_callback);
+  V4Database::Create(task_runner_, base_path_, list_infos_, db_ready_callback);
 }
 
 void V4LocalDatabaseManager::DatabaseReady(
@@ -404,11 +378,22 @@ void V4LocalDatabaseManager::DatabaseUpdated() {
   }
 }
 
+// Returns the SBThreatType corresponding to a given SafeBrowsing list.
+SBThreatType V4LocalDatabaseManager::GetSBThreatTypeForList(
+    const ListIdentifier& list_id) {
+  auto it = std::find_if(
+      std::begin(list_infos_), std::end(list_infos_),
+      [&list_id](ListInfo const& li) { return li.list_id() == list_id; });
+  DCHECK(list_infos_.end() != it);
+  DCHECK_NE(SB_THREAT_TYPE_SAFE, it->sb_threat_type());
+  return it->sb_threat_type();
+}
+
 std::unordered_set<ListIdentifier>
 V4LocalDatabaseManager::GetStoresForFullHashRequests() {
   std::unordered_set<ListIdentifier> stores_for_full_hash;
-  for (auto it : store_id_file_names_) {
-    stores_for_full_hash.insert(it.list_id);
+  for (auto it : list_infos_) {
+    stores_for_full_hash.insert(it.list_id());
   }
   return stores_for_full_hash;
 }

components/safe_browsing_db/v4_local_database_manager.h

@@ -117,6 +117,16 @@ class V4LocalDatabaseManager : public SafeBrowsingDatabaseManager {
 
   ~V4LocalDatabaseManager() override;
 
+  // Returns the SBThreatType for a given ListIdentifier.
+  SBThreatType GetSBThreatTypeForList(const ListIdentifier& list_id);
+
+  // Finds the most severe |SBThreatType| and the corresponding |metadata| from
+  // |full_hash_infos|.
+  void GetSeverestThreatTypeAndMetadata(
+      SBThreatType* result_threat_type,
+      ThreatMetadata* metadata,
+      const std::vector<FullHashInfo>& full_hash_infos);
+
   // The callback called each time the protocol manager downloads updates
   // successfully.
   void UpdateRequestCompleted(
@@ -129,7 +139,7 @@ class V4LocalDatabaseManager : public SafeBrowsingDatabaseManager {
   void SetupDatabase();
 
   // Called when the |v4_get_hash_protocol_manager_| has the full hash response
-  // avaialble for the URL that we requested. It determines the severest
+  // available for the URL that we requested. It determines the severest
   // threat type and responds to the |client| with that information.
   void OnFullHashResponse(std::unique_ptr<PendingCheck> pending_check,
                           const std::vector<FullHashInfo>& full_hash_infos);
@@ -145,13 +155,6 @@ class V4LocalDatabaseManager : public SafeBrowsingDatabaseManager {
   // of |pending_check|.
   void RespondToClient(std::unique_ptr<PendingCheck> pending_check);
 
-  // Finds the most severe |SBThreatType| and the corresponding |metadata| from
-  // |full_hash_infos|.
-  static void GetSeverestThreatTypeAndMetadata(
-      SBThreatType* result_threat_type,
-      ThreatMetadata* metadata,
-      const std::vector<FullHashInfo>& full_hash_infos);
-
   // The base directory under which to create the files that contain hashes.
   const base::FilePath base_path_;
 
@@ -162,9 +165,11 @@ class V4LocalDatabaseManager : public SafeBrowsingDatabaseManager {
   // SafeBrowsing service.
   PendingClients pending_clients_;
 
-  // The list of stores to manage (for hash prefixes and full hashes), along
-  // with the corresponding filename on disk for each of them.
-  StoreIdAndFileNames store_id_file_names_;
+  // The list of stores to manage (for hash prefixes and full hashes). Each
+  // element contains the identifier for the store, the corresponding
+  // SBThreatType, whether to fetch hash prefixes for that store, and the
+  // name of the file on disk that would contain the prefixes, if applicable.
+  ListInfos list_infos_;
 
   // The protocol manager that downloads the hash prefix updates.
   std::unique_ptr<V4UpdateProtocolManager> v4_update_protocol_manager_;

components/safe_browsing_db/v4_local_database_manager_unittest.cc

@@ -149,15 +149,15 @@ TEST_F(V4LocalDatabaseManagerTest, TestGetSeverestThreatTypeAndMetadata) {
   SBThreatType result_threat_type;
   ThreatMetadata metadata;
 
-  V4LocalDatabaseManager::GetSeverestThreatTypeAndMetadata(&result_threat_type,
-                                                           &metadata, fhis);
+  v4_local_database_manager_->GetSeverestThreatTypeAndMetadata(
+      &result_threat_type, &metadata, fhis);
   EXPECT_EQ(SB_THREAT_TYPE_URL_MALWARE, result_threat_type);
   EXPECT_EQ("malware_popid", metadata.population_id);
 
   // Reversing the list has no effect.
   std::reverse(std::begin(fhis), std::end(fhis));
-  V4LocalDatabaseManager::GetSeverestThreatTypeAndMetadata(&result_threat_type,
-                                                           &metadata, fhis);
+  v4_local_database_manager_->GetSeverestThreatTypeAndMetadata(
+      &result_threat_type, &metadata, fhis);
   EXPECT_EQ(SB_THREAT_TYPE_URL_MALWARE, result_threat_type);
   EXPECT_EQ("malware_popid", metadata.population_id);
 }

components/safe_browsing_db/v4_protocol_manager_util.cc

@@ -62,9 +62,9 @@ std::string Escape(const std::string& url) {
 }  // namespace
 
 std::ostream& operator<<(std::ostream& os, const ListIdentifier& id) {
-  os << "{hash: " << id.hash() << "; platform_type: " << id.platform_type
-     << "; threat_entry_type: " << id.threat_entry_type
-     << "; threat_type: " << id.threat_type << "}";
+  os << "{hash: " << id.hash() << "; platform_type: " << id.platform_type()
+     << "; threat_entry_type: " << id.threat_entry_type()
+     << "; threat_type: " << id.threat_type() << "}";
   return os;
 }
 
@@ -123,9 +123,9 @@ size_t StoreAndHashPrefix::hash() const {
 }
 
 bool ListIdentifier::operator==(const ListIdentifier& other) const {
-  return platform_type == other.platform_type &&
-         threat_entry_type == other.threat_entry_type &&
-         threat_type == other.threat_type;
+  return platform_type_ == other.platform_type_ &&
+         threat_entry_type_ == other.threat_entry_type_ &&
+         threat_type_ == other.threat_type_;
 }
 
 bool ListIdentifier::operator!=(const ListIdentifier& other) const {
@@ -133,9 +133,9 @@ bool ListIdentifier::operator!=(const ListIdentifier& other) const {
 }
 
 size_t ListIdentifier::hash() const {
-  std::size_t first = std::hash<unsigned int>()(platform_type);
-  std::size_t second = std::hash<unsigned int>()(threat_entry_type);
-  std::size_t third = std::hash<unsigned int>()(threat_type);
+  std::size_t first = std::hash<unsigned int>()(platform_type_);
+  std::size_t second = std::hash<unsigned int>()(threat_entry_type_);
+  std::size_t third = std::hash<unsigned int>()(threat_type_);
 
   std::size_t interim = base::HashInts(first, second);
   return base::HashInts(interim, third);
@@ -146,9 +146,9 @@ ListIdentifier::ListIdentifier() {}
 ListIdentifier::ListIdentifier(PlatformType platform_type,
                                ThreatEntryType threat_entry_type,
                                ThreatType threat_type)
-    : platform_type(platform_type),
-      threat_entry_type(threat_entry_type),
-      threat_type(threat_type) {
+    : platform_type_(platform_type),
+      threat_entry_type_(threat_entry_type),
+      threat_type_(threat_type) {
   DCHECK(PlatformType_IsValid(platform_type));
   DCHECK(ThreatEntryType_IsValid(threat_entry_type));
   DCHECK(ThreatType_IsValid(threat_type));

components/safe_browsing_db/v4_protocol_manager_util.h

@@ -63,6 +63,43 @@ struct V4ProtocolConfig {
   ~V4ProtocolConfig();
 };
 
+// Different types of threats that SafeBrowsing protects against. This is the
+// type that's returned to the clients of SafeBrowsing in Chromium.
+enum SBThreatType {
+  // No threat at all.
+  SB_THREAT_TYPE_SAFE,
+
+  // The URL is being used for phishing.
+  SB_THREAT_TYPE_URL_PHISHING,
+
+  // The URL hosts malware.
+  SB_THREAT_TYPE_URL_MALWARE,
+
+  // The URL hosts unwanted programs.
+  SB_THREAT_TYPE_URL_UNWANTED,
+
+  // The download URL is malware.
+  SB_THREAT_TYPE_BINARY_MALWARE_URL,
+
+  // Url detected by the client-side phishing model.  Note that unlike the
+  // above values, this does not correspond to a downloaded list.
+  SB_THREAT_TYPE_CLIENT_SIDE_PHISHING_URL,
+
+  // The Chrome extension or app (given by its ID) is malware.
+  SB_THREAT_TYPE_EXTENSION,
+
+  // Url detected by the client-side malware IP list. This IP list is part
+  // of the client side detection model.
+  SB_THREAT_TYPE_CLIENT_SIDE_MALWARE_URL,
+
+  // Url leads to a blacklisted resource script. Note that no warnings should be
+  // shown on this threat type, but an incident report might be sent.
+  SB_THREAT_TYPE_BLACKLISTED_RESOURCE,
+
+  // Url abuses a permission API.
+  SB_THREAT_TYPE_API_ABUSE,
+};
+
 // The information required to uniquely identify each list the client is
 // interested in maintaining and downloading from the SafeBrowsing servers.
 // For example, for digests of Malware binaries on Windows:
@@ -71,10 +108,6 @@ struct V4ProtocolConfig {
 // threat_type = MALWARE
 struct ListIdentifier {
  public:
-  PlatformType platform_type;
-  ThreatEntryType threat_entry_type;
-  ThreatType threat_type;
-
   ListIdentifier(PlatformType, ThreatEntryType, ThreatType);
   explicit ListIdentifier(const ListUpdateResponse&);
 
@@ -82,7 +115,15 @@ struct ListIdentifier {
   bool operator!=(const ListIdentifier& other) const;
   size_t hash() const;
 
+  PlatformType platform_type() const { return platform_type_; }
+  ThreatEntryType threat_entry_type() const { return threat_entry_type_; }
+  ThreatType threat_type() const { return threat_type_; }
+
  private:
+  PlatformType platform_type_;
+  ThreatEntryType threat_entry_type_;
+  ThreatType threat_type_;
+
   ListIdentifier();
 };
 

components/safe_browsing_db/v4_update_protocol_manager.cc

@@ -201,10 +201,10 @@ std::string V4UpdateProtocolManager::GetBase64SerializedUpdateRequestProto() {
     const auto& list_to_update = entry.first;
     const auto& state = entry.second;
     ListUpdateRequest* list_update_request = request.add_list_update_requests();
-    list_update_request->set_platform_type(list_to_update.platform_type);
+    list_update_request->set_platform_type(list_to_update.platform_type());
     list_update_request->set_threat_entry_type(
-        list_to_update.threat_entry_type);
-    list_update_request->set_threat_type(list_to_update.threat_type);
+        list_to_update.threat_entry_type());
+    list_update_request->set_threat_type(list_to_update.threat_type());
 
     if (!state.empty()) {
       list_update_request->set_state(state);