From 3dcc5b5fe62b4b07e6292fc6aa6ceeb59637b3b8 Mon Sep 17 00:00:00 2001 From: Julien Wajsberg Date: Wed, 23 Jun 2021 16:03:49 +0000 Subject: [PATCH] Bug 1717414 - [profiler] Add a new property to network markers to report the new channel id for redirection processes r=gerald Then this will make it possible in the UI to relate the redirected request with its redirection. With this patch a debug build of Firefox will crash when a redirection happens, but the next patch will fix this. Differential Revision: https://phabricator.services.mozilla.com/D118467 --- tools/profiler/core/platform.cpp | 13 +++++++++--- tools/profiler/public/GeckoProfiler.h | 3 ++- tools/profiler/tests/gtest/GeckoProfiler.cpp | 22 +++++++++++++++----- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/tools/profiler/core/platform.cpp b/tools/profiler/core/platform.cpp index c34d03062bde..44976b48e12d 100644 --- a/tools/profiler/core/platform.cpp +++ b/tools/profiler/core/platform.cpp @@ -5822,7 +5822,7 @@ void profiler_add_network_marker( const mozilla::net::TimingStruct* aTimings, UniquePtr aSource, const Maybe& aContentType, nsIURI* aRedirectURI, - uint32_t aRedirectFlags) { + uint32_t aRedirectFlags, uint64_t aRedirectChannelId) { if (!profiler_can_accept_markers()) { return; } @@ -5856,7 +5856,8 @@ void profiler_add_network_marker( int32_t aPri, int64_t aCount, net::CacheDisposition aCacheDisposition, const net::TimingStruct& aTimings, const ProfilerString8View& aRedirectURI, - const ProfilerString8View& aContentType, uint32_t aRedirectFlags) { + const ProfilerString8View& aContentType, uint32_t aRedirectFlags, + int64_t aRedirectChannelId) { // This payload still streams a startTime and endTime property because it // made the migration to MarkerTiming on the front-end easier. aWriter.TimeProperty("startTime", aStart); @@ -5881,7 +5882,13 @@ void profiler_add_network_marker( aWriter.BoolProperty( "isHttpToHttpsRedirect", aRedirectFlags & nsIChannelEventSink::REDIRECT_STS_UPGRADE); + + MOZ_ASSERT( + aRedirectChannelId != 0, + "aRedirectChannelId should be non-zero for a redirected request"); + aWriter.IntProperty("redirectId", aRedirectChannelId); } + aWriter.StringProperty("requestMethod", aRequestMethod); if (aContentType.Length() != 0) { @@ -5968,7 +5975,7 @@ void profiler_add_network_marker( aRequestMethod, aType, aPriority, aCount, aCacheDisposition, aTimings ? *aTimings : scEmptyNetTimingStruct, redirect_spec, aContentType ? ProfilerString8View(*aContentType) : ProfilerString8View(), - aRedirectFlags); + aRedirectFlags, aRedirectChannelId); } bool profiler_add_native_allocation_marker(int64_t aSize, diff --git a/tools/profiler/public/GeckoProfiler.h b/tools/profiler/public/GeckoProfiler.h index 8653887e7174..ef8dd653c7f4 100644 --- a/tools/profiler/public/GeckoProfiler.h +++ b/tools/profiler/public/GeckoProfiler.h @@ -492,7 +492,8 @@ void profiler_add_network_marker( const mozilla::net::TimingStruct* aTimings = nullptr, mozilla::UniquePtr aSource = nullptr, const mozilla::Maybe& aContentType = mozilla::Nothing(), - nsIURI* aRedirectURI = nullptr, uint32_t aRedirectFlags = 0); + nsIURI* aRedirectURI = nullptr, uint32_t aRedirectFlags = 0, + uint64_t aRedirectChannelId = 0); enum TracingKind { TRACING_EVENT, diff --git a/tools/profiler/tests/gtest/GeckoProfiler.cpp b/tools/profiler/tests/gtest/GeckoProfiler.cpp index d2209452eda0..92ab009460e6 100644 --- a/tools/profiler/tests/gtest/GeckoProfiler.cpp +++ b/tools/profiler/tests/gtest/GeckoProfiler.cpp @@ -936,6 +936,7 @@ TEST(GeckoProfiler, Markers) /* const mozilla::Maybe& aContentType = mozilla::Nothing() */ /* nsIURI* aRedirectURI = nullptr */ + /* uint64_t aRedirectChannelId = 0 */ ); profiler_add_network_marker( @@ -957,7 +958,8 @@ TEST(GeckoProfiler, Markers) /* const mozilla::Maybe& aContentType = mozilla::Nothing() */ Some(nsDependentCString("text/html")), - /* nsIURI* aRedirectURI = nullptr */ nullptr); + /* nsIURI* aRedirectURI = nullptr */ nullptr, + /* uint64_t aRedirectChannelId = 0 */ 0); nsCOMPtr redirectURI; ASSERT_TRUE(NS_SUCCEEDED( @@ -983,7 +985,8 @@ TEST(GeckoProfiler, Markers) mozilla::Nothing(), /* nsIURI* aRedirectURI = nullptr */ redirectURI, /* uint32_t aRedirectFlags = 0 */ - nsIChannelEventSink::REDIRECT_TEMPORARY); + nsIChannelEventSink::REDIRECT_TEMPORARY, + /* uint64_t aRedirectChannelId = 0 */ 103); profiler_add_network_marker( /* nsIURI* aURI */ uri, @@ -1006,7 +1009,8 @@ TEST(GeckoProfiler, Markers) mozilla::Nothing(), /* nsIURI* aRedirectURI = nullptr */ redirectURI, /* uint32_t aRedirectFlags = 0 */ - nsIChannelEventSink::REDIRECT_PERMANENT); + nsIChannelEventSink::REDIRECT_PERMANENT, + /* uint64_t aRedirectChannelId = 0 */ 104); profiler_add_network_marker( /* nsIURI* aURI */ uri, @@ -1028,7 +1032,8 @@ TEST(GeckoProfiler, Markers) mozilla::Nothing() */ mozilla::Nothing(), /* nsIURI* aRedirectURI = nullptr */ redirectURI, - /* uint32_t aRedirectFlags = 0 */ nsIChannelEventSink::REDIRECT_INTERNAL); + /* uint32_t aRedirectFlags = 0 */ nsIChannelEventSink::REDIRECT_INTERNAL, + /* uint64_t aRedirectChannelId = 0 */ 105); profiler_add_network_marker( /* nsIURI* aURI */ uri, @@ -1051,7 +1056,8 @@ TEST(GeckoProfiler, Markers) mozilla::Nothing(), /* nsIURI* aRedirectURI = nullptr */ redirectURI, /* uint32_t aRedirectFlags = 0 */ nsIChannelEventSink::REDIRECT_INTERNAL | - nsIChannelEventSink::REDIRECT_STS_UPGRADE); + nsIChannelEventSink::REDIRECT_STS_UPGRADE, + /* uint64_t aRedirectChannelId = 0 */ 106); MOZ_RELEASE_ASSERT(profiler_add_marker( "Text in main thread with stack", geckoprofiler::category::OTHER, @@ -1428,6 +1434,7 @@ TEST(GeckoProfiler, Markers) EXPECT_TRUE(payload["RedirectURI"].isNull()); EXPECT_TRUE(payload["redirectType"].isNull()); EXPECT_TRUE(payload["isHttpToHttpsRedirect"].isNull()); + EXPECT_TRUE(payload["redirectId"].isNull()); EXPECT_TRUE(payload["contentType"].isNull()); } else if (nameString == "Load 2: http://mozilla.org/") { @@ -1445,6 +1452,7 @@ TEST(GeckoProfiler, Markers) EXPECT_TRUE(payload["RedirectURI"].isNull()); EXPECT_TRUE(payload["redirectType"].isNull()); EXPECT_TRUE(payload["isHttpToHttpsRedirect"].isNull()); + EXPECT_TRUE(payload["redirectId"].isNull()); EXPECT_EQ_JSON(payload["contentType"], String, "text/html"); } else if (nameString == "Load 3: http://mozilla.org/") { @@ -1463,6 +1471,7 @@ TEST(GeckoProfiler, Markers) "http://example.com/"); EXPECT_EQ_JSON(payload["redirectType"], String, "Temporary"); EXPECT_EQ_JSON(payload["isHttpToHttpsRedirect"], Bool, false); + EXPECT_EQ_JSON(payload["redirectId"], Int64, 103); EXPECT_TRUE(payload["contentType"].isNull()); } else if (nameString == "Load 4: http://mozilla.org/") { @@ -1481,6 +1490,7 @@ TEST(GeckoProfiler, Markers) "http://example.com/"); EXPECT_EQ_JSON(payload["redirectType"], String, "Permanent"); EXPECT_EQ_JSON(payload["isHttpToHttpsRedirect"], Bool, false); + EXPECT_EQ_JSON(payload["redirectId"], Int64, 104); EXPECT_TRUE(payload["contentType"].isNull()); } else if (nameString == "Load 5: http://mozilla.org/") { @@ -1499,6 +1509,7 @@ TEST(GeckoProfiler, Markers) "http://example.com/"); EXPECT_EQ_JSON(payload["redirectType"], String, "Internal"); EXPECT_EQ_JSON(payload["isHttpToHttpsRedirect"], Bool, false); + EXPECT_EQ_JSON(payload["redirectId"], Int64, 105); EXPECT_TRUE(payload["contentType"].isNull()); } else if (nameString == "Load 6: http://mozilla.org/") { @@ -1519,6 +1530,7 @@ TEST(GeckoProfiler, Markers) "http://example.com/"); EXPECT_EQ_JSON(payload["redirectType"], String, "Internal"); EXPECT_EQ_JSON(payload["isHttpToHttpsRedirect"], Bool, true); + EXPECT_EQ_JSON(payload["redirectId"], Int64, 106); EXPECT_TRUE(payload["contentType"].isNull()); } else if (nameString == "Text in main thread with stack") {