From 73f6d61afadadaf6226b427b2ba8366cbb18ef2b Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Mon, 11 Oct 2021 13:26:28 +0000 Subject: [PATCH] Bug 1730919 - Add 'Extension Suspend' profiler marker from ChannelWrapper::Suspend. r=florian,mixedpuppy Differential Revision: https://phabricator.services.mozilla.com/D125722 --- dom/chrome-webidl/ChannelWrapper.webidl | 10 ++-- .../extensions/webrequest/ChannelWrapper.cpp | 19 +++++--- .../extensions/webrequest/ChannelWrapper.h | 8 ++-- .../extensions/webrequest/WebRequest.jsm | 55 ++++++++++++---------- 4 files changed, 52 insertions(+), 40 deletions(-) diff --git a/dom/chrome-webidl/ChannelWrapper.webidl b/dom/chrome-webidl/ChannelWrapper.webidl index 524fa9edb909..e21c6bffb009 100644 --- a/dom/chrome-webidl/ChannelWrapper.webidl +++ b/dom/chrome-webidl/ChannelWrapper.webidl @@ -122,17 +122,17 @@ interface ChannelWrapper : EventTarget { void upgradeToSecure(); /** - * Suspends the underlying channel. + * Suspends the underlying channel. The profilerText parameter is only used + * to annotate profiles. */ [Throws] - void suspend(); + void suspend(ByteString profileMarkerText); /** - * Resumes (un-suspends) the underlying channel. The profilerText parameter - * is only used to annotate profiles. + * Resumes (un-suspends) the underlying channel. */ [Throws] - void resume(ByteString profileText); + void resume(); /** * The content type of the request, usually as read from the Content-Type diff --git a/toolkit/components/extensions/webrequest/ChannelWrapper.cpp b/toolkit/components/extensions/webrequest/ChannelWrapper.cpp index 9d713e14ebad..80e71911131b 100644 --- a/toolkit/components/extensions/webrequest/ChannelWrapper.cpp +++ b/toolkit/components/extensions/webrequest/ChannelWrapper.cpp @@ -246,35 +246,40 @@ void ChannelWrapper::UpgradeToSecure(ErrorResult& aRv) { } } -void ChannelWrapper::Suspend(ErrorResult& aRv) { +void ChannelWrapper::Suspend(const nsCString& aProfileMarkerText, + ErrorResult& aRv) { if (!mSuspended) { nsresult rv = NS_ERROR_UNEXPECTED; if (nsCOMPtr chan = MaybeChannel()) { - mSuspendTime = mozilla::TimeStamp::Now(); rv = chan->Suspend(); } if (NS_FAILED(rv)) { aRv.Throw(rv); } else { mSuspended = true; + MOZ_ASSERT(mSuspendedMarkerText.IsVoid()); + mSuspendedMarkerText = aProfileMarkerText; + PROFILER_MARKER_TEXT("Extension Suspend", NETWORK, + MarkerOptions(MarkerTiming::IntervalStart()), + mSuspendedMarkerText); } } } -void ChannelWrapper::Resume(const nsCString& aText, ErrorResult& aRv) { +void ChannelWrapper::Resume(ErrorResult& aRv) { if (mSuspended) { nsresult rv = NS_ERROR_UNEXPECTED; if (nsCOMPtr chan = MaybeChannel()) { rv = chan->Resume(); - - PROFILER_MARKER_TEXT("Extension Suspend", NETWORK, - MarkerTiming::IntervalUntilNowFrom(mSuspendTime), - aText); } if (NS_FAILED(rv)) { aRv.Throw(rv); } else { mSuspended = false; + PROFILER_MARKER_TEXT("Extension Suspend", NETWORK, + MarkerOptions(MarkerTiming::IntervalEnd()), + mSuspendedMarkerText); + mSuspendedMarkerText = VoidCString(); } } } diff --git a/toolkit/components/extensions/webrequest/ChannelWrapper.h b/toolkit/components/extensions/webrequest/ChannelWrapper.h index 3f2299c93cb7..0ecdfd1f0079 100644 --- a/toolkit/components/extensions/webrequest/ChannelWrapper.h +++ b/toolkit/components/extensions/webrequest/ChannelWrapper.h @@ -141,8 +141,8 @@ class ChannelWrapper final : public DOMEventTargetHelper, void UpgradeToSecure(ErrorResult& aRv); bool Suspended() const { return mSuspended; } - void Suspend(ErrorResult& aRv); - void Resume(const nsCString& aText, ErrorResult& aRv); + void Suspend(const nsCString& aProfileMarkerText, ErrorResult& aRv); + void Resume(ErrorResult& aRv); void GetContentType(nsCString& aContentType) const; void SetContentType(const nsACString& aContentType); @@ -318,7 +318,9 @@ class ChannelWrapper final : public DOMEventTargetHelper, nsInterfaceHashtable, nsIRemoteTab> mAddonEntries; - mozilla::TimeStamp mSuspendTime; + // The text for the "Extension Suspend" marker, set from the Suspend method + // when called for the first time and then cleared on the Resume method. + nsCString mSuspendedMarkerText = VoidCString(); class RequestListener final : public nsIStreamListener, public nsIMultiPartChannelListener, diff --git a/toolkit/components/extensions/webrequest/WebRequest.jsm b/toolkit/components/extensions/webrequest/WebRequest.jsm index d013390c23a6..7f4b07b449df 100644 --- a/toolkit/components/extensions/webrequest/WebRequest.jsm +++ b/toolkit/components/extensions/webrequest/WebRequest.jsm @@ -944,14 +944,25 @@ HttpObserverManager = { requestHeaders, responseHeaders ) { + const { finalURL, id: chanId } = channel; let shouldResume = !channel.suspended; - let suspenders = []; - + // NOTE: if a request has been suspended before the GeckoProfiler + // has been activated and then resumed while the GeckoProfiler is active + // and collecting data, the resulting "Extension Suspend" marker will be + // recorded with an empty marker text (and so without url, chan id and + // the supenders addon ids). + let markerText = ""; + if (Services.profiler?.IsActive()) { + const suspenders = handlerResults + .filter(({ result }) => isThenable(result)) + .map(({ opts }) => opts.addonId) + .join(", "); + markerText = `${kind} ${finalURL} by ${suspenders} (chanId: ${chanId})`; + } try { for (let { opts, result } of handlerResults) { if (isThenable(result)) { - suspenders.push(opts.addonId); - channel.suspend(); + channel.suspend(markerText); try { result = await result; } catch (e) { @@ -986,17 +997,16 @@ HttpObserverManager = { } if (result.cancel) { - let text = ""; - if (Services.profiler?.IsActive()) { - text = - `${kind} ${channel.finalURL}` + - ` by ${suspenders.join(", ")} canceled`; - } - channel.resume(text); + channel.resume(); channel.cancel( Cr.NS_ERROR_ABORT, Ci.nsILoadInfo.BLOCKING_REASON_EXTENSION_WEBREQUEST ); + ChromeUtils.addProfilerMarker( + "Extension Canceled", + { category: "Network" }, + `${kind} ${finalURL} canceled by ${opts.addonId} (chanId: ${chanId})` + ); if (opts.policy) { let properties = channel.channel.QueryInterface( Ci.nsIWritablePropertyBag @@ -1008,15 +1018,14 @@ HttpObserverManager = { if (result.redirectUrl) { try { - let text = ""; - if (Services.profiler?.IsActive()) { - text = - `${kind} ${channel.finalURL}` + - ` by ${suspenders.join(", ")}` + - ` redirected to ${result.redirectUrl}`; - } - channel.resume(text); - channel.redirectTo(Services.io.newURI(result.redirectUrl)); + const { redirectUrl } = result; + channel.resume(); + channel.redirectTo(Services.io.newURI(redirectUrl)); + ChromeUtils.addProfilerMarker( + "Extension Redirected", + { category: "Network" }, + `${kind} ${finalURL} redirected to ${redirectUrl} by ${opts.addonId} (chanId: ${chanId})` + ); if (opts.policy) { let properties = channel.channel.QueryInterface( Ci.nsIWritablePropertyBag @@ -1102,11 +1111,7 @@ HttpObserverManager = { // Only resume the channel if it was suspended by this call. if (shouldResume) { - let text = ""; - if (Services.profiler?.IsActive()) { - text = `${kind} ${channel.finalURL} by ${suspenders.join(", ")}`; - } - channel.resume(text); + channel.resume(); } }, -- 2.11.4.GIT