Replace metadata.raw_metadata with parsed fields, and add population_id to ping.

This is part 3 of 3 CLs:
  1: https://codereview.chromium.org/1726403006
  2: https://chrome-internal-review.googlesource.com/#/c/249571/

BUG=589610

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

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

15 files changed, 132 insertions, 168 deletions

diff --git a/chrome/browser/safe_browsing/download_protection_service.cc b/chrome/browser/safe_browsing/download_protection_service.cc
index 71c81a77..48a42e9 100644
--- a/chrome/browser/safe_browsing/download_protection_service.cc
+++ b/chrome/browser/safe_browsing/download_protection_service.cc
@@ -192,6 +192,7 @@ class DownloadSBClient
     hit_report.threat_type = threat_type;
     // TODO(nparker) Replace this with database_manager_->GetThreatSource();
     hit_report.threat_source = safe_browsing::ThreatSource::LOCAL_PVER3;
+    // TODO(nparker) Populate hit_report.population_id once Pver4 is used here.
     hit_report.post_data = post_data;
     hit_report.is_extended_reporting = is_extended_reporting_;
     hit_report.is_metrics_reporting_active =
diff --git a/chrome/browser/safe_browsing/ping_manager.cc b/chrome/browser/safe_browsing/ping_manager.cc
index 001d08f..c7d681f 100644
--- a/chrome/browser/safe_browsing/ping_manager.cc
+++ b/chrome/browser/safe_browsing/ping_manager.cc
@@ -198,14 +198,29 @@ GURL SafeBrowsingPingManager::SafeBrowsingHitUrl(
       NOTREACHED();
   }
 
+  // Add user_population component only if it's not empty.
+  std::string user_population_comp;
+  if (!hit_report.population_id.empty()) {
+    // Population_id should be URL-safe, but escape it and size-limit it
+    // anyway since it came from outside Chrome.
+    std::string up_str =
+        net::EscapeQueryParamValue(hit_report.population_id, true);
+    if (up_str.size() > 512) {
+      DCHECK(false) << "population_id is too long: " << up_str;
+      up_str = "UP_STRING_TOO_LONG";
+    }
+
+    user_population_comp = "&up=" + up_str;
+  }
+
   return GURL(base::StringPrintf(
-      "%s&evts=%s&evtd=%s&evtr=%s&evhr=%s&evtb=%d&src=%s&m=%d", url.c_str(),
+      "%s&evts=%s&evtd=%s&evtr=%s&evhr=%s&evtb=%d&src=%s&m=%d%s", url.c_str(),
       threat_list.c_str(),
       net::EscapeQueryParamValue(hit_report.malicious_url.spec(), true).c_str(),
       net::EscapeQueryParamValue(hit_report.page_url.spec(), true).c_str(),
       net::EscapeQueryParamValue(hit_report.referrer_url.spec(), true).c_str(),
       hit_report.is_subresource, threat_source.c_str(),
-      hit_report.is_metrics_reporting_active));
+      hit_report.is_metrics_reporting_active, user_population_comp.c_str()));
 }
 
 GURL SafeBrowsingPingManager::ThreatDetailsUrl() const {
diff --git a/chrome/browser/safe_browsing/ping_manager_unittest.cc b/chrome/browser/safe_browsing/ping_manager_unittest.cc
index caabdcff..11771eb 100644
--- a/chrome/browser/safe_browsing/ping_manager_unittest.cc
+++ b/chrome/browser/safe_browsing/ping_manager_unittest.cc
@@ -137,6 +137,26 @@ TEST_F(SafeBrowsingPingManagerTest, TestSafeBrowsingHitUrl) {
             "url.com%2F&evtb=1&src=l4&m=0",
         pm.SafeBrowsingHitUrl(hp).spec());
   }
+
+  // Same as above, but add population_id
+  {
+    HitReport hp(base_hp);
+    hp.threat_type = SB_THREAT_TYPE_CLIENT_SIDE_MALWARE_URL;
+    hp.threat_source = ThreatSource::LOCAL_PVER4;
+    hp.is_extended_reporting = false;
+    hp.is_metrics_reporting_active = false;
+    hp.is_subresource = true;
+    hp.population_id = "foo bar";
+    EXPECT_EQ(
+        "https://prefix.com/foo/report?client=unittest&appver=1.0&"
+        "pver=3.0" +
+            key_param_ +
+            "&ext=0&evts=malcsdhit&"
+            "evtd=http%3A%2F%2Fmalicious.url.com%2F&"
+            "evtr=http%3A%2F%2Fpage.url.com%2F&evhr=http%3A%2F%2Freferrer."
+            "url.com%2F&evtb=1&src=l4&m=0&up=foo+bar",
+        pm.SafeBrowsingHitUrl(hp).spec());
+  }
 }
 
 TEST_F(SafeBrowsingPingManagerTest, TestThreatDetailsUrl) {
diff --git a/chrome/browser/safe_browsing/protocol_parser.cc b/chrome/browser/safe_browsing/protocol_parser.cc
index 60e3304..1a1ee8c 100644
--- a/chrome/browser/safe_browsing/protocol_parser.cc
+++ b/chrome/browser/safe_browsing/protocol_parser.cc
@@ -22,6 +22,7 @@
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "chrome/browser/safe_browsing/safe_browsing_util.h"
+#include "components/safe_browsing_db/metadata.pb.h"
 
 namespace safe_browsing {
 
@@ -137,6 +138,38 @@ class BufferReader {
   DISALLOW_COPY_AND_ASSIGN(BufferReader);
 };
 
+// Parse the |raw_metadata| string based on the |list_type| and populate
+// the appropriate field of |metadata|.  For Pver3 (which this file implements),
+// we only fill in threat_pattern_type.  Others are populated for Pver4.
+void InterpretMetadataString(const std::string& raw_metadata,
+                             ListType list_type,
+                             ThreatMetadata* metadata) {
+  if (list_type != UNWANTEDURL && list_type != MALWARE)
+    return;
+
+  if (raw_metadata.empty())
+    return;
+
+  MalwarePatternType proto;
+  if (!proto.ParseFromString(raw_metadata)) {
+    DCHECK(false) << "Bad MalwarePatternType";
+    return;
+  }
+
+  // Convert proto enum to internal enum (we'll move away from this
+  // proto in Pver4).
+  switch (proto.pattern_type()) {
+    case MalwarePatternType::LANDING:
+      metadata->threat_pattern_type = ThreatPatternType::LANDING;
+      break;
+    case MalwarePatternType::DISTRIBUTION:
+      metadata->threat_pattern_type = ThreatPatternType::DISTRIBUTION;
+      break;
+    default:
+      metadata->threat_pattern_type = ThreatPatternType::NONE;
+  }
+}
+
 bool ParseGetHashMetadata(
     size_t hash_count,
     BufferReader* reader,
@@ -155,9 +188,14 @@ bool ParseGetHashMetadata(
       return false;
 
     if (full_hashes) {
-      (*full_hashes)[full_hashes->size() - hash_count + i]
-          .metadata.raw_metadata.assign(
-              reinterpret_cast<const char*>(meta_data), meta_data_len);
+      const std::string raw_metadata(reinterpret_cast<const char*>(meta_data),
+                                     meta_data_len);
+      // Update the i'th entry in the last hash_count elements of the list.
+      SBFullHashResult* full_hash =
+          &((*full_hashes)[full_hashes->size() - hash_count + i]);
+      InterpretMetadataString(raw_metadata,
+                              static_cast<ListType>(full_hash->list_id),
+                              &full_hash->metadata);
     }
   }
   return true;
diff --git a/chrome/browser/safe_browsing/protocol_parser_unittest.cc b/chrome/browser/safe_browsing/protocol_parser_unittest.cc
index c1f04db..1f1f462 100644
--- a/chrome/browser/safe_browsing/protocol_parser_unittest.cc
+++ b/chrome/browser/safe_browsing/protocol_parser_unittest.cc
@@ -11,6 +11,7 @@
 #include "base/time/time.h"
 #include "chrome/browser/safe_browsing/protocol_parser.h"
 #include "chrome/browser/safe_browsing/safe_browsing_util.h"
+#include "components/safe_browsing_db/metadata.pb.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace safe_browsing {
@@ -432,16 +433,31 @@ TEST(SafeBrowsingProtocolParsingTest, TestGetHash) {
                    sizeof(SBFullHash)), 0);
   EXPECT_EQ(MALWARE, full_hashes[2].list_id);
 
-  // Test metadata parsing.
+  // Test metadata parsing. Make some metadata protos to fill in the message.
+  MalwarePatternType proto;
+
+  proto.set_pattern_type(MalwarePatternType::LANDING);
+  std::string metadata_pb_landing;
+  ASSERT_TRUE(proto.SerializeToString(&metadata_pb_landing));
+
+  proto.set_pattern_type(MalwarePatternType::DISTRIBUTION);
+  std::string metadata_pb_distribution;
+  ASSERT_TRUE(proto.SerializeToString(&metadata_pb_distribution));
+
   const std::string get_hash3(base::StringPrintf(
       "45\n"
       "%s:32:2:m\n"
       "zzzzyyyyxxxxwwwwvvvvuuuuttttssss"
       "00112233445566778899aabbccddeeff"
-      "2\nab2\nxy"
+      "%zu\n%s"  // meta 1
+      "%zu\n%s"  // meta 2
       "%s:32:1\n"
       "cafebeefcafebeefdeaddeaddeaddead",
       kDefaultMalwareList,
+      metadata_pb_landing.size(),
+      metadata_pb_landing.c_str(),
+      metadata_pb_distribution.size(),
+      metadata_pb_distribution.c_str(),
       kDefaultPhishList));
   EXPECT_TRUE(ParseGetHash(get_hash3.data(), get_hash3.length(),
                            &cache_lifetime, &full_hashes));
@@ -451,17 +467,22 @@ TEST(SafeBrowsingProtocolParsingTest, TestGetHash) {
                    "zzzzyyyyxxxxwwwwvvvvuuuuttttssss",
                    sizeof(SBFullHash)), 0);
   EXPECT_EQ(MALWARE, full_hashes[0].list_id);
-  EXPECT_EQ(std::string("ab"), full_hashes[0].metadata.raw_metadata);
+  EXPECT_EQ(ThreatPatternType::LANDING,
+            full_hashes[0].metadata.threat_pattern_type);
+
   EXPECT_EQ(memcmp(&full_hashes[1].hash,
                    "00112233445566778899aabbccddeeff",
                    sizeof(SBFullHash)), 0);
   EXPECT_EQ(MALWARE, full_hashes[1].list_id);
-  EXPECT_EQ(std::string("xy"), full_hashes[1].metadata.raw_metadata);
+  EXPECT_EQ(ThreatPatternType::DISTRIBUTION,
+            full_hashes[1].metadata.threat_pattern_type);
+
   EXPECT_EQ(memcmp(&full_hashes[2].hash,
                    "cafebeefcafebeefdeaddeaddeaddead",
                    sizeof(SBFullHash)), 0);
   EXPECT_EQ(PHISH, full_hashes[2].list_id);
-  EXPECT_EQ(std::string(), full_hashes[2].metadata.raw_metadata);
+  EXPECT_EQ(ThreatPatternType::NONE,
+            full_hashes[2].metadata.threat_pattern_type);
 }
 
 TEST(SafeBrowsingProtocolParsingTest, TestGetHashWithUnknownList) {
@@ -508,7 +529,7 @@ TEST(SafeBrowsingProtocolParsingTest, TestGetHashWithUnknownListAndMetadata) {
       "600\n"
       "BADLISTNAME:32:1:m\n"
       "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"
-      "8\nMETADATA"
+      "8\nMETADATA"  // not even parsed (lest the parser DCHECK's).
       "%s:32:1\n"
       "0123456789hashhashhashhashhashha",
       kDefaultMalwareList));
@@ -519,7 +540,8 @@ TEST(SafeBrowsingProtocolParsingTest, TestGetHashWithUnknownListAndMetadata) {
                    "0123456789hashhashhashhashhashha",
                    sizeof(SBFullHash)), 0);
   EXPECT_EQ(MALWARE, full_hashes[0].list_id);
-  EXPECT_EQ(std::string(), full_hashes[0].metadata.raw_metadata);
+  EXPECT_EQ(ThreatPatternType::NONE,
+            full_hashes[0].metadata.threat_pattern_type);
 }
 
 TEST(SafeBrowsingProtocolParsingTest, TestFormatHash) {
diff --git a/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc b/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
index 3d56a35..52a094f 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
@@ -686,36 +686,17 @@ class SafeBrowsingServiceTest : public InProcessBrowserTest {
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingServiceTest);
 };
 
-enum MalwareMetadataTestType {
-  METADATA_NONE,
-  METADATA_LANDING,
-  METADATA_DISTRIBUTION,
-};
-
 class SafeBrowsingServiceMetadataTest
     : public SafeBrowsingServiceTest,
-      public ::testing::WithParamInterface<MalwareMetadataTestType> {
+      public ::testing::WithParamInterface<ThreatPatternType> {
  public:
   SafeBrowsingServiceMetadataTest() {}
 
   void GenUrlFullhashResultWithMetadata(const GURL& url,
                                         SBFullHashResult* full_hash) {
     GenUrlFullhashResult(url, MALWARE, full_hash);
-
-    MalwarePatternType proto;
-    switch (GetParam()) {
-      case METADATA_NONE:
-        full_hash->metadata.raw_metadata = std::string();
-        break;
-      case METADATA_LANDING:
-        proto.set_pattern_type(MalwarePatternType::LANDING);
-        full_hash->metadata.raw_metadata = proto.SerializeAsString();
-        break;
-      case METADATA_DISTRIBUTION:
-        proto.set_pattern_type(MalwarePatternType::DISTRIBUTION);
-        full_hash->metadata.raw_metadata = proto.SerializeAsString();
-        break;
-    }
+    // We test with different threat_pattern_types.
+    full_hash->metadata.threat_pattern_type = GetParam();
   }
 
  private:
@@ -771,12 +752,12 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingServiceMetadataTest, MalwareImg) {
   SBFullHashResult malware_full_hash;
   GenUrlFullhashResultWithMetadata(img_url, &malware_full_hash);
   switch (GetParam()) {
-    case METADATA_NONE:  // Falls through.
-    case METADATA_DISTRIBUTION:
+    case ThreatPatternType::NONE:  // Falls through.
+    case ThreatPatternType::DISTRIBUTION:
       EXPECT_CALL(observer_, OnSafeBrowsingHit(IsUnsafeResourceFor(img_url)))
           .Times(1);
       break;
-    case METADATA_LANDING:
+    case ThreatPatternType::LANDING:
       // No interstitial shown, so no notifications expected.
       break;
   }
@@ -785,8 +766,8 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingServiceMetadataTest, MalwareImg) {
   // Subresource which is tagged as a landing page should not show an
   // interstitial, the other types should.
   switch (GetParam()) {
-    case METADATA_NONE:
-    case METADATA_DISTRIBUTION:
+    case ThreatPatternType::NONE:  // Falls through.
+    case ThreatPatternType::DISTRIBUTION:
       EXPECT_TRUE(ShowingInterstitialPage());
       EXPECT_TRUE(got_hit_report());
       EXPECT_EQ(img_url, hit_report().malicious_url);
@@ -794,7 +775,7 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingServiceMetadataTest, MalwareImg) {
       EXPECT_EQ(GURL(), hit_report().referrer_url);
       EXPECT_TRUE(hit_report().is_subresource);
       break;
-    case METADATA_LANDING:
+    case ThreatPatternType::LANDING:
       EXPECT_FALSE(ShowingInterstitialPage());
       EXPECT_FALSE(got_hit_report());
       break;
@@ -803,9 +784,9 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingServiceMetadataTest, MalwareImg) {
 
 INSTANTIATE_TEST_CASE_P(MaybeSetMetadata,
                         SafeBrowsingServiceMetadataTest,
-                        testing::Values(METADATA_NONE,
-                                        METADATA_LANDING,
-                                        METADATA_DISTRIBUTION));
+                        testing::Values(ThreatPatternType::NONE,
+                                        ThreatPatternType::LANDING,
+                                        ThreatPatternType::DISTRIBUTION));
 
 IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, UnwantedImgIgnored) {
   GURL main_url = embedded_test_server()->GetURL(kMalwarePage);
diff --git a/chrome/browser/safe_browsing/ui_manager.cc b/chrome/browser/safe_browsing/ui_manager.cc
index faebd87..6d6897e 100644
--- a/chrome/browser/safe_browsing/ui_manager.cc
+++ b/chrome/browser/safe_browsing/ui_manager.cc
@@ -155,14 +155,11 @@ void SafeBrowsingUIManager::DisplayBlockingPage(
     // applied to malware sites tagged as "landing sites" (see "Types of
     // Malware sites" under
     // https://developers.google.com/safe-browsing/developers_guide_v3#UserWarnings).
-    // TODO(nparker): Replace the use of raw_metadata when other fields are
-    // populated. crbug/589610
     MalwarePatternType proto;
     if (resource.threat_type == SB_THREAT_TYPE_URL_UNWANTED ||
         (resource.threat_type == SB_THREAT_TYPE_URL_MALWARE &&
-         !resource.threat_metadata.raw_metadata.empty() &&
-         proto.ParseFromString(resource.threat_metadata.raw_metadata) &&
-         proto.pattern_type() == MalwarePatternType::LANDING)) {
+         resource.threat_metadata.threat_pattern_type ==
+         ThreatPatternType::LANDING)) {
       if (!resource.callback.is_null()) {
         DCHECK(resource.callback_thread);
         resource.callback_thread->PostTask(FROM_HERE,
@@ -201,6 +198,7 @@ void SafeBrowsingUIManager::DisplayBlockingPage(
     hit_report.is_subresource = resource.is_subresource;
     hit_report.threat_type = resource.threat_type;
     hit_report.threat_source = resource.threat_source;
+    hit_report.population_id = resource.threat_metadata.population_id;
 
     NavigationEntry* entry = resource.GetNavigationEntryForResource();
     if (entry) {
diff --git a/components/safe_browsing_db/database_manager.h b/components/safe_browsing_db/database_manager.h
index f5ba2db..5fa427f 100644
--- a/components/safe_browsing_db/database_manager.h
+++ b/components/safe_browsing_db/database_manager.h
@@ -55,7 +55,7 @@ class SafeBrowsingDatabaseManager
 
     // Called when the result of checking the API blacklist is known.
     virtual void OnCheckApiBlacklistUrlResult(const GURL& url,
-                                              const std::string& metadata) {}
+                                              const ThreatMetadata& metadata) {}
 
     // Called when the result of checking the resource blacklist is known.
     virtual void OnCheckResourceUrlResult(const GURL& url,
diff --git a/components/safe_browsing_db/hit_report.h b/components/safe_browsing_db/hit_report.h
index 881a01f..158ec98 100644
--- a/components/safe_browsing_db/hit_report.h
+++ b/components/safe_browsing_db/hit_report.h
@@ -37,6 +37,10 @@ struct HitReport {
   bool is_subresource;
   SBThreatType threat_type;
   ThreatSource threat_source;
+
+  // Opaque string used for tracking Pver4-based experiments
+  std::string population_id;
+
   bool is_extended_reporting;
   bool is_metrics_reporting_active;
 
diff --git a/components/safe_browsing_db/remote_database_manager_unittest.cc b/components/safe_browsing_db/remote_database_manager_unittest.cc
index eb08a53..d91baa5 100644
--- a/components/safe_browsing_db/remote_database_manager_unittest.cc
+++ b/components/safe_browsing_db/remote_database_manager_unittest.cc
@@ -18,7 +18,7 @@ namespace {
 
 class TestSafeBrowsingApiHandler : public SafeBrowsingApiHandler {
  public:
-  void StartURLCheck(const URLCheckCallback& callback,
+  void StartURLCheck(const URLCheckCallbackMeta& callback,
                      const GURL& url,
                      const std::vector<SBThreatType>& threat_types) override {}
 };
diff --git a/components/safe_browsing_db/safe_browsing_api_handler.cc b/components/safe_browsing_db/safe_browsing_api_handler.cc
index 178a35d..528dccb 100644
--- a/components/safe_browsing_db/safe_browsing_api_handler.cc
+++ b/components/safe_browsing_db/safe_browsing_api_handler.cc
@@ -7,20 +7,6 @@
 
 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
@@ -33,14 +19,4 @@ 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 8ec5deb..fa01a60 100644
--- a/components/safe_browsing_db/safe_browsing_api_handler.h
+++ b/components/safe_browsing_db/safe_browsing_api_handler.h
@@ -27,21 +27,10 @@ class SafeBrowsingApiHandler {
                               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) {};
+                             const std::vector<SBThreatType>& threat_types) = 0;
 
   virtual ~SafeBrowsingApiHandler() {}
 
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 1c1fb3d..d780dbc 100644
--- a/components/safe_browsing_db/safe_browsing_api_handler_util.cc
+++ b/components/safe_browsing_db/safe_browsing_api_handler_util.cc
@@ -76,29 +76,6 @@ ThreatPatternType ParseThreatSubType(
   }
 }
 
-// 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) {
@@ -195,54 +172,4 @@ UmaRemoteCallResult ParseJsonFromGMSCore(const std::string& metadata_str,
   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) {
-  *worst_threat = SB_THREAT_TYPE_SAFE;  // Default to safe.
-  *metadata_pb_str = std::string();
-
-  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;
-  *metadata_pb_str = ParseExtraMetadataToPB(worst_match, *worst_threat);
-
-  return UMA_STATUS_UNSAFE;  // success
-}
-
 }  // namespace safe_browsing
diff --git a/components/safe_browsing_db/testing_util.h b/components/safe_browsing_db/testing_util.h
index 0030518..76b3194 100644
--- a/components/safe_browsing_db/testing_util.h
+++ b/components/safe_browsing_db/testing_util.h
@@ -16,8 +16,7 @@ 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;
+         lhs.population_id == rhs.population_id;
 }
 
 inline bool operator!=(const ThreatMetadata& lhs, const ThreatMetadata& rhs) {
@@ -29,8 +28,7 @@ inline std::ostream& operator<<(std::ostream& os, const ThreatMetadata& meta) {
      << ", api_permissions=[";
   for (auto p : meta.api_permissions)
     os << p << ",";
-  return os << "], population_id=" << meta.population_id
-            << ", raw_metadata=" << meta.raw_metadata << "}";
+  return os << "], population_id=" << meta.population_id;
 }
 
 }  // namespace safe_browsing
diff --git a/components/safe_browsing_db/util.h b/components/safe_browsing_db/util.h
index edf12f3..932d5bd 100644
--- a/components/safe_browsing_db/util.h
+++ b/components/safe_browsing_db/util.h
@@ -78,11 +78,6 @@ struct ThreatMetadata {
   // 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.