Bug 1774329 - If SingleLineStyle is set at construction, also remove spaces after colons and commas - r=florian

These were the last remaining JSON whitespace characters, so we can now our
regression tests can check that there are non of these left.

Differential Revision: https://phabricator.services.mozilla.com/D152607

mfbt/JSONWriter.h

@@ -222,6 +222,9 @@ class JSONWriter {
   // specified. If a collection is printed in single-line style, every nested
   // collection within it is also printed in single-line style, even if
   // multi-line style is requested.
+  // If SingleLineStyle is set in the constructer, all JSON whitespace is
+  // eliminated, including spaces after colons and commas, for the most compact
+  // encoding possible.
   enum CollectionStyle {
     MultiLineStyle,  // the default
     SingleLineStyle
@@ -239,8 +242,7 @@ class JSONWriter {
   static constexpr Span<const char> scObjectEndString = MakeStringSpan("}");
   static constexpr Span<const char> scPropertyBeginString =
       MakeStringSpan("\"");
-  static constexpr Span<const char> scPropertyEndString =
-      MakeStringSpan("\": ");
+  static constexpr Span<const char> scPropertyEndString = MakeStringSpan("\":");
   static constexpr Span<const char> scQuoteString = MakeStringSpan("\"");
   static constexpr Span<const char> scSpaceString = MakeStringSpan(" ");
   static constexpr Span<const char> scTopObjectBeginString =
@@ -269,7 +271,7 @@ class JSONWriter {
     if (mDepth > 0 && mNeedNewlines[mDepth]) {
       mWriter->Write(scNewLineString);
       Indent();
-    } else if (mNeedComma[mDepth]) {
+    } else if (mNeedComma[mDepth] && mNeedNewlines[0]) {
       mWriter->Write(scSpaceString);
     }
   }
@@ -278,6 +280,9 @@ class JSONWriter {
     mWriter->Write(scPropertyBeginString);
     mWriter->Write(EscapedString(aName).SpanRef());
     mWriter->Write(scPropertyEndString);
+    if (mNeedNewlines[0]) {
+      mWriter->Write(scSpaceString);
+    }
   }
 
   void Scalar(const Span<const char>& aMaybePropertyName,

mfbt/tests/TestJSONWriter.cpp

@@ -435,7 +435,7 @@ void TestOneLineObject() {
 void TestOneLineJson() {
   const char* expected =
       "\
-{\"i\": 1, \"array\": [null, [{}], {\"o\": {}}, \"s\"], \"d\": 3.33}\
+{\"i\":1,\"array\":[null,[{}],{\"o\":{}},\"s\"],\"d\":3.33}\
 ";
 
   JSONWriter w(MakeUnique<StringWriteFunc>(), JSONWriter::SingleLineStyle);

mozglue/tests/TestBaseProfiler.cpp

@@ -1170,14 +1170,14 @@ void CheckJSON(mozilla::baseprofiler::SpliceableJSONWriter& aWriter,
 void TestJSONTimeOutput() {
   printf("TestJSONTimeOutput...\n");
 
-#  define TEST(in, out)                                      \
-    do {                                                     \
-      mozilla::baseprofiler::SpliceableJSONWriter writer(    \
-          mozilla::MakeUnique<StringWriteFunc>());           \
-      writer.Start(mozilla::JSONWriter::SingleLineStyle);    \
-      writer.TimeDoubleMsProperty("time_ms", (in));          \
-      writer.End();                                          \
-      CheckJSON(writer, "{\"time_ms\": " out "}", __LINE__); \
+#  define TEST(in, out)                                     \
+    do {                                                    \
+      mozilla::baseprofiler::SpliceableJSONWriter writer(   \
+          mozilla::MakeUnique<StringWriteFunc>());          \
+      writer.Start(mozilla::JSONWriter::SingleLineStyle);   \
+      writer.TimeDoubleMsProperty("time_ms", (in));         \
+      writer.End();                                         \
+      CheckJSON(writer, "{\"time_ms\":" out "}", __LINE__); \
     } while (false);
 
   TEST(0, "0");
@@ -4770,14 +4770,14 @@ void TestProfiler() {
     constexpr const auto svnpos = std::string_view::npos;
     // TODO: Properly parse profile and check fields.
     // Check for some expected marker schema JSON output.
-    MOZ_RELEASE_ASSERT(profileSV.find("\"markerSchema\": [") != svnpos);
-    MOZ_RELEASE_ASSERT(profileSV.find("\"name\": \"Text\",") != svnpos);
-    MOZ_RELEASE_ASSERT(profileSV.find("\"name\": \"tracing\",") != svnpos);
-    MOZ_RELEASE_ASSERT(profileSV.find("\"name\": \"MediaSample\",") != svnpos);
-    MOZ_RELEASE_ASSERT(profileSV.find("\"display\": [") != svnpos);
+    MOZ_RELEASE_ASSERT(profileSV.find("\"markerSchema\":[") != svnpos);
+    MOZ_RELEASE_ASSERT(profileSV.find("\"name\":\"Text\",") != svnpos);
+    MOZ_RELEASE_ASSERT(profileSV.find("\"name\":\"tracing\",") != svnpos);
+    MOZ_RELEASE_ASSERT(profileSV.find("\"name\":\"MediaSample\",") != svnpos);
+    MOZ_RELEASE_ASSERT(profileSV.find("\"display\":[") != svnpos);
     MOZ_RELEASE_ASSERT(profileSV.find("\"marker-chart\"") != svnpos);
     MOZ_RELEASE_ASSERT(profileSV.find("\"marker-table\"") != svnpos);
-    MOZ_RELEASE_ASSERT(profileSV.find("\"format\": \"string\"") != svnpos);
+    MOZ_RELEASE_ASSERT(profileSV.find("\"format\":\"string\"") != svnpos);
     // TODO: Add more checks for what's expected in the profile. Some of them
     // are done in gtest's.
 
@@ -4857,9 +4857,9 @@ static void VerifyUniqueStringContents(
   std::string_view jsonStringView(jsonString.get());
   const size_t length = writer.ChunkedWriteFunc().Length();
   MOZ_RELEASE_ASSERT(length == jsonStringView.length());
-  std::string expected = "{\"data\": [";
+  std::string expected = "{\"data\":[";
   expected += aExpectedData;
-  expected += "], \"stringTable\": [";
+  expected += "],\"stringTable\":[";
   expected += aExpectedUniqueStrings;
   expected += "]}";
   if (jsonStringView != expected) {
@@ -4909,7 +4909,7 @@ void TestUniqueJSONStrings() {
         aUniqueStrings.WriteElement(aWriter, "string");
         aUniqueStrings.WriteElement(aWriter, "string");
       },
-      "0, 0", R"("string")");
+      "0,0", R"("string")");
 
   // Two single unique strings.
   VerifyUniqueStringContents(
@@ -4917,7 +4917,7 @@ void TestUniqueJSONStrings() {
         aUniqueStrings.WriteElement(aWriter, "string0");
         aUniqueStrings.WriteElement(aWriter, "string1");
       },
-      "0, 1", R"("string0", "string1")");
+      "0,1", R"("string0","string1")");
 
   // Two unique strings with repetition.
   VerifyUniqueStringContents(
@@ -4926,7 +4926,7 @@ void TestUniqueJSONStrings() {
         aUniqueStrings.WriteElement(aWriter, "string1");
         aUniqueStrings.WriteElement(aWriter, "string0");
       },
-      "0, 1, 0", R"("string0", "string1")");
+      "0,1,0", R"("string0","string1")");
 
   // Mix some object properties, for coverage.
   VerifyUniqueStringContents(
@@ -4943,8 +4943,7 @@ void TestUniqueJSONStrings() {
         aUniqueStrings.WriteElement(aWriter, "string0");
         aUniqueStrings.WriteElement(aWriter, "prop");
       },
-      R"(0, {"p0": 1, "p1": 0, "p2": 1}, 2, 0, 1)",
-      R"("string0", "prop", "string1")");
+      R"(0,{"p0":1,"p1":0,"p2":1},2,0,1)", R"("string0","prop","string1")");
 
   // Unique string table with pre-existing data.
   {
@@ -4961,7 +4960,7 @@ void TestUniqueJSONStrings() {
           aUniqueStrings.WriteElement(aWriter, "string1");
           aUniqueStrings.WriteElement(aWriter, "string0");
         },
-        "2, 3, 2", R"("external0", "external1", "string0", "string1")", &ujs);
+        "2,3,2", R"("external0","external1","string0","string1")", &ujs);
   }
 
   // Unique string table with pre-existing data from another table.
@@ -4981,7 +4980,7 @@ void TestUniqueJSONStrings() {
           aUniqueStrings.WriteElement(aWriter, "string1");
           aUniqueStrings.WriteElement(aWriter, "string0");
         },
-        "2, 3, 2", R"("external0", "external1", "string0", "string1")", &ujs);
+        "2,3,2", R"("external0","external1","string0","string1")", &ujs);
   }
 
   // Unique string table through SpliceableJSONWriter.
@@ -5001,8 +5000,7 @@ void TestUniqueJSONStrings() {
         aWriter.UniqueStringElement("prop");
         aWriter.ResetUniqueStrings();
       },
-      R"(0, {"p0": 1, "p1": 0, "p2": 1}, 2, 0, 1)",
-      R"("string0", "prop", "string1")");
+      R"(0,{"p0":1,"p1":0,"p2":1},2,0,1)", R"("string0","prop","string1")");
 
   printf("TestUniqueJSONStrings done\n");
 }

tools/profiler/tests/gtest/GeckoProfiler.cpp

@@ -1252,8 +1252,8 @@ TEST(BaseProfiler, BlocksRingBuffer)
 
 // Common JSON checks.
 
-// Check that the given JSON string only includes up to a certain proportion of
-// JSON whitespace characters (excluding those in property names and strings).
+// Check that the given JSON string include no JSON whitespace characters
+// (excluding those in property names and strings).
 void JSONWhitespaceCheck(const char* aOutput) {
   ASSERT_NE(aOutput, nullptr);
 
@@ -1288,7 +1288,8 @@ void JSONWhitespaceCheck(const char* aOutput) {
     }
   }
 
-  EXPECT_LE(double(whitespaces) / double(length), 0.1);
+  EXPECT_EQ(whitespaces, 0u);
+  EXPECT_GT(length, 0u);
 }
 
 // Does the GETTER return a non-null TYPE? (Non-critical)