PVer4: Re-use the PVer3 implementation to get full hashes

components/safe_browsing_db/v4_local_database_manager.cc

Index: components/safe_browsing_db/v4_local_database_manager.cc
diff --git a/components/safe_browsing_db/v4_local_database_manager.cc b/components/safe_browsing_db/v4_local_database_manager.cc
index b5eb1b9afc0acfe461fe5b79eeb7d1e9df2636e3..8608c6b3a2daab5a7f84f983522105c3f5442583 100644
--- a/components/safe_browsing_db/v4_local_database_manager.cc
+++ b/components/safe_browsing_db/v4_local_database_manager.cc
@@ -10,6 +10,9 @@
 #include <vector>
 
 #include "base/callback.h"
+#include "components/safe_browsing_db/v4_get_hash_protocol_manager.h"
+#include "components/safe_browsing_db/v4_protocol_manager_util.h"
+#include "components/safe_browsing_db/v4_update_protocol_manager.h"
 #include "content/public/browser/browser_thread.h"
 
 using content::BrowserThread;
@@ -147,22 +150,39 @@ bool V4LocalDatabaseManager::CheckBrowseUrl(const GURL& url, Client* client) {
     base::hash_set<UpdateListIdentifier> stores_to_look(
         {GetUrlMalwareId(), GetUrlSocEngId()});
     base::hash_set<HashPrefix> matched_hash_prefixes;
-    base::hash_set<UpdateListIdentifier> matched_stores;
+    PlatformTypeSet matched_platforms;
+    ThreatEntryTypeSet matched_threat_entry_types;
+    ThreatTypeSet matched_threat_types;
     MatchedHashPrefixMap matched_hash_prefix_map;
     for (const auto& full_hash : full_hashes) {
       v4_database_->GetStoresMatchingFullHash(full_hash, stores_to_look,
                                               &matched_hash_prefix_map);
       for (const auto& matched_pair : matched_hash_prefix_map) {
-        matched_stores.insert(matched_pair.first);
         matched_hash_prefixes.insert(matched_pair.second);
+        matched_platforms.insert(matched_pair.first.platform_type);
+        matched_threat_entry_types.insert(matched_pair.first.threat_entry_type);
+        matched_threat_types.insert(matched_pair.first.threat_type);
       }
     }
 
-    DCHECK_EQ(matched_stores.empty(), matched_hash_prefixes.empty());
-
-    // TODO(vakh): Return false and fetch full hashes for the matching hash
-    // prefixes.
-    return matched_hash_prefixes.empty();
+    DCHECK_EQ(matched_hash_prefixes.empty(), matched_platforms.empty());
+    DCHECK_EQ(matched_hash_prefixes.empty(),
+              matched_threat_entry_types.empty());
+    DCHECK_EQ(matched_hash_prefixes.empty(), matched_threat_types.empty());

[Scott Hess - ex-Googler, 2016/08/23 20:25:46]

Since they all have to match, I'd pick the shortest one as canonical.  Maybe it
would let you get away without the middle line being wrapped :-).

Alternately, since the next line checks for a specific empty(), the if() and
else could each have appropriate DCHECK() statements for empty() or !empty(). 
That would cost 4 lines, which is your current count.

+    if (matched_hash_prefixes.empty()) {
+      // No matching hash prefixes in the database, URL is good.
+      return true;
+    } else {
+      // Request full hashes.
+      // TODO(vakh): Record the information needed to respond to the client
+      // when the full hash check returns.
+      v4_get_hash_protocol_manager_->GetFullHashes(
+          matched_hash_prefixes, matched_platforms, matched_threat_entry_types,
+          matched_threat_types,
+          base::Bind(&V4LocalDatabaseManager::HandleGetHashes,
+                     base::Unretained(this)));
+      return false;
+    }
   } else {
     // TODO(vakh): Queue the check and process it when the database becomes
     // ready.
@@ -170,6 +190,14 @@ bool V4LocalDatabaseManager::CheckBrowseUrl(const GURL& url, Client* client) {
   }
 }
 
+void V4LocalDatabaseManager::HandleGetHashes(
+    const std::vector<SBFullHashResult>& full_hash_results,
+    const base::Time& negative_cache_expire) {
+  // TODO(vakh): Implement this skeleton.
+  // Ideally this should get a hash_set of FullHash objects. That will be done
+  // after http://crbug.com/616647 is fixed.
+}
+
 void V4LocalDatabaseManager::CancelCheck(Client* client) {
   // TODO(vakh): Implement this skeleton.
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -186,6 +214,8 @@ void V4LocalDatabaseManager::StartOnIOThread(
 
   SetupUpdateProtocolManager(request_context_getter, config);
 
+  SetupGetHashProtocolManager(request_context_getter, config);
+
   SetupDatabase();
 
   enabled_ = true;
@@ -201,6 +231,13 @@ void V4LocalDatabaseManager::SetupUpdateProtocolManager(
       V4UpdateProtocolManager::Create(request_context_getter, config, callback);
 }
 
+void V4LocalDatabaseManager::SetupGetHashProtocolManager(
+    net::URLRequestContextGetter* request_context_getter,
+    const V4ProtocolConfig& config) {
+  v4_get_hash_protocol_manager_.reset(
+      V4GetHashProtocolManager::Create(request_context_getter, config));
+}
+
 void V4LocalDatabaseManager::SetupDatabase() {
   DCHECK(!base_path_.empty());
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -252,6 +289,10 @@ void V4LocalDatabaseManager::StopOnIOThread(bool shutdown) {
   // and doesn't block the IO thread.
   V4Database::Destroy(std::move(v4_database_));
 
+  // Delete the V4GetHashProtocolManager.
+  // This cancels any in-flight get-hash request.
+  v4_get_hash_protocol_manager_.reset();
+
   // Delete the V4UpdateProtocolManager.
   // This cancels any in-flight update request.
   v4_update_protocol_manager_.reset();