New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 690471 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Feb 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Avoid logging post-commit histograms from FlushMetricsOnAppEnterBackground

Project Member Reported by bmcquade@chromium.org, Feb 9 2017

Issue description

DRP implements FlushMetricsOnAppEnterBackground and invokes both RecordPageSizeUMA(); and SendPingback(timing, info); from this method.

If FlushMetricsOnAppEnterBackground gets invoked before commit, we may log metrics for a non-DRP page, since DRP filtering policy is applied in OnCommit.

SendPingback checks to make sure browser_context_ is not null, which protects it from this issue since browser_context_ is only set in OnCommit.

RecordPageSizeUMA is protected by a test for num_network_requests_ == 0, which similarly can never be true before commit.

We should update the logging logic to be more explicit about this, since a bug could easily slip in.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 13 2017

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

commit 5550e325a24cf8e44fa08aa4dc298051d2b5a63a
Author: bmcquade <bmcquade@chromium.org>
Date: Mon Feb 13 22:05:49 2017

Page load metrics observers cleanup

* use consistent policy for logging bytes and resource count histograms
* define a helper macro for logging bytes and resource count histograms
* only log from FlushMetrics for committed pages
* add SimulateLoadedResource test helper
* fix logic in PopulateRequiredTimingFields

BUG= 690471 ,  690472 

Review-Url: https://codereview.chromium.org/2681193003
Cr-Commit-Position: refs/heads/master@{#450109}

[modify] https://crrev.com/5550e325a24cf8e44fa08aa4dc298051d2b5a63a/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc
[modify] https://crrev.com/5550e325a24cf8e44fa08aa4dc298051d2b5a63a/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.h
[modify] https://crrev.com/5550e325a24cf8e44fa08aa4dc298051d2b5a63a/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/5550e325a24cf8e44fa08aa4dc298051d2b5a63a/chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc
[modify] https://crrev.com/5550e325a24cf8e44fa08aa4dc298051d2b5a63a/chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.h
[modify] https://crrev.com/5550e325a24cf8e44fa08aa4dc298051d2b5a63a/chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer_unittest.cc
[modify] https://crrev.com/5550e325a24cf8e44fa08aa4dc298051d2b5a63a/chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.cc
[modify] https://crrev.com/5550e325a24cf8e44fa08aa4dc298051d2b5a63a/chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.h
[modify] https://crrev.com/5550e325a24cf8e44fa08aa4dc298051d2b5a63a/chrome/browser/page_load_metrics/page_load_metrics_util.h
[modify] https://crrev.com/5550e325a24cf8e44fa08aa4dc298051d2b5a63a/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Started)

Sign in to add a comment