Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(582)

Issue 1879413002: Provide currently_committed_url to PageLoadMetricsObservers (Closed)

Created:
4 years, 8 months ago by Bryan McQuade
Modified:
4 years, 8 months ago
Reviewers:
Charlie Harrison
CC:
chromium-reviews, csharrison+watch_chromium.org, loading-reviews+metrics_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Provide currently_committed_url to PageLoadMetricsObservers This allows us to track metrics based on where the user is navigating from. A referer can be used to accomplish this in some cases, though referers are not always available or may be sanitized to include only the origin in some cases. BUG=603948 Committed: https://crrev.com/003fade38f4d89100f517a6469abeb84cd9db165 Cr-Commit-Position: refs/heads/master@{#387649}

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : rebase #

Patch Set 5 : #

Patch Set 6 : add test #

Total comments: 2

Patch Set 7 : address #

Unified diffs Side-by-side diffs Delta from patch set Stats (+118 lines, -62 lines) Patch
M components/page_load_metrics/browser/metrics_web_contents_observer.h View 1 2 3 4 5 6 3 chunks +10 lines, -5 lines 0 comments Download
M components/page_load_metrics/browser/metrics_web_contents_observer.cc View 1 2 3 4 5 6 4 chunks +13 lines, -11 lines 0 comments Download
M components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc View 1 2 3 4 5 12 chunks +85 lines, -44 lines 0 comments Download
M components/page_load_metrics/browser/page_load_metrics_observer.h View 1 2 3 4 1 chunk +10 lines, -2 lines 0 comments Download

Messages

Total messages: 11 (5 generated)
Bryan McQuade
4 years, 8 months ago (2016-04-15 16:40:35 UTC) #3
Charlie Harrison
LGTM with comment. https://codereview.chromium.org/1879413002/diff/100001/components/page_load_metrics/browser/metrics_web_contents_observer.h File components/page_load_metrics/browser/metrics_web_contents_observer.h (right): https://codereview.chromium.org/1879413002/diff/100001/components/page_load_metrics/browser/metrics_web_contents_observer.h#newcode110 components/page_load_metrics/browser/metrics_web_contents_observer.h:110: // currently_committed_load_or_null beyond the scope of ...
4 years, 8 months ago (2016-04-15 16:51:38 UTC) #4
Bryan McQuade
Thanks! Updated comment. Will commit now. https://codereview.chromium.org/1879413002/diff/100001/components/page_load_metrics/browser/metrics_web_contents_observer.h File components/page_load_metrics/browser/metrics_web_contents_observer.h (right): https://codereview.chromium.org/1879413002/diff/100001/components/page_load_metrics/browser/metrics_web_contents_observer.h#newcode110 components/page_load_metrics/browser/metrics_web_contents_observer.h:110: // currently_committed_load_or_null beyond ...
4 years, 8 months ago (2016-04-15 17:21:44 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1879413002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1879413002/120001
4 years, 8 months ago (2016-04-15 17:22:56 UTC) #8
commit-bot: I haz the power
Committed patchset #7 (id:120001)
4 years, 8 months ago (2016-04-15 18:21:55 UTC) #9
commit-bot: I haz the power
4 years, 8 months ago (2016-04-15 18:22:59 UTC) #11
Message was sent while issue was closed.
Patchset 7 (id:??) landed as
https://crrev.com/003fade38f4d89100f517a6469abeb84cd9db165
Cr-Commit-Position: refs/heads/master@{#387649}

Powered by Google App Engine
This is Rietveld 408576698