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

Issue 1417733011: page_load_metrics symbolic constants refactor (Closed)

Created:
5 years, 1 month ago by Charlie Harrison
Modified:
5 years, 1 month ago
CC:
chromium-reviews, csharrison+watch_chromium.org, loading-reviews+metrics_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@navigation_start_final
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

page_load_metrics symbolic constants refactor We had const string constants for histogram names used for tests, but we should unify the code and use them everywhere. This change also corrects the names to be more standard (i.e. BG->Background). BUG=549295 Committed: https://crrev.com/980920bd4915931d6871cbfe62d717a9956947cb Cr-Commit-Position: refs/heads/master@{#359781}

Patch Set 1 #

Patch Set 2 : update dependent patchset #

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+120 lines, -118 lines) Patch
M chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc View 3 chunks +9 lines, -9 lines 0 comments Download
M components/page_load_metrics/browser/metrics_web_contents_observer.h View 1 2 1 chunk +28 lines, -11 lines 0 comments Download
M components/page_load_metrics/browser/metrics_web_contents_observer.cc View 1 2 5 chunks +28 lines, -44 lines 0 comments Download
M components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc View 1 2 9 chunks +55 lines, -54 lines 0 comments Download

Depends on Patchset:

Dependent Patchsets:

Messages

Total messages: 14 (5 generated)
Charlie Harrison
PTAL. cc nasko@ who brought up this issue with me.
5 years, 1 month ago (2015-11-06 18:09:19 UTC) #2
nasko
On 2015/11/06 18:09:19, csharrison wrote: > PTAL. cc nasko@ who brought up this issue with ...
5 years, 1 month ago (2015-11-06 23:33:25 UTC) #3
Bryan McQuade
lgtm
5 years, 1 month ago (2015-11-07 16:31:14 UTC) #4
Randy Smith (Not in Mondays)
Structure LGTM. I did not verify that each change was correct (i.e. that you used ...
5 years, 1 month ago (2015-11-09 20:24:09 UTC) #5
Charlie Harrison
It's okay, I just self reviewed for errors.
5 years, 1 month ago (2015-11-09 21:34:38 UTC) #8
Charlie Harrison
It's okay, I just self reviewed for errors.
5 years, 1 month ago (2015-11-09 21:34:39 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1417733011/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1417733011/40001
5 years, 1 month ago (2015-11-15 19:38:29 UTC) #12
commit-bot: I haz the power
Committed patchset #3 (id:40001)
5 years, 1 month ago (2015-11-15 19:41:44 UTC) #13
commit-bot: I haz the power
5 years, 1 month ago (2015-11-15 19:43:34 UTC) #14
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/980920bd4915931d6871cbfe62d717a9956947cb
Cr-Commit-Position: refs/heads/master@{#359781}

Powered by Google App Engine
This is Rietveld 408576698