New issue
Advanced search Search tips

Issue 852479 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 8
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task
Team-Security-UX



Sign in to add a comment

Refactor logic for suffixing histogram name by SecurityLevel out of SecurityStatePageLoadMetricsObserver

Project Member Reported by cthomp@chromium.org, Jun 13 2018

Issue description

Currently, SecurityStatePageLoadMetricsObserver [1] collects histograms for various metrics and suffixes them with a string for the current SecurityLevel [2] of the page.

However, some other metrics are being added which will be split by security level (e.g., Autofill and PageInfo metrics), and we may want to be able to use this in more places in the future.

My initial thought is to refactor this out of the individual places it is used for metrics into https://cs.chromium.org/chromium/src/components/security_state/core/security_state.h (so they can be used anywhere we would access the SecurityLevel of the page).

[1]: https://cs.chromium.org/chromium/src/chrome/browser/page_load_metrics/observers/security_state_page_load_metrics_observer.cc
[2]: https://cs.chromium.org/chromium/src/components/security_state/core/security_state.h?l=41&gsn=SecurityLevel
 
Cc: jdeblasio@chromium.org
Cc: -jdeblasio@chromium.org cthomp@chromium.org
Owner: jdeblasio@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 8

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

commit c7b95288abe3a10e8f40eac95ade92a2b9c01176
Author: Joe DeBlasio <jdeblasio@chromium.org>
Date: Thu Nov 08 18:41:37 2018

Merge duplicate histogram naming by SecurityLevel.

This CL removes three similar sections of code that generate histogram
names with a SecurityLevel suffix and replaces them with
a shared implementation in the security_state namespace. This is to
facilitate future growth of that histogram naming pattern.

Bug:  852479 
Change-Id: Ic27bc65539108e672e64168c34dd80fa27a34b02
Reviewed-on: https://chromium-review.googlesource.com/c/1318695
Reviewed-by: Christopher Thompson <cthomp@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Reviewed-by: Adrienne Porter Felt <felt@chromium.org>
Commit-Queue: Joe DeBlasio <jdeblasio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606549}
[modify] https://crrev.com/c7b95288abe3a10e8f40eac95ade92a2b9c01176/chrome/browser/page_load_metrics/observers/security_state_page_load_metrics_observer.cc
[modify] https://crrev.com/c7b95288abe3a10e8f40eac95ade92a2b9c01176/chrome/browser/ui/page_info/page_info.cc
[modify] https://crrev.com/c7b95288abe3a10e8f40eac95ade92a2b9c01176/components/autofill/core/browser/autofill_metrics.cc
[modify] https://crrev.com/c7b95288abe3a10e8f40eac95ade92a2b9c01176/components/security_state/core/security_state.cc
[modify] https://crrev.com/c7b95288abe3a10e8f40eac95ade92a2b9c01176/components/security_state/core/security_state.h

Status: Fixed (was: Started)

Sign in to add a comment