V4LDBM: Get response from GetHashManager, detect severest result

components/safe_browsing_db/v4_local_database_manager.h

Index: components/safe_browsing_db/v4_local_database_manager.h
diff --git a/components/safe_browsing_db/v4_local_database_manager.h b/components/safe_browsing_db/v4_local_database_manager.h
index a6f3b826fcc0b1e9b42cb832c3d22dcd983202d4..a06838162b0d68c5b66d8af7b57b926f6226177c 100644
--- a/components/safe_browsing_db/v4_local_database_manager.h
+++ b/components/safe_browsing_db/v4_local_database_manager.h
@@ -22,10 +22,49 @@ using content::ResourceType;
 
 namespace safe_browsing {
 
+typedef unsigned ThreatSeverity;
+
 // Manages the local, on-disk database of updates downloaded from the
 // SafeBrowsing service and interfaces with the protocol manager.
 class V4LocalDatabaseManager : public SafeBrowsingDatabaseManager {
  public:
+  enum class ClientCallbackType {
+    // This represents the case when we're trying to determine if a URL is
+    // unsafe from the following perspectives: Malware, Phishing, UwS.
+    CHECK_BROWSE_URL = 0,
+
+    // This should always be the last value.
+    CHECK_MAX
+  };
+
+  // The information we need to return the response to the SafeBrowsing client
+  // that asked for the safety reputation of a URL if we can't determine that
+  // synchronously.

[Nathan Parker, 2016/09/20 18:43:38]

Can you add a todo or note that this will be extended to manage additional data
types needed by the other callback types?

This union/reuse of the same struct for different purposes is a pattern I'm not
thrilled with (it is used in the existing pver3 db), but I don't know of a
strong alternative.  Maybe it'd help to document which fields are used for which
callback types, when you add others.

[vakh (use Gerrit instead), 2016/09/20 20:51:28]

Done.
I'm not thrilled either. One way would be to have a base class that's extended
for each Check* method but that seems like an overkill. Definitely not something
I would like to do before launch.
It might be more useful to add those details/comments when this code starts
working for other methods.

+  struct PendingCheck {
+    PendingCheck(Client* client,
+                 ClientCallbackType client_callback_type,
+                 const GURL& url);
+
+    ~PendingCheck();
+
+    // The SafeBrowsing client that's waiting for the safe/unsafe verdict.
+    Client* client;
+
+    // Determines which funtion from the |client| needs to be called once we
+    // know whether the URL in |url| is safe or unsafe.
+    ClientCallbackType client_callback_type;
+
+    // The URL that are being checked for being unsafe.

[Nathan Parker, 2016/09/20 18:43:37]

nit: s/are/is

[vakh (use Gerrit instead), 2016/09/20 20:51:28]

Done.

+    GURL url;
+
+    // The metadata associated with the full hash of the severest match found
+    // for that URL.
+    ThreatMetadata url_metadata;
+
+    // The threat verdict for the URL being checked.
+    SBThreatType result_threat_type;
+  };
+
   // Construct V4LocalDatabaseManager.
   // Must be initialized by calling StartOnIOThread() before using.
   V4LocalDatabaseManager(const base::FilePath& base_path);
@@ -68,6 +107,11 @@ class V4LocalDatabaseManager : public SafeBrowsingDatabaseManager {
       const scoped_refptr<base::SequencedTaskRunner>& task_runner) {
     task_runner_ = task_runner;
   }
+  FRIEND_TEST_ALL_PREFIXES(V4LocalDatabaseManagerTest,
+                           TestGetSeverestThreatTypeAndMetadata);
+
+  // The set of clients awaiting a full hash response.
+  typedef std::unordered_set<Client*> PendingClients;
 
   ~V4LocalDatabaseManager() override;
 
@@ -82,24 +126,39 @@ class V4LocalDatabaseManager : public SafeBrowsingDatabaseManager {
 
   void SetupDatabase();
 
-  void OnFullHashResponse(const std::vector<FullHashInfo>& full_hash_infos);
+  // Called when the |v4_get_hash_protocol_manager_| has the full hash response
+  // avaialble 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);
 
+  // Called when all the stores managed by the database have been read from
+  // disk after startup and the database is ready for use.
   void DatabaseReady(std::unique_ptr<V4Database> v4_database);
 
   // Called when the database has been updated and schedules the next update.
   void DatabaseUpdated();
 
+  // Calls the appopriate method on the |client| object, based on the contents
+  // 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_;
 
   // Whether the service is running.
   bool enabled_;
 
-  // Stores the current status of the lists to download from the SafeBrowsing
-  // servers.
-  // TODO(vakh): current_list_states_ doesn't really belong here.
-  // It should come through the database, from the various V4Stores.
-  base::hash_map<UpdateListIdentifier, std::string> current_list_states_;
+  // The set of clients that are waiting for a full hash response from the
+  // SafeBrowsing service.

[Nathan Parker, 2016/09/20 18:43:38]

Add what it's used for: For tracking when a client has canceled its request

[vakh (use Gerrit instead), 2016/09/20 20:51:28]

Done.

+  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.