From 46622f995fba3f828d7bbf342c9edc2e61a9bdb3 Mon Sep 17 00:00:00 2001 From: Andrew McCreight Date: Fri, 28 Jun 2019 14:30:18 +0000 Subject: [PATCH] Bug 1561653 - Drop URIs from remote types we use in telemetry. r=nika With Fission enabled, remote types can have the form webIsolated=. This is bad, because we store them in telemetry in some places. This patch adds and uses a method that sanitizes remote types. Differential Revision: https://phabricator.services.mozilla.com/D36068 --HG-- extra : moz-landing-system : lando --- dom/ipc/ContentChild.cpp | 6 +++++- dom/ipc/ContentChild.h | 2 ++ dom/ipc/ContentParent.cpp | 14 ++++++++++---- dom/ipc/ContentParent.h | 2 ++ .../backgroundhangmonitor/HangDetails.cpp | 6 +++++- xpcom/system/nsIXULRuntime.idl | 4 +++- 6 files changed, 27 insertions(+), 7 deletions(-) diff --git a/dom/ipc/ContentChild.cpp b/dom/ipc/ContentChild.cpp index 7d8aad60d1be..a704c4446673 100644 --- a/dom/ipc/ContentChild.cpp +++ b/dom/ipc/ContentChild.cpp @@ -1752,8 +1752,10 @@ mozilla::ipc::IPCResult ContentChild::RecvSetProcessSandbox( CrashReporter::Annotation::ContentSandboxCapabilities, static_cast(SandboxInfo::Get().AsInteger())); # endif /* XP_LINUX && !OS_ANDROID */ + // Use the prefix to avoid URIs from Fission isolated processes. + auto remoteTypePrefix = RemoteTypePrefix(GetRemoteType()); CrashReporter::AnnotateCrashReport(CrashReporter::Annotation::RemoteType, - NS_ConvertUTF16toUTF8(GetRemoteType())); + NS_ConvertUTF16toUTF8(remoteTypePrefix)); #endif /* MOZ_SANDBOX */ return IPC_OK(); @@ -2772,6 +2774,8 @@ mozilla::ipc::IPCResult ContentChild::RecvRemoteType( return IPC_OK(); } +// Call RemoteTypePrefix() on the result to remove URIs if you want to use this +// for telemetry. const nsAString& ContentChild::GetRemoteType() const { return mRemoteType; } mozilla::ipc::IPCResult ContentChild::RecvInitServiceWorkers( diff --git a/dom/ipc/ContentChild.h b/dom/ipc/ContentChild.h index ff7ed86bb6a5..95f9d7bdcba4 100644 --- a/dom/ipc/ContentChild.h +++ b/dom/ipc/ContentChild.h @@ -415,6 +415,8 @@ class ContentChild final : public PContentChild, mozilla::ipc::IPCResult RecvRemoteType(const nsString& aRemoteType); + // Call RemoteTypePrefix() on the result to remove URIs if you want to use + // this for telemetry. const nsAString& GetRemoteType() const; mozilla::ipc::IPCResult RecvInitServiceWorkers( diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 96b5610f847e..780589019b29 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -702,17 +702,23 @@ uint32_t ContentParent::GetPoolSize(const nsAString& aContentProcessType) { return *sBrowserContentParents->LookupOrAdd(aContentProcessType); } -/*static*/ -uint32_t ContentParent::GetMaxProcessCount( +const nsDependentSubstring RemoteTypePrefix( const nsAString& aContentProcessType) { // The suffix after a `=` in a remoteType is dynamic, and used to control the - // process pool to use. Max process count is based only on the prefix. + // process pool to use. int32_t equalIdx = aContentProcessType.FindChar(L'='); if (equalIdx == kNotFound) { equalIdx = aContentProcessType.Length(); } + return StringHead(aContentProcessType, equalIdx); +} + +/*static*/ +uint32_t ContentParent::GetMaxProcessCount( + const nsAString& aContentProcessType) { + // Max process count is based only on the prefix. const nsDependentSubstring processTypePrefix = - StringHead(aContentProcessType, equalIdx); + RemoteTypePrefix(aContentProcessType); // Check for the default remote type of "web", as it uses different prefs. if (processTypePrefix.EqualsLiteral("web")) { diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index c32d66982408..88888de3beaa 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -1355,6 +1355,8 @@ class ContentParent final : public PContentParent, NS_DEFINE_STATIC_IID_ACCESSOR(ContentParent, NS_CONTENTPARENT_IID) +const nsDependentSubstring RemoteTypePrefix( + const nsAString& aContentProcessType); } // namespace dom } // namespace mozilla diff --git a/toolkit/components/backgroundhangmonitor/HangDetails.cpp b/toolkit/components/backgroundhangmonitor/HangDetails.cpp index 4edbcb80a777..ae4d856078c6 100644 --- a/toolkit/components/backgroundhangmonitor/HangDetails.cpp +++ b/toolkit/components/backgroundhangmonitor/HangDetails.cpp @@ -3,6 +3,7 @@ #include "nsPrintfCString.h" #include "mozilla/gfx/GPUParent.h" #include "mozilla/dom/ContentChild.h" +#include "mozilla/dom/ContentParent.h" // For RemoteTypePrefix #include "mozilla/Unused.h" #include "mozilla/GfxMessageUtils.h" // For ParamTraits @@ -255,7 +256,10 @@ void nsHangDetails::Submit() { case GeckoProcessType_Content: { auto cc = dom::ContentChild::GetSingleton(); if (cc) { - hangDetails->mDetails.remoteType().Assign(cc->GetRemoteType()); + // Use the prefix so we don't get URIs from Fission isolated + // processes. + hangDetails->mDetails.remoteType().Assign( + dom::RemoteTypePrefix(cc->GetRemoteType())); Unused << cc->SendBHRThreadHang(hangDetails->mDetails); } break; diff --git a/xpcom/system/nsIXULRuntime.idl b/xpcom/system/nsIXULRuntime.idl index 83ceef691059..9e99e8fa0629 100644 --- a/xpcom/system/nsIXULRuntime.idl +++ b/xpcom/system/nsIXULRuntime.idl @@ -98,7 +98,9 @@ interface nsIXULRuntime : nsISupports /** * The type of remote content process we're running in. - * null if we're in the parent/chrome process. + * null if we're in the parent/chrome process. This can contain + * a URI if Fission is enabled, so don't use it for any kind of + * telemetry. */ readonly attribute AString remoteType;