Additional heuristic user interaction attribution for page load metrics

Additional heuristic user interaction attribution for page load metrics

* Add UserInitiatedInfo struct, which keeps track of information related to
  whether an action was initiated by a user.

* Use UserInputTracker to heuristically determine if an action was
  triggered by a user interaction.

* Add additional histograms to better understand what the distributions
  look like for aborts with different user interaction attributes.

BUG=633329
Committed: https://crrev.com/2adc9318634ff8c0fa0fc0a98b4be2c3aa9c49ae
Cr-Commit-Position: refs/heads/master@{#437095}

[Bryan McQuade, 2016-11-30 20:34:28]

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

[commit-bot: I haz the power, 2016-11-30 20:35:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Bryan McQuade, 2016-11-30 20:36:05]

Description was changed from

==========
Use UserInputTracker to heuristically determine if an action was
triggered by a user interaction, by checking to see if a user input
event was received shortly before that action.

BUG=
==========

to

==========
Use UserInputTracker for heuristic user input event attribution

Use UserInputTracker to heuristically determine if an action was
triggered by a user interaction, by checking to see if a user input
event was received shortly before that action.

BUG=
==========

[Bryan McQuade, 2016-11-30 20:37:37]

Description was changed from

==========
Use UserInputTracker for heuristic user input event attribution

Use UserInputTracker to heuristically determine if an action was
triggered by a user interaction, by checking to see if a user input
event was received shortly before that action.

BUG=
==========

to

==========
Use UserInputTracker for heuristic user interaction attribution

Use UserInputTracker to heuristically determine if an action was
triggered by a user interaction, by checking to see if a user input
event was received shortly before that action.

BUG=
==========

[commit-bot: I haz the power, 2016-11-30 21:22:11]

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

[commit-bot: I haz the power, 2016-11-30 21:22:12]

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

[Bryan McQuade, 2016-11-30 21:26:45]

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

[commit-bot: I haz the power, 2016-11-30 21:27:16]

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, 2016-11-30 22:22:32]

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

[commit-bot: I haz the power, 2016-11-30 22:22:33]

Dry run: This issue passed the CQ dry run.

[Bryan McQuade, 2016-12-01 12:11:04]

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

[commit-bot: I haz the power, 2016-12-01 12:11:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Bryan McQuade, 2016-12-01 12:23:59]

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

[commit-bot: I haz the power, 2016-12-01 12:24:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Bryan McQuade, 2016-12-01 13:00:46]

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

[commit-bot: I haz the power, 2016-12-01 13:00:55]

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, 2016-12-01 13:50:16]

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

[commit-bot: I haz the power, 2016-12-01 13:50:17]

Dry run: This issue passed the CQ dry run.

[Bryan McQuade, 2016-12-01 17:30:08]

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

[commit-bot: I haz the power, 2016-12-01 17:30:44]

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, 2016-12-01 18:35:06]

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

[commit-bot: I haz the power, 2016-12-01 18:35:07]

Dry run: This issue passed the CQ dry run.

[Bryan McQuade, 2016-12-01 18:51:06]

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

[commit-bot: I haz the power, 2016-12-01 18:51:23]

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, 2016-12-01 19:46:26]

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

[commit-bot: I haz the power, 2016-12-01 19:46:27]

Dry run: This issue passed the CQ dry run.

[Bryan McQuade, 2016-12-02 16:00:48]

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

[commit-bot: I haz the power, 2016-12-02 16:01:07]

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, 2016-12-02 16:47:56]

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

[commit-bot: I haz the power, 2016-12-02 16:47:57]

Dry run: This issue passed the CQ dry run.

[Bryan McQuade, 2016-12-03 00:52:50]

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

[commit-bot: I haz the power, 2016-12-03 00:53:28]

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, 2016-12-03 02:03:26]

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

[commit-bot: I haz the power, 2016-12-03 02:03:26]

Dry run: This issue passed the CQ dry run.

[Bryan McQuade, 2016-12-03 02:20:01]

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

[commit-bot: I haz the power, 2016-12-03 02:20:31]

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, 2016-12-03 03:26:32]

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

[commit-bot: I haz the power, 2016-12-03 03:26:32]

Dry run: This issue passed the CQ dry run.

[Bryan McQuade, 2016-12-06 03:41:39]

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

[commit-bot: I haz the power, 2016-12-06 03:41:59]

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, 2016-12-06 04:35:13]

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

[commit-bot: I haz the power, 2016-12-06 04:35:14]

Dry run: This issue passed the CQ dry run.

[Bryan McQuade, 2016-12-06 22:52:47]

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

[commit-bot: I haz the power, 2016-12-06 22:53:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Bryan McQuade, 2016-12-06 23:07:24]

Description was changed from

==========
Use UserInputTracker for heuristic user interaction attribution

Use UserInputTracker to heuristically determine if an action was
triggered by a user interaction, by checking to see if a user input
event was received shortly before that action.

BUG=
==========

to

==========
Add additional heuristic user interaction attribution

* Add UserInitiatedInfo struct, which keeps track of information related to
  whether an action was initiated by a user.

* Use UserInputTracker to heuristically determine if an action was
  triggered by a user interaction.

* Add additional histograms to better understand what the distributions
  look like for aborts with different user interaction attributes.

BUG=
==========

[Bryan McQuade, 2016-12-06 23:08:50]

Description was changed from

==========
Add additional heuristic user interaction attribution

* Add UserInitiatedInfo struct, which keeps track of information related to
  whether an action was initiated by a user.

* Use UserInputTracker to heuristically determine if an action was
  triggered by a user interaction.

* Add additional histograms to better understand what the distributions
  look like for aborts with different user interaction attributes.

BUG=
==========

to

==========
Additional heuristic user interaction attribution for page load metrics

* Add UserInitiatedInfo struct, which keeps track of information related to
  whether an action was initiated by a user.

* Use UserInputTracker to heuristically determine if an action was
  triggered by a user interaction.

* Add additional histograms to better understand what the distributions
  look like for aborts with different user interaction attributes.

BUG=633329
==========

[Bryan McQuade, 2016-12-06 23:10:45]

bmcquade@chromium.org changed reviewers:
+ csharrison@chromium.org

[Bryan McQuade, 2016-12-06 23:10:53]

PTAL

[commit-bot: I haz the power, 2016-12-07 00:48:09]

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

[commit-bot: I haz the power, 2016-12-07 00:48:10]

Dry run: This issue passed the CQ dry run.

[Bryan McQuade, 2016-12-07 15:06:07]

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

[commit-bot: I haz the power, 2016-12-07 15:06:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Charlie Harrison, 2016-12-07 15:45:14]

Generally looks good, just a few nits. Should we also mark these histograms as
being "temporary" until we figure out a good solution?

https://codereview.chromium.org/2545593003/diff/280001/chrome/browser/page_lo...
File
chrome/browser/page_load_metrics/observers/aborts_page_load_metrics_observer.cc
(right):

https://codereview.chromium.org/2545593003/diff/280001/chrome/browser/page_lo...
chrome/browser/page_load_metrics/observers/aborts_page_load_metrics_observer.cc:63:
const char kHistogramAbortNewNavigationBeforeCommitUserGesture[] =
optional nit: I would prefer that we just inline these at their use sites unless
they are exported to tests. Honestly I think the kConstant names are less
readable than the raw strings with dots.

https://codereview.chromium.org/2545593003/diff/300001/chrome/browser/page_lo...
File
chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc
(left):

https://codereview.chromium.org/2545593003/diff/300001/chrome/browser/page_lo...
chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc:336:
if (info.user_initiated) {
Should we move these metrics to also log when the user_input_event happens? If
so, can you add a TODO of some sort to update and re-version these?

https://codereview.chromium.org/2545593003/diff/300001/chrome/browser/page_lo...
File chrome/browser/page_load_metrics/page_load_tracker.cc (right):

https://codereview.chromium.org/2545593003/diff/300001/chrome/browser/page_lo...
chrome/browser/page_load_metrics/page_load_tracker.cc:599: // abort_type_ ==
ABORT_NONE implies !abort_user_initiated_info_
nit: Is !abort_user_initiated_info_ even defined? I would update the comment to
something like "implies abort_user_initiated_info_ is not user initiated" or
something.

[commit-bot: I haz the power, 2016-12-07 16:14:18]

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

[commit-bot: I haz the power, 2016-12-07 16:14:19]

Dry run: This issue passed the CQ dry run.

[Bryan McQuade, 2016-12-07 16:30:15]

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

[commit-bot: I haz the power, 2016-12-07 16:30:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Bryan McQuade, 2016-12-07 16:32:00]

Thanks! Addressed comments. RE: marking as temporary, that's what the goal of
putting these in PageLoad.Experimental was - anything in PageLoad.Experimental
is intended to be temporary as we tune the implementation. By using that
namespace we avoid having to add a 'temporary' comment for every one of those
histograms. How does that sound?

https://codereview.chromium.org/2545593003/diff/280001/chrome/browser/page_lo...
File
chrome/browser/page_load_metrics/observers/aborts_page_load_metrics_observer.cc
(right):

https://codereview.chromium.org/2545593003/diff/280001/chrome/browser/page_lo...
chrome/browser/page_load_metrics/observers/aborts_page_load_metrics_observer.cc:63:
const char kHistogramAbortNewNavigationBeforeCommitUserGesture[] =
On 2016/12/07 at 15:45:13, Charlie Harrison wrote:
> optional nit: I would prefer that we just inline these at their use sites
unless they are exported to tests. Honestly I think the kConstant names are less
readable than the raw strings with dots.

Yeah, that makes sense. I've inlined all of these.

https://codereview.chromium.org/2545593003/diff/300001/chrome/browser/page_lo...
File
chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc
(left):

https://codereview.chromium.org/2545593003/diff/300001/chrome/browser/page_lo...
chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc:336:
if (info.user_initiated) {
On 2016/12/07 at 15:45:13, Charlie Harrison wrote:
> Should we move these metrics to also log when the user_input_event happens? If
so, can you add a TODO of some sort to update and re-version these?

Good idea. Added a TODO

https://codereview.chromium.org/2545593003/diff/300001/chrome/browser/page_lo...
File chrome/browser/page_load_metrics/page_load_tracker.cc (right):

https://codereview.chromium.org/2545593003/diff/300001/chrome/browser/page_lo...
chrome/browser/page_load_metrics/page_load_tracker.cc:599: // abort_type_ ==
ABORT_NONE implies !abort_user_initiated_info_
On 2016/12/07 at 15:45:13, Charlie Harrison wrote:
> nit: Is !abort_user_initiated_info_ even defined? I would update the comment
to something like "implies abort_user_initiated_info_ is not user initiated" or
something.

Nope, does not compile. This was intended to be pseudocode shorthand. Your
suggestion definitely makes this clearer, thanks!

[commit-bot: I haz the power, 2016-12-07 17:33:11]

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

[commit-bot: I haz the power, 2016-12-07 17:33:12]

Dry run: This issue passed the CQ dry run.

[Charlie Harrison, 2016-12-07 17:47:59]

LGTM! Your comment about the Experimental name makes sense.

[Bryan McQuade, 2016-12-07 18:07:34]

The CQ bit was checked by bmcquade@chromium.org

[commit-bot: I haz the power, 2016-12-07 18:07:48]

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

[Bryan McQuade, 2016-12-07 18:09:26]

The CQ bit was unchecked by bmcquade@chromium.org

[Bryan McQuade, 2016-12-07 18:09:39]

bmcquade@chromium.org changed reviewers:
+ isherman@chromium.org

[Bryan McQuade, 2016-12-07 18:09:53]

isherman, PTAL for histograms.xml, thanks!

[Ilya Sherman, 2016-12-07 22:43:14]

Metrics lgtm, thanks.

[Bryan McQuade, 2016-12-07 22:46:04]

The CQ bit was checked by bmcquade@chromium.org

[commit-bot: I haz the power, 2016-12-07 22:46:33]

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, 2016-12-07 22:52:18]

CQ is committing da patch.

Bot data: {"patchset_id": 320001, "attempt_start_ts": 1481150764073610,
"parent_rev": "2345a5d5a2458d379a990c883161da600de41d2d", "commit_rev":
"3768c2aca12c6b09f6d34777df4ee675b7678775"}

[commit-bot: I haz the power, 2016-12-07 22:52:49]

Committed patchset #17 (id:320001)

[commit-bot: I haz the power, 2016-12-07 22:54:46]

Description was changed from

==========
Additional heuristic user interaction attribution for page load metrics

* Add UserInitiatedInfo struct, which keeps track of information related to
  whether an action was initiated by a user.

* Use UserInputTracker to heuristically determine if an action was
  triggered by a user interaction.

* Add additional histograms to better understand what the distributions
  look like for aborts with different user interaction attributes.

BUG=633329
==========

to

==========
Additional heuristic user interaction attribution for page load metrics

* Add UserInitiatedInfo struct, which keeps track of information related to
  whether an action was initiated by a user.

* Use UserInputTracker to heuristically determine if an action was
  triggered by a user interaction.

* Add additional histograms to better understand what the distributions
  look like for aborts with different user interaction attributes.

BUG=633329

Committed: https://crrev.com/2adc9318634ff8c0fa0fc0a98b4be2c3aa9c49ae
Cr-Commit-Position: refs/heads/master@{#437095}
==========

[commit-bot: I haz the power, 2016-12-07 22:54:48]

Patchset 17 (id:??) landed as
https://crrev.com/2adc9318634ff8c0fa0fc0a98b4be2c3aa9c49ae
Cr-Commit-Position: refs/heads/master@{#437095}