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

components/safe_browsing_db/v4_get_hash_protocol_manager.cc

Index: components/safe_browsing_db/v4_get_hash_protocol_manager.cc
diff --git a/components/safe_browsing_db/v4_get_hash_protocol_manager.cc b/components/safe_browsing_db/v4_get_hash_protocol_manager.cc
index 4b5f911d3aea69310fb8be800b29b7dff126aac1..5de4445c57b50cac4206f2a2fcf0e97c4909238b 100644
--- a/components/safe_browsing_db/v4_get_hash_protocol_manager.cc
+++ b/components/safe_browsing_db/v4_get_hash_protocol_manager.cc
@@ -298,6 +298,34 @@ bool V4GetHashProtocolManager::ParseHashResponse(
 }
 
 void V4GetHashProtocolManager::GetFullHashes(
+    const base::hash_set<HashPrefix>& prefixes,
+    const PlatformTypeSet& platforms,
+    const ThreatEntryTypeSet& threat_entry_types,
+    const ThreatTypeSet& threat_types,
+    FullHashCallback callback) {

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

I think these are in the opposite order from the header?

I think having the function overloaded has strong potential for confusion. 
IMHO, you should probably just rename the V3 version to GetFullHashesV3 or
something, which will make it clear which call sites need conversion.  Yeah, it
means you'll touch more files than you might like, but *shrug*, you'll have to
touch them eventually.

+  DCHECK(CalledOnValidThread());
+  DCHECK(!prefixes.empty());
+  DCHECK(!platforms.empty());
+  DCHECK(!threat_entry_types.empty());
+  DCHECK(!threat_types.empty());
+
+  // TODO(vakh): The following code currently relies on the PVer3
+  // implementation but should not. This will be fixed after
+  // http://crbug.com/616647 is fixed.
+  std::vector<SBPrefix> pver3_prefixes;
+  for (const auto& prefix : prefixes) {
+    // This code assumes 4-byte hash prefixes but PVer4 prefixes can be longer.
+    SBPrefix pver3_prefix =
+        prefix[0] | prefix[1] << 8 | prefix[2] << 16 | prefix[3] << 24;
+    pver3_prefixes.push_back(pver3_prefix);
+  }
+  std::vector<PlatformType> platforms_list(platforms.begin(), platforms.end());
+
+  GetFullHashes(pver3_prefixes, platforms_list, *(threat_types.begin()),
+                callback);

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

I'm a tiny bit nervous about DCHECK(!thread_types.empty()) plus
*(thread_types.begin()).  If someone breaks the contract in production, then
that becomes an undefined something.  Upgrading to a CHECK() would make sense to
me (though in the long term DCHECK makes more sense).

+}
+
+void V4GetHashProtocolManager::GetFullHashes(
     const std::vector<SBPrefix>& prefixes,
     const std::vector<PlatformType>& platforms,
     ThreatType threat_type,