Some new unittests for ReplaceStringPlaceholders, inspired by

some of the tricky cases for ReplaceSubstringsAfterOffsets.

These tests uncovered a correctness bug involving a sorted vector,
which is also fixed.

Bug: None
Change-Id: Ic96d709e70732705e7bf3be9655e5427bdfe396f
Reviewed-on: https://chromium-review.googlesource.com/648215Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Nick Carter <nick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504824}

base/strings/string_util.cc

@@ -1039,12 +1039,11 @@ OutStringType DoReplaceStringPlaceholders(
           uintptr_t index = *i - '1';
           if (offsets) {
             ReplacementOffset r_offset(index,
-                static_cast<int>(formatted.size()));
-            r_offsets.insert(std::lower_bound(r_offsets.begin(),
-                                              r_offsets.end(),
-                                              r_offset,
-                                              &CompareParameter),
-                             r_offset);
+                                       static_cast<int>(formatted.size()));
+            r_offsets.insert(
+                std::upper_bound(r_offsets.begin(), r_offsets.end(), r_offset,
+                                 &CompareParameter),
+                r_offset);
           }
           if (index < substitutions)
             formatted.append(subst.at(index));

base/strings/string_util_unittest.cc

@@ -992,6 +992,54 @@ TEST(StringUtilTest, ReplaceStringPlaceholders) {
   EXPECT_EQ(ASCIIToUTF16("9aa,8bb,7cc,6dd,5ee,4ff,3gg,2hh,1ii"), formatted);
 }
 
+TEST(StringUtilTest, ReplaceStringPlaceholdersNetExpansionWithContraction) {
+  // In this test, some of the substitutions are shorter than the placeholders,
+  // but overall the string gets longer.
+  std::vector<string16> subst;
+  subst.push_back(ASCIIToUTF16("9a____"));
+  subst.push_back(ASCIIToUTF16("B"));
+  subst.push_back(ASCIIToUTF16("7c___"));
+  subst.push_back(ASCIIToUTF16("d"));
+  subst.push_back(ASCIIToUTF16("5e____"));
+  subst.push_back(ASCIIToUTF16("F"));
+  subst.push_back(ASCIIToUTF16("3g___"));
+  subst.push_back(ASCIIToUTF16("h"));
+  subst.push_back(ASCIIToUTF16("1i_____"));
+
+  string16 original = ASCIIToUTF16("$1a,$2b,$3c,$4d,$5e,$6f,$7g,$8h,$9i");
+  string16 expected =
+      ASCIIToUTF16("9a____a,Bb,7c___c,dd,5e____e,Ff,3g___g,hh,1i_____i");
+
+  EXPECT_EQ(expected, ReplaceStringPlaceholders(original, subst, nullptr));
+
+  std::vector<size_t> offsets;
+  EXPECT_EQ(expected, ReplaceStringPlaceholders(original, subst, &offsets));
+  std::vector<size_t> expected_offsets = {0, 8, 11, 18, 21, 29, 32, 39, 42};
+  EXPECT_EQ(offsets.size(), subst.size());
+  EXPECT_EQ(expected_offsets, offsets);
+  for (size_t i = 0; i < offsets.size(); i++) {
+    EXPECT_EQ(expected.substr(expected_offsets[i], subst[i].length()),
+              subst[i]);
+  }
+}
+
+TEST(StringUtilTest, ReplaceStringPlaceholdersNetContractionWithExpansion) {
+  // In this test, some of the substitutions are longer than the placeholders,
+  // but overall the string gets smaller. Additionally, the placeholders appear
+  // in a permuted order.
+  std::vector<string16> subst;
+  subst.push_back(ASCIIToUTF16("z"));
+  subst.push_back(ASCIIToUTF16("y"));
+  subst.push_back(ASCIIToUTF16("XYZW"));
+  subst.push_back(ASCIIToUTF16("x"));
+  subst.push_back(ASCIIToUTF16("w"));
+
+  string16 formatted =
+      ReplaceStringPlaceholders(ASCIIToUTF16("$3_$4$2$1$5"), subst, nullptr);
+
+  EXPECT_EQ(ASCIIToUTF16("XYZW_xyzw"), formatted);
+}
+
 TEST(StringUtilTest, ReplaceStringPlaceholdersOneDigit) {
   std::vector<string16> subst;
   subst.push_back(ASCIIToUTF16("1a"));
@@ -1027,6 +1075,22 @@ TEST(StringUtilTest, StdStringReplaceStringPlaceholders) {
   EXPECT_EQ("9aa,8bb,7cc,6dd,5ee,4ff,3gg,2hh,1ii", formatted);
 }
 
+TEST(StringUtilTest, StdStringReplaceStringPlaceholdersMultipleMatches) {
+  std::vector<std::string> subst;
+  subst.push_back("4");   // Referenced twice.
+  subst.push_back("?");   // Unreferenced.
+  subst.push_back("!");   // Unreferenced.
+  subst.push_back("16");  // Referenced once.
+
+  std::string original = "$1 * $1 == $4";
+  std::string expected = "4 * 4 == 16";
+  EXPECT_EQ(expected, ReplaceStringPlaceholders(original, subst, nullptr));
+  std::vector<size_t> offsets;
+  EXPECT_EQ(expected, ReplaceStringPlaceholders(original, subst, &offsets));
+  std::vector<size_t> expected_offsets = {0, 4, 9};
+  EXPECT_EQ(expected_offsets, offsets);
+}
+
 TEST(StringUtilTest, ReplaceStringPlaceholdersConsecutiveDollarSigns) {
   std::vector<std::string> subst;
   subst.push_back("a");