Add memory and CPU histogram for OOPIF

Add memory and CPU histogram for OOPIF

Current histograms have no information about out-of-process iframes. But
it can be useful to be able to see memory and CPU usage of OOPIF
renderers. For that reason was created new histograms for OOPIF memory
and CPU.

BUG=694254
R=jochen@chromium.org

[jochen (gone - plz use gerrit), 2017-02-17 15:28:40]

I can't approve this histograms.xml part, sorry

[apisarev, 2017-02-17 15:32:34]

On 2017/02/17 15:28:40, jochen wrote:
> I can't approve this histograms.xml part, sorry

Ok, I add someone else to review it.

[jochen (gone - plz use gerrit), 2017-02-17 15:32:46]

jochen@chromium.org changed reviewers:
+ nasko@chromium.org

[jochen (gone - plz use gerrit), 2017-02-17 15:32:47]

+nasko to look at the content API

Is it possible to write a test for this?

https://codereview.chromium.org/2694363007/diff/1/chrome/browser/memory_detai...
File chrome/browser/memory_details.h (right):

https://codereview.chromium.org/2694363007/diff/1/chrome/browser/memory_detai...
chrome/browser/memory_details.h:69: bool contains_oopif = false;
please initialize this field in the ctor instead of inline as all other fields
are initialized in the ctor

https://codereview.chromium.org/2694363007/diff/1/chrome/browser/performance_...
File chrome/browser/performance_monitor/process_metrics_history.h (right):

https://codereview.chromium.org/2694363007/diff/1/chrome/browser/performance_...
chrome/browser/performance_monitor/process_metrics_history.h:32: bool
contains_oopif = false;
same here

[apisarev, 2017-02-17 15:37:28]

Description was changed from

==========
Add memory and CPU histogram for OOPIF

Current histograms have no information about out-of-process iframes. But
it can be useful to be able to see memory and CPU usage of OOPIF
renderers. For that reason was created new histograms for OOPIF memory
and CPU.

BUG=None
R=jochen@chromium.org
==========

to

==========
Add memory and CPU histogram for OOPIF

Current histograms have no information about out-of-process iframes. But
it can be useful to be able to see memory and CPU usage of OOPIF
renderers. For that reason was created new histograms for OOPIF memory
and CPU.

BUG=None
R=jochen@chromium.org
==========

[apisarev, 2017-02-17 15:37:28]

apisarev@yandex-team.ru changed reviewers:
+ creis@chromium.org

[apisarev, 2017-02-17 15:39:38]

On 2017/02/17 15:32:47, jochen wrote:
> +nasko to look at the content API
> 
> Is it possible to write a test for this?
> 
>
https://codereview.chromium.org/2694363007/diff/1/chrome/browser/memory_detai...
> File chrome/browser/memory_details.h (right):
> 
>
https://codereview.chromium.org/2694363007/diff/1/chrome/browser/memory_detai...
> chrome/browser/memory_details.h:69: bool contains_oopif = false;
> please initialize this field in the ctor instead of inline as all other fields
> are initialized in the ctor
> 
>
https://codereview.chromium.org/2694363007/diff/1/chrome/browser/performance_...
> File chrome/browser/performance_monitor/process_metrics_history.h (right):
> 
>
https://codereview.chromium.org/2694363007/diff/1/chrome/browser/performance_...
> chrome/browser/performance_monitor/process_metrics_history.h:32: bool
> contains_oopif = false;
> same here

+ creis to review histogram

I not sure about tests, but I see if it's possible.

[nasko, 2017-02-17 16:48:27]

On 2017/02/17 15:39:38, apisarev wrote:
> On 2017/02/17 15:32:47, jochen wrote:
> > +nasko to look at the content API
> > 
> > Is it possible to write a test for this?
> > 
> >
>
https://codereview.chromium.org/2694363007/diff/1/chrome/browser/memory_detai...
> > File chrome/browser/memory_details.h (right):
> > 
> >
>
https://codereview.chromium.org/2694363007/diff/1/chrome/browser/memory_detai...
> > chrome/browser/memory_details.h:69: bool contains_oopif = false;
> > please initialize this field in the ctor instead of inline as all other
fields
> > are initialized in the ctor
> > 
> >
>
https://codereview.chromium.org/2694363007/diff/1/chrome/browser/performance_...
> > File chrome/browser/performance_monitor/process_metrics_history.h (right):
> > 
> >
>
https://codereview.chromium.org/2694363007/diff/1/chrome/browser/performance_...
> > chrome/browser/performance_monitor/process_metrics_history.h:32: bool
> > contains_oopif = false;
> > same here
> 
> + creis to review histogram
> 
> I not sure about tests, but I see if it's possible.

I'll defer to creis@, since I will be out starting later today.

[Charlie Reis, 2017-02-17 20:42:38]

On 2017/02/17 16:48:27, nasko (out until Feb 27th) wrote:
> I'll defer to creis@, since I will be out starting later today.

Thanks for starting this!  Seems worthwhile to have something along these lines.

I have several thoughts about the names and how to gather the data, but I want
to see which question you're trying to answer first.  Keep in mind that OOPIFs
are often placed in normal renderer or extension processes (e.g., an extension
OOPIF will go into the existing extension process, and even processes created
for web OOPIFs might open a same-site main frame in the same process).  In this
sense, processes tend to contain a mix of main frames and subframes.

I mention this because we wouldn't want to interpret this data as saying "OOPIF
processes are really big" if the processes we're looking at also frequently
include other things (and may have been created for other reasons, like
extension background pages).  I'm not even sure if it's the common case that a
process with an OOPIF only contains OOPIFs.

Let me know what question you'd like to answer and we can figure out how to
gather and name the histograms.

(Please also file a bug for this, so we can track it after the CL lands.)

[Charlie Reis, 2017-02-17 21:14:13]

On 2017/02/17 20:42:38, Charlie Reis wrote:
> On 2017/02/17 16:48:27, nasko (out until Feb 27th) wrote:
> > I'll defer to creis@, since I will be out starting later today.
> 
> Thanks for starting this!  Seems worthwhile to have something along these
lines.
> 
> I have several thoughts about the names and how to gather the data, but I want
> to see which question you're trying to answer first.  Keep in mind that OOPIFs
> are often placed in normal renderer or extension processes (e.g., an extension
> OOPIF will go into the existing extension process, and even processes created
> for web OOPIFs might open a same-site main frame in the same process).  In
this
> sense, processes tend to contain a mix of main frames and subframes.
> 
> I mention this because we wouldn't want to interpret this data as saying
"OOPIF
> processes are really big" if the processes we're looking at also frequently
> include other things (and may have been created for other reasons, like
> extension background pages).  I'm not even sure if it's the common case that a
> process with an OOPIF only contains OOPIFs.
> 
> Let me know what question you'd like to answer and we can figure out how to
> gather and name the histograms.
> 
> (Please also file a bug for this, so we can track it after the CL lands.)

Oh, and I meant to mention-- you can look at site_details_browsertest.cc for a
way to test this (though we'll probably want to figure out exactly what to
collect before writing the tests).

[apisarev, 2017-02-20 14:11:44]

Description was changed from

==========
Add memory and CPU histogram for OOPIF

Current histograms have no information about out-of-process iframes. But
it can be useful to be able to see memory and CPU usage of OOPIF
renderers. For that reason was created new histograms for OOPIF memory
and CPU.

BUG=None
R=jochen@chromium.org
==========

to

==========
Add memory and CPU histogram for OOPIF

Current histograms have no information about out-of-process iframes. But
it can be useful to be able to see memory and CPU usage of OOPIF
renderers. For that reason was created new histograms for OOPIF memory
and CPU.

BUG=694254
R=jochen@chromium.org
==========

[apisarev, 2017-02-20 14:33:06]

On 2017/02/17 20:42:38, Charlie Reis wrote:
> On 2017/02/17 16:48:27, nasko (out until Feb 27th) wrote:
> > I'll defer to creis@, since I will be out starting later today.
> 
> Thanks for starting this!  Seems worthwhile to have something along these
lines.
> 
> I have several thoughts about the names and how to gather the data, but I want
> to see which question you're trying to answer first.  Keep in mind that OOPIFs
> are often placed in normal renderer or extension processes (e.g., an extension
> OOPIF will go into the existing extension process, and even processes created
> for web OOPIFs might open a same-site main frame in the same process).  In
this
> sense, processes tend to contain a mix of main frames and subframes.
> 
> I mention this because we wouldn't want to interpret this data as saying
"OOPIF
> processes are really big" if the processes we're looking at also frequently
> include other things (and may have been created for other reasons, like
> extension background pages).  I'm not even sure if it's the common case that a
> process with an OOPIF only contains OOPIFs.
> 
> Let me know what question you'd like to answer and we can figure out how to
> gather and name the histograms.
> 
> (Please also file a bug for this, so we can track it after the CL lands.)

I create a new issue and link it to review.
The idea was in collecting information about OOPIF resource consumption.
I found the only way to get memory and CPU information about the whole process.
And I don't know how it can be split into smaller parts, per (sub) frame.
Currently, I handle only one case when OOPIF used for ad placed in separate
renderer process. And with such assumption, my code seems legit. In more
complicated cases it wouldn't work, of course.
If you can give me advice how to do it, I would try to improve my solution.

Thanks.