New issue
Advanced search Search tips

Issue 803540 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Components:
EstimatedDays: ----
NextAction: 2018-03-08
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Task

Blocked on:
issue 803544

Blocking:
issue 803501



Sign in to add a comment

EV Study: Add navigation metrics

Project Member Reported by cthomp@chromium.org, Jan 18 2018

Issue description

For our EV removal study (issue 803138) we would like to have metrics for navigations to and from EV pages.

- The number of page loads with EV
- Navigations to/from an EV page
- Time spent on an EV page (time on unload - time on load).

This should likely be implemented as tracking (SecurityLevel, PageLoaded) pairs, so that the metric can be used for other studies as well.

A likely way to do this is to implement a WebContentsObserver in chrome/browser/ssl to track navigations to/from EV pages. This may be bookended by WebContentsObserver::DidStartNavigation or  and either another navigation or WebConententsDestroyed(). We may need to account for various navigation failure conditions as well.

This observer will also likely be helpful for our tracking of Site Engagement metrics.

We also want to track the number of EV page loads as a UKM metric.
 

Comment 1 by cthomp@chromium.org, Jan 18 2018

Blocking: 803544

Comment 2 by cthomp@chromium.org, Jan 26 2018

Status: Started (was: Assigned)

Comment 3 by cthomp@chromium.org, Feb 12 2018

Labels: -Restrict-View-Google

Comment 4 by cthomp@chromium.org, Feb 13 2018

Blocking: -803544

Comment 5 by cthomp@chromium.org, Feb 13 2018

Blockedon: 803544
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 26 2018

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

commit 6db2eb37f98b5830ce957977945c1417bba11dc8
Author: Christopher Thompson <cthomp@chromium.org>
Date: Mon Feb 26 22:01:18 2018

Add PageEndReason metrics

This adds a histogram for tracking the PageEndReason in the
SecurityStatePageLoadMetricsObserver for pages during OnComplete (so
tracking committed page loads only), split up by the final SecurityLevel
of the page. This also adds histograms for tracking the time on page split
by final SecurityLevel, and the security level at OnCommit and at
OnComplete.

Modifies the PageEndReason enum to be explicitly numbered, and added it
to enums.xml for histogram tracking.

Adds histogram tracking to each relevant browser test for the SSPLMO,
and adds a new test for the page being reloaded.

This also changes the behavior of the SSPLMO slightly. Before, if the
SiteEngagementService did not exist, the SSPLMO would not get created.
Now, it gets created, but the SiteEngagement tracking pieces are gated
on the SiteEngagementService existing.

Bug:  803540 
Change-Id: Ic368dd3cb86557f147f49a7206c9919c93ec1a3b
Reviewed-on: https://chromium-review.googlesource.com/914468
Commit-Queue: Christopher Thompson <cthomp@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539278}
[modify] https://crrev.com/6db2eb37f98b5830ce957977945c1417bba11dc8/chrome/browser/page_load_metrics/observers/security_state_page_load_metrics_observer.cc
[modify] https://crrev.com/6db2eb37f98b5830ce957977945c1417bba11dc8/chrome/browser/page_load_metrics/observers/security_state_page_load_metrics_observer.h
[modify] https://crrev.com/6db2eb37f98b5830ce957977945c1417bba11dc8/chrome/browser/page_load_metrics/observers/security_state_page_load_metrics_observer_browsertest.cc
[modify] https://crrev.com/6db2eb37f98b5830ce957977945c1417bba11dc8/chrome/browser/page_load_metrics/page_load_metrics_observer.h
[modify] https://crrev.com/6db2eb37f98b5830ce957977945c1417bba11dc8/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/6db2eb37f98b5830ce957977945c1417bba11dc8/tools/metrics/histograms/histograms.xml

Comment 7 by cthomp@chromium.org, Feb 26 2018

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 5 2018

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

commit fa844a3fa25c1d7176250e15880bc1a746abab6c
Author: Christopher Thompson <cthomp@chromium.org>
Date: Mon Mar 05 21:35:39 2018

Add missing Security histograms to histograms.xml

This adds three missing histograms to histograms.xml that should have
been introduced in https://crrev.com/c/914468: two enums,
Security.SecurityLevel.OnCommit and Security.SecurityLevel.OnComplete,
and one suffixed histogram, Security.TimeOnPage.

Bug:  803540 
Change-Id: I608604067c422703efa130dbc16f271529d0bbf6
Reviewed-on: https://chromium-review.googlesource.com/949608
Commit-Queue: Christopher Thompson <cthomp@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540945}
[modify] https://crrev.com/fa844a3fa25c1d7176250e15880bc1a746abab6c/tools/metrics/histograms/histograms.xml

We will need to merge the CL in #8 to M66 once it has baked in canary for a day or two. It is a very simple change that adds three missing entries to histograms.xml. To manually verify it, we will check that the histograms are being correctly reported in the UMA dashboard for Canary.
NextAction: 2018-03-08
The NextAction date has arrived: 2018-03-08
Labels: Merge-Request-66
I've verified that the histograms for the CL in #8 are appearing in the UMA dashboard for the latest Canary. Requesting merge to M66 as these metrics are needed for an experiment we are running in M66.
Project Member

Comment 13 by sheriffbot@chromium.org, Mar 9 2018

Labels: -Merge-Request-66 Merge-Approved-66 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M66. Please go ahead and merge the CL to branch 3359 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 14 by bugdroid1@chromium.org, Mar 9 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d5eb31fc03a2a74b7acc8414135f75620bebd6a8

commit d5eb31fc03a2a74b7acc8414135f75620bebd6a8
Author: Christopher Thompson <cthomp@chromium.org>
Date: Fri Mar 09 23:01:58 2018

Add missing Security histograms to histograms.xml

This adds three missing histograms to histograms.xml that should have
been introduced in https://crrev.com/c/914468: two enums,
Security.SecurityLevel.OnCommit and Security.SecurityLevel.OnComplete,
and one suffixed histogram, Security.TimeOnPage.

Bug:  803540 
Change-Id: I608604067c422703efa130dbc16f271529d0bbf6
Reviewed-on: https://chromium-review.googlesource.com/949608
Commit-Queue: Christopher Thompson <cthomp@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#540945}(cherry picked from commit fa844a3fa25c1d7176250e15880bc1a746abab6c)
Reviewed-on: https://chromium-review.googlesource.com/957383
Reviewed-by: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#139}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/d5eb31fc03a2a74b7acc8414135f75620bebd6a8/tools/metrics/histograms/histograms.xml

Sign in to add a comment