Skip to content

Commit

Permalink
Implement metrics for OOPSIF.
Browse files Browse the repository at this point in the history
This CL adds four out-of-process sandboxed iframes (OOPSIFs) metrics,
the first three of which are independent of the state of the kIsolateSandboxedIframes flag:

1) SiteIsolation.IsolatableSandboxedIframes is a count of the total
   number of sandboxed iframes that are eligible to be process-isolated
   (this gives us an upper bound on the number of overhead processes we
   could have if we gave every OOPSIF its own process).

2) SiteIsolation.IsolatableSandboxedIframes.UniqueOrigins is a count of
   the total number of unique precursor origins found among the
   isolatable sandboxed iframes (this gives an estimate of process
   overhead if the isolation model was switched to per-origin).

3) SiteIsolation.IsolatableSandboxedIframes.UniqueSites is the
   count of the total number of unique sites found among the
   isolatable sandboxed iframes.

4) Memory.RenderProcessHost.Count.SandboxedIframeOverhead counts the
   number of RenderProcessHosts used by SiteInstances with the
   is_sandboxed flag set. This is the actual process overhead under the
   current process model, which varies depending on the value of the
   kIsolateSandboxedIframes flag.

Bug: 510122
Change-Id: I0eebe1155d7bb48b78b9191aa8e4429718c20124
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3582503
Reviewed-by: Caitlin Fischer <caitlinfischer@google.com>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Chris Thompson <cthomp@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: James Maclean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002212}
  • Loading branch information
W. James MacLean authored and Chromium LUCI CQ committed May 11, 2022
1 parent 8ad8d82 commit d236c49
Show file tree
Hide file tree
Showing 8 changed files with 864 additions and 0 deletions.
659 changes: 659 additions & 0 deletions chrome/browser/site_isolation/isolated_sandboxed_iframe_browsertest.cc

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions chrome/browser/site_isolation/site_details.cc
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,11 @@ void SiteDetails::UpdateHistograms(
base::UmaHistogramCounts100("SiteIsolation.OutOfProcessIframes", num_oopifs);
base::UmaHistogramCounts100("SiteIsolation.OutOfProcessInnerFrameTrees",
num_oop_inner_frame_trees);

// Log metrics related to the actual & potential process overhead of isolated
// sandboxed iframes.
RenderFrameHost::LogSandboxedIframesIsolationMetrics();

if (!content::SiteIsolationPolicy::
IsProcessIsolationForOriginAgentClusterEnabled()) {
return;
Expand Down
1 change: 1 addition & 0 deletions chrome/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1996,6 +1996,7 @@ if (!is_android) {
"../browser/site_isolation/chrome_site_per_process_browsertest.cc",
"../browser/site_isolation/chrome_site_per_process_test.cc",
"../browser/site_isolation/chrome_site_per_process_test.h",
"../browser/site_isolation/isolated_sandboxed_iframe_browsertest.cc",
"../browser/site_isolation/origin_agent_cluster_browsertest.cc",
"../browser/site_isolation/site_details_browsertest.cc",
"../browser/spellchecker/spellcheck_service_browsertest.cc",
Expand Down
120 changes: 120 additions & 0 deletions content/browser/renderer_host/render_frame_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,14 @@ typedef std::unordered_map<GlobalRenderFrameHostId,
base::LazyInstance<RoutingIDFrameMap>::DestructorAtExit g_routing_id_frame_map =
LAZY_INSTANCE_INITIALIZER;

// A global set of all sandboxed RenderFrameHosts that could be isolated from
// the rest of their SiteInstance.
typedef std::unordered_set<GlobalRenderFrameHostId,
GlobalRenderFrameHostIdHasher>
RoutingIDIsolatableSandboxedIframesSet;
base::LazyInstance<RoutingIDIsolatableSandboxedIframesSet>::DestructorAtExit
g_routing_id_isolatable_sandboxed_iframes_set = LAZY_INSTANCE_INITIALIZER;

using TokenFrameMap = std::unordered_map<blink::LocalFrameToken,
RenderFrameHostImpl*,
blink::LocalFrameToken::Hasher>;
Expand Down Expand Up @@ -1577,6 +1585,10 @@ RenderFrameHostImpl::~RenderFrameHostImpl() {
g_routing_id_frame_map.Get().erase(
GlobalRenderFrameHostId(GetProcess()->GetID(), routing_id_));

// Remove this object from the isolatable sandboxed iframe set as well, if
// necessary.
g_routing_id_isolatable_sandboxed_iframes_set.Get().erase(GetGlobalId());

// When a RenderFrameHostImpl is deleted, it may still contain children. This
// can happen with the unload timer. It causes a RenderFrameHost to delete
// itself even if it is still waiting for its children to complete their
Expand Down Expand Up @@ -10835,6 +10847,112 @@ blink::mojom::ReferrerPtr GetReferrerForDidCommitParams(
return request->GetReferrer().Clone();
}

// static
// This function logs metrics about potentially isolatable sandboxed iframes
// that are tracked through calls to UpdateIsolatableSandboxedIframeTracking().
// In addition to reporting the number of potential OOPSIFs, it also reports the
// number of unique origins encountered (to give insight into potential
// behavior if a per-origin isolation model was implemented), and it counts the
// actual number of RenderProcessHosts isolating OOPSIFs using the current
// per-site isolation model.
void RenderFrameHost::LogSandboxedIframesIsolationMetrics() {
RoutingIDIsolatableSandboxedIframesSet* oopsifs =
g_routing_id_isolatable_sandboxed_iframes_set.Pointer();

base::UmaHistogramCounts1000("SiteIsolation.IsolatableSandboxedIframes",
oopsifs->size());

// Count the number of unique origins across all the isolatable sandboxed
// iframes. This will give us a sense of the potential process overhead if we
// chose a per-origin process model for isolating these frames instead of the
// per-site model we plan to use. We use the precursor SchemeHostPort rather
// than the url::Origin, which is always opaque in these cases.
{
std::set<SiteInfo> sandboxed_site_infos;
std::set<url::SchemeHostPort> sandboxed_origins;
for (auto rfh_global_id : *oopsifs) {
auto* rfhi = RenderFrameHostImpl::FromID(rfh_global_id);
DCHECK(rfhi->GetLastCommittedOrigin().opaque());
sandboxed_origins.insert(
rfhi->GetLastCommittedOrigin().GetTupleOrPrecursorTupleIfOpaque());
sandboxed_site_infos.insert(rfhi->GetSiteInstance()->GetSiteInfo());
}
base::UmaHistogramCounts1000(
"SiteIsolation.IsolatableSandboxedIframes.UniqueOrigins",
sandboxed_origins.size());
base::UmaHistogramCounts1000(
"SiteIsolation.IsolatableSandboxedIframes.UniqueSites",
sandboxed_site_infos.size());
}

// Walk the set and count the number of unique RenderProcessHosts. Using a set
// allows us to accurately measure process overhead, including cases where
// SiteInstances from multiple BrowsingInstances are coalesced into a single
// RenderProcess.
std::set<RenderProcessHost*> sandboxed_rphs;
for (auto rfh_global_id : *oopsifs) {
auto* rfhi = FromID(rfh_global_id);
DCHECK(rfhi);
auto* site_instance =
static_cast<SiteInstanceImpl*>(rfhi->GetSiteInstance());
DCHECK(site_instance->HasProcess());
if (site_instance->GetSiteInfo().is_sandboxed())
sandboxed_rphs.insert(site_instance->GetProcess());
}
// There should be no sandboxed RPHs if the feature isn't enabled.
DCHECK(SiteIsolationPolicy::AreIsolatedSandboxedIframesEnabled() ||
sandboxed_rphs.size() == 0);
base::UmaHistogramCounts1000(
"Memory.RenderProcessHost.Count.SandboxedIframeOverhead",
sandboxed_rphs.size());
}

void RenderFrameHostImpl::UpdateIsolatableSandboxedIframeTracking() {
RoutingIDIsolatableSandboxedIframesSet* oopsifs =
g_routing_id_isolatable_sandboxed_iframes_set.Pointer();
GlobalRenderFrameHostId global_id = GetGlobalId();

// Check if the flags are correct.
DCHECK(policy_container_host_);
bool frame_is_isolatable =
IsSandboxed(network::mojom::WebSandboxFlags::kOrigin);

if (frame_is_isolatable) {
// Limit the "isolatable" sandboxed frames to those that are either in the
// same SiteInstance as their parent/opener (and thus could be isolated), or
// that are already isolated due to sandbox flags.
GURL url = GetLastCommittedURL();
if (url.IsAboutBlank() || url.is_empty()) {
frame_is_isolatable = false;
} else {
// Since this frame could be a main frame, we need to consider the
// SiteInstance of either the parent or opener (if either exists) of this
// frame, to see if the url can be placed in an OOPSIF, i.e. it's not
// already isolated because of being cross-site.
RenderFrameHost* frame_owner = GetParent();
if (!frame_owner && frame_tree_node_->opener())
frame_owner = frame_tree_node_->opener()->current_frame_host();

if (!frame_owner) {
frame_is_isolatable = false;
} else if (GetSiteInstance()->GetSiteInfo().is_sandboxed()) {
DCHECK(frame_is_isolatable);
} else if (frame_owner->GetSiteInstance() != GetSiteInstance()) {
// If this host's SiteInstance isn't already marked as is_sandboxed
// (with a frame owner), and yet the SiteInstance doesn't match that of
// our parent/opener, then it is already isolated for some other reason
// (cross-site, origin-keyed, etc.).
frame_is_isolatable = false;
}
}
}

if (frame_is_isolatable)
oopsifs->insert(global_id);
else
oopsifs->erase(global_id);
}

bool RenderFrameHostImpl::DidCommitNavigationInternal(
std::unique_ptr<NavigationRequest> navigation_request,
mojom::DidCommitProvisionalLoadParamsPtr params,
Expand Down Expand Up @@ -11192,6 +11310,8 @@ void RenderFrameHostImpl::DidCommitNewDocument(
media_device_id_salt_base_ = BrowserContext::CreateRandomMediaDeviceIDSalt();

accessibility_fatal_error_count_ = 0;

UpdateIsolatableSandboxedIframeTracking();
}

// TODO(arthursonzogni): Below, many NavigationRequest's objects are passed from
Expand Down
16 changes: 16 additions & 0 deletions content/browser/renderer_host/render_frame_host_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -3114,6 +3114,22 @@ class CONTENT_EXPORT RenderFrameHostImpl
mojom::DidCommitProvisionalLoadParamsPtr params,
mojom::DidCommitProvisionalLoadInterfaceParamsPtr interface_params);

// Updates tracking of potentially isolatable sandboxed iframes, i.e. iframes
// that could be isolated if features::kIsolateSandboxedIframes is enabled.
// A frame can only be an out-of-process sandboxed iframe (OOPSIF) if, in
// addition to the iframe being sandboxed, the url being committed is not
// about:blank and is same-site to the parent's site (i.e. is not already an
// OOPIF). Also, the sandbox permissions must not include 'allow-same-origin'.
// Anytime the commit is a potential OOPSIF, this RenderFrameHostImpl will be
// tracked in a global list from which we can determine how many potential
// OOPSIFs exist at any instant in time. Metrics based on the tracked
// isolatable frames are generated in LogSandboxedIframesIsolationMetrics()
// when it is called by the metrics recording codepath. Note: sandboxed main
// frames that have been opened by an OOPSIF are considered isolatable for the
// purposes of this function, since they could lead to process overhead under
// a per-origin isolation model. Assumes that `policy_container_host_` is set.
void UpdateIsolatableSandboxedIframeTracking();

// Called when we receive the confirmation that a navigation committed in the
// renderer. Used by both DidCommitSameDocumentNavigation and
// DidCommitNavigation.
Expand Down
3 changes: 3 additions & 0 deletions content/public/browser/render_frame_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,9 @@ class CONTENT_EXPORT RenderFrameHost : public IPC::Listener,
const base::android::JavaRef<jobject>& jrender_frame_host_android);
#endif

// Logs UMA metrics related to isolatable sandboxed iframes.
static void LogSandboxedIframesIsolationMetrics();

~RenderFrameHost() override = default;

// Returns the route id for this frame.
Expand Down
13 changes: 13 additions & 0 deletions tools/metrics/histograms/metadata/memory/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2554,6 +2554,19 @@ chromium-metrics-reviews@google.com.
</summary>
</histogram>

<histogram name="Memory.RenderProcessHost.Count.SandboxedIframeOverhead"
units="processes" expires_after="2023-05-09">
<owner>wjmaclean@chromium.org</owner>
<owner>alexmos@chromium.org</owner>
<owner>creis@chromium.org</owner>
<summary>
The number of renderer processes that represent sandboxed iframes, computed
by counting the number of RenderProcessHosts allocated to them. This will be
zero when SiteIsolationPolicy::AreIsolatedSandboxedIframesEnabled() is
false. Recorded once per UMA ping.
</summary>
</histogram>

<histogram name="Memory.RenderProcessHost.Percent.OriginAgentClusterOverhead"
units="%" expires_after="2022-09-28">
<owner>wjmaclean@chromium.org</owner>
Expand Down
47 changes: 47 additions & 0 deletions tools/metrics/histograms/metadata/security/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,53 @@ chromium-metrics-reviews@google.com.
</summary>
</histogram>

<histogram name="SiteIsolation.IsolatableSandboxedIframes" units="processes"
expires_after="2023-05-09">
<owner>wjmaclean@chromium.org</owner>
<owner>alexmos@chromium.org</owner>
<owner>creis@chromium.org</owner>
<summary>
The number of sandboxed iframes that (i) are same-site (and therefore not
already process-isolated) and (ii) eligible to be process isolated if
SiteIsolationPolicy::AreIsolatedSandboxedIframesEnabled() is true. A
sandboxed iframe is eligible for process-isolation so long as it doesn't
have `allow-same-origin` among its sandbox permissions, and it isn't an
about:blank url. This metric measures the upper bound on the process
overhead of isolating sandboxed iframes, namely the case where every
sandboxed iframe is placed in its own process. Note that the computation of
this metric is independent of the current process model. Recorded once per
UMA ping.
</summary>
</histogram>

<histogram name="SiteIsolation.IsolatableSandboxedIframes.UniqueOrigins"
units="processes" expires_after="2023-05-09">
<owner>wjmaclean@chromium.org</owner>
<owner>alexmos@chromium.org</owner>
<owner>creis@chromium.org</owner>
<summary>
The number of unique origins among the sandboxed iframes reported in
SiteIsolation.IsolatableSandboxedIframes. Used to estimate the potential
process overhead if a per-origin process model is used for isolating the
sandboxed iframes instead of the (current) per-site process model. Note that
the computation of this metric is independent of the current process model.
Recorded once per UMA ping.
</summary>
</histogram>

<histogram name="SiteIsolation.IsolatableSandboxedIframes.UniqueSites"
units="processes" expires_after="2023-05-09">
<owner>wjmaclean@chromium.org</owner>
<owner>alexmos@chromium.org</owner>
<owner>creis@chromium.org</owner>
<summary>
The number of unique sites among the sandboxed iframes reported in
SiteIsolation.IsolatableSandboxedIframes. Note that the computation of this
metric is independent of the current process model. Recorded once per UMA
ping.
</summary>
</histogram>

<histogram name="SiteIsolation.IsPasswordFormSubmittedInDedicatedProcess"
enum="SiteIsolationIsDedicatedProcess" expires_after="2022-08-07">
<owner>alexmos@chromium.org</owner>
Expand Down

0 comments on commit d236c49

Please sign in to comment.