NOT YET READY: UMA recording the kind of target frame when extensions pierce browsing instance.

UMA recording the kind of target frame when extensions pierce browsing instance.

Extensions violate browsing instance boundaries.  The UMA metrics added
in this CL should help confirm or deny the theory that violating
browsing instance boundaries is only needed for looking up background
contents / pages (of VIEW_TYPE_BACKGROUND_CONTENTS type).

BUG=718489

[Łukasz Anforowicz, 2017-06-07 23:24:35]

lukasza@chromium.org changed reviewers:
+ bcwhite@chromium.org

[Łukasz Anforowicz, 2017-06-07 23:24:36]

bcwhite@, could you please help me with one aspect of this WIP CL?  I am trying
to use base::HistogramTester from a browser test and failing - the UMA I want to
test is recorded from a renderer and apparently the histogram tester (in the
browser-side of a browser test) doesn't see the recorded data... :-/

PS. I am tagging you based on recommendation from dcheng@ :-) ("I think you're
going to have to force the UMAs to be flushed by exiting the renderer.  Maybe
there's some IPC to do it.  bcwhite@ would know").

https://codereview.chromium.org/2925193002/diff/1/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_functional_browsertest.cc (right):

https://codereview.chromium.org/2925193002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_functional_browsertest.cc:177: // for
|tab1_popup|.
How would a browser test force a sync in a canonical way?  I guess I can try
closing all the tabs or something like that, but maybe there is a better, more
direct way of syncing the data?

FWIW, right now the new verification below fails (GetAllSamples returns an empty
vector), despite the fact that I clearly see (via LOG(ERROR) added in
extension_frame_helper.cc) that the new UMA are getting recorder.

[bcwhite, 2017-06-08 13:01:11]

> bcwhite@, could you please help me with one aspect of this WIP CL?  I am
trying
> to use base::HistogramTester from a browser test and failing - the UMA I want
to
> test is recorded from a renderer and apparently the histogram tester (in the
> browser-side of a browser test) doesn't see the recorded data... :-/

Call SubprocessMetricsProvider::MergeHistogramDeltasForTesting() in your test to
import those values.  No need for the Renderer to exit.

[Łukasz Anforowicz, 2017-06-08 16:24:40]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-08 16:25:09]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-08 16:27:56]

The CQ bit was unchecked by commit-bot@chromium.org

[Łukasz Anforowicz, 2017-06-08 16:27:56]

On 2017/06/08 13:01:11, bcwhite wrote:
> > bcwhite@, could you please help me with one aspect of this WIP CL?  I am
> trying
> > to use base::HistogramTester from a browser test and failing - the UMA I
want
> to
> > test is recorded from a renderer and apparently the histogram tester (in the
> > browser-side of a browser test) doesn't see the recorded data... :-/
> 
> Call SubprocessMetricsProvider::MergeHistogramDeltasForTesting() in your test
to
> import those values.  No need for the Renderer to exit.

Excellent - this works great.  Thank you for your help!

In the latest patchset I've tried adding a comment to
base/test/histogram_tester.h that mentions
SubprocessMetricsProvider::MergeHistogramDeltasForTesting().  I hope this looks
good (I was wondering whether it in some way violates layering"by referring to
//chrome functions from a //base comment, but in the end I think that it is
better to have too much rather than too little documentation and hints in the
code).

[commit-bot: I haz the power, 2017-06-08 16:27:57]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Łukasz Anforowicz, 2017-06-08 17:26:32]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-08 17:26:57]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-08 19:45:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-08 19:45:46]

Dry run: This issue passed the CQ dry run.

[Łukasz Anforowicz, 2017-06-09 16:21:36]

Description was changed from

==========
UMA recording the kind of target frame when extensions pierce browsing instance.

Extensions violate browsing instance boundaries.  The UMA metrics added
in this CL should help confirm or deny the theory that violating
browsing instance boundaries is only needed for looking up background
contents / pages (of VIEW_TYPE_BACKGROUND_CONTENTS type).

BUG=718489
==========

to

==========
UMA recording the kind of target frame when extensions pierce browsing instance.

Extensions violate browsing instance boundaries.  The UMA metrics added
in this CL should help confirm or deny the theory that violating
browsing instance boundaries is only needed for looking up background
contents / pages (of VIEW_TYPE_BACKGROUND_CONTENTS type).

BUG=718489
==========