Added regression test for base::JoinString given a null StringPiece.

base/strings/string_util_unittest.cc

Index: base/strings/string_util_unittest.cc
diff --git a/base/strings/string_util_unittest.cc b/base/strings/string_util_unittest.cc
index 6c054f8327e4718366c19fe5e8dd5f0f0778667c..6ac307ec2b4f1113d53af03ef4a655f2b1f32657 100644
--- a/base/strings/string_util_unittest.cc
+++ b/base/strings/string_util_unittest.cc
@@ -676,6 +676,10 @@ TEST(StringUtilTest, JoinString) {
   std::vector<std::string> parts;
   EXPECT_EQ(std::string(), JoinString(parts, separator));
 
+  parts.push_back(std::string());
+  EXPECT_EQ(std::string(), JoinString(parts, separator));
+  parts.clear();
+
   parts.push_back("a");
   EXPECT_EQ("a", JoinString(parts, separator));
 
@@ -694,6 +698,10 @@ TEST(StringUtilTest, JoinString16) {
   std::vector<string16> parts;
   EXPECT_EQ(string16(), JoinString(parts, separator));
 
+  parts.push_back(string16());
+  EXPECT_EQ(string16(), JoinString(parts, separator));
+  parts.clear();
+
   parts.push_back(ASCIIToUTF16("a"));
   EXPECT_EQ(ASCIIToUTF16("a"), JoinString(parts, separator));
 
@@ -709,8 +717,13 @@ TEST(StringUtilTest, JoinString16) {
 
 TEST(StringUtilTest, JoinStringPiece) {
   std::string separator(", ");
-  std::vector<base::StringPiece> parts;
-  EXPECT_EQ(base::StringPiece(), JoinString(parts, separator));
+  std::vector<StringPiece> parts;
+  EXPECT_EQ(std::string(), JoinString(parts, separator));
+
+  // Test empty first part (https://crbug.com/698073).
+  parts.push_back(StringPiece());
+  EXPECT_EQ(std::string(), JoinString(parts, separator));
+  parts.clear();
 
   parts.push_back("a");
   EXPECT_EQ("a", JoinString(parts, separator));
@@ -719,7 +732,7 @@ TEST(StringUtilTest, JoinStringPiece) {
   parts.push_back("c");
   EXPECT_EQ("a, b, c", JoinString(parts, separator));
 
-  parts.push_back(base::StringPiece());
+  parts.push_back(StringPiece());
   EXPECT_EQ("a, b, c, ", JoinString(parts, separator));
   parts.push_back(" ");
   EXPECT_EQ("a|b|c|| ", JoinString(parts, "|"));
@@ -727,8 +740,13 @@ TEST(StringUtilTest, JoinStringPiece) {
 
 TEST(StringUtilTest, JoinStringPiece16) {
   string16 separator = ASCIIToUTF16(", ");
-  std::vector<base::StringPiece16> parts;
-  EXPECT_EQ(base::StringPiece16(), JoinString(parts, separator));
+  std::vector<StringPiece16> parts;
+  EXPECT_EQ(string16(), JoinString(parts, separator));
+
+  // Test empty first part (https://crbug.com/698073).
+  parts.push_back(StringPiece16());
+  EXPECT_EQ(string16(), JoinString(parts, separator));
+  parts.clear();
 
   const string16 kA = ASCIIToUTF16("a");
   parts.push_back(kA);
@@ -740,7 +758,7 @@ TEST(StringUtilTest, JoinStringPiece16) {
   parts.push_back(kC);
   EXPECT_EQ(ASCIIToUTF16("a, b, c"), JoinString(parts, separator));
 
-  parts.push_back(base::StringPiece16());
+  parts.push_back(StringPiece16());
   EXPECT_EQ(ASCIIToUTF16("a, b, c, "), JoinString(parts, separator));
   const string16 kSpace = ASCIIToUTF16(" ");
   parts.push_back(kSpace);
@@ -749,13 +767,16 @@ TEST(StringUtilTest, JoinStringPiece16) {
 
 TEST(StringUtilTest, JoinStringInitializerList) {
   std::string separator(", ");
-  EXPECT_EQ(base::StringPiece(), JoinString({}, separator));
+  EXPECT_EQ(std::string(), JoinString({}, separator));
+
+  // Test empty first part (https://crbug.com/698073).
+  EXPECT_EQ(std::string(), JoinString({StringPiece()}, separator));
 
   // With const char*s.
   EXPECT_EQ("a", JoinString({"a"}, separator));
   EXPECT_EQ("a, b, c", JoinString({"a", "b", "c"}, separator));
-  EXPECT_EQ("a, b, c, ", JoinString({"a", "b", "c", ""}, separator));
-  EXPECT_EQ("a|b|c|| ", JoinString({"a", "b", "c", "", " "}, "|"));
+  EXPECT_EQ("a, b, c, ", JoinString({"a", "b", "c", StringPiece()}, separator));
+  EXPECT_EQ("a|b|c|| ", JoinString({"a", "b", "c", StringPiece(), " "}, "|"));
 
   // With std::strings.
   const std::string kA = "a";
@@ -770,7 +791,10 @@ TEST(StringUtilTest, JoinStringInitializerList) {
 
 TEST(StringUtilTest, JoinStringInitializerList16) {
   string16 separator = ASCIIToUTF16(", ");
-  EXPECT_EQ(base::StringPiece16(), JoinString({}, separator));
+  EXPECT_EQ(string16(), JoinString({}, separator));
+
+  // Test empty first part (https://crbug.com/698073).
+  EXPECT_EQ(string16(), JoinString({StringPiece16()}, separator));
 
   // With string16s.
   const string16 kA = ASCIIToUTF16("a");
@@ -780,12 +804,12 @@ TEST(StringUtilTest, JoinStringInitializerList16) {
   const string16 kC = ASCIIToUTF16("c");
   EXPECT_EQ(ASCIIToUTF16("a, b, c"), JoinString({kA, kB, kC}, separator));
 
-  const string16 kEmpty = ASCIIToUTF16("");
   EXPECT_EQ(ASCIIToUTF16("a, b, c, "),
-            JoinString({kA, kB, kC, kEmpty}, separator));
+            JoinString({kA, kB, kC, StringPiece16()}, separator));

[dcheng, 2017/03/03 09:11:08]

This doesn't seem quite equivalent, though I'm OK with it.

[Matt Giuca, 2017/03/06 02:39:57]

It's not equivalent; I deliberately changed it to test the edge case of a null
StringPiece that *isn't* in the first position (since there was a special case
for the first position, and may be again).

(StringPiece() and StringPiece("") are essentially the same, but as we've seen,
StringPiece() has this edge case where data() returns nullptr, so it's more
important to test for that.)

   const string16 kSpace = ASCIIToUTF16(" ");
-  EXPECT_EQ(ASCIIToUTF16("a|b|c|| "),
-            JoinString({kA, kB, kC, kEmpty, kSpace}, ASCIIToUTF16("|")));
+  EXPECT_EQ(
+      ASCIIToUTF16("a|b|c|| "),
+      JoinString({kA, kB, kC, StringPiece16(), kSpace}, ASCIIToUTF16("|")));
 
   // With StringPiece16s.
   const StringPiece16 kPieceA = kA;