Failure to parse full hash metadata shouldn't discard the response

components/safe_browsing_db/v4_get_hash_protocol_manager_unittest.cc

 // Copyright 2016 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.
 
 #include "components/safe_browsing_db/v4_get_hash_protocol_manager.h"
 
 #include <memory>
 #include <vector>
 
 #include "base/base64.h"
@@ skipping 452 matching lines @@
     invalid->mutable_threat()->set_hash(full_hash);
     ThreatEntryMetadata::MetadataEntry* invalid_meta =
         invalid->mutable_threat_entry_metadata()->add_entries();
     invalid_meta->set_key("pha_pattern_type");
     invalid_meta->set_value("INVALIDE_VALUE");
 
     std::string invalid_data;
     invalid_res.SerializeToString(&invalid_data);
     std::vector<FullHashInfo> full_hash_infos;
     base::Time cache_expire;
-    EXPECT_FALSE(
+    EXPECT_TRUE(
         pm->ParseHashResponse(invalid_data, &full_hash_infos, &cache_expire));
-    EXPECT_EQ(0ul, full_hash_infos.size());
+
+    ASSERT_EQ(1ul, full_hash_infos.size());

[Nathan Parker, 2016/10/27 22:06:49]

nit: Add a comment: The PHA threat still remains since it's valid.
(same below)

[vakh (use Gerrit instead), 2016/10/27 22:40:05]

Done.

+    const auto& fhi = full_hash_infos[0];
+    EXPECT_EQ(full_hash, fhi.full_hash);
+    EXPECT_EQ(
+        ListIdentifier(CHROME_PLATFORM, URL, POTENTIALLY_HARMFUL_APPLICATION),
+        fhi.list_id);
+    EXPECT_EQ(ThreatPatternType::NONE, fhi.metadata.threat_pattern_type);
   }
 }
 
 // Adds metadata with a key value that is not "permission".
 TEST_F(V4GetHashProtocolManagerTest,
        TestParseHashResponseNonPermissionMetadata) {
   std::unique_ptr<V4GetHashProtocolManager> pm(CreateProtocolManager());
 
   base::Time now = base::Time::UnixEpoch();
   SetTestClock(now, pm.get());
 
+  FullHash full_hash("Not to fret.");
   FindFullHashesResponse res;
   res.mutable_negative_cache_duration()->set_seconds(600);
   ThreatMatch* m = res.add_matches();
   m->set_threat_type(API_ABUSE);
   m->set_platform_type(CHROME_PLATFORM);
   m->set_threat_entry_type(URL);
-  m->mutable_threat()->set_hash(FullHash("Not to fret."));
+  m->mutable_threat()->set_hash(full_hash);
   ThreatEntryMetadata::MetadataEntry* e =
       m->mutable_threat_entry_metadata()->add_entries();
   e->set_key("notpermission");
   e->set_value("NOTGEOLOCATION");
 
   // Serialize.
   std::string res_data;
   res.SerializeToString(&res_data);
 
   std::vector<FullHashInfo> full_hash_infos;
   base::Time cache_expire;
-  EXPECT_FALSE(
-      pm->ParseHashResponse(res_data, &full_hash_infos, &cache_expire));
+  EXPECT_TRUE(pm->ParseHashResponse(res_data, &full_hash_infos, &cache_expire));
 
   EXPECT_EQ(now + base::TimeDelta::FromSeconds(600), cache_expire);
-  EXPECT_EQ(0ul, full_hash_infos.size());
+  ASSERT_EQ(1ul, full_hash_infos.size());
+  const auto& fhi = full_hash_infos[0];
+  EXPECT_EQ(full_hash, fhi.full_hash);
+  EXPECT_EQ(GetChromeUrlApiId(), fhi.list_id);
+  EXPECT_TRUE(fhi.metadata.api_permissions.empty());
 }
 
 TEST_F(V4GetHashProtocolManagerTest,
        TestParseHashResponseInconsistentThreatTypes) {
   std::unique_ptr<V4GetHashProtocolManager> pm(CreateProtocolManager());
 
   FindFullHashesResponse res;
   res.mutable_negative_cache_duration()->set_seconds(600);
   ThreatMatch* m1 = res.add_matches();
   m1->set_threat_type(API_ABUSE);
@@ skipping 206 matching lines @@
   full_hash = FullHash("Everything's shiny, Cap'n.");
   info = ResponseInfo(full_hash, GetChromeUrlApiId());
   info.key_values.emplace_back("permission", "GEOLOCATION");
   infos.push_back(info);
   SetupFetcherToReturnOKResponse(factory, infos);
 
   EXPECT_TRUE(callback_called());
 }
 
 }  // namespace safe_browsing