From 5a5ec63e4c288d1688ba6dfdd164ec07125396c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Fri, 14 Jun 2024 13:24:01 +0000 Subject: [PATCH] Bug 1901016 - Remove descriptions from text and tracing marker schemas r=profiler-reviewers,julienw It's really not possible to add a single meaningful description to these marker types since they can be used by various marker types. Previous descriptions were not really descriptive or helpful for the users. They were just explaining some implementation details which our users don't need to know. For the implementation details, we already check if this `Description` field is nullptr or not, and if it's falsy we don't serialize this field at all: https://searchfox.org/mozilla-central/rev/d3fea1aa852bb3a353a0a4a875c3685da11ce39b/mozglue/baseprofiler/public/BaseProfilerMarkersPrerequisites.h#1064-1066 Differential Revision: https://phabricator.services.mozilla.com/D212956 --- mozglue/baseprofiler/public/BaseProfilerMarkers.h | 8 ++++++-- tools/profiler/tests/gtest/GeckoProfiler.cpp | 8 ++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/mozglue/baseprofiler/public/BaseProfilerMarkers.h b/mozglue/baseprofiler/public/BaseProfilerMarkers.h index 6d0b4e74e769..00ed8c1ed8e8 100644 --- a/mozglue/baseprofiler/public/BaseProfilerMarkers.h +++ b/mozglue/baseprofiler/public/BaseProfilerMarkers.h @@ -140,7 +140,9 @@ namespace mozilla::baseprofiler::markers { // Most common marker type. Others are in BaseProfilerMarkerTypes.h. struct TextMarker : public BaseMarkerType { static constexpr const char* Name = "Text"; - static constexpr const char* Description = "Generic text marker"; + // It's not possible to add a single meaningful description to this marker + // type since it can be used by various different markers. + static constexpr const char* Description = nullptr; static constexpr bool StoreName = true; @@ -167,7 +169,9 @@ struct TextMarker : public BaseMarkerType { // counterpart. struct Tracing : public BaseMarkerType { static constexpr const char* Name = "tracing"; - static constexpr const char* Description = "Generic tracing marker"; + // It's not possible to add a single meaningful description to this marker + // type since it can be used by various different markers. + static constexpr const char* Description = nullptr; static constexpr bool StoreName = true; diff --git a/tools/profiler/tests/gtest/GeckoProfiler.cpp b/tools/profiler/tests/gtest/GeckoProfiler.cpp index 4e07f03339be..3c36e53d67bb 100644 --- a/tools/profiler/tests/gtest/GeckoProfiler.cpp +++ b/tools/profiler/tests/gtest/GeckoProfiler.cpp @@ -3231,14 +3231,12 @@ TEST(GeckoProfiler, Markers) EXPECT_EQ(display[0u].asString(), "marker-chart"); EXPECT_EQ(display[1u].asString(), "marker-table"); - ASSERT_EQ(data.size(), 2u); + ASSERT_EQ(data.size(), 1u); ASSERT_TRUE(data[0u].isObject()); EXPECT_EQ_JSON(data[0u]["key"], String, "name"); EXPECT_EQ_JSON(data[0u]["label"], String, "Details"); EXPECT_EQ_JSON(data[0u]["format"], String, "string"); - EXPECT_EQ_JSON(data[1u]["label"], String, "Description"); - EXPECT_EQ_JSON(data[1u]["value"], String, "Generic text marker"); } else if (nameString == "NoPayloadUserData") { // TODO: Remove this when bug 1646714 lands. @@ -3257,14 +3255,12 @@ TEST(GeckoProfiler, Markers) EXPECT_EQ(display[1u].asString(), "marker-table"); EXPECT_EQ(display[2u].asString(), "timeline-overview"); - ASSERT_EQ(data.size(), 2u); + ASSERT_EQ(data.size(), 1u); ASSERT_TRUE(data[0u].isObject()); EXPECT_EQ_JSON(data[0u]["key"], String, "category"); EXPECT_EQ_JSON(data[0u]["label"], String, "Type"); EXPECT_EQ_JSON(data[0u]["format"], String, "string"); - EXPECT_EQ_JSON(data[1u]["label"], String, "Description"); - EXPECT_EQ_JSON(data[1u]["value"], String, "Generic tracing marker"); } else if (nameString == "BHR-detected hang") { EXPECT_EQ(display.size(), 2u);