Fix off-by-one in WriteHashRangeToFile()

In the last_hash < first_hash case, the first write (first_hash->EOF)
writes one fingerprint too many.

The bug causes the file to be too large by sizeof(Fingerprint) and will
fail validation on the following load (on browser initialization), since
ReadFileHeader() makes sure the table size matches the file size. The
browser will still manage to start since the table will be rebuilt from
history, but this rebuild is redundant.

R=sky@chromium.org

Change-Id: I428a5229cd971b3c1856a3a3079a5e6572a1aa66
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4208189
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1110489}

AUTHORS

@@ -138,6 +138,7 @@ Arnaud Mandy <arnaud.mandy@intel.com>
 Arnaud Renevier <a.renevier@samsung.com>
 Arpita Bahuguna <a.bah@samsung.com>
 Arthur Lussos <developer0420@gmail.com>
+Artin Lindqvist <artin.lindqvist.chromium@gmail.com>
 Artur Akerberg <artur.aker@gmail.com>
 Arun Kulkarni <kulkarni.a@samsung.com>
 Arun Kumar <arun87.kumar@samsung.com>

components/visitedlink/browser/visitedlink_writer.cc

@@ -1104,7 +1104,7 @@ void VisitedLinkWriter::WriteHashRangeToFile(Hash first_hash, Hash last_hash) {
     WriteToFile(scoped_file_holder_.get(),
                 first_hash * sizeof(Fingerprint) + kFileHeaderSize,
                 &hash_table_[first_hash],
-                (table_length_ - first_hash + 1) * sizeof(Fingerprint));
+                (table_length_ - first_hash) * sizeof(Fingerprint));
 
     // Now do 0->last_lash.
     WriteToFile(scoped_file_holder_.get(), kFileHeaderSize, hash_table_,

components/visitedlink/browser/visitedlink_writer.h

@@ -177,6 +177,7 @@ class VisitedLinkWriter : public VisitedLinkCommon {
   FRIEND_TEST_ALL_PREFIXES(VisitedLinkTest, Delete);
   FRIEND_TEST_ALL_PREFIXES(VisitedLinkTest, BigDelete);
   FRIEND_TEST_ALL_PREFIXES(VisitedLinkTest, BigImport);
+  FRIEND_TEST_ALL_PREFIXES(VisitedLinkTest, HashRangeWraparound);
 
   // Keeps the result of loading the table from the database file to the UI
   // thread.

components/visitedlink/test/visitedlink_unittest.cc

@@ -26,6 +26,7 @@
 #include "components/visitedlink/browser/visitedlink_event_listener.h"
 #include "components/visitedlink/browser/visitedlink_writer.h"
 #include "components/visitedlink/common/visitedlink.mojom.h"
+#include "components/visitedlink/common/visitedlink_common.h"
 #include "components/visitedlink/renderer/visitedlink_reader.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/browser_task_environment.h"
@@ -544,6 +545,39 @@ TEST_F(VisitedLinkTest, Listener) {
   EXPECT_EQ(1, listener->completely_reset_count());
 }
 
+TEST_F(VisitedLinkTest, HashRangeWraparound) {
+  ASSERT_TRUE(InitVisited(0, true, true));
+
+  // Create two fingerprints that, when added, will create a wraparound hash
+  // range.
+  const VisitedLinkCommon::Fingerprint kFingerprint0 =
+      writer_->DefaultTableSize() - 1;
+  const VisitedLinkCommon::Fingerprint kFingerprint1 = kFingerprint0 + 1;
+
+  // Add the two fingerprints.
+  const VisitedLinkCommon::Hash hash0 =
+      writer_->AddFingerprint(kFingerprint0, false);
+  const VisitedLinkCommon::Hash hash1 =
+      writer_->AddFingerprint(kFingerprint1, false);
+
+  // Verify the hashes form a range that wraps around.
+  EXPECT_EQ(hash0, VisitedLinkCommon::Hash(writer_->DefaultTableSize() - 1));
+  EXPECT_EQ(hash1, 0);
+
+  // Write the database to file.
+  writer_->WriteUsedItemCountToFile();
+  writer_->WriteHashRangeToFile(hash0, hash1);
+
+  // Close and reopen the database.
+  ClearDB();
+  ASSERT_TRUE(InitVisited(0, true, true));
+
+  // Verify database contents.
+  ASSERT_EQ(writer_->GetUsedCount(), 2);
+  ASSERT_TRUE(writer_->IsVisited(kFingerprint0));
+  ASSERT_TRUE(writer_->IsVisited(kFingerprint1));
+}
+
 class VisitCountingContext : public mojom::VisitedLinkNotificationSink {
  public:
   VisitCountingContext()