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

Issue 2359523003: Remove the Timing2 histograms that are deprecated. (Closed)

Created:
4 years, 3 months ago by Bryan McQuade
Modified:
4 years, 2 months ago
CC:
chromium-reviews, csharrison+watch_chromium.org, loading-reviews+metrics_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove the Timing2 histograms that are deprecated. The document.write observer was still logging a few Timing2 histograms, which prevented us from deprecating them. Now that the new versions of these histograms have rolled out to stable, we can safely remove the old ones and mark the base Timing2 histograms as deprecated. BUG=648726 Committed: https://crrev.com/e915bc7618c1e48299060bf8a45d1c8c53c6ef25 Cr-Commit-Position: refs/heads/master@{#420985}

Patch Set 1 #

Patch Set 2 : clean up formatting #

Patch Set 3 : mark core Timing2 histograms as deprecated #

Total comments: 3

Patch Set 4 : remove WasParseInForeground #

Unified diffs Side-by-side diffs Delta from patch set Stats (+111 lines, -358 lines) Patch
M chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.h View 3 chunks +0 lines, -15 lines 0 comments Download
M chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc View 1 5 chunks +59 lines, -289 lines 0 comments Download
M chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer_unittest.cc View 1 5 chunks +4 lines, -35 lines 0 comments Download
M chrome/browser/page_load_metrics/page_load_metrics_util.h View 1 2 3 1 chunk +0 lines, -7 lines 0 comments Download
M chrome/browser/page_load_metrics/page_load_metrics_util.cc View 1 2 3 1 chunk +0 lines, -12 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 15 chunks +48 lines, -0 lines 0 comments Download

Messages

Total messages: 33 (21 generated)
Bryan McQuade
PTAL, thanks!
4 years, 3 months ago (2016-09-21 20:34:55 UTC) #10
shivanisha
https://codereview.chromium.org/2359523003/diff/40001/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc File chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc (left): https://codereview.chromium.org/2359523003/diff/40001/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc#oldcode449 chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc:449: if (WasParseInForeground(timing.parse_start, timing.parse_stop, info)) { WasParseInForeground also considers incomplete ...
4 years, 2 months ago (2016-09-26 15:54:38 UTC) #13
Bryan McQuade
Thanks! I replied to your comment. PTAL, thanks! https://codereview.chromium.org/2359523003/diff/40001/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc File chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc (left): https://codereview.chromium.org/2359523003/diff/40001/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc#oldcode449 chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc:449: if ...
4 years, 2 months ago (2016-09-26 16:03:08 UTC) #15
shivanisha
On 2016/09/26 at 16:03:08, bmcquade wrote: > Thanks! I replied to your comment. PTAL, thanks! ...
4 years, 2 months ago (2016-09-26 16:52:17 UTC) #17
shivanisha
https://codereview.chromium.org/2359523003/diff/40001/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc File chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc (left): https://codereview.chromium.org/2359523003/diff/40001/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc#oldcode449 chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc:449: if (WasParseInForeground(timing.parse_start, timing.parse_stop, info)) { Removing those metrics sound ...
4 years, 2 months ago (2016-09-26 16:52:35 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2359523003/60001
4 years, 2 months ago (2016-09-26 20:08:09 UTC) #22
Bryan McQuade
gayane, PTAL for histograms.xml, thanks!
4 years, 2 months ago (2016-09-26 20:14:33 UTC) #24
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/267019)
4 years, 2 months ago (2016-09-26 20:19:32 UTC) #26
gayane -on leave until 09-2017
LGTM I find the CL description confusing because it seems like all Timing2 histograms got ...
4 years, 2 months ago (2016-09-26 20:36:53 UTC) #27
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2359523003/60001
4 years, 2 months ago (2016-09-26 20:38:31 UTC) #30
commit-bot: I haz the power
Committed patchset #4 (id:60001)
4 years, 2 months ago (2016-09-26 20:47:05 UTC) #31
commit-bot: I haz the power
4 years, 2 months ago (2016-09-26 20:50:40 UTC) #33
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/e915bc7618c1e48299060bf8a45d1c8c53c6ef25
Cr-Commit-Position: refs/heads/master@{#420985}

Powered by Google App Engine
This is Rietveld 408576698