Switch Safe Browsing's metadata from string to struct.

This is needed so we can add more types to it.  This CL plumbs through
the struct but keeps the string in-tact.  This is backward compatible with clank/'s current implementation.

I've added parsing methods in the appropriate place in the stack:
  - For Pver4: v4_get_hash_protocol_manager.cc
  - For XLB: safe_browsing_api_handler_util.cc
  - For Pver3: protocol_parser.cc [separate CL]

The next two CLs:
  - Switch clank/ to use the struct. https://chrome-internal-review.googlesource.com/#/c/249571/
  - Enable parsing for Pver3, switch ui_manager.cc to use the parsed values, and clean up.

BUG=589610

Review URL: https://codereview.chromium.org/1726403006

Cr-Commit-Position: refs/heads/master@{#378082}

12 files changed, 274 insertions, 56 deletions

diff --git a/components/safe_browsing_db/database_manager.h b/components/safe_browsing_db/database_manager.h
index 660a6ee..f5ba2db 100644
--- a/components/safe_browsing_db/database_manager.h
+++ b/components/safe_browsing_db/database_manager.h
@@ -43,7 +43,7 @@ class SafeBrowsingDatabaseManager
     // Called when the result of checking a browse URL is known.
     virtual void OnCheckBrowseUrlResult(const GURL& url,
                                         SBThreatType threat_type,
-                                        const std::string& metadata) {}
+                                        const ThreatMetadata& metadata) {}
 
     // Called when the result of checking a download URL is known.
     virtual void OnCheckDownloadUrlResult(const std::vector<GURL>& url_chain,
diff --git a/components/safe_browsing_db/remote_database_manager.cc b/components/safe_browsing_db/remote_database_manager.cc
index 74c6c0c..47d8062 100644
--- a/components/safe_browsing_db/remote_database_manager.cc
+++ b/components/safe_browsing_db/remote_database_manager.cc
@@ -42,9 +42,9 @@ class RemoteSafeBrowsingDatabaseManager::ClientRequest {
 
   static void OnRequestDoneWeak(const base::WeakPtr<ClientRequest>& req,
                                 SBThreatType matched_threat_type,
-                                const std::string& metadata);
+                                const ThreatMetadata& metadata);
   void OnRequestDone(SBThreatType matched_threat_type,
-                     const std::string& metadata);
+                     const ThreatMetadata& metadata);
 
   // Accessors
   Client* client() const { return client_; }
@@ -74,7 +74,7 @@ RemoteSafeBrowsingDatabaseManager::ClientRequest::ClientRequest(
 void RemoteSafeBrowsingDatabaseManager::ClientRequest::OnRequestDoneWeak(
     const base::WeakPtr<ClientRequest>& req,
     SBThreatType matched_threat_type,
-    const std::string& metadata) {
+    const ThreatMetadata& metadata) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (!req)
     return;  // Previously canceled
@@ -83,7 +83,7 @@ void RemoteSafeBrowsingDatabaseManager::ClientRequest::OnRequestDoneWeak(
 
 void RemoteSafeBrowsingDatabaseManager::ClientRequest::OnRequestDone(
     SBThreatType matched_threat_type,
-    const std::string& metadata) {
+    const ThreatMetadata& metadata) {
   DVLOG(1) << "OnRequestDone took " << timer_.Elapsed().InMilliseconds()
            << " ms for client " << client_ << " and URL " << url_;
   client_->OnCheckBrowseUrlResult(url_, matched_threat_type, metadata);
@@ -251,7 +251,7 @@ bool RemoteSafeBrowsingDatabaseManager::CheckBrowseUrl(const GURL& url,
   DVLOG(1) << "Checking for client " << client << " and URL " << url;
   SafeBrowsingApiHandler* api_handler = SafeBrowsingApiHandler::GetInstance();
   // This shouldn't happen since SafeBrowsingResourceThrottle checks
-  // IsSupported() ealier.
+  // IsSupported() earlier.
   DCHECK(api_handler) << "SafeBrowsingApiHandler was never constructed";
   api_handler->StartURLCheck(
       base::Bind(&ClientRequest::OnRequestDoneWeak, req->GetWeakPtr()), url,
@@ -298,7 +298,7 @@ void RemoteSafeBrowsingDatabaseManager::StopOnIOThread(bool shutdown) {
   std::vector<ClientRequest*> to_callback(current_requests_);
   for (auto req : to_callback) {
     DVLOG(1) << "Stopping: Invoking unfinished req for URL " << req->url();
-    req->OnRequestDone(SB_THREAT_TYPE_SAFE, std::string());
+    req->OnRequestDone(SB_THREAT_TYPE_SAFE, ThreatMetadata());
   }
   enabled_ = false;
 
diff --git a/components/safe_browsing_db/safe_browsing_api_handler.cc b/components/safe_browsing_db/safe_browsing_api_handler.cc
index fd3c3ce..178a35d 100644
--- a/components/safe_browsing_db/safe_browsing_api_handler.cc
+++ b/components/safe_browsing_db/safe_browsing_api_handler.cc
@@ -2,10 +2,25 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/bind.h"
 #include "components/safe_browsing_db/safe_browsing_api_handler.h"
 
 namespace safe_browsing {
 
+namespace {
+
+// TODO(nparker): Remove this as part of crbug/589610.
+void OnRequestDoneShim(
+    const SafeBrowsingApiHandler::URLCheckCallbackMeta& callback,
+    SBThreatType sb_threat_type,
+    const std::string& raw_metadata) {
+  ThreatMetadata metadata_struct;
+  metadata_struct.raw_metadata = raw_metadata;
+  callback.Run(sb_threat_type, metadata_struct);
+}
+
+}  // namespace
+
 SafeBrowsingApiHandler* SafeBrowsingApiHandler::instance_ = NULL;
 
 // static
@@ -18,4 +33,14 @@ SafeBrowsingApiHandler* SafeBrowsingApiHandler::GetInstance() {
   return instance_;
 }
 
+// TODO(nparker): Remove this as part of crbug/589610.
+// Default impl since clank/ code doesn't yet support this.
+void SafeBrowsingApiHandler::StartURLCheck(
+    const URLCheckCallbackMeta& callback,
+    const GURL& url,
+    const std::vector<SBThreatType>& threat_types) {
+  URLCheckCallback impl_callback = base::Bind(OnRequestDoneShim, callback);
+  StartURLCheck(impl_callback, url, threat_types);
+}
+
 }  // namespace safe_browsing
diff --git a/components/safe_browsing_db/safe_browsing_api_handler.h b/components/safe_browsing_db/safe_browsing_api_handler.h
index 0ffdd6b..8ec5deb 100644
--- a/components/safe_browsing_db/safe_browsing_api_handler.h
+++ b/components/safe_browsing_db/safe_browsing_api_handler.h
@@ -24,13 +24,24 @@ class SafeBrowsingApiHandler {
   static SafeBrowsingApiHandler* GetInstance();
 
   typedef base::Callback<void(SBThreatType sb_threat_type,
-                              const std::string& metadata)>
+                              const ThreatMetadata& metadata)>
+      URLCheckCallbackMeta;
+
+  // TODO(nparker): Remove this as part of crbug/589610.
+  typedef base::Callback<void(SBThreatType sb_threat_type,
+                              const std::string& metadata_str)>
       URLCheckCallback;
 
   // Makes Native->Java call and invokes callback when check is done.
+  // TODO(nparker): Switch this back to pure virtual. crbug/589610.
+  virtual void StartURLCheck(const URLCheckCallbackMeta& callback,
+                             const GURL& url,
+                             const std::vector<SBThreatType>& threat_types);
+
+  // TODO(nparker): Remove this as part of crbug/589610.
   virtual void StartURLCheck(const URLCheckCallback& callback,
                              const GURL& url,
-                             const std::vector<SBThreatType>& threat_types) = 0;
+                             const std::vector<SBThreatType>& threat_types) {};
 
   virtual ~SafeBrowsingApiHandler() {}
 
diff --git a/components/safe_browsing_db/safe_browsing_api_handler_unittest.cc b/components/safe_browsing_db/safe_browsing_api_handler_unittest.cc
index 20cbe05..15b5063 100644
--- a/components/safe_browsing_db/safe_browsing_api_handler_unittest.cc
+++ b/components/safe_browsing_db/safe_browsing_api_handler_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "components/safe_browsing_db/metadata.pb.h"
 #include "components/safe_browsing_db/safe_browsing_api_handler_util.h"
+#include "components/safe_browsing_db/testing_util.h"
 #include "components/safe_browsing_db/util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -12,65 +13,61 @@ namespace safe_browsing {
 class SafeBrowsingApiHandlerUtilTest : public ::testing::Test {
  protected:
   SBThreatType threat_;
-  std::string pb_str_;
+  ThreatMetadata meta_;
+  const ThreatMetadata empty_meta_;
 
   UmaRemoteCallResult ResetAndParseJson(const std::string& json) {
     threat_ = SB_THREAT_TYPE_EXTENSION;  // Should never be seen
-    pb_str_ = "unitialized value";
-    return ParseJsonToThreatAndPB(json, &threat_, &pb_str_);
+    meta_ = ThreatMetadata();
+    return ParseJsonFromGMSCore(json, &threat_, &meta_);
   }
 
-  MalwarePatternType::PATTERN_TYPE ParsePatternType() {
-    MalwarePatternType proto;
-    EXPECT_TRUE(proto.ParseFromString(pb_str_));
-    return proto.pattern_type();
-  }
 };
 
 TEST_F(SafeBrowsingApiHandlerUtilTest, BadJson) {
   EXPECT_EQ(UMA_STATUS_JSON_EMPTY, ResetAndParseJson(""));
   EXPECT_EQ(SB_THREAT_TYPE_SAFE, threat_);
-  EXPECT_TRUE(pb_str_.empty());
+  EXPECT_EQ(empty_meta_, meta_);
 
   EXPECT_EQ(UMA_STATUS_JSON_FAILED_TO_PARSE, ResetAndParseJson("{"));
   EXPECT_EQ(SB_THREAT_TYPE_SAFE, threat_);
-  EXPECT_TRUE(pb_str_.empty());
+  EXPECT_EQ(empty_meta_, meta_);
 
   EXPECT_EQ(UMA_STATUS_JSON_FAILED_TO_PARSE, ResetAndParseJson("[]"));
   EXPECT_EQ(SB_THREAT_TYPE_SAFE, threat_);
-  EXPECT_TRUE(pb_str_.empty());
+  EXPECT_EQ(empty_meta_, meta_);
 
   EXPECT_EQ(UMA_STATUS_JSON_FAILED_TO_PARSE,
             ResetAndParseJson("{\"matches\":\"foo\"}"));
   EXPECT_EQ(SB_THREAT_TYPE_SAFE, threat_);
-  EXPECT_TRUE(pb_str_.empty());
+  EXPECT_EQ(empty_meta_, meta_);
 
   EXPECT_EQ(UMA_STATUS_JSON_UNKNOWN_THREAT,
             ResetAndParseJson("{\"matches\":[{}]}"));
   EXPECT_EQ(SB_THREAT_TYPE_SAFE, threat_);
-  EXPECT_TRUE(pb_str_.empty());
+  EXPECT_EQ(empty_meta_, meta_);
 
   EXPECT_EQ(UMA_STATUS_JSON_UNKNOWN_THREAT,
             ResetAndParseJson("{\"matches\":[{\"threat_type\":\"junk\"}]}"));
   EXPECT_EQ(SB_THREAT_TYPE_SAFE, threat_);
-  EXPECT_TRUE(pb_str_.empty());
+  EXPECT_EQ(empty_meta_, meta_);
 
   EXPECT_EQ(UMA_STATUS_JSON_UNKNOWN_THREAT,
             ResetAndParseJson("{\"matches\":[{\"threat_type\":\"999\"}]}"));
   EXPECT_EQ(SB_THREAT_TYPE_SAFE, threat_);
-  EXPECT_TRUE(pb_str_.empty());
+  EXPECT_EQ(empty_meta_, meta_);
 }
 
 TEST_F(SafeBrowsingApiHandlerUtilTest, BasicThreats) {
   EXPECT_EQ(UMA_STATUS_UNSAFE,
             ResetAndParseJson("{\"matches\":[{\"threat_type\":\"4\"}]}"));
   EXPECT_EQ(SB_THREAT_TYPE_URL_MALWARE, threat_);
-  EXPECT_TRUE(pb_str_.empty());
+  EXPECT_EQ(empty_meta_, meta_);
 
   EXPECT_EQ(UMA_STATUS_UNSAFE,
             ResetAndParseJson("{\"matches\":[{\"threat_type\":\"5\"}]}"));
   EXPECT_EQ(SB_THREAT_TYPE_URL_PHISHING, threat_);
-  EXPECT_TRUE(pb_str_.empty());
+  EXPECT_EQ(empty_meta_, meta_);
 }
 
 TEST_F(SafeBrowsingApiHandlerUtilTest, MultipleThreats) {
@@ -79,45 +76,68 @@ TEST_F(SafeBrowsingApiHandlerUtilTest, MultipleThreats) {
       ResetAndParseJson(
           "{\"matches\":[{\"threat_type\":\"4\"}, {\"threat_type\":\"5\"}]}"));
   EXPECT_EQ(SB_THREAT_TYPE_URL_MALWARE, threat_);
-  EXPECT_TRUE(pb_str_.empty());
+  EXPECT_EQ(empty_meta_, meta_);
 }
 
 TEST_F(SafeBrowsingApiHandlerUtilTest, PhaSubType) {
+  ThreatMetadata expected;
+
   EXPECT_EQ(UMA_STATUS_UNSAFE,
             ResetAndParseJson("{\"matches\":[{\"threat_type\":\"4\", "
                               "\"pha_pattern_type\":\"LANDING\"}]}"));
   EXPECT_EQ(SB_THREAT_TYPE_URL_MALWARE, threat_);
-  EXPECT_EQ(MalwarePatternType::LANDING, ParsePatternType());
+  expected.threat_pattern_type = ThreatPatternType::LANDING;
+  EXPECT_EQ(expected, meta_);
+  // Test the ThreatMetadata comparitor for this field.
+  EXPECT_NE(empty_meta_, meta_);
 
   EXPECT_EQ(UMA_STATUS_UNSAFE,
             ResetAndParseJson("{\"matches\":[{\"threat_type\":\"4\", "
                               "\"pha_pattern_type\":\"DISTRIBUTION\"}]}"));
   EXPECT_EQ(SB_THREAT_TYPE_URL_MALWARE, threat_);
-  EXPECT_EQ(MalwarePatternType::DISTRIBUTION, ParsePatternType());
+  expected.threat_pattern_type = ThreatPatternType::DISTRIBUTION;
+  EXPECT_EQ(expected, meta_);
 
   EXPECT_EQ(UMA_STATUS_UNSAFE,
             ResetAndParseJson("{\"matches\":[{\"threat_type\":\"4\", "
                               "\"pha_pattern_type\":\"junk\"}]}"));
-  EXPECT_TRUE(pb_str_.empty());
+  EXPECT_EQ(empty_meta_, meta_);
 }
 
 TEST_F(SafeBrowsingApiHandlerUtilTest, SocialEngineeringSubType) {
+  ThreatMetadata expected;
+
   EXPECT_EQ(UMA_STATUS_UNSAFE,
             ResetAndParseJson("{\"matches\":[{\"threat_type\":\"5\", "
                               "\"se_pattern_type\":\"LANDING\"}]}"));
   EXPECT_EQ(SB_THREAT_TYPE_URL_PHISHING, threat_);
-  EXPECT_EQ(MalwarePatternType::LANDING, ParsePatternType());
+  expected.threat_pattern_type = ThreatPatternType::LANDING;
+  EXPECT_EQ(expected, meta_);
 
   EXPECT_EQ(UMA_STATUS_UNSAFE,
             ResetAndParseJson("{\"matches\":[{\"threat_type\":\"5\", "
                               "\"se_pattern_type\":\"DISTRIBUTION\"}]}"));
   EXPECT_EQ(SB_THREAT_TYPE_URL_PHISHING, threat_);
-  EXPECT_EQ(MalwarePatternType::DISTRIBUTION, ParsePatternType());
+  expected.threat_pattern_type = ThreatPatternType::DISTRIBUTION;
+  EXPECT_EQ(expected, meta_);
 
   EXPECT_EQ(UMA_STATUS_UNSAFE,
             ResetAndParseJson("{\"matches\":[{\"threat_type\":\"5\", "
                               "\"se_pattern_type\":\"junk\"}]}"));
-  EXPECT_TRUE(pb_str_.empty());
+  EXPECT_EQ(empty_meta_, meta_);
+}
+
+TEST_F(SafeBrowsingApiHandlerUtilTest, PopulationId) {
+  ThreatMetadata expected;
+
+  EXPECT_EQ(UMA_STATUS_UNSAFE,
+            ResetAndParseJson("{\"matches\":[{\"threat_type\":\"4\", "
+                              "\"UserPopulation\":\"foobarbazz\"}]}"));
+  EXPECT_EQ(SB_THREAT_TYPE_URL_MALWARE, threat_);
+  expected.population_id = "foobarbazz";
+  EXPECT_EQ(expected, meta_);
+  // Test the ThreatMetadata comparator for this field.
+  EXPECT_NE(empty_meta_, meta_);
 }
 
 }  // namespace safe_browsing
diff --git a/components/safe_browsing_db/safe_browsing_api_handler_util.cc b/components/safe_browsing_db/safe_browsing_api_handler_util.cc
index 181b3a0..1c1fb3d 100644
--- a/components/safe_browsing_db/safe_browsing_api_handler_util.cc
+++ b/components/safe_browsing_db/safe_browsing_api_handler_util.cc
@@ -45,14 +45,11 @@ void ReportUmaThreatSubType(SBThreatType threat_type,
   }
 }
 
-// Parse the extra key/value pair(s) added by Safe Browsing backend.  To make
-// it binary compatible with the Pver3 metadata that UIManager expects to
-// deserialize, we convert it to a MalwarePatternType.
-//
-// TODO(nparker):  When chrome desktop is converted to use Pver4, convert this
-// to the new proto instead.
-const std::string ParseExtraMetadataToPB(const base::DictionaryValue* match,
-                                         SBThreatType threat_type) {
+// Parse the appropriate "*_pattern_type" key from the metadata.
+// Returns NONE if no pattern type was found.
+ThreatPatternType ParseThreatSubType(
+    const base::DictionaryValue* match,
+    SBThreatType threat_type) {
   std::string pattern_key;
   if (threat_type == SB_THREAT_TYPE_URL_MALWARE) {
     pattern_key = "pha_pattern_type";
@@ -64,24 +61,54 @@ const std::string ParseExtraMetadataToPB(const base::DictionaryValue* match,
   std::string pattern_type;
   if (!match->GetString(pattern_key, &pattern_type)) {
     ReportUmaThreatSubType(threat_type, UMA_THREAT_SUB_TYPE_NOT_SET);
-    return std::string();
+    return ThreatPatternType::NONE;
   }
 
-  MalwarePatternType pb;
   if (pattern_type == "LANDING") {
-    pb.set_pattern_type(MalwarePatternType::LANDING);
     ReportUmaThreatSubType(threat_type, UMA_THREAT_SUB_TYPE_LANDING);
+    return ThreatPatternType::LANDING;
   } else if (pattern_type == "DISTRIBUTION") {
-    pb.set_pattern_type(MalwarePatternType::DISTRIBUTION);
     ReportUmaThreatSubType(threat_type, UMA_THREAT_SUB_TYPE_DISTRIBUTION);
+    return ThreatPatternType::DISTRIBUTION;
   } else {
     ReportUmaThreatSubType(threat_type, UMA_THREAT_SUB_TYPE_UNKNOWN);
-    return std::string();
+    return ThreatPatternType::NONE;
+  }
+}
+
+// DEPRECATED
+// TODO(nparker): Remove this as part of crbug/589610
+// Convert back to PB for compatibility with clank, until we land
+// a CL there to use ParseJson().
+const std::string ParseExtraMetadataToPB(const base::DictionaryValue* match,
+                                         SBThreatType threat_type) {
+  ThreatPatternType pattern_type = ParseThreatSubType(match, threat_type);
+
+  MalwarePatternType pb;
+  switch (pattern_type) {
+    case ThreatPatternType::LANDING:
+      pb.set_pattern_type(MalwarePatternType::LANDING);
+      break;
+    case ThreatPatternType::DISTRIBUTION:
+      pb.set_pattern_type(MalwarePatternType::DISTRIBUTION);
+      break;
+    default:
+      return std::string();
   }
 
   return pb.SerializeAsString();
 }
 
+// Parse the optional "UserPopulation" key from the metadata.
+// Returns empty string if none was found.
+std::string ParseUserPopulation(const base::DictionaryValue* match) {
+  std::string population_id;
+  if (!match->GetString("UserPopulation", &population_id))
+    return std::string();
+  else
+    return population_id;
+}
+
 int GetThreatSeverity(int java_threat_num) {
   // Assign higher numbers to more severe threats.
   switch (java_threat_num) {
@@ -114,6 +141,62 @@ SBThreatType JavaToSBThreatType(int java_threat_num) {
 //   or
 // {"matches":[{"threat_type":"4"},
 //             {"threat_type":"5", "se_pattern_type":"LANDING"}]}
+//   or
+// {"matches":[{"threat_type":"4", "UserPopulation":"YXNvZWZpbmFqO..."}]
+UmaRemoteCallResult ParseJsonFromGMSCore(const std::string& metadata_str,
+                                         SBThreatType* worst_threat,
+                                         ThreatMetadata* metadata) {
+  *worst_threat = SB_THREAT_TYPE_SAFE;  // Default to safe.
+  *metadata = ThreatMetadata();  // Default values.
+
+  if (metadata_str.empty())
+    return UMA_STATUS_JSON_EMPTY;
+
+  // Pick out the "matches" list.
+  scoped_ptr<base::Value> value = base::JSONReader::Read(metadata_str);
+  const base::ListValue* matches = nullptr;
+  if (!value.get() || !value->IsType(base::Value::TYPE_DICTIONARY) ||
+      !(static_cast<base::DictionaryValue*>(value.get()))
+           ->GetList(kJsonKeyMatches, &matches) ||
+      !matches) {
+    return UMA_STATUS_JSON_FAILED_TO_PARSE;
+  }
+
+  // Go through each matched threat type and pick the most severe.
+  int worst_threat_num = -1;
+  const base::DictionaryValue* worst_match = nullptr;
+  for (size_t i = 0; i < matches->GetSize(); i++) {
+    // Get the threat number
+    const base::DictionaryValue* match;
+    std::string threat_num_str;
+    int java_threat_num = -1;
+    if (!matches->GetDictionary(i, &match) ||
+        !match->GetString(kJsonKeyThreatType, &threat_num_str) ||
+        !base::StringToInt(threat_num_str, &java_threat_num)) {
+      continue;  // Skip malformed list entries
+    }
+
+    if (GetThreatSeverity(java_threat_num) >
+        GetThreatSeverity(worst_threat_num)) {
+      worst_threat_num = java_threat_num;
+      worst_match = match;
+    }
+  }
+
+  *worst_threat = JavaToSBThreatType(worst_threat_num);
+  if (*worst_threat == SB_THREAT_TYPE_SAFE || !worst_match)
+    return UMA_STATUS_JSON_UNKNOWN_THREAT;
+
+  // Fill in the metadata
+  metadata->threat_pattern_type =
+      ParseThreatSubType(worst_match, *worst_threat);
+  metadata->population_id = ParseUserPopulation(worst_match);
+
+  return UMA_STATUS_UNSAFE;  // success
+}
+
+// DEPRECATED
+// TODO(nparker): Remove this as part of crbug/589610
 UmaRemoteCallResult ParseJsonToThreatAndPB(const std::string& metadata_str,
                                            SBThreatType* worst_threat,
                                            std::string* metadata_pb_str) {
diff --git a/components/safe_browsing_db/safe_browsing_api_handler_util.h b/components/safe_browsing_db/safe_browsing_api_handler_util.h
index 5733c66..0cb5e57 100644
--- a/components/safe_browsing_db/safe_browsing_api_handler_util.h
+++ b/components/safe_browsing_db/safe_browsing_api_handler_util.h
@@ -43,13 +43,15 @@ enum UmaRemoteCallResult {
 
 // This parses the JSON from the GMSCore API and then:
 //   1) Picks the most severe threat type
-//   2) Parses remaining key/value pairs into a MalwarePatternType PB
-//      so DisplayBlockingPage() can unmarshal it.  We make this string
-//      is binary compatible with the Pver3 API's metadata string even
-//      though it comes from Pver4.
+//   2) Parses that threat's key/value pairs into the metadata struct.
 //
 // If anything fails to parse, this sets the threat to "safe".  The caller
 // should report the return value via UMA.
+UmaRemoteCallResult ParseJsonFromGMSCore(const std::string& metadata_str,
+                                         SBThreatType* worst_threat,
+                                         ThreatMetadata* metadata);
+
+// DEPRECATED.  Will be removed.
 UmaRemoteCallResult ParseJsonToThreatAndPB(const std::string& metadata_str,
                                            SBThreatType* worst_threat,
                                            std::string* metadata_pb_str);
diff --git a/components/safe_browsing_db/testing_util.h b/components/safe_browsing_db/testing_util.h
new file mode 100644
index 0000000..0030518
--- /dev/null
+++ b/components/safe_browsing_db/testing_util.h
@@ -0,0 +1,38 @@
+// Copyright (c) 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Utilities to be used in tests of safe_browsing_db/ component.
+
+#ifndef COMPONENTS_SAFE_BROWSING_DB_TESTING_UTIL_H_
+#define COMPONENTS_SAFE_BROWSING_DB_TESTING_UTIL_H_
+
+#include "components/safe_browsing_db/util.h"
+
+#include <ostream>
+
+namespace safe_browsing {
+
+inline bool operator==(const ThreatMetadata& lhs, const ThreatMetadata& rhs) {
+  return lhs.threat_pattern_type == rhs.threat_pattern_type &&
+         lhs.api_permissions == rhs.api_permissions &&
+         lhs.population_id == rhs.population_id &&
+         lhs.raw_metadata == rhs.raw_metadata;
+}
+
+inline bool operator!=(const ThreatMetadata& lhs, const ThreatMetadata& rhs) {
+  return !(lhs == rhs);
+}
+
+inline std::ostream& operator<<(std::ostream& os, const ThreatMetadata& meta) {
+  os << "{threat_pattern_type=" << static_cast<int>(meta.threat_pattern_type)
+     << ", api_permissions=[";
+  for (auto p : meta.api_permissions)
+    os << p << ",";
+  return os << "], population_id=" << meta.population_id
+            << ", raw_metadata=" << meta.raw_metadata << "}";
+}
+
+}  // namespace safe_browsing
+
+#endif  // COMPONENTS_SAFE_BROWSING_DB_TESTING_UTIL_H_
diff --git a/components/safe_browsing_db/util.cc b/components/safe_browsing_db/util.cc
index 18dd9f8..153da70 100644
--- a/components/safe_browsing_db/util.cc
+++ b/components/safe_browsing_db/util.cc
@@ -29,6 +29,12 @@ bool IsKnownList(const std::string& name) {
 }
 }  // namespace
 
+// ThreatMetadata ------------------------------------------------------------
+ThreatMetadata::ThreatMetadata()
+    : threat_pattern_type(ThreatPatternType::NONE) {}
+
+ThreatMetadata::~ThreatMetadata() {}
+
 // SBCachedFullHashResult ------------------------------------------------------
 
 SBCachedFullHashResult::SBCachedFullHashResult() {}
diff --git a/components/safe_browsing_db/util.h b/components/safe_browsing_db/util.h
index 8f82c60..edf12f3 100644
--- a/components/safe_browsing_db/util.h
+++ b/components/safe_browsing_db/util.h
@@ -53,6 +53,37 @@ enum SBThreatType {
   SB_THREAT_TYPE_BLACKLISTED_RESOURCE,
 };
 
+// Metadata that indicates what kind of URL match this is.
+enum class ThreatPatternType {
+  NONE,          // Pattern type didn't appear in the metadata
+  LANDING,       // The match is a landing page
+  DISTRIBUTION,  // The match is a distribution page
+};
+
+// Metadata that was returned by a GetFullHash call. This is the parsed version
+// of the PB (from Pver3, or Pver4 local) or JSON (from Pver4 via GMSCore).
+// Some fields are only applicable to certain lists.
+struct ThreatMetadata {
+  ThreatMetadata();
+  ~ThreatMetadata();
+
+  // Type of blacklisted page. Used on malware and UwS lists.
+  // This will be NONE if it wasn't present in the reponse.
+  ThreatPatternType threat_pattern_type;
+
+  // List of permissions blocked. Used with threat_type API_ABUSE.
+  // This will be empty if it wasn't present in the response.
+  std::vector<std::string> api_permissions;
+
+  // Opaque base64 string used for user-population experiments in pver4.
+  // This will be empty if it wasn't present in the response.
+  std::string population_id;
+
+  // This is the only field currently populated, and it'll be removed
+  // when the others are used.
+  // TODO(nparker): Remove this as part of crbug/589610.
+  std::string raw_metadata;
+};
 
 // A truncated hash's type.
 typedef uint32_t SBPrefix;
@@ -68,7 +99,7 @@ struct SBFullHashResult {
   SBFullHash hash;
   // TODO(shess): Refactor to allow ListType here.
   int list_id;
-  std::string metadata;
+  ThreatMetadata metadata;
   // Used only for V4 results. The cache lifetime for this result. The response
   // must not be cached for more than this duration to avoid false positives.
   base::TimeDelta cache_duration;
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 b78a4b5..55196ba 100644
--- a/components/safe_browsing_db/v4_get_hash_protocol_manager.cc
+++ b/components/safe_browsing_db/v4_get_hash_protocol_manager.cc
@@ -211,11 +211,11 @@ bool V4GetHashProtocolManager::ParseHashResponse(
       if (match.has_platform_type() &&
           match.platform_type() == CHROME_PLATFORM) {
         if (match.has_threat_entry_metadata()) {
-          // For API Abuse, store a csv of the returned permissions.
+          // For API Abuse, store a list of the returned permissions.
           for (const ThreatEntryMetadata::MetadataEntry& m :
                match.threat_entry_metadata().entries()) {
             if (m.key() == "permission") {
-              result.metadata += m.value() + ",";
+              result.metadata.api_permissions.push_back(m.value());
             }
           }
         } else {
diff --git a/components/safe_browsing_db/v4_get_hash_protocol_manager_unittest.cc b/components/safe_browsing_db/v4_get_hash_protocol_manager_unittest.cc
index 0a177ff..ba76637 100644
--- a/components/safe_browsing_db/v4_get_hash_protocol_manager_unittest.cc
+++ b/components/safe_browsing_db/v4_get_hash_protocol_manager_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
 #include "components/safe_browsing_db/safebrowsing.pb.h"
+#include "components/safe_browsing_db/testing_util.h"
 #include "components/safe_browsing_db/util.h"
 #include "components/safe_browsing_db/v4_get_hash_protocol_manager.h"
 #include "net/base/escape.h"
@@ -142,7 +143,7 @@ TEST_F(SafeBrowsingV4GetHashProtocolManagerTest, TestGetHashErrorHandlingOK) {
   std::vector<SBFullHashResult> expected_full_hashes;
   SBFullHashResult hash_result;
   hash_result.hash = SBFullHashForString("Everything's shiny, Cap'n.");
-  hash_result.metadata = "NOTIFICATIONS,";
+  hash_result.metadata.api_permissions.push_back("NOTIFICATIONS");
   hash_result.cache_duration = base::TimeDelta::FromSeconds(300);
   expected_full_hashes.push_back(hash_result);
   base::TimeDelta expected_cache_duration = base::TimeDelta::FromSeconds(600);
@@ -229,7 +230,8 @@ TEST_F(SafeBrowsingV4GetHashProtocolManagerTest, TestParseHashResponse) {
   EXPECT_EQ(1ul, full_hashes.size());
   EXPECT_TRUE(SBFullHashEqual(SBFullHashForString("Everything's shiny, Cap'n."),
                               full_hashes[0].hash));
-  EXPECT_EQ("NOTIFICATIONS,", full_hashes[0].metadata);
+  EXPECT_EQ(1ul, full_hashes[0].metadata.api_permissions.size());
+  EXPECT_EQ("NOTIFICATIONS", full_hashes[0].metadata.api_permissions[0]);
   EXPECT_EQ(base::TimeDelta::FromSeconds(300), full_hashes[0].cache_duration);
   EXPECT_LE(now + base::TimeDelta::FromSeconds(400), pm->next_gethash_time_);
 }
@@ -318,7 +320,7 @@ TEST_F(SafeBrowsingV4GetHashProtocolManagerTest,
   EXPECT_TRUE(SBFullHashEqual(SBFullHashForString("Not to fret."),
                               full_hashes[0].hash));
   // Metadata should be empty.
-  EXPECT_EQ("", full_hashes[0].metadata);
+  EXPECT_EQ(0ul, full_hashes[0].metadata.api_permissions.size());
   EXPECT_EQ(base::TimeDelta::FromSeconds(0), full_hashes[0].cache_duration);
 }