New issue
Advanced search Search tips

Issue 828072 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Trace viewer: Non top-level render processes (e.g. OOPIFs, extensions) need better names

Project Member Reported by fmea...@chromium.org, Apr 2 2018

Issue description

"Now that site isolation is rolling out, this is critical to help understand control flow when viewing traces.

Ideally, we could have subframes be labeled with their origins / URLs, along with a pointer to the PID of their parent."

Issue originally filed by csharrison@ in catapult
https://github.com/catapult-project/catapult/issues/4366
 
Components: Internals>Sandbox>SiteIsolation
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c52e741c0c48826d0a1c0d20abc04980a0f3b1fa

commit c52e741c0c48826d0a1c0d20abc04980a0f3b1fa
Author: Fadi Meawad <fmeawad@chromium.org>
Date: Tue Apr 03 14:39:57 2018

[Tracing] Add process label for SubFrames in tracing

With the addition of site isolation, we now can have render
processes that are not assigned to a top level page, hence
they do not get a descriptive title in tracing.

This CL adds information about all the hosted subframes to
the process label in tracing.

Bug: chromium:828072
Change-Id: Ia6f617dc936f0c92183684faee39c65447bfd37c
Reviewed-on: https://chromium-review.googlesource.com/990553
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547700}
[modify] https://crrev.com/c52e741c0c48826d0a1c0d20abc04980a0f3b1fa/content/renderer/render_frame_impl.cc

fmeawad: Are there any particular length constraints to process labels? It would be nice to be able to annotate parent-child relationships so I can know if e.g. a subframe from https://optimizely.com is coming from cnn.com or nytimes.com (both of which use optimizely).


It looks like this update is done in the renderer, so parent PID is probably out of the question. Maybe we could just annotate with the parent's security origin?
Cc: -csharrison@chromium.org fmea...@chromium.org
Owner: csharrison@chromium.org
Status: Assigned (was: Started)
>> Are there any particular length constraints to process labels? 

No, but if it gets too long then it does not become readable.

>> It would be nice to be able to annotate parent-child relationships so I can know if e.g. a subframe from https://optimizely.com is coming from cnn.com or nytimes.com (both of which use optimizely).

Tracing users tend to trace one website at a time. Hence, even with site-isolation enabled, those labels are typically enough for the average case.
In an attempt to help with the question you have, instead of stripping it to just https://optimizely.com as in the TaskManager, I left it as https://abc.optimizely.com and https://def.optimizely.com. For each routing_id, the process will get another label and they all get displayed in trace-viewer. (i.e. for each frame hosted, it gets a label)

>> It looks like this update is done in the renderer, so parent PID is probably out of the question. Maybe we could just annotate with the parent's security origin?

Instead, we should add flow events (cross process events) that tracks the relation between the creation and communication of these processes whenever possible.


I am happy reviewing any extensions to this change as well as discussing any proposals, I will assign it to you and leave it open.
That makes sense to me, I'm happy to own any followups. Even a single flow event on frame creation would be enough to determine a relationship like this, so adding more complexity to the name doesn't really make sense.

Thanks for your help on this.

Sign in to add a comment