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

base/strings/string_util_unittest.cc

 // Copyright 2013 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 "base/strings/string_util.h"
 
 #include <math.h>
 #include <stdarg.h>
 #include <stddef.h>
 #include <stdint.h>
@@ skipping 658 matching lines @@
   EXPECT_EQ(13, HexDigitToInt('d'));
   EXPECT_EQ(14, HexDigitToInt('e'));
   EXPECT_EQ(15, HexDigitToInt('f'));
 }
 
 TEST(StringUtilTest, JoinString) {
   std::string separator(", ");
   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));
 
   parts.push_back("b");
   parts.push_back("c");
   EXPECT_EQ("a, b, c", JoinString(parts, separator));
 
   parts.push_back(std::string());
   EXPECT_EQ("a, b, c, ", JoinString(parts, separator));
   parts.push_back(" ");
   EXPECT_EQ("a|b|c|| ", JoinString(parts, "|"));
 }
 
 TEST(StringUtilTest, JoinString16) {
   string16 separator = ASCIIToUTF16(", ");
   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));
 
   parts.push_back(ASCIIToUTF16("b"));
   parts.push_back(ASCIIToUTF16("c"));
   EXPECT_EQ(ASCIIToUTF16("a, b, c"), JoinString(parts, separator));
 
   parts.push_back(ASCIIToUTF16(""));
   EXPECT_EQ(ASCIIToUTF16("a, b, c, "), JoinString(parts, separator));
   parts.push_back(ASCIIToUTF16(" "));
   EXPECT_EQ(ASCIIToUTF16("a|b|c|| "), JoinString(parts, ASCIIToUTF16("|")));
 }
 
 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));
 
   parts.push_back("b");
   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, "|"));
 }
 
 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);
   EXPECT_EQ(ASCIIToUTF16("a"), JoinString(parts, separator));
 
   const string16 kB = ASCIIToUTF16("b");
   parts.push_back(kB);
   const string16 kC = ASCIIToUTF16("c");
   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);
   EXPECT_EQ(ASCIIToUTF16("a|b|c|| "), JoinString(parts, ASCIIToUTF16("|")));
 }
 
 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";
   const std::string kB = "b";
   EXPECT_EQ("a, b", JoinString({kA, kB}, separator));
 
   // With StringPieces.
   const StringPiece kPieceA = kA;
   const StringPiece kPieceB = kB;
   EXPECT_EQ("a, b", JoinString({kPieceA, kPieceB}, separator));
 }
 
 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");
   EXPECT_EQ(ASCIIToUTF16("a"), JoinString({kA}, separator));
 
   const string16 kB = ASCIIToUTF16("b");
   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;
   const StringPiece16 kPieceB = kB;
   EXPECT_EQ(ASCIIToUTF16("a, b"), JoinString({kPieceA, kPieceB}, separator));
 }
 
 TEST(StringUtilTest, StartsWith) {
   EXPECT_TRUE(StartsWith("javascript:url", "javascript",
                          base::CompareCase::SENSITIVE));
@@ skipping 408 matching lines @@
   const std::string live = kLive;
   std::string dead = live;
   strncpy(WriteInto(&dead, 5), kDead, 4);
   EXPECT_EQ(kDead, dead);
   EXPECT_EQ(4u, dead.size());
   EXPECT_EQ(kLive, live);
   EXPECT_EQ(4u, live.size());
 }
 
 }  // namespace base