Move full hash caching logic to v4_get_hash_protocol_manager

components/safe_browsing_db/v4_get_hash_protocol_manager.h

Index: components/safe_browsing_db/v4_get_hash_protocol_manager.h
diff --git a/components/safe_browsing_db/v4_get_hash_protocol_manager.h b/components/safe_browsing_db/v4_get_hash_protocol_manager.h
index c80f2da773f2d1489438d41e85060667c1b4f8f6..d92769d5f98ba5715564aa052f665559f7b35e93 100644
--- a/components/safe_browsing_db/v4_get_hash_protocol_manager.h
+++ b/components/safe_browsing_db/v4_get_hash_protocol_manager.h
@@ -83,6 +83,15 @@ class V4GetHashProtocolManager : public net::URLFetcherDelegate,
   // Overrides the clock used to check the time.
   void SetClockForTests(std::unique_ptr<base::Clock> clock);
 
+  // Looks up the cached results for |threat_type|. Fills |prefixes| with the
+  // prefixes that need a request. Fills |cached_results| with the cached
+  // results.
+  void GetFullHashCachedResults(const SBThreatType& threat_type,
+                                const std::vector<SBFullHash>& full_hashes,
+                                base::Time now,
+                                std::vector<SBPrefix>* prefixes,
+                                std::vector<SBFullHashResult>* cached_results);
+
  protected:
   // Constructs a V4GetHashProtocolManager that issues
   // network requests using |request_context_getter|.
@@ -90,24 +99,25 @@ class V4GetHashProtocolManager : public net::URLFetcherDelegate,
                            const V4ProtocolConfig& config);
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingV4GetHashProtocolManagerTest,
+  FRIEND_TEST_ALL_PREFIXES(V4GetHashProtocolManagerTest,
                            TestGetHashRequest);
-  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingV4GetHashProtocolManagerTest,
+  FRIEND_TEST_ALL_PREFIXES(V4GetHashProtocolManagerTest,
                            TestParseHashResponse);
-  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingV4GetHashProtocolManagerTest,
+  FRIEND_TEST_ALL_PREFIXES(V4GetHashProtocolManagerTest,
                            TestParseHashResponseWrongThreatEntryType);
-  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingV4GetHashProtocolManagerTest,
+  FRIEND_TEST_ALL_PREFIXES(V4GetHashProtocolManagerTest,
                            TestParseHashThreatPatternType);
-  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingV4GetHashProtocolManagerTest,
+  FRIEND_TEST_ALL_PREFIXES(V4GetHashProtocolManagerTest,
                            TestParseHashResponseNonPermissionMetadata);
-  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingV4GetHashProtocolManagerTest,
+  FRIEND_TEST_ALL_PREFIXES(V4GetHashProtocolManagerTest,
                            TestParseHashResponseInconsistentThreatTypes);
-  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingV4GetHashProtocolManagerTest,
+  FRIEND_TEST_ALL_PREFIXES(V4GetHashProtocolManagerTest,
                            TestGetHashErrorHandlingOK);
-  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingV4GetHashProtocolManagerTest,
+  FRIEND_TEST_ALL_PREFIXES(V4GetHashProtocolManagerTest,
                            TestGetHashErrorHandlingNetwork);
-  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingV4GetHashProtocolManagerTest,
+  FRIEND_TEST_ALL_PREFIXES(V4GetHashProtocolManagerTest,
                            TestGetHashErrorHandlingResponseCode);
+  FRIEND_TEST_ALL_PREFIXES(V4GetHashProtocolManagerTest, GetCachedResults);
   friend class V4GetHashProtocolManagerFactoryImpl;
 
   void GetHashUrlAndHeaders(const std::string& request_base64,
@@ -136,9 +146,20 @@ class V4GetHashProtocolManager : public net::URLFetcherDelegate,
   // the current time is |now|.
   void HandleGetHashError(const base::Time& now);
 
+  // TODO(vakh): Use HashPrefix and FullHash instead.
+  // TODO(vakh): Use base::hash_map instead of std::map
+  typedef std::map<SBPrefix, SBCachedFullHashResult> PrefixToFullHashResultsMap;
+
+  typedef std::map<SBThreatType, PrefixToFullHashResultsMap>
+      ThreatTypeToResultsMap;
+
+  ThreatTypeToResultsMap* v4_full_hash_cache(){

[Nathan Parker, 2016/08/15 19:54:47]

Does this accessor add value?  You could just make the member protected.

+    return &v4_full_hash_cache_;
+  }
+
  private:
   // Map of GetHash requests to parameters which created it.
-  typedef base::hash_map<const net::URLFetcher*, FullHashCallback> HashRequests;
+  typedef base::hash_map<const net::URLFetcher*, std::pair<std::vector<SBPrefix>, FullHashCallback>> PendingHashRequests;
 
   // The factory that controls the creation of V4GetHashProtocolManager.
   // This is used by tests.
@@ -156,7 +177,7 @@ class V4GetHashProtocolManager : public net::URLFetcherDelegate,
   // Multiplier for the backoff error after the second.
   size_t gethash_back_off_mult_;
 
-  HashRequests hash_requests_;
+  PendingHashRequests pending_hash_requests_;
 
   // For v4, the next gethash time is set to the backoff time is the last
   // response was an error, or the minimum wait time if the last response was
@@ -175,6 +196,9 @@ class V4GetHashProtocolManager : public net::URLFetcherDelegate,
   // The clock used to vend times.
   std::unique_ptr<base::Clock> clock_;
 
+  // A cache of V4 full hash results for api checks.

[Nathan Parker, 2016/08/15 19:54:47]

Is this just for API requests?

+  ThreatTypeToResultsMap v4_full_hash_cache_;
+
   DISALLOW_COPY_AND_ASSIGN(V4GetHashProtocolManager);
 };