Drop support for reading PrefixSet v1.

r253924 "Convert safe-browsing SBPrefix from int32 to uint32." added
support for reading the v1 file and re-sorting.  Remove that support and
let the files just be dropped on read, since few users are hitting the
code.

BUG=346405

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@276312 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/safe_browsing/prefix_set.cc

@@ -28,7 +28,8 @@ static uint32 kMagic = 0x864088dd;
 // Version 2 layout is identical to version 1.  The sort order of |index_|
 // changed from |int32| to |uint32| to match the change of |SBPrefix|.
 // Version 3 adds storage for full hashes.
-static uint32 kVersion = 0x3;
+static uint32 kVersion = 3;
+static uint32 kDeprecatedVersion = 1;  // And lower.
 
 typedef struct {
   uint32 magic;
@@ -194,12 +195,11 @@ scoped_ptr<PrefixSet> PrefixSet::LoadFile(const base::FilePath& filter_name) {
   // Track version read to inform removal of support for older versions.
   UMA_HISTOGRAM_SPARSE_SLOWLY("SB2.PrefixSetVersionRead", header.version);
 
-  // TODO(shess): Version 1 and 2 use the same file structure, with version 1
-  // data using a signed sort.  For M-35, the data is re-sorted before return.
-  // After M-36, just drop v1 support. <http://crbug.com/346405>
   // TODO(shess): <http://crbug.com/368044> for removing v2 support.
   size_t header_size = sizeof(header);
-  if (header.version == 2 || header.version == 1) {
+  if (header.version <= kDeprecatedVersion) {
+    return scoped_ptr<PrefixSet>();
+  } else if (header.version == 2) {
     // Rewind the file and restart building the digest with the old header
     // structure.
     FileHeader_v2 v2_header;
@@ -291,16 +291,6 @@ scoped_ptr<PrefixSet> PrefixSet::LoadFile(const base::FilePath& filter_name) {
   if (0 != memcmp(&file_digest, &calculated_digest, sizeof(file_digest)))
     return scoped_ptr<PrefixSet>();
 
-  // For version 1, fetch the prefixes and re-sort.
-  if (header.version == 1) {
-    std::vector<SBPrefix> prefixes;
-    PrefixSet(&index, &deltas, &full_hashes).GetPrefixes(&prefixes);
-    std::sort(prefixes.begin(), prefixes.end());
-
-    // v1 cannot have full hashes, so no need to propagate a copy here.
-    return PrefixSetBuilder(prefixes).GetPrefixSetNoHashes().Pass();
-  }
-
   // Steals vector contents using swap().
   return scoped_ptr<PrefixSet>(new PrefixSet(&index, &deltas, &full_hashes));
 }

chrome/browser/safe_browsing/prefix_set_unittest.cc

@@ -631,8 +631,8 @@ TEST_F(PrefixSetTest, FullHashBuild) {
   EXPECT_FALSE(prefix_set->PrefixExists(kHash6.prefix));
 }
 
-// Test that a version 1 file is re-ordered correctly on read.
-TEST_F(PrefixSetTest, ReadWriteSigned) {
+// Test that a version 1 file is discarded on read.
+TEST_F(PrefixSetTest, ReadSigned) {
   base::FilePath filename;
   ASSERT_TRUE(GetPrefixSetFile(&filename));
 
@@ -672,25 +672,9 @@ TEST_F(PrefixSetTest, ReadWriteSigned) {
   CleanChecksum(file.get());
   file.reset();  // Flush updates.
 
-  scoped_ptr<PrefixSet> prefix_set = PrefixSet::LoadFile(filename);
-  ASSERT_TRUE(prefix_set.get());
-
-  // |PrefixExists()| uses |std::upper_bound()| to find a starting point, which
-  // assumes |index_| is sorted.  Depending on how |upper_bound()| is
-  // implemented, if the actual list is sorted by |int32|, then one of these
-  // test pairs should fail.
-  EXPECT_TRUE(prefix_set->PrefixExists(1000u));
-  EXPECT_TRUE(prefix_set->PrefixExists(1023u));
-  EXPECT_TRUE(prefix_set->PrefixExists(static_cast<uint32>(-1000)));
-  EXPECT_TRUE(prefix_set->PrefixExists(static_cast<uint32>(-1000 + 23)));
-
-  std::vector<SBPrefix> prefixes_copy;
-  prefix_set->GetPrefixes(&prefixes_copy);
-  EXPECT_EQ(prefixes_copy.size(), 4u);
-  EXPECT_EQ(prefixes_copy[0], 1000u);
-  EXPECT_EQ(prefixes_copy[1], 1023u);
-  EXPECT_EQ(prefixes_copy[2], static_cast<uint32>(-1000));
-  EXPECT_EQ(prefixes_copy[3], static_cast<uint32>(-1000 + 23));
+  scoped_ptr<safe_browsing::PrefixSet>
+      prefix_set(safe_browsing::PrefixSet::LoadFile(filename));
+  ASSERT_FALSE(prefix_set.get());
 }
 
 // Test that a golden v2 file can be read by the current code.  All platforms